From 3d3154de861ae02663a665735e055412353dffcf Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 6 Feb 2025 12:38:51 -0800 Subject: [PATCH] chore(tracing): look up snapshot resources only in the same context (#34645) --- .../src/server/trace/recorder/tracing.ts | 3 ++- .../server/trace/test/inMemorySnapshotter.ts | 4 +-- packages/trace-viewer/src/sw/main.ts | 9 ++++--- .../trace-viewer/src/sw/snapshotServer.ts | 16 ++++++------ .../trace-viewer/src/sw/snapshotStorage.ts | 25 +++++++++++++------ packages/trace-viewer/src/sw/traceModel.ts | 3 ++- .../trace-viewer/src/sw/traceModernizer.ts | 8 +++--- packages/trace-viewer/src/types/entries.ts | 1 + packages/trace/src/trace.ts | 1 + 9 files changed, 45 insertions(+), 25 deletions(-) diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index fa57a14df1..6f6724e368 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -103,7 +103,8 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps wallTime: 0, monotonicTime: 0, sdkLanguage: context.attribution.playwright.options.sdkLanguage, - testIdAttributeName + testIdAttributeName, + contextId: context.guid, }; if (context instanceof BrowserContext) { this._snapshotter = new Snapshotter(context, this); diff --git a/packages/playwright-core/src/server/trace/test/inMemorySnapshotter.ts b/packages/playwright-core/src/server/trace/test/inMemorySnapshotter.ts index 8de07c2aad..d5da0e4229 100644 --- a/packages/playwright-core/src/server/trace/test/inMemorySnapshotter.ts +++ b/packages/playwright-core/src/server/trace/test/inMemorySnapshotter.ts @@ -72,7 +72,7 @@ export class InMemorySnapshotter implements SnapshotterDelegate, HarTracerDelega } onEntryFinished(entry: har.Entry) { - this._storage.addResource(entry); + this._storage.addResource('', entry); } onContentBlob(sha1: string, buffer: Buffer) { @@ -85,7 +85,7 @@ export class InMemorySnapshotter implements SnapshotterDelegate, HarTracerDelega onFrameSnapshot(snapshot: FrameSnapshot): void { ++this._snapshotCount; - const renderer = this._storage.addFrameSnapshot(snapshot, []); + const renderer = this._storage.addFrameSnapshot('', snapshot, []); this._snapshotReadyPromises.get(snapshot.snapshotName || '')?.resolve(renderer); } diff --git a/packages/trace-viewer/src/sw/main.ts b/packages/trace-viewer/src/sw/main.ts index cda3ba8cad..cd23082d91 100644 --- a/packages/trace-viewer/src/sw/main.ts +++ b/packages/trace-viewer/src/sw/main.ts @@ -120,14 +120,16 @@ async function doFetch(event: FetchEvent): Promise { const { snapshotServer } = loadedTraces.get(traceUrl!) || {}; if (!snapshotServer) return new Response(null, { status: 404 }); - return snapshotServer.serveSnapshotInfo(relativePath, url.searchParams); + const pageOrFrameId = relativePath.substring('/snapshotInfo/'.length); + return snapshotServer.serveSnapshotInfo(pageOrFrameId, url.searchParams); } if (relativePath.startsWith('/snapshot/')) { const { snapshotServer } = loadedTraces.get(traceUrl!) || {}; if (!snapshotServer) return new Response(null, { status: 404 }); - const response = snapshotServer.serveSnapshot(relativePath, url.searchParams, url.href); + const pageOrFrameId = relativePath.substring('/snapshot/'.length); + const response = snapshotServer.serveSnapshot(pageOrFrameId, url.searchParams, url.href); if (isDeployedAsHttps) response.headers.set('Content-Security-Policy', 'upgrade-insecure-requests'); return response; @@ -137,7 +139,8 @@ async function doFetch(event: FetchEvent): Promise { const { snapshotServer } = loadedTraces.get(traceUrl!) || {}; if (!snapshotServer) return new Response(null, { status: 404 }); - return snapshotServer.serveClosestScreenshot(relativePath, url.searchParams); + const pageOrFrameId = relativePath.substring('/closest-screenshot/'.length); + return snapshotServer.serveClosestScreenshot(pageOrFrameId, url.searchParams); } if (relativePath.startsWith('/sha1/')) { diff --git a/packages/trace-viewer/src/sw/snapshotServer.ts b/packages/trace-viewer/src/sw/snapshotServer.ts index e1978c79b6..64a8693540 100644 --- a/packages/trace-viewer/src/sw/snapshotServer.ts +++ b/packages/trace-viewer/src/sw/snapshotServer.ts @@ -31,8 +31,8 @@ export class SnapshotServer { this._resourceLoader = resourceLoader; } - serveSnapshot(pathname: string, searchParams: URLSearchParams, snapshotUrl: string): Response { - const snapshot = this._snapshot(pathname.substring('/snapshot'.length), searchParams); + serveSnapshot(pageOrFrameId: string, searchParams: URLSearchParams, snapshotUrl: string): Response { + const snapshot = this._snapshot(pageOrFrameId, searchParams); if (!snapshot) return new Response(null, { status: 404 }); @@ -41,16 +41,16 @@ export class SnapshotServer { return new Response(renderedSnapshot.html, { status: 200, headers: { 'Content-Type': 'text/html; charset=utf-8' } }); } - async serveClosestScreenshot(pathname: string, searchParams: URLSearchParams): Promise { - const snapshot = this._snapshot(pathname.substring('/closest-screenshot'.length), searchParams); + async serveClosestScreenshot(pageOrFrameId: string, searchParams: URLSearchParams): Promise { + const snapshot = this._snapshot(pageOrFrameId, searchParams); const sha1 = snapshot?.closestScreenshot(); if (!sha1) return new Response(null, { status: 404 }); return new Response(await this._resourceLoader(sha1)); } - serveSnapshotInfo(pathname: string, searchParams: URLSearchParams): Response { - const snapshot = this._snapshot(pathname.substring('/snapshotInfo'.length), searchParams); + serveSnapshotInfo(pageOrFrameId: string, searchParams: URLSearchParams): Response { + const snapshot = this._snapshot(pageOrFrameId, searchParams); return this._respondWithJson(snapshot ? { viewport: snapshot.viewport(), url: snapshot.snapshot().frameUrl, @@ -61,9 +61,9 @@ export class SnapshotServer { }); } - private _snapshot(pathname: string, params: URLSearchParams) { + private _snapshot(pageOrFrameId: string, params: URLSearchParams) { const name = params.get('name')!; - return this._snapshotStorage.snapshotByName(pathname.slice(1), name); + return this._snapshotStorage.snapshotByName(pageOrFrameId, name); } private _respondWithJson(object: any): Response { diff --git a/packages/trace-viewer/src/sw/snapshotStorage.ts b/packages/trace-viewer/src/sw/snapshotStorage.ts index d15bfc4f41..a0adadefb1 100644 --- a/packages/trace-viewer/src/sw/snapshotStorage.ts +++ b/packages/trace-viewer/src/sw/snapshotStorage.ts @@ -20,19 +20,19 @@ import type { PageEntry } from '../types/entries'; import { LRUCache } from './lruCache'; export class SnapshotStorage { - private _resources: ResourceSnapshot[] = []; private _frameSnapshots = new Map(); private _cache = new LRUCache(100_000_000); // 100MB per each trace + private _contextToResources = new Map(); - addResource(resource: ResourceSnapshot): void { + addResource(contextId: string, resource: ResourceSnapshot): void { resource.request.url = rewriteURLForCustomProtocol(resource.request.url); - this._resources.push(resource); + this._ensureResourcesForContext(contextId).push(resource); } - addFrameSnapshot(snapshot: FrameSnapshot, screencastFrames: PageEntry['screencastFrames']) { + addFrameSnapshot(contextId: string, snapshot: FrameSnapshot, screencastFrames: PageEntry['screencastFrames']) { for (const override of snapshot.resourceOverrides) override.url = rewriteURLForCustomProtocol(override.url); let frameSnapshots = this._frameSnapshots.get(snapshot.frameId); @@ -46,7 +46,8 @@ export class SnapshotStorage { this._frameSnapshots.set(snapshot.pageId, frameSnapshots); } frameSnapshots.raw.push(snapshot); - const renderer = new SnapshotRenderer(this._cache, this._resources, frameSnapshots.raw, screencastFrames, frameSnapshots.raw.length - 1); + const resources = this._ensureResourcesForContext(contextId); + const renderer = new SnapshotRenderer(this._cache, resources, frameSnapshots.raw, screencastFrames, frameSnapshots.raw.length - 1); frameSnapshots.renderers.push(renderer); return renderer; } @@ -62,6 +63,16 @@ export class SnapshotStorage { finalize() { // Resources are not necessarily sorted in the trace file, so sort them now. - this._resources.sort((a, b) => (a._monotonicTime || 0) - (b._monotonicTime || 0)); + for (const resources of this._contextToResources.values()) + resources.sort((a, b) => (a._monotonicTime || 0) - (b._monotonicTime || 0)); + } + + private _ensureResourcesForContext(contextId: string): ResourceSnapshot[] { + let resources = this._contextToResources.get(contextId); + if (!resources) { + resources = []; + this._contextToResources.set(contextId, resources); + } + return resources; } } diff --git a/packages/trace-viewer/src/sw/traceModel.ts b/packages/trace-viewer/src/sw/traceModel.ts index 8230824f5d..012c7e5bb0 100644 --- a/packages/trace-viewer/src/sw/traceModel.ts +++ b/packages/trace-viewer/src/sw/traceModel.ts @@ -105,7 +105,7 @@ export class TraceModel { this.contextEntries.push(contextEntry); } - this._snapshotStorage!.finalize(); + this._snapshotStorage.finalize(); } async hasEntry(filename: string): Promise { @@ -153,5 +153,6 @@ function createEmptyContext(): ContextEntry { errors: [], stdio: [], hasSource: false, + contextId: '', }; } diff --git a/packages/trace-viewer/src/sw/traceModernizer.ts b/packages/trace-viewer/src/sw/traceModernizer.ts index 6b4b268cb2..16bf3ad2db 100644 --- a/packages/trace-viewer/src/sw/traceModernizer.ts +++ b/packages/trace-viewer/src/sw/traceModernizer.ts @@ -92,6 +92,7 @@ export class TraceModernizer { contextEntry.sdkLanguage = event.sdkLanguage; contextEntry.options = event.options; contextEntry.testIdAttributeName = event.testIdAttributeName; + contextEntry.contextId = event.contextId ?? ''; break; } case 'screencast-frame': { @@ -156,11 +157,11 @@ export class TraceModernizer { break; } case 'resource-snapshot': - this._snapshotStorage.addResource(event.snapshot); + this._snapshotStorage.addResource(this._contextEntry.contextId, event.snapshot); contextEntry.resources.push(event.snapshot); break; case 'frame-snapshot': - this._snapshotStorage.addFrameSnapshot(event.snapshot, this._pageEntry(event.snapshot.pageId).screencastFrames); + this._snapshotStorage.addFrameSnapshot(this._contextEntry.contextId, event.snapshot, this._pageEntry(event.snapshot.pageId).screencastFrames); break; } // Make sure there is a page entry for each page, even without screencast frames, @@ -388,12 +389,13 @@ export class TraceModernizer { wallTime: 0, monotonicTime: 0, sdkLanguage: 'javascript', + contextId: '', }; result.push(event); } for (const event of events) { if (event.type === 'context-options') { - result.push({ ...event, monotonicTime: 0, origin: 'library' }); + result.push({ ...event, monotonicTime: 0, origin: 'library', contextId: '' }); continue; } // Take wall and monotonic time from the first event. diff --git a/packages/trace-viewer/src/types/entries.ts b/packages/trace-viewer/src/types/entries.ts index e7b7a0b977..123bd0b4b9 100644 --- a/packages/trace-viewer/src/types/entries.ts +++ b/packages/trace-viewer/src/types/entries.ts @@ -40,6 +40,7 @@ export type ContextEntry = { stdio: trace.StdioTraceEvent[]; errors: trace.ErrorTraceEvent[]; hasSource: boolean; + contextId: string; }; export type PageEntry = { diff --git a/packages/trace/src/trace.ts b/packages/trace/src/trace.ts index 6c9c7be1a0..65843de1ec 100644 --- a/packages/trace/src/trace.ts +++ b/packages/trace/src/trace.ts @@ -44,6 +44,7 @@ export type ContextCreatedTraceEvent = { options: BrowserContextEventOptions, sdkLanguage?: Language, testIdAttributeName?: string, + contextId?: string, }; export type ScreencastFrameTraceEvent = {