Bug 1552290 - Preview fails after selecting another location. r=davidwalsh

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

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

devtools/client/debugger/src/actions/ast.js

@@ -1,47 +0,0 @@
-/* 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
-
-import { getSourceWithContent, getSelectedLocation } from "../selectors";
-
-import { setInScopeLines } from "./ast/setInScopeLines";
-
-import type { Context } from "../types";
-import type { ThunkArgs, Action } from "./types";
-
-export function setOutOfScopeLocations(cx: Context) {
-  return async ({ dispatch, getState, parser }: ThunkArgs) => {
-    const location = getSelectedLocation(getState());
-    if (!location) {
-      return;
-    }
-
-    const { source, content } = getSourceWithContent(
-      getState(),
-      location.sourceId
-    );
-
-    if (!content) {
-      return;
-    }
-
-    let locations = null;
-    if (location.line && source && !source.isWasm) {
-      locations = await parser.findOutOfScopeLocations(
-        source.id,
-        ((location: any): parser.AstPosition)
-      );
-    }
-
-    dispatch(
-      ({
-        type: "OUT_OF_SCOPE_LOCATIONS",
-        cx,
-        locations,
-      }: Action)
-    );
-    dispatch(setInScopeLines(cx));
-  };
-}

devtools/client/debugger/src/actions/ast/index.js

@@ -0,0 +1,7 @@
+/* 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 { setInScopeLines } from "./setInScopeLines";

devtools/client/debugger/src/actions/ast/moz.build

@@ -8,5 +8,6 @@ DIRS += [
 ]
 
 CompiledModules(
+    'index.js',
     'setInScopeLines.js',
 )

devtools/client/debugger/src/actions/ast/setInScopeLines.js

@@ -5,9 +5,11 @@
 // @flow
 
 import {
-  getOutOfScopeLocations,
-  getSelectedSourceWithContent,
+  hasInScopeLines,
+  getSourceWithContent,
+  getVisibleSelectedFrame,
 } from "../../selectors";
+
 import { getSourceLineCount } from "../../utils/source";
 
 import { range, flatMap, uniq, without } from "lodash";
@@ -29,31 +31,52 @@ function getOutOfScopeLines(outOfScopeLocations: ?(AstLocation[])) {
   );
 }
 
-export function setInScopeLines(cx: Context) {
-  return ({ dispatch, getState }: ThunkArgs) => {
-    const sourceWithContent = getSelectedSourceWithContent(getState());
-    const outOfScopeLocations = getOutOfScopeLocations(getState());
+async function getInScopeLines(cx, location, { dispatch, getState, parser }) {
+  const { source, content } = getSourceWithContent(
+    getState(),
+    location.sourceId
+  );
 
-    if (!sourceWithContent || !sourceWithContent.content) {
+  let locations = null;
+  if (location.line && source && !source.isWasm) {
+    locations = await parser.findOutOfScopeLocations(
+      source.id,
+      ((location: any): parser.AstPosition)
+    );
+  }
+
+  const linesOutOfScope = getOutOfScopeLines(locations);
+  const sourceNumLines =
+    !content || !isFulfilled(content) ? 0 : getSourceLineCount(content.value);
+
+  const sourceLines = range(1, sourceNumLines + 1);
+
+  return !linesOutOfScope
+    ? sourceLines
+    : without(sourceLines, ...linesOutOfScope);
+}
+
+export function setInScopeLines(cx: Context) {
+  return async (thunkArgs: ThunkArgs) => {
+    const { getState, dispatch } = thunkArgs;
+    const visibleFrame = getVisibleSelectedFrame(getState());
+
+    if (!visibleFrame) {
       return;
     }
-    const content = sourceWithContent.content;
 
-    const linesOutOfScope = getOutOfScopeLines(outOfScopeLocations);
+    const { location } = visibleFrame;
+    if (hasInScopeLines(getState(), location)) {
+      return;
+    }
 
-    const sourceNumLines = isFulfilled(content)
-      ? getSourceLineCount(content.value)
-      : 0;
-    const sourceLines = range(1, sourceNumLines + 1);
-
-    const inScopeLines = !linesOutOfScope
-      ? sourceLines
-      : without(sourceLines, ...linesOutOfScope);
+    const lines = await getInScopeLines(cx, location, thunkArgs);
 
     dispatch({
       type: "IN_SCOPE_LINES",
       cx,
-      lines: inScopeLines,
+      location,
+      lines,
     });
   };
 }

devtools/client/debugger/src/actions/ast/tests/__snapshots__/setInScopeLines.spec.js.snap

@@ -0,0 +1,16 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`getInScopeLine with selected line 1`] = `
+Array [
+  1,
+  2,
+  3,
+  4,
+  5,
+  6,
+  7,
+  10,
+  11,
+  12,
+]
+`;

devtools/client/debugger/src/actions/ast/tests/setInScopeLines.spec.js

@@ -0,0 +1,67 @@
+/* eslint max-nested-callbacks: ["error", 6] */
+/* 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
+import readFixture from "../../tests/helpers/readFixture";
+
+import { makeMockFrame, makeMockSource } from "../../../utils/test-mockup";
+import {
+  createStore,
+  selectors,
+  actions,
+  makeSource,
+  waitForState,
+} from "../../../utils/test-head";
+
+const { getInScopeLines } = selectors;
+
+const sourceTexts = {
+  "scopes.js": readFixture("scopes.js"),
+};
+
+const threadClient = {
+  sourceContents: async ({ source }) => ({
+    source: sourceTexts[source],
+    contentType: "text/javascript",
+  }),
+  evaluateExpressions: async () => {},
+  getFrameScopes: async () => {},
+  getBreakpointPositions: async () => ({}),
+  getBreakableLines: async () => [],
+};
+
+describe("getInScopeLine", () => {
+  it("with selected line", async () => {
+    const store = createStore(threadClient);
+    const { dispatch, getState } = store;
+    const source = makeMockSource("scopes.js", "scopes.js");
+
+    await dispatch(actions.newGeneratedSource(makeSource("scopes.js")));
+
+    await dispatch(
+      actions.selectLocation(selectors.getContext(getState()), {
+        sourceId: "scopes.js",
+        line: 5,
+      })
+    );
+
+    const frame = makeMockFrame("scopes-4", source);
+    await dispatch(
+      actions.paused({
+        thread: "FakeThread",
+        why: { type: "debuggerStatement" },
+        frame,
+        frames: [frame],
+      })
+    );
+    await dispatch(actions.setInScopeLines(selectors.getContext(getState())));
+
+    await waitForState(store, state => getInScopeLines(state, frame.location));
+
+    const lines = getInScopeLines(getState(), frame.location);
+
+    expect(lines).toMatchSnapshot();
+  });
+});

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

@@ -4,6 +4,7 @@
 
 // @flow
 
+import * as ast from "./ast";
 import * as breakpoints from "./breakpoints";
 import * as expressions from "./expressions";
 import * as eventListeners from "./event-listeners";
@@ -11,7 +12,6 @@ import * as pause from "./pause";
 import * as navigation from "./navigation";
 import * as ui from "./ui";
 import * as fileSearch from "./file-search";
-import * as ast from "./ast";
 import * as projectTextSearch from "./project-text-search";
 import * as quickOpen from "./quick-open";
 import * as sourceTree from "./source-tree";
@@ -23,6 +23,7 @@ import * as toolbox from "./toolbox";
 import * as preview from "./preview";
 
 export default {
+  ...ast,
   ...navigation,
   ...breakpoints,
   ...expressions,
@@ -33,7 +34,6 @@ export default {
   ...pause,
   ...ui,
   ...fileSearch,
-  ...ast,
   ...projectTextSearch,
   ...quickOpen,
   ...sourceTree,

devtools/client/debugger/src/actions/moz.build

@@ -12,7 +12,6 @@ DIRS += [
 ]
 
 CompiledModules(
-    'ast.js',
     'debuggee.js',
     'event-listeners.js',
     'expressions.js',

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

@@ -28,7 +28,6 @@ export function selectFrame(cx: ThreadContext, frame: Frame) {
     });
 
     dispatch(selectLocation(cx, frame.location));
-
     dispatch(evaluateExpressions(cx));
     dispatch(fetchScopes(cx));
   };

devtools/client/debugger/src/actions/pause/tests/pause.spec.js

@@ -85,6 +85,7 @@ function createPauseInfo(
       { id: mockFrameId, sourceId: frameLocation.sourceId },
       {
         location: frameLocation,
+        generatedLocation: frameLocation,
         ...frameOpts,
       }
     ),

devtools/client/debugger/src/actions/sources/select.js

@@ -13,8 +13,8 @@ import { isOriginalId } from "devtools-source-map";
 
 import { getSourceFromId, getSourceWithContent } from "../../reducers/sources";
 import { getSourcesForTabs } from "../../reducers/tabs";
-import { setOutOfScopeLocations } from "../ast";
 import { setSymbols } from "./symbols";
+import { setInScopeLines } from "../ast";
 import { closeActiveSearch, updateActiveFileSearch } from "../ui";
 import { isFulfilled } from "../../utils/async-value";
 import { togglePrettyPrint } from "./prettyPrint";
@@ -189,7 +189,7 @@ export function selectLocation(
     }
 
     dispatch(setSymbols({ cx, source: loadedSource }));
-    dispatch(setOutOfScopeLocations(cx));
+    dispatch(setInScopeLines(cx));
 
     // If a new source is selected update the file search results
     const newSource = getSelectedSource(getState());

devtools/client/debugger/src/actions/sources/tests/select.spec.js

@@ -20,7 +20,6 @@ const {
   getSourceCount,
   getSelectedSource,
   getSourceTabs,
-  getOutOfScopeLocations,
   getSelectedLocation,
 } = selectors;
 
@@ -67,10 +66,6 @@ describe("sources", () => {
       throw new Error("bad source");
     }
     expect(source.id).toEqual("foo1");
-
-    await waitForState(store, state => getOutOfScopeLocations(state));
-    const locations = getOutOfScopeLocations(getState());
-    expect(locations).toHaveLength(1);
   });
 
   it("should select next tab on tab closed if no previous tab", async () => {

devtools/client/debugger/src/actions/tests/__snapshots__/ast.spec.js.snap

@@ -1,45 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`ast getOutOfScopeLocations with selected line 1`] = `
-Array [
-  Object {
-    "end": Object {
-      "column": 16,
-      "line": 1,
-    },
-    "start": Object {
-      "column": 0,
-      "line": 1,
-    },
-  },
-  Object {
-    "end": Object {
-      "column": 3,
-      "line": 10,
-    },
-    "start": Object {
-      "column": 22,
-      "line": 8,
-    },
-  },
-]
-`;
-
-exports[`ast getOutOfScopeLocations with selected line 2`] = `
-Array [
-  1,
-  2,
-  3,
-  4,
-  5,
-  6,
-  7,
-  10,
-  11,
-  12,
-]
-`;
-
 exports[`ast setSymbols when the source is loaded should be able to set symbols 1`] = `
 Object {
   "callExpressions": Array [],

devtools/client/debugger/src/actions/tests/ast.spec.js

@@ -11,20 +11,11 @@ import {
   actions,
   makeSource,
   makeOriginalSource,
-  makeFrame,
   waitForState,
 } from "../../utils/test-head";
 
 import readFixture from "./helpers/readFixture";
-const {
-  getSymbols,
-  getOutOfScopeLocations,
-  getInScopeLines,
-  isSymbolsLoading,
-  getFramework,
-} = selectors;
-
-import { prefs } from "../../utils/prefs";
+const { getSymbols, isSymbolsLoading, getFramework } = selectors;
 
 const threadClient = {
   sourceContents: async ({ source }) => ({
@@ -52,7 +43,6 @@ const sourceMaps = {
 const sourceTexts = {
   "base.js": "function base(boo) {}",
   "foo.js": "function base(boo) { return this.bazz; } outOfScope",
-  "scopes.js": readFixture("scopes.js"),
   "reactComponent.js/originalSource": readFixture("reactComponent.js"),
   "reactFuncComponent.js/originalSource": readFixture("reactFuncComponent.js"),
 };
@@ -135,64 +125,4 @@ describe("ast", () => {
       });
     });
   });
-
-  describe("getOutOfScopeLocations", () => {
-    beforeEach(async () => {
-      prefs.autoPrettyPrint = false;
-    });
-
-    it("with selected line", async () => {
-      const store = createStore(threadClient);
-      const { dispatch, getState, cx } = store;
-      const source = await dispatch(
-        actions.newGeneratedSource(makeSource("scopes.js"))
-      );
-
-      await dispatch(
-        actions.selectLocation(cx, { sourceId: "scopes.js", line: 5 })
-      );
-
-      // Make sure the state has finished updating before pausing.
-      await waitForState(store, state => {
-        const symbols = getSymbols(state, source);
-        return symbols && !symbols.loading && getOutOfScopeLocations(state);
-      });
-
-      const frame = makeFrame({ id: "1", sourceId: "scopes.js" });
-      await dispatch(
-        actions.paused({
-          thread: "FakeThread",
-          why: { type: "debuggerStatement" },
-          frame,
-          frames: [frame],
-        })
-      );
-
-      const ncx = selectors.getThreadContext(getState());
-      await dispatch(actions.setOutOfScopeLocations(ncx));
-
-      await waitForState(store, state => getOutOfScopeLocations(state));
-
-      const locations = getOutOfScopeLocations(getState());
-      const lines = getInScopeLines(getState());
-
-      expect(locations).toMatchSnapshot();
-      expect(lines).toMatchSnapshot();
-    });
-
-    it("without a selected line", async () => {
-      const { dispatch, getState, cx } = createStore(threadClient);
-      await dispatch(actions.newGeneratedSource(makeSource("base.js")));
-      await dispatch(actions.selectSource(cx, "base.js"));
-
-      const locations = getOutOfScopeLocations(getState());
-      // const lines = getInScopeLines(getState());
-
-      expect(locations).toEqual(null);
-
-      // This check is disabled as locations that are in/out of scope may not
-      // have completed yet when the selectSource promise finishes.
-      // expect(lines).toEqual([1]);
-    });
-  });
 });

devtools/client/debugger/src/actions/types/ASTAction.js

@@ -4,9 +4,9 @@
 
 // @flow
 
-import type { SymbolDeclarations, AstLocation } from "../../workers/parser";
+import type { SymbolDeclarations } from "../../workers/parser";
 import type { PromiseAction } from "../utils/middleware/promise";
-import type { Context } from "../../types";
+import type { Context, SourceLocation } from "../../types";
 
 export type ASTAction =
   | PromiseAction<
@@ -17,13 +17,9 @@ export type ASTAction =
       |},
       SymbolDeclarations
     >
-  | {|
-      +type: "OUT_OF_SCOPE_LOCATIONS",
-      +cx: Context,
-      +locations: ?(AstLocation[]),
-    |}
   | {|
       +type: "IN_SCOPE_LINES",
       +cx: Context,
-      +lines: number[],
+      location: SourceLocation,
+      lines: Array<number>,
     |};

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

@@ -9,9 +9,11 @@
  * @module reducers/ast
  */
 
-import type { AstLocation, SymbolDeclarations } from "../workers/parser";
+import { makeBreakpointId } from "../utils/breakpoint";
 
-import type { Source } from "../types";
+import type { SymbolDeclarations } from "../workers/parser";
+
+import type { Source, SourceLocation } from "../types";
 import type { Action, DonePromiseAction } from "../actions/types";
 
 type EmptyLinesType = number[];
@@ -30,15 +32,13 @@ export type SourceMetaDataMap = { [k: string]: SourceMetaDataType };
 
 export type ASTState = {
   +symbols: SymbolsMap,
-  +outOfScopeLocations: ?Array<AstLocation>,
-  +inScopeLines: ?Array<number>,
+  +inScopeLines: { [string]: Array<number> },
 };
 
 export function initialASTState(): ASTState {
   return {
     symbols: {},
-    outOfScopeLocations: null,
-    inScopeLines: null,
+    inScopeLines: {},
   };
 }
 
@@ -60,16 +60,18 @@ function update(state: ASTState = initialASTState(), action: Action): ASTState {
       };
     }
 
-    case "OUT_OF_SCOPE_LOCATIONS": {
-      return { ...state, outOfScopeLocations: action.locations };
-    }
-
     case "IN_SCOPE_LINES": {
-      return { ...state, inScopeLines: action.lines };
+      return {
+        ...state,
+        inScopeLines: {
+          ...state.inScopeLines,
+          [makeBreakpointId(action.location)]: action.lines,
+        },
+      };
     }
 
     case "RESUME": {
-      return { ...state, outOfScopeLocations: null };
+      return { ...state, inScopeLines: {} };
     }
 
     case "NAVIGATE": {
@@ -120,17 +122,13 @@ export function isSymbolsLoading(state: OuterState, source: ?Source): boolean {
   return symbols.loading;
 }
 
-export function getOutOfScopeLocations(state: OuterState) {
-  return state.ast.outOfScopeLocations;
+export function getInScopeLines(state: OuterState, location: SourceLocation) {
+  const inScopeLines = state.ast.inScopeLines;
+  return inScopeLines[makeBreakpointId(location)];
 }
 
-export function getInScopeLines(state: OuterState) {
-  return state.ast.inScopeLines;
-}
-
-export function isLineInScope(state: OuterState, line: number) {
-  const linesInScope = state.ast.inScopeLines;
-  return linesInScope && linesInScope.includes(line);
+export function hasInScopeLines(state: OuterState, location: SourceLocation) {
+  return !!getInScopeLines(state, location);
 }
 
 export default update;

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

@@ -44,6 +44,7 @@ export { inComponent } from "./inComponent";
 export { isSelectedFrameVisible } from "./isSelectedFrameVisible";
 export { getCallStackFrames } from "./getCallStackFrames";
 export { getBreakpointSources } from "./breakpointSources";
+export { isLineInScope } from "./isLineInScope";
 export { getXHRBreakpoints, shouldPauseOnAnyXHR } from "./breakpoints";
 export * from "./visibleColumnBreakpoints";
 export {

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

@@ -0,0 +1,26 @@
+/* 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
+
+import { getInScopeLines } from "../reducers/ast";
+import { getVisibleSelectedFrame } from "./pause";
+
+import type { State } from "../reducers/types";
+
+// Checks if a line is considered in scope
+// We consider all lines in scope, if we do not have lines in scope.
+export function isLineInScope(state: State, line: number) {
+  const frame = getVisibleSelectedFrame(state);
+  if (!frame) {
+    return false;
+  }
+
+  const lines = getInScopeLines(state, frame.location);
+  if (!lines) {
+    return true;
+  }
+
+  return lines.includes(line);
+}

devtools/client/debugger/src/selectors/moz.build

@@ -14,6 +14,7 @@ CompiledModules(
     'getCallStackFrames.js',
     'inComponent.js',
     'index.js',
+    'isLineInScope.js',
     'isSelectedFrameVisible.js',
     'pause.js',
     'visibleBreakpoints.js',

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

@@ -48,10 +48,11 @@ export const getVisibleSelectedFrame: Selector<?{
       return null;
     }
 
-    const { id } = selectedFrame;
+    const { id, displayName } = selectedFrame;
 
     return {
       id,
+      displayName,
       location: _getSelectedLocation(selectedFrame, selectedLocation),
     };
   }

devtools/client/debugger/test/mochitest/.eslintrc

@@ -71,6 +71,7 @@
     "assertNotPaused": false,
     "assertSourceCount": false,
     "assertPausedAtSourceAndLine": false,
+    "assertPreviews": false,
     "assertPreviewTextValue": false,
     "assertEditorBreakpoint": false,
     "assertBreakpointSnippet": false,

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

@@ -743,6 +743,8 @@ skip-if = (verify && debug && (os == 'mac')) || (os == 'linux' && debug && bits
 [browser_dbg-pretty-print-paused.js]
 [browser_dbg-preview.js]
 skip-if = os == "win"
+[browser_dbg-preview-frame.js]
+skip-if = os == "win"
 [browser_dbg-preview-module.js]
 skip-if = os == "win"
 [browser_dbg-preview-source-maps.js]

devtools/client/debugger/test/mochitest/browser_dbg-preview-frame.js

@@ -0,0 +1,48 @@
+/* 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 waitForSelectedFrame(dbg, displayName) {
+  const { getInScopeLines, getVisibleSelectedFrame } = dbg.selectors;
+  return waitForState(dbg, state => {
+    const frame = getVisibleSelectedFrame();
+
+    return (
+      frame &&
+      frame.displayName == displayName &&
+      getInScopeLines(frame.location)
+    );
+  });
+}
+
+async function assertFunctionPreview(dbg, line, column, displayName) {
+  const previewEl = await tryHovering(dbg, line, column, "tooltip");
+  is(previewEl.innerText, displayName);
+}
+
+// Test hovering in a selected frame
+add_task(async function() {
+  const dbg = await initDebugger("doc-script-switching.html");
+
+  const found = findElement(dbg, "callStackBody");
+  is(found, null, "Call stack is hidden");
+
+  invokeInTab("firstCall");
+  await waitForPaused(dbg);
+
+  info("Preview a variable in the second frame");
+  clickElement(dbg, "frame", 2);
+  await waitForSelectedFrame(dbg, "firstCall");
+  await assertFunctionPreview(dbg, 8, 4, "secondCall()");
+
+  info("Preview should still work after selecting different locations");
+  const frame = dbg.selectors.getVisibleSelectedFrame();
+  const inScopeLines = dbg.selectors.getInScopeLines(frame.location);
+  await selectSource(dbg, "switching-01");
+  await assertFunctionPreview(dbg, 8, 4, "secondCall()");
+  is(
+    dbg.selectors.getInScopeLines(frame.location),
+    inScopeLines,
+    "In scope lines"
+  );
+});

devtools/client/debugger/test/mochitest/browser_dbg-preview-source-maps.js

@@ -41,10 +41,6 @@ add_task(async function() {
     "times2.js",
     "opts.js"
   );
-  const {
-    selectors: { getSelectedSource },
-    getState
-  } = dbg;
 
   await selectSource(dbg, "times2");
   await addBreakpoint(dbg, "times2", 2);
@@ -55,14 +51,14 @@ add_task(async function() {
 
   info("Test previewing in the original location");
   await assertPreviews(dbg, [
-    { line: 2, column: 10, result: 4, expression: "x" }
+    { line: 2, column: 10, result: 4, expression: "x" },
   ]);
 
   info("Test previewing in the generated location");
   await dbg.actions.jumpToMappedSelectedLocation(getContext(dbg));
   await waitForSelectedSource(dbg, "bundle.js");
   await assertPreviews(dbg, [
-    { line: 70, column: 11, result: 4, expression: "x" }
+    { line: 70, column: 11, result: 4, expression: "x" },
   ]);
 
   info("Test that you can not preview in another original file");