skip source maps for replaced scripts (#286)

Author: vzaidman

front_end/models/bindings/CompilerScriptMapping.test.ts

@@ -45,6 +45,28 @@ describeWithMockConnection('CompilerScriptMapping', () => {
             });
       });
 
+  /**
+   * Holds back loading the source map at `url` until the returned function is called,
+   * so that source maps can finish loading out of parse order. Calling the returned
+   * function also restores `loadResource`.
+   */
+  const delaySourceMapLoad = (url: string): () => void => {
+    const {promise: sourceMapLoadReleased, resolve} = Promise.withResolvers<void>();
+    const pageResourceLoader = SDK.PageResourceLoader.PageResourceLoader.instance();
+    const loadResource = pageResourceLoader.loadResource.bind(pageResourceLoader);
+    const loadResourceStub =
+        sinon.stub(pageResourceLoader, 'loadResource').callsFake(async (resourceUrl, initiator) => {
+          if (resourceUrl === url) {
+            await sourceMapLoadReleased;
+          }
+          return await loadResource(resourceUrl, initiator);
+        });
+    return () => {
+      loadResourceStub.restore();
+      resolve();
+    };
+  };
+
   it('creates UISourceCodes with the correct content type', async () => {
     const target = createTarget();
 
@@ -518,6 +540,111 @@ describeWithMockConnection('CompilerScriptMapping', () => {
               originalScript.debuggerModel.createRawLocation(originalScript, 0, 0)),
           originalLibUISourceCode.uiLocation(0, 0));
     });
+
+    it('supports script replacement when source maps finish loading out of order', async () => {
+      // When DevTools attaches to a page where a script replaced code of an earlier script
+      // (e.g. on hot reload), the scripts are reported in parse order, but the small source
+      // map of the replacement script sometimes finishes loading before the large source map
+      // of the original bundle. The replacement script must keep mapping to `app.js` regardless.
+      const target = createTarget();
+      const sourceRoot = 'webpack:///src';
+
+      const bundleScriptInfo = {
+        url: 'http://example.com/bundle.js',
+        content: 'const f = console.log;\nf("Hello from the original bundle");',
+      };
+      const bundleSourceMapInfo = {
+        url: `${bundleScriptInfo.url}.map`,
+        content: encodeSourceMap(
+            [
+              '0:0 => lib.js:0:0',
+              'lib.js: const f = console.log;',
+              '1:0 => app.js:0:0',
+              'app.js: f("Hello from the original bundle")',
+            ],
+            sourceRoot),
+      };
+      const replacementScriptInfo = {
+        url: 'http://example.com/replacement.js',
+        content: 'f("Hello from the replacement");',
+      };
+      const replacementSourceMapInfo = {
+        url: `${replacementScriptInfo.url}.map`,
+        content: encodeSourceMap(['0:0 => app.js:0:0', 'app.js: f("Hello from the replacement")'], sourceRoot),
+      };
+
+      const loadBundleSourceMap = delaySourceMapLoad(bundleSourceMapInfo.url);
+
+      const bundleScriptPromise = backend.addScript(target, bundleScriptInfo, bundleSourceMapInfo);
+      const [replacementAppUISourceCode, replacementScript] = await Promise.all([
+        waitForUISourceCodeAdded(`${sourceRoot}/app.js`, target),
+        backend.addScript(target, replacementScriptInfo, replacementSourceMapInfo),
+      ]);
+
+      const [libUISourceCode, bundleScript] = await Promise.all([
+        waitForUISourceCodeAdded(`${sourceRoot}/lib.js`, target),
+        bundleScriptPromise,
+        loadBundleSourceMap(),
+      ]);
+
+      // The replacement script still maps to `app.js`...
+      assert.strictEqual(workspace.uiSourceCodeForURL(urlString`${sourceRoot}/app.js`), replacementAppUISourceCode);
+      assert.deepEqual(await debuggerWorkspaceBinding.rawLocationToUILocation(
+                           replacementScript.debuggerModel.createRawLocation(replacementScript, 0, 0)),
+                       replacementAppUISourceCode.uiLocation(0, 0));
+      assert.deepEqual(await debuggerWorkspaceBinding.uiLocationToRawLocations(replacementAppUISourceCode, 0, 0), [
+        replacementScript.debuggerModel.createRawLocation(replacementScript, 0, 0),
+      ]);
+
+      // ...the stale `app.js` mapping of `bundle.js` is not used...
+      const {uiSourceCode} = (await debuggerWorkspaceBinding.rawLocationToUILocation(
+          bundleScript.debuggerModel.createRawLocation(bundleScript, 1, 0)))!;
+      assert.notStrictEqual(uiSourceCode, replacementAppUISourceCode);
+
+      // ...while `bundle.js` still maps to `lib.js`.
+      assert.deepEqual(await debuggerWorkspaceBinding.rawLocationToUILocation(
+                           bundleScript.debuggerModel.createRawLocation(bundleScript, 0, 0)),
+                       libUISourceCode.uiLocation(0, 0));
+    });
+
+    it('shares UISourceCodes with compatible source maps that finish loading out of order', async () => {
+      const target = createTarget();
+      const sourceRoot = 'webpack:///src';
+
+      const route1ScriptInfo = {
+        url: 'http://example.com/route1.js',
+        content: 'function f(x){}\nf(1)',
+      };
+      const route1SourceMapInfo = {
+        url: `${route1ScriptInfo.url}.map`,
+        content: encodeSourceMap(['0:0 => shared.js:0:0', '1:0 => route1.js:0:0'], sourceRoot),
+      };
+      const route2ScriptInfo = {
+        url: 'http://example.com/route2.js',
+        content: 'function f(x){}\nf(2)',
+      };
+      const route2SourceMapInfo = {
+        url: `${route2ScriptInfo.url}.map`,
+        content: encodeSourceMap(['0:0 => shared.js:0:0', '1:0 => route2.js:0:0'], sourceRoot),
+      };
+
+      const loadRoute1SourceMap = delaySourceMapLoad(route1SourceMapInfo.url);
+
+      const route1ScriptPromise = backend.addScript(target, route1ScriptInfo, route1SourceMapInfo);
+      const [, route2Script] = await Promise.all([
+        waitForUISourceCodeAdded(`${sourceRoot}/shared.js`, target),
+        backend.addScript(target, route2ScriptInfo, route2SourceMapInfo),
+      ]);
+
+      const [sharedUISourceCode, route1Script] = await Promise.all([
+        waitForUISourceCodeAdded(`${sourceRoot}/shared.js`, target),
+        route1ScriptPromise,
+        loadRoute1SourceMap(),
+      ]);
+
+      const sharedLocations = await debuggerWorkspaceBinding.uiLocationToRawLocations(sharedUISourceCode, 0, 0);
+      assert.sameMembers(sharedLocations.map(location => location.script()), [route1Script, route2Script]);
+    });
   });
 
   it('assumes UTF-8 encoding for source files embedded in source maps', async () => {

front_end/models/bindings/CompilerScriptMapping.ts

@@ -359,7 +359,8 @@ export class CompilerScriptMapping implements DebuggerSourceMapping {
    * and copies over all the mappings from the other source maps that were registered for the
    * same URL and which are compatible (agree on the content and ignore-list hint for the given
    * URL). If they are considered incompatible, the original `UISourceCode` will simply be
-   * removed and a new mapping will be established.
+   * removed and a new mapping will be established, unless the original `UISourceCode` is
+   * mapped by a newer script, which happens when source maps finish loading out of parse order.
    *
    * @param event holds the {@link SDK.Script.Script} and its {@link SDK.SourceMap.SourceMap}.
    */
@@ -384,7 +385,16 @@ export class CompilerScriptMapping implements DebuggerSourceMapping {
     }
     this.#sourceMapToProject.set(sourceMap, project);
 
+    const scriptParseOrder = new Map(script.debuggerModel.scripts().map((script, index) => [script, index]));
     for (const url of sourceMap.sourceURLs()) {
+      const existingUISourceCode = project.uiSourceCodeForURL(url);
+      // Source maps can finish loading out of parse order (e.g. when DevTools attaches after a
+      // script was replaced on hot reload), so a stale source map must not override the mapping
+      // of a newer script.
+      if (!this.#shouldUseSourceMapForURL(existingUISourceCode, url, script, sourceMap, scriptParseOrder)) {
+        continue;
+      }
+
       const contentType = Common.ResourceType.resourceTypes.SourceMapScript;
       const uiSourceCode = project.createUISourceCode(url, contentType);
       if (sourceMap.hasIgnoreListHint(url)) {
@@ -417,18 +427,17 @@ export class CompilerScriptMapping implements DebuggerSourceMapping {
       // added scripts in case of code-splitting, since it won't find out about these new mappings.
       // By removing and (re)adding a `UISourceCode` for the `url` we effectively force restoration
       // of breakpoints.
-      const otherUISourceCode = project.uiSourceCodeForURL(url);
-      if (otherUISourceCode !== null) {
-        // Copy the existing source mappings from the `otherUISourceCode` over as long as
+      if (existingUISourceCode !== null) {
+        // Copy the existing source mappings from the `existingUISourceCode` over as long as
         // they are compatible with the `sourceMap` wrt. `url`. While doing so, also clean
-        // up the `otherUISourceCode` (in particular its frame attributions).
-        for (const otherSourceMap of this.#uiSourceCodeToSourceMaps.get(otherUISourceCode)) {
-          this.#uiSourceCodeToSourceMaps.delete(otherUISourceCode, otherSourceMap);
+        // up the `existingUISourceCode` (in particular its frame attributions).
+        for (const otherSourceMap of this.#uiSourceCodeToSourceMaps.get(existingUISourceCode)) {
+          this.#uiSourceCodeToSourceMaps.delete(existingUISourceCode, otherSourceMap);
           const otherScript = this.#sourceMapManager.clientForSourceMap(otherSourceMap);
           if (!otherScript) {
             continue;
           }
-          NetworkProject.removeFrameAttribution(otherUISourceCode, otherScript.frameId);
+          NetworkProject.removeFrameAttribution(existingUISourceCode, otherScript.frameId);
           if (sourceMap.compatibleForURL(url, otherSourceMap)) {
             this.#uiSourceCodeToSourceMaps.set(uiSourceCode, otherSourceMap);
             NetworkProject.addFrameAttribution(uiSourceCode, otherScript.frameId);
@@ -444,6 +453,29 @@ export class CompilerScriptMapping implements DebuggerSourceMapping {
         .then(() => this.sourceMapAttachedForTest(sourceMap));
   }
 
+  #shouldUseSourceMapForURL(existingUISourceCode: Workspace.UISourceCode.UISourceCode|null,
+                            url: Platform.DevToolsPath.UrlString, script: SDK.Script.Script,
+                            sourceMap: SDK.SourceMap.SourceMap,
+                            scriptParseOrder: Map<SDK.Script.Script, number>): boolean {
+    if (existingUISourceCode === null) {
+      return true;
+    }
+    for (const existingSourceMap of this.#uiSourceCodeToSourceMaps.get(existingUISourceCode)) {
+      const existingScript = this.#sourceMapManager.clientForSourceMap(existingSourceMap);
+      // Compatible source maps agree on the source code of `url` (e.g. code splitting) and
+      // complement each other. Incompatible ones mean a different script replaced the source code
+      // of `url` (e.g. on hot reload), so only the source map of the newest script should be used.
+      if (!existingScript || sourceMap.compatibleForURL(url, existingSourceMap)) {
+        continue;
+      }
+      const isExistingScriptNewer = (scriptParseOrder.get(existingScript) ?? -1) > (scriptParseOrder.get(script) ?? -1);
+      if (isExistingScriptNewer) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   /**
    * Invoked by the {@link SDK.SourceMapManager.SourceMapManager} when the source map for a given
    * {@link SDK.Script.Script} is removed, which could be either because the target is execution