Bug 1450323 - Debugger: Update Pause Points. r=jimb draft
authorJason Laster <jason.laster.11@gmail.com>
Fri, 30 Mar 2018 14:36:33 -0400
changeset 776186 e5d78203987f0869d740d8bc999f5f9ff961ae25
parent 775082 7fdb14ac881161d8dc429460d0aa373f4bbd9f39
push id104825
push userbmo:jlaster@mozilla.com
push dateMon, 02 Apr 2018 19:12:50 +0000
reviewersjimb
bugs1450323
milestone61.0a1
Bug 1450323 - Debugger: Update Pause Points. r=jimb
devtools/client/debugger/new/debugger.js
devtools/client/debugger/new/parser-worker.js
devtools/server/actors/source.js
devtools/server/actors/thread.js
devtools/server/tests/unit/test_stepping-with-pause-points.js
--- a/devtools/client/debugger/new/debugger.js
+++ b/devtools/client/debugger/new/debugger.js
@@ -4835,17 +4835,16 @@ function update(state = initialASTState(
         }
         return state.setIn(["symbols", source.id], action.value);
       }
 
     case "SET_PAUSE_POINTS":
       {
         const { source, pausePoints } = action;
         const emptyLines = (0, _ast.findEmptyLines)(source, pausePoints);
-
         return state.setIn(["pausePoints", source.id], pausePoints).setIn(["emptyLines", source.id], emptyLines);
       }
 
     case "OUT_OF_SCOPE_LOCATIONS":
       {
         return state.set("outOfScopeLocations", action.locations);
       }
 
@@ -26428,27 +26427,42 @@ function findBestMatchExpression(symbols
     }
 
     return found;
   }, null);
 } /* This Source Code Form is subject to the terms of the Mozilla Public
    * License, v. 2.0. If a copy of the MPL was not distributed with this
    * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
+function convertToList(pausePoints) {
+  return Object.keys(pausePoints).reduce(
+    (nodes, line) =>
+      nodes.concat(
+        Object.entries(pausePoints[line]).map(([column, point]) => ({
+          location: { line: parseInt(line, 10), column: parseInt(column, 10) },
+          types: point
+        }))
+      ),
+    []
+  );
+}
+
 function findEmptyLines(selectedSource, pausePoints) {
-  if (!pausePoints || pausePoints.length == 0 || !selectedSource) {
+  if (!pausePoints || !selectedSource) {
     return [];
   }
 
-  const breakpoints = pausePoints.filter(point => point.types.breakpoint);
+  const pausePointsList = convertToList(pausePoints);
+  const breakpoints = pausePointsList.filter(point => point.types.break);
   const breakpointLines = breakpoints.map(point => point.location.line);
 
   if (!selectedSource.text) {
     return [];
   }
+
   const lineCount = selectedSource.text.split("\n").length;
   const sourceLines = (0, _lodash.range)(1, lineCount + 1);
   return (0, _lodash.without)(sourceLines, ...breakpointLines);
 }
 
 function containsPosition(a, b) {
   const startsBefore = a.start.line < b.line || a.start.line === b.line && a.start.column <= b.column;
   const endsAfter = a.end.line > b.line || a.end.line === b.line && a.end.column >= b.column;
--- a/devtools/client/debugger/new/parser-worker.js
+++ b/devtools/client/debugger/new/parser-worker.js
@@ -21125,126 +21125,162 @@ var _uniqBy2 = _interopRequireDefault(_u
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 
 function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) newObj[key] = obj[key]; } } newObj.default = obj; return newObj; } }
 
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
-const isControlFlow = node => t.isForStatement(node) || t.isWhileStatement(node) || t.isIfStatement(node) || t.isSwitchCase(node) || t.isSwitchStatement(node);
-
-const isAssignment = node => t.isVariableDeclarator(node) || t.isAssignmentExpression(node);
+
+const isControlFlow = node =>
+  t.isForStatement(node) ||
+  t.isWhileStatement(node) ||
+  t.isIfStatement(node) ||
+  t.isSwitchCase(node) ||
+  t.isSwitchStatement(node);
+
+const isAssignment = node =>
+  t.isVariableDeclarator(node) || t.isAssignmentExpression(node);
 
 const isImport = node => t.isImport(node) || t.isImportDeclaration(node);
 const isReturn = node => t.isReturnStatement(node);
 const isCall = node => t.isCallExpression(node) || t.isJSXElement(node);
 
-const inExpression = (parent, grandParent) => t.isArrayExpression(parent) || t.isObjectProperty(parent) || t.isCallExpression(parent) || t.isJSXElement(parent) || t.isJSXAttribute(grandParent) || t.isTemplateLiteral(parent);
-
-const isExport = node => t.isExportNamedDeclaration(node) || t.isExportDefaultDeclaration(node);
-
-function removeDuplicatePoints(state) {
-  return (0, _uniqBy2.default)(state, ({ location }) => `${location.line}-$${location.column}`);
-}
+const inStepExpression = parent =>
+  t.isArrayExpression(parent) ||
+  t.isObjectProperty(parent) ||
+  t.isCallExpression(parent) ||
+  t.isJSXElement(parent);
+
+const inExpression = (parent, grandParent) =>
+  inStepExpression(parent) ||
+  t.isJSXAttribute(grandParent) ||
+  t.isTemplateLiteral(parent);
+
+const isExport = node =>
+  t.isExportNamedDeclaration(node) || t.isExportDefaultDeclaration(node);
 
 function getPausePoints(sourceId) {
-  const state = [];
-  (0, _ast.traverseAst)(sourceId, { enter: onEnter }, state);
-  const uniqPoints = removeDuplicatePoints(state);
-  return uniqPoints;
-}
-
+  const state = {};
+   (0, _ast.traverseAst)(sourceId, { enter: onEnter }, state);
+  return state;
+}
+
+/* eslint-disable complexity */
 function onEnter(node, ancestors, state) {
   const parent = ancestors[ancestors.length - 1];
+  const parentNode = parent && parent.node;
   const grandParent = ancestors[ancestors.length - 2];
   const startLocation = node.loc.start;
 
-  if (isImport(node) || t.isClassDeclaration(node) || isExport(node) || t.isDebuggerStatement(node)) {
-    addPoint(state, startLocation);
+  if (
+    isImport(node) ||
+    t.isClassDeclaration(node) ||
+    isExport(node) ||
+    t.isDebuggerStatement(node)
+  ) {
+    return addStopPoint(state, startLocation);
   }
 
   if (isControlFlow(node)) {
     addEmptyPoint(state, startLocation);
 
     const test = node.test || node.discriminant;
     if (test) {
-      addPoint(state, test.loc.start);
-    }
+      addStopPoint(state, test.loc.start);
+    }
+    return;
   }
 
   if (isReturn(node)) {
     // We do not want to pause at the return and the call e.g. return foo()
     if (isCall(node.argument)) {
-      addEmptyPoint(state, startLocation);
-    } else {
-      addPoint(state, startLocation);
-    }
+      return addEmptyPoint(state, startLocation);
+    }
+    return addStopPoint(state, startLocation);
   }
 
   if (isAssignment(node)) {
     // We only want to pause at literal assignments `var a = foo()`
     const value = node.right || node.init;
     if (!isCall(value)) {
-      addPoint(state, startLocation);
+      return addStopPoint(state, startLocation);
     }
   }
 
   if (isCall(node)) {
     let location = startLocation;
 
     // When functions are chained, we want to use the property location
     // e.g `foo().bar()`
     if (t.isMemberExpression(node.callee)) {
       location = node.callee.property.loc.start;
     }
 
     // NOTE: we do not want to land inside an expression e.g. [], {}, call
-    const stepOver = !inExpression(parent.node, grandParent && grandParent.node);
+    const step = !inExpression(parent.node, grandParent && grandParent.node);
 
     // NOTE: we add a point at the beginning of the expression
     // and each of the calls because the engine does not support
     // column-based member expression calls.
-    addPoint(state, startLocation, { breakpoint: true, stepOver });
+    addPoint(state, startLocation, { break: true, step });
     if (location && !(0, _isEqual2.default)(location, startLocation)) {
-      addPoint(state, location, { breakpoint: true, stepOver });
-    }
+      addPoint(state, location, { break: true, step });
+    }
+
+    return;
   }
 
   if (t.isClassProperty(node)) {
-    addBreakPoint(state, startLocation);
+    return addBreakPoint(state, startLocation);
   }
 
   if (t.isFunction(node)) {
     const { line, column } = node.loc.end;
     addBreakPoint(state, startLocation);
-    addPoint(state, { line, column: column - 1 });
+    return addStopPoint(state, { line, column: column - 1 });
   }
 
   if (t.isProgram(node)) {
     const lastStatement = node.body[node.body.length - 1];
-    addPoint(state, lastStatement.loc.end);
-  }
-}
-
-function formatNode(location, types) {
-  return { location, types };
-}
-
-function addPoint(state, location, types = { breakpoint: true, stepOver: true }) {
-  state.push(formatNode(location, types));
+    if (lastStatement) {
+      return addStopPoint(state, lastStatement.loc.end);
+    }
+  }
+
+  if (!hasPoint(state, startLocation) && inStepExpression(parentNode)) {
+    return addEmptyPoint(state, startLocation);
+  }
+}
+
+function hasPoint(state, { line, column }) {
+  return state[line] && state[line][column];
+}
+
+function addPoint(state, { line, column }, types) {
+  if (!state[line]) {
+    state[line] = {};
+  }
+  state[line][column] = types;
+  return state;
+}
+
+function addStopPoint(state, location) {
+  return addPoint(state, location, { break: true, step: true });
 }
 
 function addEmptyPoint(state, location) {
-  addPoint(state, location, { breakpoint: false, stepOver: false });
+  return addPoint(state, location, {});
 }
 
 function addBreakPoint(state, location) {
-  addPoint(state, location, { breakpoint: true, stepOver: false });
-}
+  return addPoint(state, location, { break: true });
+}
+
 
 /***/ }),
 
 /***/ 3613:
 /***/ (function(module, exports, __webpack_require__) {
 
 "use strict";
 
--- a/devtools/server/actors/source.js
+++ b/devtools/server/actors/source.js
@@ -660,21 +660,22 @@ let SourceActor = ActorClassWithSpec(sou
   unblackbox: function() {
     this.threadActor.sources.unblackBox(this.url);
   },
 
   /**
    * Handler for the "setPausePoints" packet.
    *
    * @param Array pausePoints
-   *        A list of pausePoint objects
+   *        A dictionary of pausePoint objects
    *
-   *        type PausePoint = {
-   *          location: { line: number, column: number }
-   *          types: { breakpoint: boolean, stepOver: boolean }
+   *        type PausePoints = {
+   *          line: {
+   *            column: { break?: boolean, step?: boolean }
+   *          }
    *        }
    */
   setPausePoints: function(pausePoints) {
     this.pausePoints = pausePoints;
   },
 
   /**
    * Handle a request to set a breakpoint.
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -514,37 +514,43 @@ const ThreadActor = ActorClassWithSpec(t
       }
 
       // Cases 1.1, 1.2
       if (this !== startFrame || startLocation.originalUrl !== newLocation.originalUrl) {
         return pauseAndRespond(this);
       }
 
       const pausePoints = newLocation.originalSourceActor.pausePoints;
+      const lineChanged = startLocation.originalLine !== newLocation.originalLine;
+      const columnChanged = startLocation.originalColumn !== newLocation.originalColumn;
 
       if (!pausePoints) {
         // Case 1.4
-        if (startLocation.originalLine !== newLocation.originalLine) {
+        if (lineChanged) {
           return pauseAndRespond(this);
         }
+
         return undefined;
       }
 
       // Case 1.3
-      if (
-        startLocation.originalLine === newLocation.originalLine
-        && startLocation.originalColumn === newLocation.originalColumn
-      ) {
+      if (!lineChanged && !columnChanged) {
         return undefined;
       }
 
       // When pause points are specified for the source,
       // we should pause when we are at a stepOver pause point
       const pausePoint = findPausePointForLocation(pausePoints, newLocation);
-      if (pausePoint && pausePoint.types.stepOver) {
+      if (pausePoint) {
+        if (pausePoint.step) {
+          return pauseAndRespond(this);
+        }
+      } else if (lineChanged) {
+        // NOTE: if we do not find a pause point we want to
+        // fall back on the old behavior (1.3)
         return pauseAndRespond(this);
       }
 
       // Otherwise, let execution continue (we haven't executed enough code to
       // consider this a "step" yet).
       return undefined;
     };
   },
@@ -1908,20 +1914,18 @@ function findEntryPointsForLine(scripts,
     if (offsets.length) {
       entryPoints.push({ script, offsets });
     }
   }
   return entryPoints;
 }
 
 function findPausePointForLocation(pausePoints, location) {
-  return pausePoints.find(pausePoint =>
-    pausePoint.location.line === location.originalLine
-    && pausePoint.location.column === location.originalColumn
-  );
+  const { originalLine: line, originalColumn: column } = location;
+  return pausePoints[line] && pausePoints[line][column];
 }
 
 /**
  * Unwrap a global that is wrapped in a |Debugger.Object|, or if the global has
  * become a dead object, return |undefined|.
  *
  * @param Debugger.Object wrappedGlobal
  *        The |Debugger.Object| which wraps a global.
--- a/devtools/server/tests/unit/test_stepping-with-pause-points.js
+++ b/devtools/server/tests/unit/test_stepping-with-pause-points.js
@@ -57,22 +57,32 @@ async function test_simple_stepping() {
   equal(step1.type, "paused");
   equal(step1.why.type, "resumeLimit");
   equal(step1.frame.where.line, 3);
   equal(step1.frame.where.column, 8);
 
   equal(gDebuggee.a, undefined);
   equal(gDebuggee.b, undefined);
 
+  dumpn("Step Over to line 4");
+  const step2 = await stepOver(gClient, threadClient);
+  equal(step2.type, "paused");
+  equal(step2.why.type, "resumeLimit");
+  equal(step2.frame.where.line, 4);
+  equal(step2.frame.where.column, 8);
+
+  equal(gDebuggee.a, 1);
+  equal(gDebuggee.b, undefined);
+
   dumpn("Step Over to the end of line 4");
-  const step4 = await stepOver(gClient, threadClient);
-  equal(step4.type, "paused");
-  equal(step4.why.type, "resumeLimit");
-  equal(step4.frame.where.line, 4);
-  equal(step4.frame.where.column, 14);
+  const step3 = await stepOver(gClient, threadClient);
+  equal(step3.type, "paused");
+  equal(step3.why.type, "resumeLimit");
+  equal(step3.frame.where.line, 4);
+  equal(step3.frame.where.column, 14);
   equal(gDebuggee.a, 1);
   equal(gDebuggee.b, 2);
 
   finishClient(gClient, gCallback);
 }
 
 function evaluateTestCode() {
   /* eslint-disable */