From e4690da13b19ed906f6c573200ce54dda918cc9c Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Wed, 14 Oct 2026 09:48:36 +0100 Subject: [PATCH 1/2] [DevTools] Fix Stale Source Maps Replacing Newer Source Maps. When a newer script replaces code from an earlier script (e.g. on hot reload), both scripts' source maps claim the same original source file but disagree on its content. `CompilerScriptMapping.sourceMapAttached` resolves such conflicts in favor of whichever source map **finishes loading** last. That works while DevTools is attached during the replacement, because the replacement script's source map arrives last and attaches last. But when DevTools attaches (or reloads) after the replacement happened, all scripts are reported at once in parse order. The (potentially) small source map of the replacement script can then finish loading before the large source map of the original bundle. The bundle's stale source map then takes over the original file, and the replacement script loses its mapping. Breakpoints and `debugger;` statements in the replaced code then pause in the generated script instead of the original source. In React Native this reproduces reliably with larger apps: 1. Add a `debugger;` statement and let Fast Refresh apply it. 2. Reload DevTools. 3. The pause shows the generated bundle instead of the source file. The cause is that Metro's main bundle map for larger apps is much bigger than a hot-update map, so the main bundle map consistently arrives after the hot-update map. This change makes conflict resolution depend on script parse order rather than source map load order. A source map no longer takes over a URL when that URL is already provided by an incompatible source map of a newer script. Compatible source maps (source maps that are added together when they agree on the content of the file receiving the source maps) are still merged into one `UISourceCode` as before. Fixed: 571671607 Change-Id: I75f40532b2e35f5371b0dff2d516add335bc03ce Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/8549555 Reviewed-by: Eric Leese Reviewed-by: Benedikt Meurer Commit-Queue: Benedikt Meurer --- .../bindings/CompilerScriptMapping.test.ts | 127 ++++++++++++++++++ .../models/bindings/CompilerScriptMapping.ts | 48 +++++-- 2 files changed, 167 insertions(+), 8 deletions(-) diff --git a/front_end/models/bindings/CompilerScriptMapping.test.ts b/front_end/models/bindings/CompilerScriptMapping.test.ts index 3499719bcc12..a6e65e148e49 100644 --- a/front_end/models/bindings/CompilerScriptMapping.test.ts +++ b/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(); + const {pageResourceLoader} = backend.universe; + 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 = backend.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 = backend.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 () => { diff --git a/front_end/models/bindings/CompilerScriptMapping.ts b/front_end/models/bindings/CompilerScriptMapping.ts index 7e728f144ec4..a8e741102b57 100644 --- a/front_end/models/bindings/CompilerScriptMapping.ts +++ b/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(this.#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): 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 From 2672c21ab75397df8aff7a29152917c7dc51d2ff Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Thu, 15 Oct 2026 11:47:19 +0100 Subject: [PATCH 2/2] [DevTools] Fix Stale Source Maps Replacing Newer Source Maps - cherry pick adjustments --- front_end/models/bindings/CompilerScriptMapping.test.ts | 6 +++--- front_end/models/bindings/CompilerScriptMapping.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/front_end/models/bindings/CompilerScriptMapping.test.ts b/front_end/models/bindings/CompilerScriptMapping.test.ts index a6e65e148e49..ca81122f9eed 100644 --- a/front_end/models/bindings/CompilerScriptMapping.test.ts +++ b/front_end/models/bindings/CompilerScriptMapping.test.ts @@ -52,7 +52,7 @@ describeWithMockConnection('CompilerScriptMapping', () => { */ const delaySourceMapLoad = (url: string): () => void => { const {promise: sourceMapLoadReleased, resolve} = Promise.withResolvers(); - const {pageResourceLoader} = backend.universe; + const pageResourceLoader = SDK.PageResourceLoader.PageResourceLoader.instance(); const loadResource = pageResourceLoader.loadResource.bind(pageResourceLoader); const loadResourceStub = sinon.stub(pageResourceLoader, 'loadResource').callsFake(async (resourceUrl, initiator) => { @@ -546,7 +546,7 @@ describeWithMockConnection('CompilerScriptMapping', () => { // (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 = backend.createTarget(); + const target = createTarget(); const sourceRoot = 'webpack:///src'; const bundleScriptInfo = { @@ -608,7 +608,7 @@ describeWithMockConnection('CompilerScriptMapping', () => { }); it('shares UISourceCodes with compatible source maps that finish loading out of order', async () => { - const target = backend.createTarget(); + const target = createTarget(); const sourceRoot = 'webpack:///src'; const route1ScriptInfo = { diff --git a/front_end/models/bindings/CompilerScriptMapping.ts b/front_end/models/bindings/CompilerScriptMapping.ts index a8e741102b57..0d742066b83c 100644 --- a/front_end/models/bindings/CompilerScriptMapping.ts +++ b/front_end/models/bindings/CompilerScriptMapping.ts @@ -385,7 +385,7 @@ export class CompilerScriptMapping implements DebuggerSourceMapping { } this.#sourceMapToProject.set(sourceMap, project); - const scriptParseOrder = new Map(this.#debuggerModel.scripts().map((script, index) => [script, index])); + 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