Bug 1818495 - [devtools] Use location source and sourceActor attribute instead of re-querying the selectors. r=bomsy,devtools-reviewers

This may keep running async code whose source have been removed,
but this help remove various selectors lookups.

If we expect to check in async code if the source is still registered,
we should probably have an explicit API for this.

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

devtools/client/debugger/dist/parser-worker.js

@@ -44246,20 +44246,20 @@ function sortByStart(a, b) {
  */
 
 
-function findOutOfScopeLocations(sourceId, position) {
+function findOutOfScopeLocations(location) {
   const {
     functions,
     comments
-  } = findSymbols(sourceId);
+  } = findSymbols(location.source.id);
   const commentLocations = comments.map(c => c.location);
   const locations = functions.map(getLocation).concat(commentLocations).sort(sortByStart);
-  const innerLocations = getInnerLocations(locations, position);
+  const innerLocations = getInnerLocations(locations, location);
   const outerLocations = locations.filter(loc => {
     if (innerLocations.includes(loc)) {
       return false;
     }
 
-    return !(0, _contains.containsPosition)(loc, position);
+    return !(0, _contains.containsPosition)(loc, location);
   });
   return removeOverlaps(outerLocations);
 }

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

@@ -4,7 +4,6 @@
 
 import {
   hasInScopeLines,
-  getLocationSource,
   getSourceTextContent,
   getVisibleSelectedFrame,
 } from "../../selectors";
@@ -33,12 +32,11 @@ async function getInScopeLines(
   location,
   { dispatch, getState, parserWorker }
 ) {
-  const source = getLocationSource(getState(), location);
   const sourceTextContent = getSourceTextContent(getState(), location);
 
   let locations = null;
-  if (location.line && source && !source.isWasm) {
-    locations = await parserWorker.findOutOfScopeLocations(source.id, location);
+  if (location.line && location.source && !location.source.isWasm) {
+    locations = await parserWorker.findOutOfScopeLocations(location);
   }
 
   const linesOutOfScope = getOutOfScopeLines(locations);

devtools/client/debugger/src/actions/breakpoints/modify.js

@@ -11,7 +11,6 @@ import {
   getBreakpoint,
   getBreakpointPositionsForLocation,
   getFirstBreakpointPosition,
-  getLocationSource,
   getSettledSourceTextContent,
   getBreakpointsList,
   getPendingBreakpointList,
@@ -64,7 +63,7 @@ async function clientSetBreakpoint(
   );
   const shouldMapBreakpointExpressions =
     isMapScopesEnabled(getState()) &&
-    getLocationSource(getState(), breakpoint.location).isOriginal &&
+    breakpoint.location.source.isOriginal &&
     (breakpoint.options.logValue || breakpoint.options.condition);
 
   if (shouldMapBreakpointExpressions) {
@@ -107,14 +106,15 @@ export function addBreakpoint(
     const { dispatch, getState, client } = thunkArgs;
     recordEvent("add_breakpoint");
 
-    const { column, line } = initialLocation;
-    const initialSource = getLocationSource(getState(), initialLocation);
-
     await dispatch(
-      setBreakpointPositions({ cx, sourceId: initialSource.id, line })
+      setBreakpointPositions({
+        cx,
+        sourceId: initialLocation.source.id,
+        line: initialLocation.line,
+      })
     );
 
-    const position = column
+    const position = initialLocation.column
       ? getBreakpointPositionsForLocation(getState(), initialLocation)
       : getFirstBreakpointPosition(getState(), initialLocation);
 
@@ -126,23 +126,20 @@ export function addBreakpoint(
 
     const { location, generatedLocation } = position;
 
-    const source = getLocationSource(getState(), location);
-    const generatedSource = getLocationSource(getState(), generatedLocation);
-
-    if (!source || !generatedSource) {
+    if (!location.source || !generatedLocation.source) {
       return null;
     }
 
     const originalContent = getSettledSourceTextContent(getState(), location);
     const originalText = getTextAtPosition(
-      source.id,
+      location.source.id,
       originalContent,
       location
     );
 
     const content = getSettledSourceTextContent(getState(), generatedLocation);
     const text = getTextAtPosition(
-      generatedSource.id,
+      generatedLocation.source.id,
       content,
       generatedLocation
     );
@@ -150,7 +147,7 @@ export function addBreakpoint(
     const id = makeBreakpointId(location);
     const breakpoint = createBreakpoint({
       id,
-      thread: generatedSource.thread,
+      thread: generatedLocation.source.thread,
       disabled,
       options,
       location,

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

@@ -7,7 +7,6 @@ import {
   getExpressions,
   getSelectedFrame,
   getSelectedFrameId,
-  getLocationSource,
   getSelectedSource,
   getSelectedScopeMappings,
   getSelectedFrameBindings,
@@ -135,11 +134,13 @@ function evaluateExpression(cx, expression) {
     const frame = getSelectedFrame(getState(), cx.thread);
 
     if (frame) {
-      const source = getLocationSource(getState(), frame.location);
-
       const selectedSource = getSelectedSource(getState());
 
-      if (selectedSource && source.isOriginal && selectedSource.isOriginal) {
+      if (
+        selectedSource &&
+        frame.location.source.isOriginal &&
+        selectedSource.isOriginal
+      ) {
         const mapResult = await dispatch(getMappedExpression(input));
         if (mapResult) {
           input = mapResult.expression;

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

@@ -4,7 +4,6 @@
 
 import {
   getSymbols,
-  getLocationSource,
   getSelectedFrame,
   getCurrentThread,
 } from "../../selectors";
@@ -46,12 +45,11 @@ export function highlightCalls(cx) {
       return null;
     }
 
-    const source = getLocationSource(getState(), frame.location);
-    if (!source) {
+    if (!frame.location.source) {
       return null;
     }
 
-    const symbols = getSymbols(getState(), source);
+    const symbols = getSymbols(getState(), frame.location.source);
 
     if (!symbols) {
       return null;

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

@@ -2,7 +2,7 @@
  * 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/>. */
 
-import { getFrames, getSymbols, getLocationSource } from "../../selectors";
+import { getFrames, getSymbols } from "../../selectors";
 
 import { findClosestFunction } from "../../utils/ast";
 
@@ -11,13 +11,11 @@ function mapDisplayName(frame, { getState }) {
     return frame;
   }
 
-  const source = getLocationSource(getState(), frame.location);
-
-  if (!source) {
+  if (!frame.location.source) {
     return frame;
   }
 
-  const symbols = getSymbols(getState(), source);
+  const symbols = getSymbols(getState(), frame.location.source);
 
   if (!symbols || !symbols.functions) {
     return frame;

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

@@ -6,7 +6,6 @@ import {
   getSource,
   getFrames,
   getBlackBoxRanges,
-  getLocationSource,
   getSelectedFrame,
 } from "../../selectors";
 
@@ -22,20 +21,12 @@ function getSelectedFrameId(state, thread, frames) {
   let selectedFrame = getSelectedFrame(state, thread);
   const blackboxedRanges = getBlackBoxRanges(state);
 
-  if (
-    selectedFrame &&
-    !isFrameBlackBoxed(
-      selectedFrame,
-      getLocationSource(state, selectedFrame.location),
-      blackboxedRanges
-    )
-  ) {
+  if (selectedFrame && !isFrameBlackBoxed(selectedFrame, blackboxedRanges)) {
     return selectedFrame.id;
   }
 
   selectedFrame = frames.find(frame => {
-    const frameSource = getLocationSource(state, frame.location);
-    return !isFrameBlackBoxed(frame, frameSource, blackboxedRanges);
+    return !isFrameBlackBoxed(frame, blackboxedRanges);
   });
   return selectedFrame?.id;
 }
@@ -66,12 +57,8 @@ function isWasmOriginalSourceFrame(frame, getState) {
   if (isGeneratedId(frame.location.sourceId)) {
     return false;
   }
-  const generatedSource = getLocationSource(
-    getState(),
-    frame.generatedLocation
-  );
 
-  return Boolean(generatedSource?.isWasm);
+  return Boolean(frame.generatedLocation?.source.isWasm);
 }
 
 async function expandFrames(frames, { getState, sourceMapLoader }) {

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

@@ -5,7 +5,6 @@
 import {
   getSelectedFrameId,
   getSource,
-  getLocationSource,
   getSettledSourceTextContent,
   isMapScopesEnabled,
   getSelectedFrame,
@@ -135,12 +134,9 @@ export function mapScopes(cx, scopes, frame) {
 export function getMappedScopes(cx, scopes, frame) {
   return async function(thunkArgs) {
     const { getState, dispatch } = thunkArgs;
-    const generatedSource = getLocationSource(
-      getState(),
-      frame.generatedLocation
-    );
+    const generatedSource = frame.generatedLocation.source;
 
-    const source = getLocationSource(getState(), frame.location);
+    const source = frame.location.source;
 
     if (
       !isMapScopesEnabled(getState()) ||

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

@@ -33,7 +33,6 @@ import {
   getSelectedSource,
   canPrettyPrintSource,
   getIsCurrentThreadPaused,
-  getLocationSource,
   getSourceTextContent,
   tabExists,
 } from "../../selectors";
@@ -131,7 +130,7 @@ export function selectLocation(cx, location, { keepContext = true } = {}) {
       return;
     }
 
-    let source = getLocationSource(getState(), location);
+    let source = location.source;
 
     if (!source) {
       // If there is no source we deselect the current selected source
@@ -161,7 +160,7 @@ export function selectLocation(cx, location, { keepContext = true } = {}) {
       // getRelatedMapLocation will just convert to the related generated/original location.
       // i.e if the original location is passed, the related generated location will be returned and vice versa.
       location = await getRelatedMapLocation(location, thunkArgs);
-      source = getLocationSource(getState(), location);
+      source = location.source;
     }
 
     let sourceActor = location.sourceActor;

devtools/client/debugger/src/components/PrimaryPanes/SourcesTree.js

@@ -19,8 +19,6 @@ import {
   getContext,
   getGeneratedSourceByURL,
   getBlackBoxRanges,
-  getSourceActor,
-  getSource,
 } from "../../selectors";
 
 // Actions
@@ -32,6 +30,7 @@ import ManagedTree from "../shared/ManagedTree";
 
 // Utils
 import { getRawSourceURL } from "../../utils/source";
+import { createLocation } from "../../utils/location";
 
 function shouldAutoExpand(item, mainThreadHost) {
   // There is only one case where we want to force auto expand,
@@ -126,12 +125,12 @@ class SourcesTree extends Component {
     // We might fail to find the source if its thread is registered late,
     // so that we should re-search the selected source if highlightItems is empty.
     if (
-      nextProps.selectedTreeLocation.source &&
-      (nextProps.selectedTreeLocation.source != selectedTreeLocation.source ||
+      nextProps.selectedTreeLocation?.source &&
+      (nextProps.selectedTreeLocation.source != selectedTreeLocation?.source ||
         (nextProps.selectedTreeLocation.source ===
-          selectedTreeLocation.source &&
+          selectedTreeLocation?.source &&
           nextProps.selectedTreeLocation.sourceActor !=
-            selectedTreeLocation.sourceActor) ||
+            selectedTreeLocation?.sourceActor) ||
         !this.state.highlightItems?.length)
     ) {
       let parentDirectory = getDirectoryForSource(
@@ -375,17 +374,24 @@ class SourcesTree extends Component {
 }
 
 function getTreeLocation(state, location) {
-  let source = location ? getSource(state, location.sourceId) : null;
-  const sourceActor = location
-    ? getSourceActor(state, location.sourceActorId)
-    : null;
-
-  if (source && source.isPrettyPrinted) {
-    source =
-      getGeneratedSourceByURL(state, getRawSourceURL(source.url)) || null;
+  // In the SourceTree, we never show the pretty printed sources and only
+  // the minified version, so if we are selecting a pretty file, fake selecting
+  // the minified version.
+  if (location?.source.isPrettyPrinted) {
+    const source = getGeneratedSourceByURL(
+      state,
+      getRawSourceURL(location.source.url)
+    );
+    if (source) {
+      return createLocation({
+        source,
+        // A source actor is required by getDirectoryForSource
+        // in order to know in which thread this source relates to.
+        sourceActor: location.sourceActor,
+      });
+    }
   }
-
-  return { source, sourceActor };
+  return location;
 }
 
 const mapStateToProps = state => {

devtools/client/debugger/src/components/SecondaryPanes/index.js

@@ -24,7 +24,6 @@ import {
   getThreadContext,
   getPauseReason,
   getShouldBreakpointsPaneOpenOnPause,
-  getLocationSource,
   getSkipPausing,
   shouldLogEventBreakpoints,
 } from "../../selectors";
@@ -524,7 +523,7 @@ const mapStateToProps = state => {
     workers: getThreads(state),
     skipPausing: getSkipPausing(state),
     logEventBreakpoints: shouldLogEventBreakpoints(state),
-    source: selectedFrame && getLocationSource(state, selectedFrame.location),
+    source: selectedFrame && selectedFrame.location.source,
     pauseReason: pauseReason?.type ?? "",
     shouldBreakpointsPaneOpenOnPause,
     thread,

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

@@ -42,7 +42,7 @@ export function formatCallStackFrames(
   const formattedFrames = frames
     .filter(frame => getSourceForFrame(sourcesMap, frame))
     .map(frame => appendSource(sourcesMap, frame, selectedSource))
-    .filter(frame => !isFrameBlackBoxed(frame, frame.source, blackboxedRanges));
+    .filter(frame => !isFrameBlackBoxed(frame, blackboxedRanges));
 
   return annotateFrames(formattedFrames);
 }

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

@@ -42,10 +42,6 @@ export function getSourceFromId(state, id) {
   return source;
 }
 
-export function getLocationSource(state, location) {
-  return getSource(state, location.sourceId);
-}
-
 export function getSourceByActorId(state, actorId) {
   if (!hasSourceActor(state, actorId)) {
     return null;
@@ -242,7 +238,7 @@ export function canPrettyPrintSource(state, location) {
 }
 
 export function getPrettyPrintMessage(state, location) {
-  const source = getSource(state, location.sourceId);
+  const source = location.source;
   if (!source) {
     return L10N.getStr("sourceTabs.prettyPrint");
   }

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

@@ -6,11 +6,7 @@ import { createSelector } from "reselect";
 import { shallowEqual } from "../utils/shallow-equal";
 import { getPrettySourceURL } from "../utils/source";
 
-import {
-  getLocationSource,
-  getSpecificSourceByURL,
-  getSourcesMap,
-} from "./sources";
+import { getSpecificSourceByURL, getSourcesMap } from "./sources";
 import { isOriginalId } from "devtools/client/shared/source-map-loader/index";
 import { isSimilarTab } from "../utils/tabs";
 
@@ -54,13 +50,13 @@ export function getNewSelectedSource(state, tabList) {
     return null;
   }
 
-  const selectedTab = getLocationSource(state, selectedLocation);
-  if (!selectedTab) {
+  const selectedSource = selectedLocation.source;
+  if (!selectedSource) {
     return null;
   }
 
   const matchingTab = availableTabs.find(tab =>
-    isSimilarTab(tab, selectedTab.url, isOriginalId(selectedLocation.sourceId))
+    isSimilarTab(tab, selectedSource.url, isOriginalId(selectedSource.id))
   );
 
   if (matchingTab) {
@@ -69,21 +65,24 @@ export function getNewSelectedSource(state, tabList) {
       return null;
     }
 
-    const selectedSource = getSpecificSourceByURL(
+    const specificSelectedSource = getSpecificSourceByURL(
       state,
-      selectedTab.url,
-      selectedTab.isOriginal
+      selectedSource.url,
+      selectedSource.isOriginal
     );
 
-    if (selectedSource) {
-      return selectedSource;
+    if (specificSelectedSource) {
+      return specificSelectedSource;
     }
 
     return null;
   }
 
   const tabUrls = tabList.map(tab => tab.url);
-  const leftNeighborIndex = Math.max(tabUrls.indexOf(selectedTab.url) - 1, 0);
+  const leftNeighborIndex = Math.max(
+    tabUrls.indexOf(selectedSource.url) - 1,
+    0
+  );
   const lastAvailbleTabIndex = availableTabs.length - 1;
   const newSelectedTabIndex = Math.min(leftNeighborIndex, lastAvailbleTabIndex);
   const availableTab = availableTabs[newSelectedTabIndex];

devtools/client/debugger/src/utils/breakpoint/index.js

@@ -2,7 +2,7 @@
  * 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/>. */
 
-import { getLocationSource, getSourceActorsForSource } from "../../selectors";
+import { getSourceActorsForSource } from "../../selectors";
 import { isGenerated } from "../source";
 import { sortSelectedLocations } from "../location";
 import assert from "../assert";
@@ -30,7 +30,7 @@ export function makePendingLocationId(location) {
 }
 
 export function makeBreakpointLocation(state, location) {
-  const source = getLocationSource(state, location);
+  const source = location.source;
   if (!source) {
     throw new Error("no source");
   }

devtools/client/debugger/src/utils/source-maps.js

@@ -3,7 +3,7 @@
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 import { isOriginalId } from "devtools/client/shared/source-map-loader/index";
-import { getSource, getLocationSource } from "../selectors";
+import { getSource } from "../selectors";
 import { createLocation } from "./location";
 
 /**
@@ -76,10 +76,7 @@ export async function getOriginalLocation(location, thunkArgs) {
 }
 
 export async function getMappedLocation(location, thunkArgs) {
-  const { getState } = thunkArgs;
-  const source = getLocationSource(getState(), location);
-
-  if (!source) {
+  if (!location.source) {
     throw new Error(`no source ${location.sourceId}`);
   }
 
@@ -106,6 +103,10 @@ export async function getMappedLocation(location, thunkArgs) {
  * related location in the generated source.
  */
 export async function getRelatedMapLocation(location, thunkArgs) {
+  if (!location.source) {
+    return location;
+  }
+
   if (isOriginalId(location.sourceId)) {
     return getGeneratedLocation(location, thunkArgs);
   }

devtools/client/debugger/src/utils/source.js

@@ -65,20 +65,18 @@ export function shouldBlackbox(source) {
  *
  * @param {Object}  frame
  *                  The current frame
- * @param {Object}  source
- *                  The source related to the frame
  * @param {Object}  blackboxedRanges
  *                  The currently blackboxedRanges for all the sources.
  * @param {Boolean} isFrameBlackBoxed
  *                  If the frame is within the blackboxed range
  *                  or not.
  */
-export function isFrameBlackBoxed(frame, source, blackboxedRanges) {
+export function isFrameBlackBoxed(frame, blackboxedRanges) {
   return (
-    source &&
-    !!blackboxedRanges[source.url] &&
-    (!blackboxedRanges[source.url].length ||
-      !!findBlackBoxRange(source, blackboxedRanges, {
+    frame.source &&
+    !!blackboxedRanges[frame.source.url] &&
+    (!blackboxedRanges[frame.source.url].length ||
+      !!findBlackBoxRange(frame.source, blackboxedRanges, {
         start: frame.location.line,
         end: frame.location.line,
       }))

devtools/client/debugger/src/workers/parser/findOutOfScopeLocations.js

@@ -110,21 +110,21 @@ function sortByStart(a, b) {
  * Returns an array of locations that are considered out of scope for the given
  * location.
  */
-function findOutOfScopeLocations(sourceId, position) {
-  const { functions, comments } = findSymbols(sourceId);
+function findOutOfScopeLocations(location) {
+  const { functions, comments } = findSymbols(location.source.id);
   const commentLocations = comments.map(c => c.location);
   const locations = functions
     .map(getLocation)
     .concat(commentLocations)
     .sort(sortByStart);
 
-  const innerLocations = getInnerLocations(locations, position);
+  const innerLocations = getInnerLocations(locations, location);
   const outerLocations = locations.filter(loc => {
     if (innerLocations.includes(loc)) {
       return false;
     }
 
-    return !containsPosition(loc, position);
+    return !containsPosition(loc, location);
   });
   return removeOverlaps(outerLocations);
 }

devtools/client/debugger/src/workers/parser/tests/findOutOfScopeLocations.spec.js

@@ -19,7 +19,8 @@ function formatLines(actual) {
 describe("Parser.findOutOfScopeLocations", () => {
   it("should exclude non-enclosing function blocks", () => {
     const source = populateSource("outOfScope");
-    const actual = findOutOfScopeLocations(source.id, {
+    const actual = findOutOfScopeLocations({
+      source,
       line: 5,
       column: 5,
     });
@@ -29,7 +30,8 @@ describe("Parser.findOutOfScopeLocations", () => {
 
   it("should roll up function blocks", () => {
     const source = populateSource("outOfScope");
-    const actual = findOutOfScopeLocations(source.id, {
+    const actual = findOutOfScopeLocations({
+      source,
       line: 24,
       column: 0,
     });
@@ -39,7 +41,8 @@ describe("Parser.findOutOfScopeLocations", () => {
 
   it("should exclude function for locations on declaration", () => {
     const source = populateSource("outOfScope");
-    const actual = findOutOfScopeLocations(source.id, {
+    const actual = findOutOfScopeLocations({
+      source,
       line: 3,
       column: 12,
     });
@@ -49,7 +52,8 @@ describe("Parser.findOutOfScopeLocations", () => {
 
   it("should treat comments as out of scope", () => {
     const source = populateSource("outOfScopeComment");
-    const actual = findOutOfScopeLocations(source.id, {
+    const actual = findOutOfScopeLocations({
+      source,
       line: 3,
       column: 2,
     });
@@ -61,7 +65,8 @@ describe("Parser.findOutOfScopeLocations", () => {
 
   it("should not exclude in-scope inner locations", () => {
     const source = populateSource("outOfScope");
-    const actual = findOutOfScopeLocations(source.id, {
+    const actual = findOutOfScopeLocations({
+      source,
       line: 61,
       column: 0,
     });