Use original source location to automatically stepOver locations with useless mappings. (#5485)

Author: loganfsmyth

.prettierignore

@@ -1,5 +1,6 @@
 src/workers/parser/tests/fixtures/functionNames.js
 src/workers/parser/tests/fixtures/scopes/*.js
+src/workers/parser/tests/fixtures/pause/*.js
 src/test/mochitest/examples/babel/polyfill-bundle.js
 src/test/mochitest/examples/babel/fixtures/*/input.js
 src/test/mochitest/examples/babel/fixtures/*/output.js

src/actions/pause/mapFrames.js

@@ -5,7 +5,7 @@ import { getFrames } from "../../selectors";
 import type { Frame } from "../../types";
 import type { ThunkArgs } from "../types";
 
-function updateFrameLocation(frame: Frame, sourceMaps: any) {
+export function updateFrameLocation(frame: Frame, sourceMaps: any) {
   return sourceMaps.getOriginalLocation(frame.location).then(loc => ({
     ...frame,
     location: loc,

src/actions/pause/paused.js

@@ -10,19 +10,28 @@ import {
   getHiddenBreakpointLocation,
   isEvaluatingExpression,
   getSelectedFrame,
-  getVisibleSelectedFrame
+  getVisibleSelectedFrame,
+  getSources
 } from "../../selectors";
+
 import { mapFrames } from ".";
 import { removeBreakpoint } from "../breakpoints";
 import { evaluateExpressions } from "../expressions";
-import { selectLocation } from "../sources";
+import { selectLocation, loadSourceText } from "../sources";
 import { togglePaneCollapse } from "../ui";
+import { command } from "./commands";
+import { shouldStep } from "../../utils/pause";
+
+import { updateFrameLocation } from "./mapFrames";
 
 import { fetchScopes } from "./fetchScopes";
 
-import type { Pause } from "../../types";
+import type { Pause, Frame } from "../../types";
 import type { ThunkArgs } from "../types";
 
+async function getOriginalSourceForFrame(state, frame: Frame) {
+  return getSources(state).get(frame.location.sourceId);
+}
 /**
  * Debugger has just paused
  *
@@ -33,12 +42,26 @@ import type { ThunkArgs } from "../types";
 export function paused(pauseInfo: Pause) {
   return async function({ dispatch, getState, client, sourceMaps }: ThunkArgs) {
     const { frames, why, loadedObjects } = pauseInfo;
+    const rootFrame = frames.length > 0 ? frames[0] : null;
+
+    if (rootFrame) {
+      const mappedFrame = await updateFrameLocation(rootFrame, sourceMaps);
+      const source = await getOriginalSourceForFrame(getState(), mappedFrame);
+
+      // Ensure that the original file has loaded if there is one.
+      await dispatch(loadSourceText(source));
+
+      if (await shouldStep(mappedFrame, getState(), sourceMaps)) {
+        dispatch(command("stepOver"));
+        return;
+      }
+    }
 
     dispatch({
       type: "PAUSED",
       why,
       frames,
-      selectedFrameId: frames[0] ? frames[0].id : undefined,
+      selectedFrameId: rootFrame ? rootFrame.id : undefined,
       loadedObjects: loadedObjects || []
     });
 

src/reducers/pause.js

@@ -17,7 +17,7 @@ import { getSelectedSource } from "./sources";
 
 import type { OriginalScope } from "../actions/pause/mapScopes";
 import type { Action } from "../actions/types";
-import type { Why, Scope, SourceId, FrameId } from "../types";
+import type { Why, Scope, SourceId, FrameId, Location } from "../types";
 
 export type PauseState = {
   why: ?Why,
@@ -43,7 +43,11 @@ export type PauseState = {
   shouldIgnoreCaughtExceptions: boolean,
   canRewind: boolean,
   debuggeeUrl: string,
-  command: string
+  command: string,
+  previousLocation: ?{
+    location: Location,
+    generatedLocation: Location
+  }
 };
 
 export const createPauseState = (): PauseState => ({
@@ -60,7 +64,8 @@ export const createPauseState = (): PauseState => ({
   shouldIgnoreCaughtExceptions: prefs.ignoreCaughtExceptions,
   canRewind: false,
   debuggeeUrl: "",
-  command: ""
+  command: "",
+  previousLocation: null
 });
 
 const emptyPauseState = {
@@ -71,7 +76,8 @@ const emptyPauseState = {
     original: {}
   },
   selectedFrameId: null,
-  loadedObjects: {}
+  loadedObjects: {},
+  previousLocation: null
 };
 
 function update(
@@ -190,10 +196,16 @@ function update(
         shouldIgnoreCaughtExceptions
       };
 
-    case "COMMAND":
+    case "COMMAND": {
       return action.status === "start"
-        ? { ...state, ...emptyPauseState, command: action.command }
+        ? {
+            ...state,
+            ...emptyPauseState,
+            command: action.command,
+            previousLocation: buildPreviousLocation(state, action)
+          }
         : { ...state, command: "" };
+    }
 
     case "RESUME":
       // We clear why on resume because we need it to decide if
@@ -213,6 +225,24 @@ function update(
   return state;
 }
 
+function buildPreviousLocation(state, action) {
+  const { frames, previousLocation } = state;
+
+  if (action.command !== "stepOver") {
+    return null;
+  }
+
+  const frame = frames && frames.length > 0 ? frames[0] : null;
+  if (!frame) {
+    return previousLocation;
+  }
+
+  return {
+    location: frame.location,
+    generatedLocation: frame.generatedLocation
+  };
+}
+
 // Selectors
 
 // Unfortunately, it's really hard to make these functions accept just
@@ -243,6 +273,10 @@ export function isPaused(state: OuterState) {
   return !!getFrames(state);
 }
 
+export function getPreviousPauseFrameLocation(state: OuterState) {
+  return state.pause.previousLocation;
+}
+
 export function isEvaluatingExpression(state: OuterState) {
   return state.pause.command === "expression";
 }

src/test/mochitest/browser.ini

@@ -117,8 +117,9 @@ support-files =
 
 [browser_dbg-asm.js]
 [browser_dbg-async-stepping.js]
-[browser_dbg-babel.js]
+[browser_dbg-babel-scopes.js]
 skip-if = (os == "win" && ccov) # bug 1438797
+[browser_dbg-babel-stepping.js]
 [browser_dbg-breaking.js]
 [browser_dbg-breaking-from-console.js]
 [browser_dbg-breakpoints.js]

src/test/mochitest/browser_dbg-babel.js …st/mochitest/browser_dbg-babel-scopes.jssrc/test/mochitest/browser_dbg-babel.js renamed to src/test/mochitest/browser_dbg-babel-scopes.js src/test/mochitest/browser_dbg-babel.js renamed to src/test/mochitest/browser_dbg-babel-scopes.js

@@ -0,0 +1,154 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests for stepping through Babel's compile output.
+
+async function breakpointSteps(dbg, fixture, { line, column }, steps) {
+  const { selectors: { getBreakpoint, getBreakpoints }, getState } = dbg;
+
+  const filename = `fixtures/${fixture}/input.js`;
+  await waitForSources(dbg, filename);
+
+  ok(true, "Original sources exist");
+  const source = findSource(dbg, filename);
+
+  await selectSource(dbg, source);
+
+  // Test that breakpoint is not off by a line.
+  await addBreakpoint(dbg, source, line);
+
+  is(getBreakpoints(getState()).size, 1, "One breakpoint exists");
+  ok(
+    getBreakpoint(getState(), { sourceId: source.id, line, column }),
+    "Breakpoint has correct line"
+  );
+
+  const fnName = fixture.replace(/-([a-z])/g, (s, c) => c.toUpperCase());
+
+  const invokeResult = invokeInTab(fnName);
+
+  let invokeFailed = await Promise.race([
+    waitForPaused(dbg),
+    invokeResult.then(() => new Promise(() => {}), () => true)
+  ]);
+
+  if (invokeFailed) {
+    return invokeResult;
+  }
+
+  assertPausedLocation(dbg);
+
+  await removeBreakpoint(dbg, source.id, line, column);
+
+  is(getBreakpoints(getState()).size, 0, "Breakpoint reverted");
+
+  await runSteps(dbg, source, steps);
+
+  await resume(dbg);
+
+  // If the invoke errored later somehow, capture here so the error is
+  // reported nicely.
+  await invokeResult;
+
+  ok(true, `Ran tests for ${fixture} at line ${line} column ${column}`);
+}
+
+async function runSteps(dbg, source, steps) {
+  const { selectors: { getVisibleSelectedFrame }, getState } = dbg;
+
+  for (const [i, [type, position]] of steps.entries()) {
+    switch (type) {
+      case "stepOver":
+        await stepOver(dbg);
+        break;
+      case "stepIn":
+        await stepIn(dbg);
+        break;
+      default:
+        throw new Error("Unknown stepping type");
+    }
+
+    const { location } = getVisibleSelectedFrame(getState());
+
+    is(location.sourceId, source.id, `Step ${i} has correct sourceId`);
+    is(location.line, position.line, `Step ${i} has correct line`);
+    is(location.column, position.column, `Step ${i} has correct column`);
+
+    assertPausedLocation(dbg);
+  }
+}
+
+add_task(async function() {
+  requestLongerTimeout(2);
+
+  const dbg = await initDebugger("doc-babel.html");
+
+  await breakpointSteps(dbg, "step-over-for-of", { line: 4, column: 2 }, [
+    ["stepOver", { line: 6, column: 2 }],
+    ["stepOver", { line: 7, column: 4 }],
+    ["stepOver", { line: 6, column: 2 }],
+    ["stepOver", { line: 7, column: 4 }],
+    ["stepOver", { line: 6, column: 2 }],
+    ["stepOver", { line: 10, column: 2 }]
+  ]);
+
+  // This codifies the current behavior, but stepping twice over the for
+  // header isn't ideal.
+  await breakpointSteps(dbg, "step-over-for-of-array", { line: 3, column: 2 }, [
+    ["stepOver", { line: 5, column: 2 }],
+    ["stepOver", { line: 5, column: 7 }],
+    ["stepOver", { line: 6, column: 4 }],
+    ["stepOver", { line: 5, column: 2 }],
+    ["stepOver", { line: 5, column: 7 }],
+    ["stepOver", { line: 6, column: 4 }],
+    ["stepOver", { line: 5, column: 2 }],
+    ["stepOver", { line: 9, column: 2 }]
+  ]);
+
+  // The closure means it isn't actually possible to step into the for body,
+  // and Babel doesn't map the _loop() call, so we step past it automatically.
+  await breakpointSteps(
+    dbg,
+    "step-over-for-of-closure",
+    { line: 6, column: 2 },
+    [
+      ["stepOver", { line: 8, column: 2 }],
+      ["stepOver", { line: 12, column: 2 }]
+    ]
+  );
+
+  // Same as the previous, not possible to step into the body. The less
+  // complicated array logic makes it possible to step into the header at least,
+  // but this does end up double-visiting the for head.
+  await breakpointSteps(
+    dbg,
+    "step-over-for-of-array-closure",
+    { line: 3, column: 2 },
+    [
+      ["stepOver", { line: 5, column: 2 }],
+      ["stepOver", { line: 5, column: 7 }],
+      ["stepOver", { line: 5, column: 2 }],
+      ["stepOver", { line: 5, column: 7 }],
+      ["stepOver", { line: 5, column: 2 }],
+      ["stepOver", { line: 9, column: 2 }]
+    ]
+  );
+
+  await breakpointSteps(
+    dbg,
+    "step-over-function-params",
+    { line: 6, column: 2 },
+    [["stepOver", { line: 7, column: 2 }], ["stepIn", { line: 2, column: 2 }]]
+  );
+
+  await breakpointSteps(
+    dbg,
+    "step-over-regenerator-await",
+    { line: 2, column: 2 },
+    [
+      // Won't work until a fix to regenerator lands and we rebuild.
+      // https://github.com/facebook/regenerator/issues/342
+      // ["stepOver", { line: 4, column: 2 }],
+    ]
+  );
+});

src/test/mochitest/browser_dbg-babel-stepping.js

@@ -26,7 +26,7 @@ add_task(async function() {
   const { selectors: { getSelectedSource }, getState } = dbg;
 
   // Make sure the thread is paused in the right source and location
-  await waitForDispatch(dbg, "LOAD_SOURCE_TEXT");
+  await waitForPaused(dbg);
   is(dbg.win.cm.getValue(), "debugger");
   const source = getSelectedSource(getState()).toJS();
   assertPausedLocation(dbg);

src/test/mochitest/browser_dbg-breaking-from-console.js

@@ -0,0 +1,8 @@
+/* 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/>. */
+
+// @flow
+
+export * from "./why";
+export * from "./stepping";