diff --git a/front_end/models/bindings/CompilerScriptMapping.test.ts b/front_end/models/bindings/CompilerScriptMapping.test.ts index 3499719bcc12..ca81122f9eed 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 = 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 () => { diff --git a/front_end/models/bindings/CompilerScriptMapping.ts b/front_end/models/bindings/CompilerScriptMapping.ts index 7e728f144ec4..0d742066b83c 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(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): 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