Fix some lingering POE + Exception bugs (firefox-devtools#4225)

src/actions/pause.js

@@ -5,10 +5,12 @@ import { PROMISE } from "../utils/redux/middleware/promise";
 
 import {
   getPause,
+  pausedInEval,
   getLoadedObject,
   isStepping,
   isPaused,
-  getSelectedSource
+  getSelectedSource,
+  isEvaluatingExpression
 } from "../selectors";
 import {
   updateFrameLocations,
@@ -57,12 +59,14 @@ export function resumed() {
       return;
     }
 
+    const wasPausedInEval = pausedInEval(getState());
+
     dispatch({
       type: "RESUME",
       value: undefined
     });
 
-    if (!isStepping(getState())) {
+    if (!isStepping(getState()) && !wasPausedInEval) {
       dispatch(evaluateExpressions());
     }
   };
@@ -120,7 +124,11 @@ export function paused(pauseInfo: Pause) {
     if (hiddenBreakpointLocation) {
       dispatch(removeBreakpoint(hiddenBreakpointLocation));
     }
-    dispatch(evaluateExpressions());
+
+    if (!isEvaluatingExpression(getState())) {
+      dispatch(evaluateExpressions());
+    }
+
     dispatch(
       selectSource(frame.location.sourceId, { line: frame.location.line })
     );

src/components/WelcomeBox.css

@@ -63,7 +63,6 @@ html .welcomebox .toggle-button-end.collapsed {
 }
 
 @media (max-width: 430px) {
-
   .welcomebox .small-size-layout {
     display: inline-block;
   }
@@ -80,4 +79,3 @@ html .welcomebox .toggle-button-end.collapsed {
     display: block;
   }
 }
-

src/reducers/pause.js

@@ -194,6 +194,19 @@ export function isEvaluatingExpression(state: OuterState) {
   return state.pause.command === "expression";
 }
 
+export function pausedInEval(state: OuterState) {
+  if (!state.pause.pause) {
+    return false;
+  }
+
+  const exception = state.pause.pause.why.exception;
+  if (!exception) {
+    return false;
+  }
+
+  return exception.preview.fileName === "debugger eval code";
+}
+
 export function getLoadedObject(state: OuterState, objectId: string) {
   return getLoadedObjects(state)[objectId];
 }

src/test/mochitest/browser.ini

@@ -64,7 +64,6 @@ support-files =
 skip-if = true # Bug 1383576
 [browser_dbg-breakpoints-cond.js]
 [browser_dbg-call-stack.js]
-[browser_dbg-expressions.js]
 [browser_dbg-scopes.js]
 [browser_dbg-chrome-create.js]
 [browser_dbg-chrome-debugging.js]
@@ -74,6 +73,8 @@ skip-if = debug # bug 1374187
 [browser_dbg-editor-gutter.js]
 [browser_dbg-editor-select.js]
 [browser_dbg-editor-highlight.js]
+[browser_dbg-expressions.js]
+[browser_dbg-expressions-error.js]
 [browser_dbg-iframes.js]
 [browser_dbg_keyboard_navigation.js]
 [browser_dbg_keyboard-shortcuts.js]
@@ -102,4 +103,4 @@ skip-if = true # Bug 1393121, 1393299
 [browser_dbg-tabs.js]
 [browser_dbg-toggling-tools.js]
 [browser_dbg-wasm-sourcemaps.js]
-skip-if = true 
+skip-if = true

src/test/mochitest/browser_dbg-expressions-error.js

@@ -0,0 +1,90 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * test pausing on an errored watch expression
+ * assert that you can:
+ * 1. resume
+ * 2. still evalutate expressions
+ * 3. expand properties
+ */
+
+const expressionSelectors = {
+  input: "input.input-expression"
+};
+
+function getLabel(dbg, index) {
+  return findElement(dbg, "expressionNode", index).innerText;
+}
+
+function getValue(dbg, index) {
+  return findElement(dbg, "expressionValue", index).innerText;
+}
+
+function assertEmptyValue(dbg, index) {
+  const value = findElement(dbg, "expressionValue", index);
+  if (value) {
+    is(value.innerText, "");
+    return;
+  }
+
+  is(value, null);
+}
+
+function toggleExpression(dbg, index) {
+  findElement(dbg, "expressionNode", index).click();
+}
+
+async function addExpression(dbg, input) {
+  info("Adding an expression");
+  findElementWithSelector(dbg, expressionSelectors.input).focus();
+  type(dbg, input);
+  pressKey(dbg, "Enter");
+
+  await waitForDispatch(dbg, "EVALUATE_EXPRESSION");
+}
+
+async function editExpression(dbg, input) {
+  info("updating the expression");
+  dblClickElement(dbg, "expressionNode", 1);
+  // Position cursor reliably at the end of the text.
+  pressKey(dbg, "End");
+  type(dbg, input);
+  pressKey(dbg, "Enter");
+  await waitForDispatch(dbg, "EVALUATE_EXPRESSION");
+}
+
+/*
+ * When we add a bad expression, we'll pause,
+ * resume, and wait for the expression to finish being evaluated.
+ */
+async function addBadExpression(dbg, input) {
+  const paused = waitForPaused(dbg);
+  const added = addExpression(dbg, input);
+
+  await paused;
+  ok(dbg.selectors.isEvaluatingExpression(dbg.getState()));
+  await resume(dbg);
+  await added;
+}
+
+add_task(async function() {
+  const dbg = await initDebugger("doc-script-switching.html");
+
+  await togglePauseOnExceptions(dbg, true, false);
+
+  // add a good expression, 2 bad expressions, and another good one
+  await addExpression(dbg, "location");
+  await addBadExpression(dbg, "foo.bar");
+  await addBadExpression(dbg, "foo.batt");
+  await addExpression(dbg, "2");
+
+  // check the value of
+  is(getValue(dbg, 2), "(unavailable)");
+  is(getValue(dbg, 3), "(unavailable)");
+  is(getValue(dbg, 4), 2);
+
+  toggleExpression(dbg, 1);
+  await waitForDispatch(dbg, "LOAD_OBJECT_PROPERTIES");
+  is(findAllElements(dbg, "expressionNodes").length, 20);
+});

src/test/mochitest/head.js

@@ -504,7 +504,7 @@ function stepOut(dbg) {
 function resume(dbg) {
   info("Resuming");
   dbg.actions.resume();
-  return waitForThreadEvents(dbg, "resumed");
+  return waitForState(dbg, state => !dbg.selectors.isPaused(state));
 }
 
 function deleteExpression(dbg, input) {

src/utils/expressions.js

@@ -1,5 +1,6 @@
 // @flow
 
+import { correctIndentation } from "./indentation";
 import type { Expression } from "debugger-html";
 
 // replace quotes and slashes that could interfere with the evaluation.
@@ -14,13 +15,13 @@ export function sanitizeInput(input: string) {
  * NOTE: we add line after the expression to protect against comments.
 */
 export function wrapExpression(input: string) {
-  return `eval(\`
+  return correctIndentation(`
     try {
       ${sanitizeInput(input)}
     } catch (e) {
       e
     }
-  \`)`.trim();
+  `);
 }
 
 export function getValue(expression: Expression) {

src/utils/function.js

@@ -1,20 +1,5 @@
 import { findClosestScope } from "./breakpoint/astBreakpointLocation";
-
-function getIndentation(lines) {
-  const firstLine = lines[0];
-  const secondLine = lines[1];
-  const lastLine = lines[lines.length - 1];
-
-  const _getIndentation = line => line && line.match(/^\s*/)[0].length;
-
-  const indentations = [
-    _getIndentation(firstLine),
-    _getIndentation(secondLine),
-    _getIndentation(lastLine)
-  ];
-
-  return Math.max(...indentations, 0);
-}
+import { correctIndentation } from "./indentation";
 
 export function findFunctionText(line, source, symbols) {
   const func = findClosestScope(symbols.functions, { line, column: Infinity });
@@ -27,12 +12,8 @@ export function findFunctionText(line, source, symbols) {
   const firstLine = lines[start.line - 1].slice(start.column);
   const lastLine = lines[end.line - 1].slice(0, end.column);
   const middle = lines.slice(start.line, end.line - 1);
-  const functionLines = [firstLine, ...middle, lastLine];
-
-  const indentation = getIndentation(functionLines);
-  const formattedLines = functionLines.map(_line =>
-    _line.replace(new RegExp(`^\\s{0,${indentation - 1}}`), "")
-  );
+  const functionText = [firstLine, ...middle, lastLine].join("\n");
+  const indentedFunctionText = correctIndentation(functionText);
 
-  return formattedLines.join("\n").trim();
+  return indentedFunctionText;
 }

src/utils/indentation.js

@@ -0,0 +1,25 @@
+function getIndentation(lines) {
+  const firstLine = lines[0];
+  const secondLine = lines[1];
+  const lastLine = lines[lines.length - 1];
+
+  const _getIndentation = line => line && line.match(/^\s*/)[0].length;
+
+  const indentations = [
+    _getIndentation(firstLine),
+    _getIndentation(secondLine),
+    _getIndentation(lastLine)
+  ];
+
+  return Math.max(...indentations);
+}
+
+export function correctIndentation(text) {
+  const lines = text.trim().split("\n");
+  const indentation = getIndentation(lines);
+  const formattedLines = lines.map(_line =>
+    _line.replace(new RegExp(`^\\s{0,${indentation - 1}}`), "")
+  );
+
+  return formattedLines.join("\n");
+}

src/utils/redux/middleware/log.js

@@ -4,9 +4,7 @@
  */
 export function log({ dispatch, getState }) {
   return next => action => {
-    const actionText = JSON.stringify(action, null, 2);
-    const truncatedActionText = `${actionText.slice(0, 1000)}...`;
-    console.log(`[DISPATCH ${action.type}]`, action, truncatedActionText);
+    console.log(`[DISPATCH ${action.type}]`, action);
     next(action);
   };
 }

src/utils/tests/__snapshots__/expressions.spec.js.snap

@@ -1,21 +1,17 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`expressions wrap exxpression should wrap an expression 1`] = `
-"eval(\`
-    try {
-      foo
-    } catch (e) {
-      e
-    }
-  \`)"
+"try {
+ foo
+} catch (e) {
+ e
+}"
 `;
 
 exports[`expressions wrap exxpression should wrap expression with a comment 1`] = `
-"eval(\`
-    try {
-      foo // yo yo
-    } catch (e) {
-      e
-    }
-  \`)"
+"try {
+ foo // yo yo
+} catch (e) {
+ e
+}"
 `;

src/utils/tests/__snapshots__/indentation.spec.js.snap

@@ -0,0 +1,27 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`indentation mad indentation 1`] = `
+"try {
+console.log(\\"yo\\")
+} catch (e) {
+console.log(\\"yo\\")
+ }"
+`;
+
+exports[`indentation one function 1`] = `
+"function foo() {
+ console.log(\\"yo\\")
+}"
+`;
+
+exports[`indentation one line 1`] = `"foo"`;
+
+exports[`indentation simple 1`] = `"foo"`;
+
+exports[`indentation try catch 1`] = `
+"try {
+ console.log(\\"yo\\")
+} catch (e) {
+ console.log(\\"yo\\")
+}"
+`;