-
Notifications
You must be signed in to change notification settings - Fork 74
fix(selectivity): script id matching on page change #1290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
KuznetsovRoman
wants to merge
1
commit into
master
Choose a base branch
from
TESTPLANE-1061
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,27 @@ import type { SelectivityMapSourceMapUrlFn } from "../../../config/types"; | |
|
|
||
| const SOURCE_CODE_EXTENSIONS = [".js", ".jsx", ".ts", ".tsx", ".mjs", ".cjs", ".mts", ".cts"]; | ||
|
|
||
| const PRECISE_COVERAGE_PARAMS = { callCount: false, detailed: false, allowTriggeredUpdates: false } as const; | ||
|
|
||
| const isSourceCodeFile = (sourceFileName: string): boolean => { | ||
| return SOURCE_CODE_EXTENSIONS.some(ext => sourceFileName.endsWith(ext)); | ||
| }; | ||
|
|
||
| /** | ||
| * CDP "scriptId" is NOT stable across a test: it is reused for different scripts over time (a cross-process | ||
| * navigation restarts the counter; bfcache restore re-issues the previous document's ids; freed ids get reassigned | ||
| * to new scripts). "Profiler.takePreciseCoverage" reports only the raw "scriptId", with no way to tell which script | ||
| * a given id currently means. Therefore coverage can only be attributed correctly if it is resolved WHILE the | ||
| * scripts are still live and the "scriptId -> source" mapping is current. | ||
| * | ||
| * The orchestration in index.ts pauses the renderer at two boundaries and drives this class: | ||
| * - on "beforeunload": takeCoverageSnapshot() — resolve the leaving page while its isolate is still alive | ||
| * (required for cross-process navigation, where the old isolate is gone by the time the next document starts); | ||
| * - at the new document start (and bfcache "pageshow"): flushPage() — resolve whatever is left of the previous | ||
| * page (same-process, incl. code that ran during unload), then RESET precise coverage and CLEAR the script maps | ||
| * so the next page parses into an empty map and its coverage window cannot mix with the previous page's. | ||
| * Because the maps are cleared at every page boundary, plain "scriptId" keys are unambiguous within a page. | ||
| */ | ||
| export class JSSelectivity { | ||
| private readonly _cdp: CDP; | ||
| private readonly _sessionId: CDPSessionId; | ||
|
|
@@ -30,11 +47,14 @@ export class JSSelectivity { | |
| private _debuggerOnScriptParsedFn: | ||
| | ((params: DebuggerEvents["scriptParsed"], cdpSessionId?: CDPSessionId) => void) | ||
| | null = null; | ||
| private readonly _scriptsSource: Record<CDPRuntimeScriptId, SelectivityAssetState> = {}; | ||
| private readonly _scriptsSourceMap: Record<CDPRuntimeScriptId, SelectivityAssetState> = {}; | ||
| private readonly _scriptIdToSourceUrl: Record<CDPRuntimeScriptId, string | null> = {}; | ||
| private readonly _scriptIdToSourceMapUrl: Record<CDPRuntimeScriptId, string | null> = {}; | ||
| private readonly _coverageResult: CDPScriptCoverage[] = []; | ||
| private _scriptsSource: Record<CDPRuntimeScriptId, SelectivityAssetState> = {}; | ||
| private _scriptsSourceMap: Record<CDPRuntimeScriptId, SelectivityAssetState> = {}; | ||
| private _scriptIdToSourceUrl: Record<CDPRuntimeScriptId, string | null> = {}; | ||
| private _scriptIdToSourceMapUrl: Record<CDPRuntimeScriptId, string | null> = {}; | ||
| /** Last seen content hash per "scriptId" — used to detect a "scriptId" reused for a different script */ | ||
| private _scriptIdToHash: Record<CDPRuntimeScriptId, string> = {}; | ||
| /** Source files collected across all pages of the current test */ | ||
| private readonly _dependingSourceFiles = new Set<string>(); | ||
|
|
||
| constructor( | ||
| cdp: CDP, | ||
|
|
@@ -49,13 +69,27 @@ export class JSSelectivity { | |
| } | ||
|
|
||
| private _processScript( | ||
| { scriptId, url, sourceMapURL }: DebuggerEvents["scriptParsed"], | ||
| { scriptId, url, sourceMapURL, hash }: DebuggerEvents["scriptParsed"], | ||
| cdpSessionId?: CDPSessionId, | ||
| ): void { | ||
| if (!this._sessionId || cdpSessionId !== this._sessionId) { | ||
| return; | ||
| } | ||
|
|
||
| // A "scriptId" can be reused for a DIFFERENT script within a single page window (a cross-process forward | ||
| // navigation may restart ids before the next boundary clears the maps). Detect reuse by content hash and drop | ||
| // the now-stale mapping, so the "||=" below repopulates it for the new script instead of keeping the old one. | ||
| const previousHash = this._scriptIdToHash[scriptId]; | ||
|
|
||
| if (previousHash !== undefined && previousHash !== hash) { | ||
| delete this._scriptsSource[scriptId]; | ||
| delete this._scriptsSourceMap[scriptId]; | ||
| delete this._scriptIdToSourceUrl[scriptId]; | ||
| delete this._scriptIdToSourceMapUrl[scriptId]; | ||
| } | ||
|
|
||
| this._scriptIdToHash[scriptId] = hash; | ||
|
|
||
| this._scriptIdToSourceUrl[scriptId] ||= url; | ||
|
|
||
| if (!url || !sourceMapURL || url.startsWith("chrome-error://")) { | ||
|
|
@@ -248,27 +282,133 @@ export class JSSelectivity { | |
| ]); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves the given coverage into original source files and accumulates them into "_dependingSourceFiles". | ||
| * Must run while the "scriptId -> source" maps still describe the scripts the coverage was taken from | ||
| * (i.e. before the page is navigated away and the maps are cleared). | ||
| */ | ||
| private async _resolveCoverageToDeps(coverageScripts: CDPScriptCoverage[]): Promise<void> { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pulled from L286 - L352 |
||
| const grouppedByScriptCoverage = groupBy(coverageScripts, "scriptId"); | ||
| const scriptIds = Object.keys(grouppedByScriptCoverage); | ||
|
|
||
| await Promise.all( | ||
| scriptIds.map(async scriptId => { | ||
| const [source, sourceMaps] = await Promise.all([ | ||
| this._scriptsSource[scriptId], | ||
| this._scriptsSourceMap[scriptId], | ||
| ]); | ||
| const sourceMapUrl = this._scriptIdToSourceMapUrl[scriptId]; | ||
| // Every "scriptId" has only one uniq "url" | ||
| const sourceUrl = this._scriptIdToSourceUrl[scriptId] || grouppedByScriptCoverage[scriptId][0].url; | ||
|
|
||
| // Function was called, but source maps were not generated for the file | ||
| // Or its anonymous script, without source url | ||
| if (!source || !sourceMaps || !sourceUrl) { | ||
| return; | ||
| } | ||
|
|
||
| if (source instanceof Error) { | ||
| throw new Error( | ||
| [`JS Selectivity: Couldn't load source code from ${sourceUrl}:`, String(source)].join("\n"), | ||
| ); | ||
| } | ||
|
|
||
| if (sourceMaps instanceof Error) { | ||
| throw new Error( | ||
| [`JS Selectivity: Couldn't load source maps from ${sourceMapUrl}`, String(sourceMaps)].join( | ||
| "\n", | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| if (isCachedOnFs(sourceMaps) && !sourceMapUrl) { | ||
| throw new Error("Assertation failed: souce map url has to present if source maps are fs-cached"); | ||
| } | ||
|
|
||
| const [sourceString, sourceMapsString] = await Promise.all([ | ||
| isCachedOnFs(source) ? getCachedSelectivityFile(CacheType.Asset, sourceUrl) : source, | ||
| isCachedOnFs(sourceMaps) | ||
| ? getCachedSelectivityFile(CacheType.Asset, sourceMapUrl as string) | ||
| : sourceMaps, | ||
| ]); | ||
|
|
||
| if (!sourceString || !sourceMapsString) { | ||
| throw new Error(`JS Selectivity: fs-cache is broken for ${sourceUrl}`); | ||
| } | ||
|
|
||
| const parsedSourceMapRanges = await parseSourceMapRanges( | ||
| sourceString, | ||
| sourceMapsString, | ||
| this._sourceRoot, | ||
| ); | ||
| const dependingSourceFiles = extractSourceFilesDeps( | ||
| parsedSourceMapRanges, | ||
| grouppedByScriptCoverage[scriptId], | ||
| isSourceCodeFile, | ||
| ); | ||
|
|
||
| for (const sourceFile of dependingSourceFiles.values()) { | ||
| this._dependingSourceFiles.add(sourceFile); | ||
| } | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| async start(): Promise<void> { | ||
| const debuggerOnScriptParsedFn = (this._debuggerOnScriptParsedFn = this._processScript.bind(this)); | ||
| const sessionId = this._sessionId; | ||
|
|
||
| this._cdp.debugger.on("scriptParsed", debuggerOnScriptParsedFn); | ||
|
|
||
| await this._cdp.profiler.startPreciseCoverage(sessionId, { | ||
| callCount: false, | ||
| detailed: false, | ||
| allowTriggeredUpdates: false, | ||
| }); | ||
| await this._cdp.profiler.startPreciseCoverage(sessionId, PRECISE_COVERAGE_PARAMS); | ||
| } | ||
|
|
||
| /** | ||
| * Called on "beforeunload", while the leaving page is still alive (its isolate not yet destroyed). Resolves the | ||
| * page's coverage into dependencies. Does NOT reset coverage or clear maps — that happens later, at the next | ||
| * page boundary (flushPage), once the leaving page has fully finished executing. | ||
| */ | ||
| async takeCoverageSnapshot(): Promise<void> { | ||
| const coveragePart = await this._cdp.profiler.takePreciseCoverage(this._sessionId); | ||
|
|
||
| this._ensureScriptsAreLoading(coveragePart.result); | ||
|
|
||
| await this._waitForLoadingScripts(); | ||
|
|
||
| this._coverageResult.push(...coveragePart.result); | ||
| // Missing a dependency is worse than failing the test, so resolution errors are propagated (and turned into a | ||
| // test failure by the orchestrator in index.ts), never swallowed into a partial dump. | ||
| await this._resolveCoverageToDeps(coveragePart.result); | ||
| } | ||
|
|
||
| /** | ||
| * Called at a page boundary — the new document's start, and on bfcache "pageshow" — while the renderer is paused | ||
| * before the incoming page's scripts run. Resolves whatever is left of the previous page (same-process code that | ||
| * ran during unload), then RESETS precise coverage and CLEARS the script maps. This guarantees the next page's | ||
| * coverage window and "scriptId" namespace start empty, so reused ids cannot be mistaken for the previous page's. | ||
| */ | ||
| async flushPage(): Promise<void> { | ||
| const coveragePart = await this._cdp.profiler.takePreciseCoverage(this._sessionId); | ||
|
|
||
| this._ensureScriptsAreLoading(coveragePart.result); | ||
|
|
||
| await this._waitForLoadingScripts(); | ||
|
|
||
| // Missing a dependency is worse than failing the test, so resolution and coverage-reset errors are | ||
| // propagated (turned into a test failure by index.ts), never swallowed into a partial dump. | ||
| await this._resolveCoverageToDeps(coveragePart.result); | ||
|
|
||
| // Reset coverage so the next page's window starts empty (a persisting same-process isolate would otherwise | ||
| // keep re-reporting this page's scripts under ids that the next page reuses). | ||
| await this._cdp.profiler.stopPreciseCoverage(this._sessionId); | ||
| await this._cdp.profiler.startPreciseCoverage(this._sessionId, PRECISE_COVERAGE_PARAMS); | ||
|
|
||
| // Clear the script maps so the next page parses into an empty namespace and reused "scriptId"s can't hit a | ||
| // stale mapping from the previous page. | ||
| this._scriptsSource = {}; | ||
| this._scriptsSourceMap = {}; | ||
| this._scriptIdToSourceUrl = {}; | ||
| this._scriptIdToSourceMapUrl = {}; | ||
| this._scriptIdToHash = {}; | ||
| } | ||
|
|
||
| /** @param drop only performs cleanup without providing actual deps. Should be "true" if test is failed */ | ||
|
|
@@ -279,79 +419,14 @@ export class JSSelectivity { | |
| } | ||
|
|
||
| const coverageLastPart = await this._cdp.profiler.takePreciseCoverage(this._sessionId); | ||
| const coverageScripts = [...this._coverageResult, ...coverageLastPart.result]; | ||
|
|
||
| this._ensureScriptsAreLoading(coverageLastPart.result); | ||
|
|
||
| const totalDependingSourceFiles = new Set<string>(); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to _resolveCoverageToDeps method |
||
| const grouppedByScriptCoverage = groupBy(coverageScripts, "scriptId"); | ||
| const scriptIds = Object.keys(grouppedByScriptCoverage); | ||
|
|
||
| await Promise.all( | ||
| scriptIds.map(async scriptId => { | ||
| const [source, sourceMaps] = await Promise.all([ | ||
| this._scriptsSource[scriptId], | ||
| this._scriptsSourceMap[scriptId], | ||
| ]); | ||
| const sourceMapUrl = this._scriptIdToSourceMapUrl[scriptId]; | ||
| // Every "scriptId" has only one uniq "url" | ||
| const sourceUrl = this._scriptIdToSourceUrl[scriptId] || grouppedByScriptCoverage[scriptId][0].url; | ||
|
|
||
| // Function was called, but source maps were not generated for the file | ||
| // Or its anonymous script, without source url | ||
| if (!source || !sourceMaps || !sourceUrl) { | ||
| return; | ||
| } | ||
|
|
||
| if (source instanceof Error) { | ||
| throw new Error( | ||
| [`JS Selectivity: Couldn't load source code from ${sourceUrl}:`, String(source)].join("\n"), | ||
| ); | ||
| } | ||
|
|
||
| if (sourceMaps instanceof Error) { | ||
| throw new Error( | ||
| [`JS Selectivity: Couldn't load source maps from ${sourceMapUrl}`, String(sourceMaps)].join( | ||
| "\n", | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| if (isCachedOnFs(sourceMaps) && !sourceMapUrl) { | ||
| throw new Error( | ||
| "Assertation failed: souce map url has to present if source maps are fs-cached", | ||
| ); | ||
| } | ||
|
|
||
| const [sourceString, sourceMapsString] = await Promise.all([ | ||
| isCachedOnFs(source) ? getCachedSelectivityFile(CacheType.Asset, sourceUrl) : source, | ||
| isCachedOnFs(sourceMaps) | ||
| ? getCachedSelectivityFile(CacheType.Asset, sourceMapUrl as string) | ||
| : sourceMaps, | ||
| ]); | ||
|
|
||
| if (!sourceString || !sourceMapsString) { | ||
| throw new Error(`JS Selectivity: fs-cache is broken for ${sourceUrl}`); | ||
| } | ||
|
|
||
| const parsedSourceMapRanges = await parseSourceMapRanges( | ||
| sourceString, | ||
| sourceMapsString, | ||
| this._sourceRoot, | ||
| ); | ||
| const dependingSourceFiles = extractSourceFilesDeps( | ||
| parsedSourceMapRanges, | ||
| grouppedByScriptCoverage[scriptId], | ||
| isSourceCodeFile, | ||
| ); | ||
| await this._waitForLoadingScripts(); | ||
|
|
||
| for (const sourceFile of dependingSourceFiles.values()) { | ||
| totalDependingSourceFiles.add(sourceFile); | ||
| } | ||
| }), | ||
| ); | ||
| await this._resolveCoverageToDeps(coverageLastPart.result); | ||
|
|
||
| return totalDependingSourceFiles; | ||
| return this._dependingSourceFiles; | ||
| } finally { | ||
| this._debuggerOnScriptParsedFn && this._cdp.debugger.off("scriptParsed", this._debuggerOnScriptParsedFn); | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iframe guard