Bug 1541633 - Part 5: Move sourceMapURL field out of Source object. r=jlast

Differential Revision: https://phabricator.services.mozilla.com/D42937 --HG-- extra : moz-landing-system : lando
2019-08-23 14:34:55 +00:00 · 2019-08-23 14:34:55 +00:00 · 2c43ec001a
--- a/devtools/client/debugger/src/actions/breakpoints/tests/snapshots/breakpoints.spec.js.snap
+++ b/devtools/client/debugger/src/actions/breakpoints/tests/snapshots/breakpoints.spec.js.snap
@ -44,7 +44,6 @@ Array [
      "isPrettyPrinted": false,
      "isWasm": false,
      "relativeUrl": "/examples/a",
-      "sourceMapURL": null,
      "url": "http://localhost:8000/examples/a",
    },
  },
@ -127,7 +126,6 @@ Array [
      "isPrettyPrinted": false,
      "isWasm": false,
      "relativeUrl": "/examples/a",
-      "sourceMapURL": null,
      "url": "http://localhost:8000/examples/a",
    },
  },
--- a/devtools/client/debugger/src/actions/sources/newSources.js
+++ b/devtools/client/debugger/src/actions/sources/newSources.js
@ -25,7 +25,6 @@ import { selectLocation, setBreakableLines } from "../sources";
 import {
  getRawSourceURL,
  isPrettyURL,
-  isOriginal,
  isUrlExtension,
  isInlineScript,
 } from "../../utils/source";
@ -34,6 +33,7 @@ import {
  getSource,
  getSourceFromId,
  hasSourceActor,
+  getSourceByActorId,
  getPendingSelectedLocation,
  getPendingBreakpointsForSource,
  getContext,
@ -46,7 +46,6 @@ import { validateNavigateContext, ContextError } from "../../utils/context";

 import type {
  Source,
-  SourceId,
  Context,
  OriginalSourceData,
  GeneratedSourceData,
@ -54,15 +53,17 @@ import type {
 } from "../../types";
 import type { Action, ThunkArgs } from "../types";

-function loadSourceMaps(cx: Context, sources: Source[]) {
+function loadSourceMaps(cx: Context, sources: SourceActor[]) {
  return async function({
    dispatch,
    sourceMaps,
  }: ThunkArgs): Promise<?(Promise<Source>[])> {
    try {
      const sourceList = await Promise.all(
-        sources.map(async ({ id }) => {
-          const originalSources = await dispatch(loadSourceMap(cx, id));
+        sources.map(async sourceActor => {
+          const originalSources = await dispatch(
+            loadSourceMap(cx, sourceActor)
+          );
          sourceQueue.queueSources(
            originalSources.map(data => ({
              type: "original",
@ -75,13 +76,6 @@ function loadSourceMaps(cx: Context, sources: Source[]) {

      await sourceQueue.flush();

-      // We would like to sync breakpoints after we are done
-      // loading source maps as sometimes generated and original
-      // files share the same paths.
-      for (const source of sources) {
-        dispatch(checkPendingBreakpoints(cx, source.id));
-      }
-
      return flatten(sourceList);
    } catch (error) {
      if (!(error instanceof ContextError)) {
@ -95,20 +89,13 @@ function loadSourceMaps(cx: Context, sources: Source[]) {
 * @memberof actions/sources
 * @static
 */
-function loadSourceMap(cx: Context, sourceId: SourceId) {
+function loadSourceMap(cx: Context, sourceActor: SourceActor) {
  return async function({
    dispatch,
    getState,
    sourceMaps,
  }: ThunkArgs): Promise<OriginalSourceData[]> {
-    const source = getSource(getState(), sourceId);
-
-    if (
-      !prefs.clientSourceMapsEnabled ||
-      !source ||
-      isOriginal(source) ||
-      !source.sourceMapURL
-    ) {
+    if (!prefs.clientSourceMapsEnabled || !sourceActor.sourceMapURL) {
      return [];
    }

@ -116,21 +103,29 @@ function loadSourceMap(cx: Context, sourceId: SourceId) {
    try {
      // Unable to correctly type the result of a spread on a union type.
      // See https://github.com/facebook/flow/pull/7298
-      let url = source.url;
-      if (!source.url && typeof source.introductionUrl === "string") {
+      let url = sourceActor.url || "";
+      if (!sourceActor.url && typeof sourceActor.introductionUrl === "string") {
        // If the source was dynamically generated (via eval, dynamically
        // created script elements, and so forth), it won't have a URL, so that
        // it is not collapsed into other sources from the same place. The
        // introduction URL will include the point it was constructed at,
        // however, so use that for resolving any source maps in the source.
-        url = source.introductionUrl;
+        url = sourceActor.introductionUrl;
+      }
+
+      // Ignore sourceMapURL on scripts that are part of HTML files, since
+      // we currently treat sourcemaps as Source-wide, not SourceActor-specific.
+      const source = getSourceByActorId(getState(), sourceActor.id);
+      if (source) {
+        data = await sourceMaps.getOriginalURLs({
+          // Using source ID here is historical and eventually we'll want to
+          // switch to all of this being per-source-actor.
+          id: source.id,
+          url,
+          sourceMapURL: sourceActor.sourceMapURL || "",
+          isWasm: sourceActor.introductionType === "wasm",
+        });
      }
-      data = await sourceMaps.getOriginalURLs({
-        id: source.id,
-        url,
-        sourceMapURL: source.sourceMapURL || "",
-        isWasm: source.isWasm,
-      });
    } catch (e) {
      console.error(e);
    }
@ -139,9 +134,9 @@ function loadSourceMap(cx: Context, sourceId: SourceId) {
      // If this source doesn't have a sourcemap, enable it for pretty printing
      dispatch(
        ({
-          type: "CLEAR_SOURCE_MAP_URL",
+          type: "CLEAR_SOURCE_ACTOR_MAP_URL",
          cx,
-          sourceId,
+          id: sourceActor.id,
        }: Action)
      );
      return [];
@ -276,6 +271,10 @@ export function newOriginalSources(sourceInfo: Array<OriginalSourceData>) {

    await dispatch(checkNewSources(cx, sources));

+    for (const source of sources) {
+      dispatch(checkPendingBreakpoints(cx, source.id));
+    }
+
    return sources;
  };
 }
@ -302,19 +301,18 @@ export function newGeneratedSources(sourceInfo: Array<GeneratedSourceData>) {
      const newId = id || makeSourceId(source);

      if (!getSource(getState(), newId) && !newSourcesObj[newId]) {
-        newSourcesObj[newId] = ({
+        newSourcesObj[newId] = {
          id: newId,
          url: source.url,
          relativeUrl: source.url,
          isPrettyPrinted: false,
          extensionName: source.extensionName,
-          sourceMapURL: source.sourceMapURL,
          introductionUrl: source.introductionUrl,
          introductionType: source.introductionType,
          isBlackBoxed: false,
          isWasm: !!supportsWasm && source.introductionType === "wasm",
          isExtension: (source.url && isUrlExtension(source.url)) || false,
-        }: any);
+        };
      }

      const actorId = stringToSourceActorId(source.actor);
@ -360,6 +358,17 @@ export function newGeneratedSources(sourceInfo: Array<GeneratedSourceData>) {
    }
    await dispatch(checkNewSources(cx, newSources));

+    (async () => {
+      await dispatch(loadSourceMaps(cx, newSourceActors));
+
+      // We would like to sync breakpoints after we are done
+      // loading source maps as sometimes generated and original
+      // files share the same paths.
+      for (const source of newSources) {
+        dispatch(checkPendingBreakpoints(cx, source.id));
+      }
+    })();
+
    return resultIds.map(id => getSourceFromId(getState(), id));
  };
 }
@ -377,7 +386,6 @@ function checkNewSources(cx, sources: Source[]) {
    }

    dispatch(restoreBlackBoxedSources(cx, sources));
-    dispatch(loadSourceMaps(cx, sources));

    return sources;
  };
--- a/devtools/client/debugger/src/actions/tests/pending-breakpoints.spec.js
+++ b/devtools/client/debugger/src/actions/tests/pending-breakpoints.spec.js
@ -381,7 +381,7 @@ describe("adding sources", () => {
          url: sourceURL,
        },
      ],
-      getOriginalSourceText: async () => ({ source: "" }),
+      getOriginalSourceText: async () => ({ text: "" }),
      getGeneratedLocation: async location => ({
        line: location.line,
        column: location.column,
@ -391,7 +391,11 @@ describe("adding sources", () => {
      getGeneratedRangesForOriginal: async () => [
        { start: { line: 0, column: 0 }, end: { line: 10, column: 10 } },
      ],
-      getOriginalLocations: async items => items,
+      getOriginalLocations: async items =>
+        items.map(item => ({
+          ...item,
+          sourceId: sourceMaps.generatedToOriginalId(item.sourceId, sourceURL),
+        })),
    });

    const { getState, dispatch } = store;
--- a/devtools/client/debugger/src/actions/types/SourceAction.js
+++ b/devtools/client/debugger/src/actions/types/SourceAction.js
@ -4,7 +4,7 @@

 // @flow

-import type { SourceId, Source, SourceLocation, Context } from "../../types";
+import type { Source, SourceLocation, Context } from "../../types";
 import type { PromiseAction } from "../utils/middleware/promise";
 import type { SourceBase } from "../../reducers/sources";

@ -32,11 +32,6 @@ export type SourceAction =
      +cx: Context,
      +sources: Array<SourceBase>,
    |}
-  | {|
-      +type: "CLEAR_SOURCE_MAP_URL",
-      +cx: Context,
-      +sourceId: SourceId,
-    |}
  | {|
      +type: "SET_SELECTED_LOCATION",
      +cx: Context,
--- a/devtools/client/debugger/src/actions/types/SourceActorAction.js
+++ b/devtools/client/debugger/src/actions/types/SourceActorAction.js
@ -4,6 +4,7 @@

 // @flow

+import type { Context } from "../../types";
 import { type PromiseAction } from "../utils/middleware/promise";
 import type {
  SourceActorId,
@ -18,6 +19,11 @@ export type SourceActorsRemoveAction = {|
  type: "REMOVE_SOURCE_ACTORS",
  items: Array<SourceActor>,
 |};
+export type SourceActorClearMapAction = {|
+  +type: "CLEAR_SOURCE_ACTOR_MAP_URL",
+  +cx: Context,
+  +id: SourceActorId,
+|};

 export type SourceActorBreakpointColumnsAction = PromiseAction<
  {|
@ -40,4 +46,5 @@ export type SourceActorAction =
  | SourceActorsInsertAction
  | SourceActorsRemoveAction
  | SourceActorBreakpointColumnsAction
-  | SourceActorBreakableLinesAction;
+  | SourceActorBreakableLinesAction
+  | SourceActorClearMapAction;
--- a/devtools/client/debugger/src/components/PrimaryPanes/tests/SourcesTree.spec.js
+++ b/devtools/client/debugger/src/components/PrimaryPanes/tests/SourcesTree.spec.js
@ -83,8 +83,7 @@ describe("SourcesTree", () => {
        const newSource = createMockSource(
          "server1.conn13.child1/43",
          "http://mdn.com/four.js",
-          true,
-          ""
+          true
        );

        const newThreadSources = {
@ -149,8 +148,7 @@ describe("SourcesTree", () => {
            "server1.conn13.child1/41": createMockSource(
              "server1.conn13.child1/41",
              "http://mdn.com/three.js",
-              true,
-              ""
+              true
            ),
          },
        };
@ -177,8 +175,7 @@ describe("SourcesTree", () => {
        const newSource = createMockSource(
          "server1.conn13.child1/43",
          "http://mdn.com/four.js",
-          true,
-          ""
+          true
        );

        const newThreadSources = {
@ -227,8 +224,7 @@ describe("SourcesTree", () => {
        const mockSource = createMockSource(
          "server1.conn13.child1/41",
          "http://mdn.com/three.js",
-          false,
-          null
+          false
        );
        await component.setProps({
          ...props,
@ -363,32 +359,26 @@ function generateDefaults(overrides: Object) {
      "server1.conn13.child1/39": createMockSource(
        "server1.conn13.child1/39",
        "http://mdn.com/one.js",
-        false,
-        null
+        false
      ),
      "server1.conn13.child1/40": createMockSource(
        "server1.conn13.child1/40",
        "http://mdn.com/two.js",
-        false,
-        null
+        false
      ),
      "server1.conn13.child1/41": createMockSource(
        "server1.conn13.child1/41",
        "http://mdn.com/three.js",
-        false,
-        null
+        false
      ),
      "server1.conn13.child1/42/originalSource-sha": createMockSource(
        "server1.conn13.child1/42/originalSource-sha",
        "http://mdn.com/four.js",
-        false,
-        null
+        false
      ),
      "server1.conn13.child1/42": createMockSource(
        "server1.conn13.child1/42",
-        "http://mdn.com/four.js",
-        false,
-        "data:application/json?charset=utf?dsffewrsf"
+        "http://mdn.com/four.js"
      ),
    },
  };
@ -430,11 +420,10 @@ function render(overrides = {}) {
  return { component, props, defaultState, instance };
 }

-function createMockSource(id, url, isBlackBoxed = false, sourceMapURL = null) {
+function createMockSource(id, url, isBlackBoxed = false) {
  return {
    ...makeMockSource(url, id),
    isBlackBoxed,
-    sourceMapURL,
  };
 }

--- a/devtools/client/debugger/src/components/PrimaryPanes/tests/snapshots/SourcesTree.spec.js.snap
+++ b/devtools/client/debugger/src/components/PrimaryPanes/tests/snapshots/SourcesTree.spec.js.snap
@ -144,7 +144,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
              "isPrettyPrinted": false,
              "isWasm": false,
              "relativeUrl": "http://mdn.com/three.js",
-              "sourceMapURL": null,
              "url": "http://mdn.com/three.js",
            },
            "name": "three.js",
@ -164,7 +163,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                  "isPrettyPrinted": false,
                  "isWasm": false,
                  "relativeUrl": "http://mdn.com/four.js",
-                  "sourceMapURL": "data:application/json?charset=utf?dsffewrsf",
                  "url": "http://mdn.com/four.js",
                },
                "name": "four.js",
@ -182,7 +180,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                  "isPrettyPrinted": false,
                  "isWasm": false,
                  "relativeUrl": "http://mdn.com/four.js",
-                  "sourceMapURL": null,
                  "url": "http://mdn.com/four.js",
                },
                "name": "four.js",
@ -200,7 +197,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                  "isPrettyPrinted": false,
                  "isWasm": false,
                  "relativeUrl": "http://mdn.com/one.js",
-                  "sourceMapURL": null,
                  "url": "http://mdn.com/one.js",
                },
                "name": "one.js",
@ -218,7 +214,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                  "isPrettyPrinted": false,
                  "isWasm": false,
                  "relativeUrl": "http://mdn.com/three.js",
-                  "sourceMapURL": null,
                  "url": "http://mdn.com/three.js",
                },
                "name": "three.js",
@ -236,7 +231,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                  "isPrettyPrinted": false,
                  "isWasm": false,
                  "relativeUrl": "http://mdn.com/two.js",
-                  "sourceMapURL": null,
                  "url": "http://mdn.com/two.js",
                },
                "name": "two.js",
@ -263,7 +257,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                      "isPrettyPrinted": false,
                      "isWasm": false,
                      "relativeUrl": "http://mdn.com/four.js",
-                      "sourceMapURL": "data:application/json?charset=utf?dsffewrsf",
                      "url": "http://mdn.com/four.js",
                    },
                    "name": "four.js",
@ -281,7 +274,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                      "isPrettyPrinted": false,
                      "isWasm": false,
                      "relativeUrl": "http://mdn.com/four.js",
-                      "sourceMapURL": null,
                      "url": "http://mdn.com/four.js",
                    },
                    "name": "four.js",
@ -299,7 +291,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                      "isPrettyPrinted": false,
                      "isWasm": false,
                      "relativeUrl": "http://mdn.com/one.js",
-                      "sourceMapURL": null,
                      "url": "http://mdn.com/one.js",
                    },
                    "name": "one.js",
@ -317,7 +308,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                      "isPrettyPrinted": false,
                      "isWasm": false,
                      "relativeUrl": "http://mdn.com/three.js",
-                      "sourceMapURL": null,
                      "url": "http://mdn.com/three.js",
                    },
                    "name": "three.js",
@ -335,7 +325,6 @@ exports[`SourcesTree on receiving new props updates highlighted items updates hi
                      "isPrettyPrinted": false,
                      "isWasm": false,
                      "relativeUrl": "http://mdn.com/two.js",
-                      "sourceMapURL": null,
                      "url": "http://mdn.com/two.js",
                    },
                    "name": "two.js",
--- a/devtools/client/debugger/src/reducers/source-actors.js
+++ b/devtools/client/debugger/src/reducers/source-actors.js
@ -109,11 +109,31 @@ export default function update(
    case "SET_SOURCE_ACTOR_BREAKABLE_LINES":
      state = updateBreakableLines(state, action);
      break;
+
+    case "CLEAR_SOURCE_ACTOR_MAP_URL":
+      state = clearSourceActorMapURL(state, action.id);
+      break;
  }

  return state;
 }

+function clearSourceActorMapURL(
+  state: SourceActorsState,
+  id: SourceActorId
+): SourceActorsState {
+  if (!hasResource(state, id)) {
+    return state;
+  }
+
+  return updateResources(state, [
+    {
+      id,
+      sourceMapURL: "",
+    },
+  ]);
+}
+
 function updateBreakpointColumns(
  state: SourceActorsState,
  action: SourceActorBreakpointColumnsAction
--- a/devtools/client/debugger/src/reducers/sources.js
+++ b/devtools/client/debugger/src/reducers/sources.js
@ -89,7 +89,6 @@ type PlainUrlsMap = { [string]: string[] };
 export type SourceBase = {|
  +id: SourceId,
  +url: string,
-  +sourceMapURL?: string,
  +isBlackBoxed: boolean,
  +isPrettyPrinted: boolean,
  +relativeUrl: string,
@ -159,8 +158,6 @@ function update(
  let location = null;

  switch (action.type) {
-    case "CLEAR_SOURCE_MAP_URL":
-      return clearSourceMaps(state, action.sourceId);
    case "ADD_SOURCE":
      return addSources(state, [action.source]);

@ -453,25 +450,6 @@ function updateLoadedState(
  };
 }

-function clearSourceMaps(
-  state: SourcesState,
-  sourceId: SourceId
-): SourcesState {
-  if (!hasResource(state.sources, sourceId)) {
-    return state;
-  }
-
-  return {
-    ...state,
-    sources: updateResources(state.sources, [
-      {
-        id: sourceId,
-        sourceMapURL: "",
-      },
-    ]),
-  };
-}
-
 /*
 * Update a source when its state changes
 * e.g. the text was loaded, it was blackboxed
@ -939,8 +917,9 @@ export function isSourceWithMap(
  state: OuterState & SourceActorOuterState,
  id: SourceId
 ): boolean {
-  const source = getSource(state, id);
-  return source ? !!source.sourceMapURL : false;
+  return getSourceActorsForSource(state, id).some(
+    soureActor => soureActor.sourceMapURL
+  );
 }

 export function canPrettyPrintSource(
--- a/devtools/client/debugger/src/types.js
+++ b/devtools/client/debugger/src/types.js
@ -400,7 +400,6 @@ export type SourceWithContentAndType<+Content: SourceContent> = $ReadOnly<{
 export type Source = {
  +id: SourceId,
  +url: string,
-  +sourceMapURL?: string,
  +isBlackBoxed: boolean,
  +isPrettyPrinted: boolean,
  +relativeUrl: string,
--- a/devtools/client/debugger/src/utils/test-head.js
+++ b/devtools/client/debugger/src/utils/test-head.js
@ -89,7 +89,6 @@ function makeFrame({ id, sourceId }: Object, opts: Object = {}) {
 function createSourceObject(
  filename: string,
  props: {
-    sourceMapURL?: string,
    introductionType?: string,
    introductionUrl?: string,
    isBlackBoxed?: boolean,