Bug 1577673 - Continue to Here does not pause. r=davidwalsh

Differential Revision: https://phabricator.services.mozilla.com/D48178

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/packages/devtools-reps/src/object-inspector/utils/node.js

@@ -826,7 +826,7 @@ function getChildren(options: {
 // e.g. `b` in { a: { b: 2 } } resolves to `a.b`
 function getPathExpression(item) {
   if (item && item.parent) {
-    let parent = nodeIsBucket(item.parent) ? item.parent.parent : item.parent;
+    const parent = nodeIsBucket(item.parent) ? item.parent.parent : item.parent;
     return `${getPathExpression(parent)}.${item.name}`;
   }
 

devtools/client/debugger/src/actions/pause/continueToHere.js

@@ -8,19 +8,20 @@ import {
   getSelectedSource,
   getSelectedFrame,
   getCanRewind,
+  getClosestBreakpointPosition,
+  getBreakpoint,
 } from "../../selectors";
 import { addHiddenBreakpoint } from "../breakpoints";
+import { setBreakpointPositions } from "../breakpoints/breakpointPositions";
+
 import { resume, rewind } from "./commands";
 
 import type { ThunkArgs } from "../types";
-import type { ThreadContext } from "../../types";
+import type { ThreadContext, SourceLocation } from "../../types";
 
-export function continueToHere(
-  cx: ThreadContext,
-  line: number,
-  column?: number
-) {
+export function continueToHere(cx: ThreadContext, location: SourceLocation) {
   return async function({ dispatch, getState }: ThunkArgs) {
+    const { line, column, sourceId } = location;
     const selectedSource = getSelectedSource(getState());
     const selectedFrame = getSelectedFrame(getState(), cx.thread);
 
@@ -29,20 +30,40 @@ export function continueToHere(
     }
 
     const debugLine = selectedFrame.location.line;
-    if (debugLine == line) {
+    // If the user selects a line to continue to,
+    // it must be different than the currently paused line.
+    if (!column && debugLine == line) {
       return;
     }
 
+    await dispatch(setBreakpointPositions({ cx, sourceId, line }));
+    const position = getClosestBreakpointPosition(getState(), location);
+
+    // If the user selects a location in the editor,
+    // there must be a place we can pause on that line.
+    if (column && !position) {
+      return;
+    }
+
+    const pauseLocation = column && position ? position.location : location;
+
+    // If we're replaying and the user selects a line above the currently
+    // paused line, lets rewind to it. NOTE: this ignores a couple important
+    // cases like loops, and wanting to play forward to the next function call.
     const action =
       getCanRewind(getState()) && line < debugLine ? rewind : resume;
 
-    await dispatch(
-      addHiddenBreakpoint(cx, {
-        line,
-        column,
-        sourceId: selectedSource.id,
-      })
-    );
+    // Set a hidden breakpoint if we do not already have a breakpoint
+    // at the closest position
+    if (!getBreakpoint(getState(), pauseLocation)) {
+      await dispatch(
+        addHiddenBreakpoint(cx, {
+          sourceId: selectedSource.id,
+          line: pauseLocation.line,
+          column: pauseLocation.column,
+        })
+      );
+    }
 
     dispatch(action(cx));
   };

devtools/client/debugger/src/components/Editor/menus/editor.js

@@ -37,7 +37,7 @@ export const continueToHereItem = (
 ) => ({
   accesskey: L10N.getStr("editor.continueToHere.accesskey"),
   disabled: !isPaused,
-  click: () => editorActions.continueToHere(cx, location.line, location.column),
+  click: () => editorActions.continueToHere(cx, location),
   id: "node-menu-continue-to-here",
   label: L10N.getStr("editor.continueToHere.label"),
 });

devtools/client/debugger/src/reducers/sources.js

@@ -69,6 +69,7 @@ import type {
   SourceWithContent,
   ThreadId,
   MappedLocation,
+  BreakpointPosition,
   BreakpointPositions,
 } from "../types";
 import type { PendingSelectedLocation, Selector } from "./types";
@@ -968,13 +969,21 @@ export function hasBreakpointPositions(
   return !!getBreakpointPositionsForSource(state, sourceId);
 }
 
+export function getBreakpointPositionsForLine(
+  state: OuterState,
+  sourceId: string,
+  line: number
+): ?Array<BreakpointPosition> {
+  const positions = getBreakpointPositionsForSource(state, sourceId);
+  return positions && positions[line];
+}
+
 export function hasBreakpointPositionsForLine(
   state: OuterState,
   sourceId: string,
   line: number
 ): boolean {
-  const positions = getBreakpointPositionsForSource(state, sourceId);
-  return !!(positions && positions[line]);
+  return !!getBreakpointPositionsForLine(state, sourceId, line);
 }
 
 export function getBreakpointPositionsForLocation(

devtools/client/debugger/src/selectors/breakpointAtLocation.js

@@ -4,11 +4,14 @@
 
 // @flow
 
-import { getSelectedSource } from "../reducers/sources";
+import {
+  getSelectedSource,
+  getBreakpointPositionsForLine,
+} from "../reducers/sources";
 import { getBreakpointsList } from "../reducers/breakpoints";
 import { isGenerated } from "../utils/source";
 
-import type { Breakpoint } from "../types";
+import type { Breakpoint, BreakpointPosition, PartialPosition } from "../types";
 import type { State } from "../reducers/types";
 
 function getColumn(column, selectedSource) {
@@ -56,23 +59,25 @@ function findBreakpointAtLocation(
   });
 }
 
-// returns the closest active column breakpoint to current cursorPosition in editor.
-function findClosestBreakpointToCursor(lineBreakpoints, cursorPosition) {
-  const closestBreakpoint = lineBreakpoints.reduce((closestBp, currentBp) => {
-    // check that editor is re-rendered and breakpoints are assigned.
-    if (typeof closestBp === "object") {
-      const currentColumn = currentBp.generatedLocation.column;
-      const closestColumn = closestBp.generatedLocation.column;
-      // check that breakpoint has a column.
-      if (currentColumn && closestColumn) {
-        const currentDistance = Math.abs(currentColumn - cursorPosition.column);
-        const closestDistance = Math.abs(closestColumn - cursorPosition.column);
+// returns the closest active column breakpoint
+function findClosestBreakpoint(breakpoints, column) {
+  if (!breakpoints || breakpoints.length == 0) {
+    return null;
+  }
 
-        return currentDistance < closestDistance ? currentBp : closestBp;
-      }
+  const firstBreakpoint = breakpoints[0];
+  return breakpoints.reduce((closestBp, currentBp) => {
+    const currentColumn = currentBp.generatedLocation.column;
+    const closestColumn = closestBp.generatedLocation.column;
+    // check that breakpoint has a column.
+    if (column && currentColumn && closestColumn) {
+      const currentDistance = Math.abs(currentColumn - column);
+      const closestDistance = Math.abs(closestColumn - column);
+
+      return currentDistance < closestDistance ? currentBp : closestBp;
     }
-  }, lineBreakpoints[0] || {});
-  return closestBreakpoint;
+    return closestBp;
+  }, firstBreakpoint);
 }
 
 /*
@@ -104,7 +109,30 @@ export function getBreakpointsAtLine(state: State, line: number): Breakpoint[] {
   );
 }
 
-export function getClosestBreakpoint(state: State, cursorPosition: Object) {
-  const lineBreakpoints = getBreakpointsAtLine(state, cursorPosition.line);
-  return findClosestBreakpointToCursor(lineBreakpoints, cursorPosition);
+export function getClosestBreakpoint(
+  state: State,
+  position: PartialPosition
+): ?Breakpoint {
+  const columnBreakpoints = getBreakpointsAtLine(state, position.line);
+
+  const breakpoint = findClosestBreakpoint(columnBreakpoints, position.column);
+  return (breakpoint: any);
+}
+
+export function getClosestBreakpointPosition(
+  state: State,
+  position: PartialPosition
+): ?BreakpointPosition {
+  const selectedSource = getSelectedSource(state);
+  if (!selectedSource) {
+    throw new Error("no selectedSource");
+  }
+
+  const columnBreakpoints = getBreakpointPositionsForLine(
+    state,
+    selectedSource.id,
+    position.line
+  );
+
+  return findClosestBreakpoint(columnBreakpoints, position.column);
 }

devtools/client/debugger/src/selectors/index.js

@@ -36,6 +36,7 @@ export {
   getClosestBreakpoint,
   getBreakpointAtLocation,
   getBreakpointsAtLine,
+  getClosestBreakpointPosition,
 } from "./breakpointAtLocation";
 export {
   getVisibleBreakpoints,

devtools/client/debugger/test/mochitest/browser.ini

@@ -42,6 +42,7 @@ skip-if = debug # Window leaks: bug 1575332
 [browser_dbg-breakpoints-duplicate-functions.js]
 [browser_dbg-browser-content-toolbox.js]
 skip-if = !e10s || verify # This test is only valid in e10s
+[browser_dbg-continue-to-here.js]
 [browser_dbg-breakpoints-reloading.js]
 [browser_dbg-breakpoint-skipping.js]
 [browser_dbg-breakpoint-skipping-console.js]

devtools/client/debugger/test/mochitest/browser_dbg-continue-to-here.js

@@ -0,0 +1,44 @@
+/* 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/>. */
+
+async function continueToLine(dbg, line) {
+  rightClickElement(dbg, "gutter", line);
+  selectContextMenuItem(dbg, selectors.editorContextMenu.continueToHere);
+  await waitForDispatch(dbg, "RESUME");
+  return waitForPaused(dbg);
+}
+
+async function continueToColumn(dbg, pos) {
+  await rightClickAtPos(dbg, pos);
+
+  selectContextMenuItem(dbg, selectors.editorContextMenu.continueToHere);
+  await waitForDispatch(dbg, "RESUME");
+  await waitForPaused(dbg);
+  await waitForInlinePreviews(dbg);
+}
+
+add_task(async function() {
+  const dbg = await initDebugger("doc-pause-points.html", "pause-points.js");
+  await selectSource(dbg, "pause-points.js");
+  await waitForSelectedSource(dbg, "pause-points.js");
+
+  info("Test continuing to a line");
+  clickElementInTab("#sequences");
+  await waitForPaused(dbg);
+  await waitForInlinePreviews(dbg);
+
+  await continueToColumn(dbg, { line: 31, ch: 7 });
+  assertDebugLine(dbg, 31, 4);
+  await resume(dbg);
+
+  info("Test continuing to a column");
+  clickElementInTab("#sequences");
+  await waitForPaused(dbg);
+  await waitForInlinePreviews(dbg);
+
+  await continueToColumn(dbg, { line: 31, ch: 7 });
+
+  assertDebugLine(dbg, 31, 4);
+  await resume(dbg);
+});

devtools/client/debugger/test/mochitest/examples/doc-pause-points.html

@@ -2,12 +2,12 @@
 <html>
   <head></head>
   <body>
-     <header>
+    <header>
       <h1>
         Debugger Pause Points
       </h1>
       <button onclick="statements()">statements</button>
-      <button onclick="sequences()">sequences</button>
+      <button id="sequences" onclick="sequences()">sequences</button>
       <button onclick="expressions()">expressions</button>
       <button onclick="flow()">flow</button>
     </header>

devtools/client/debugger/test/mochitest/helpers.js

@@ -334,7 +334,7 @@ function assertPausedLocation(dbg) {
   ok(isVisibleInEditor(dbg, getCM(dbg).display.gutters), "gutter is visible");
 }
 
-function assertDebugLine(dbg, line) {
+function assertDebugLine(dbg, line, column) {
   // Check the debug line
   const lineInfo = getCM(dbg).lineInfo(line - 1);
   const source = dbg.selectors.getSelectedSourceWithContent() || {};
@@ -379,6 +379,11 @@ function assertDebugLine(dbg, line) {
           span.marker.className.includes("debug-expression")
       ).length > 0;
 
+    if (column) {
+      const frame = dbg.selectors.getVisibleSelectedFrame();
+      is(frame.location.column, column, `Paused at column ${column}`);
+    }
+
     ok(classMatch, "expression is highlighted as paused");
   }
 }
@@ -492,6 +497,10 @@ async function waitForPaused(dbg, url) {
   await waitForSelectedSource(dbg, url);
 }
 
+function waitForInlinePreviews(dbg) {
+  return waitForState(dbg, () => dbg.selectors.getSelectedInlinePreviews())
+}
+
 function waitForCondition(dbg, condition) {
   return waitForState(dbg, state =>
     dbg.selectors
@@ -1269,6 +1278,9 @@ const selectors = {
     removeOthers: "#node-menu-delete-other",
     removeCondition: "#node-menu-remove-condition",
   },
+  editorContextMenu: {
+    continueToHere: "#node-menu-continue-to-here",
+  },
   columnBreakpoints: ".column-breakpoint",
   scopes: ".scopes-list",
   scopeNode: i => `.scopes-list .tree-node:nth-child(${i}) .object-label`,
@@ -1540,7 +1552,7 @@ function getCoordsFromPosition(cm, { line, ch }) {
   return cm.charCoords({ line: ~~line, ch: ~~ch });
 }
 
-async function getTokenFromPosition(dbg, {line, ch}) {
+async function getTokenFromPosition(dbg, { line, ch }) {
   info(`Get token at ${line}, ${ch}`);
   const cm = getCM(dbg);
   cm.scrollIntoView({ line: line - 1, ch }, 0);
@@ -1558,10 +1570,7 @@ async function getTokenFromPosition(dbg, {line, ch}) {
   // https://github.com/firefox-devtools/debugger/pull/7934
   const lineHeightOffset = 3;
 
-  return dbg.win.document.elementFromPoint(
-    left,
-    top + lineHeightOffset
-  );
+  return dbg.win.document.elementFromPoint(left, top + lineHeightOffset);
 }
 
 async function waitForScrolling(codeMirror) {
@@ -1609,18 +1618,31 @@ async function clickAtPos(dbg, pos) {
   }
 
   const { top, left } = tokenEl.getBoundingClientRect();
-  info(`Clicking on token ${tokenEl.innerText} in line ${tokenEl.parentNode.innerText}`);
+  info(
+    `Clicking on token ${tokenEl.innerText} in line ${
+      tokenEl.parentNode.innerText
+    }`
+  );
   tokenEl.dispatchEvent(
     new MouseEvent("click", {
       bubbles: true,
       cancelable: true,
       view: dbg.win,
       clientX: left,
-      clientY: top
+      clientY: top,
     })
   );
 }
 
+async function rightClickAtPos(dbg, pos) {
+  const el = await getTokenFromPosition(dbg, pos);
+  if (!el) {
+    return false;
+  }
+
+  EventUtils.synthesizeMouseAtCenter(el, { type: "contextmenu" }, dbg.win);
+}
+
 async function hoverAtPos(dbg, pos) {
   const tokenEl = await getTokenFromPosition(dbg, pos);