chore(trace): remove snapshot pointers from action events

packages/isomorphic/trace/entries.ts

@@ -39,6 +39,7 @@ export type ContextEntry = {
   actions: ActionEntry[];
   screenshots: trace.ScreenshotTraceEvent[];
   ariaSnapshots: trace.AriaSnapshotTraceEvent[];
+  domSnapshots: { callId: string, phase: trace.ActionPhase }[];
   videos: trace.VideoTraceEvent[];
   events: (trace.EventTraceEvent | trace.ConsoleMessageTraceEvent)[];
   stdio: trace.StdioTraceEvent[];

packages/isomorphic/trace/snapshotRenderer.ts

@@ -326,7 +326,7 @@ function snapshotScript(viewport: ViewportSize, ...targetIds: (string | undefine
     const canvasElements: HTMLCanvasElement[] = [];
 
     let topSnapshotWindow: Window = win;
-    while (topSnapshotWindow !== topSnapshotWindow.parent && !topSnapshotWindow.location.pathname.match(/\/page@[a-z0-9]+$/))
+    while (topSnapshotWindow !== topSnapshotWindow.parent && new URLSearchParams(topSnapshotWindow.location.search).has('frameId'))
       topSnapshotWindow = topSnapshotWindow.parent;
 
     const visit = (root: Document | ShadowRoot) => {
@@ -401,13 +401,12 @@ function snapshotScript(viewport: ViewportSize, ...targetIds: (string | undefine
         if (!src) {
           iframe.setAttribute('src', blankSnapshotUrl);
         } else {
-          // Retain query parameters to inherit name=, time=, pointX=, pointY= and other values from parent.
+          // The attribute value is recorded as `/snapshot/<frameId>` by the snapshotter.
+          const frameId = src.substring(src.lastIndexOf('/') + 1);
+          // All frames of a page share the snapshot name in the path, so we only swap the frame id.
+          // Retain query parameters to inherit time=, pointX=, pointY= and other values from parent.
           const url = new URL(win.location.href);
-          // We can be loading iframe from within iframe, reset base to be absolute.
-          const index = url.pathname.lastIndexOf('/snapshot/');
-          if (index !== -1)
-            url.pathname = url.pathname.substring(0, index + 1);
-          url.pathname += src.substring(1);
+          url.searchParams.set('frameId', frameId);
           iframe.setAttribute('src', url.toString());
         }
       }

packages/isomorphic/trace/snapshotServer.ts

@@ -18,6 +18,7 @@ import type { URLSearchParams } from 'url';
 import type { SnapshotRenderer } from './snapshotRenderer';
 import type { SnapshotStorage } from './snapshotStorage';
 import type { ResourceSnapshot } from '@trace/snapshot';
+import type { ActionPhase } from '@trace/trace';
 
 export class SnapshotServer {
   private _snapshotStorage: SnapshotStorage;
@@ -29,8 +30,8 @@ export class SnapshotServer {
     this._resourceLoader = resourceLoader;
   }
 
-  serveSnapshot(pageOrFrameId: string, searchParams: URLSearchParams, snapshotUrl: string): Response {
-    const snapshot = this._snapshot(pageOrFrameId, searchParams);
+  serveSnapshot(callId: string, searchParams: URLSearchParams, snapshotUrl: string): Response {
+    const snapshot = this._snapshot(callId, searchParams);
     if (!snapshot)
       return new Response(null, { status: 404 });
 
@@ -39,16 +40,16 @@ export class SnapshotServer {
     return new Response(renderedSnapshot.html, { status: 200, headers: { 'Content-Type': 'text/html; charset=utf-8' } });
   }
 
-  async serveClosestScreenshot(pageOrFrameId: string, searchParams: URLSearchParams): Promise<Response> {
-    const snapshot = this._snapshot(pageOrFrameId, searchParams);
+  async serveClosestScreenshot(callId: string, searchParams: URLSearchParams): Promise<Response> {
+    const snapshot = this._snapshot(callId, searchParams);
     const file = snapshot?.closestScreenshot();
     if (!file)
       return new Response(null, { status: 404 });
     return new Response(await this._resourceLoader(file));
   }
 
-  serveSnapshotInfo(pageOrFrameId: string, searchParams: URLSearchParams): Response {
-    const snapshot = this._snapshot(pageOrFrameId, searchParams);
+  serveSnapshotInfo(callId: string, searchParams: URLSearchParams): Response {
+    const snapshot = this._snapshot(callId, searchParams);
     return this._respondWithJson(snapshot ? {
       viewport: snapshot.viewport(),
       url: snapshot.snapshot().frameUrl,
@@ -59,9 +60,8 @@ export class SnapshotServer {
     });
   }
 
-  private _snapshot(pageOrFrameId: string, params: URLSearchParams) {
-    const name = params.get('name')!;
-    return this._snapshotStorage.snapshotByName(pageOrFrameId, name);
+  private _snapshot(callId: string, params: URLSearchParams) {
+    return this._snapshotStorage.snapshotForCall(callId, params.get('phase') as ActionPhase, params.get('frameId') || undefined);
   }
 
   private _respondWithJson(object: any): Response {

packages/isomorphic/trace/snapshotStorage.ts

@@ -18,14 +18,13 @@ import { rewriteURLForCustomProtocol, SnapshotRenderer } from './snapshotRendere
 import { LRUCache } from '../lruCache';
 
 import type { FrameSnapshot, ResourceSnapshot } from '@trace/snapshot';
+import type { ActionPhase } from '@trace/trace';
 import type { PageEntry } from './entries';
 
 
 export class SnapshotStorage {
-  private _frameSnapshots = new Map<string, {
-    raw: FrameSnapshot[],
-    renderers: SnapshotRenderer[],
-  }>();
+  private _snapshotsByFrameId = new Map<string, FrameSnapshot[]>();
+  private _renderersByCallIdAndPhase = new Map<string, SnapshotRenderer[]>();
   private _cache = new LRUCache<SnapshotRenderer, string>(100_000_000);  // 100MB per each trace
   private _resources: ResourceSnapshot[] = [];
   private _resourceUrlsWithOverrides = new Set<string>();
@@ -38,38 +37,41 @@ export class SnapshotStorage {
   addFrameSnapshot(snapshot: FrameSnapshot, screencastFrames: PageEntry['screencastFrames']) {
     for (const override of snapshot.resourceOverrides)
       override.url = rewriteURLForCustomProtocol(override.url);
-    let frameSnapshots = this._frameSnapshots.get(snapshot.frameId);
+    let frameSnapshots = this._snapshotsByFrameId.get(snapshot.frameId);
     if (!frameSnapshots) {
-      frameSnapshots = {
-        raw: [],
-        renderers: [],
-      };
-      this._frameSnapshots.set(snapshot.frameId, frameSnapshots);
-      if (snapshot.isMainFrame)
-        this._frameSnapshots.set(snapshot.pageId, frameSnapshots);
+      frameSnapshots = [];
+      this._snapshotsByFrameId.set(snapshot.frameId, frameSnapshots);
+    }
+    frameSnapshots.push(snapshot);
+    const renderer = new SnapshotRenderer(this._cache, this._resources, frameSnapshots, screencastFrames, frameSnapshots.length - 1);
+    if (snapshot.phase) {
+      const key = callIdAndPhase(snapshot.callId, snapshot.phase);
+      let renderers = this._renderersByCallIdAndPhase.get(key);
+      if (!renderers) {
+        renderers = [];
+        this._renderersByCallIdAndPhase.set(key, renderers);
+      }
+      renderers.push(renderer);
     }
-    frameSnapshots.raw.push(snapshot);
-    const renderer = new SnapshotRenderer(this._cache, this._resources, frameSnapshots.raw, screencastFrames, frameSnapshots.raw.length - 1);
-    frameSnapshots.renderers.push(renderer);
     return renderer;
   }
 
-  snapshotByName(pageOrFrameId: string, snapshotName: string): SnapshotRenderer | undefined {
-    const snapshot = this._frameSnapshots.get(pageOrFrameId);
-    return snapshot?.renderers.find(r => r.snapshotName === snapshotName);
+  snapshotForCall(callId: string, phase: ActionPhase, frameId?: string): SnapshotRenderer | undefined {
+    const renderers = this._renderersByCallIdAndPhase.get(callIdAndPhase(callId, phase)) || [];
+    return renderers.find(r => frameId ? r.snapshot().frameId === frameId : r.snapshot().isMainFrame);
   }
 
   snapshotsForTest() {
-    return [...this._frameSnapshots.keys()];
+    return [...this._renderersByCallIdAndPhase.keys()];
   }
 
   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));
     // Resources that have overrides should not be cached, otherwise we might get stale content
     // while serving snapshots with different override values.
-    for (const frameSnapshots of this._frameSnapshots.values()) {
-      for (const snapshot of frameSnapshots.raw) {
+    for (const frameSnapshots of this._snapshotsByFrameId.values()) {
+      for (const snapshot of frameSnapshots) {
         for (const override of snapshot.resourceOverrides)
           this._resourceUrlsWithOverrides.add(override.url);
       }
@@ -80,3 +82,7 @@ export class SnapshotStorage {
     return this._resourceUrlsWithOverrides.has(url);
   }
 }
+
+function callIdAndPhase(callId: string, phase: ActionPhase): string {
+  return `${callId}/${phase}`;
+}

packages/isomorphic/trace/traceLoader.ts

@@ -152,6 +152,7 @@ function createEmptyContext(): ContextEntry {
     actions: [],
     screenshots: [],
     ariaSnapshots: [],
+    domSnapshots: [],
     videos: [],
     events: [],
     errors: [],

packages/isomorphic/trace/traceModel.ts

@@ -94,6 +94,7 @@ export class TraceModel {
   private _eventsForAction = new Map<ActionEntry, (trace.EventTraceEvent | trace.ConsoleMessageTraceEvent)[]>();
   private _screenshots = new Map<string, trace.ScreenshotTraceEvent>();
   private _ariaSnapshots = new Map<string, trace.AriaSnapshotTraceEvent>();
+  private _domSnapshots = new Set<string>();
 
   constructor(traceUri: string, contexts: ContextEntry[]) {
     const libraryContext = contexts.find(context => context.origin === 'library');
@@ -131,9 +132,11 @@ export class TraceModel {
         this._screenshots.set(`${event.callId}/${event.phase}`, event);
       for (const event of context.ariaSnapshots || [])
         this._ariaSnapshots.set(`${event.callId}/${event.phase}`, event);
+      for (const entry of context.domSnapshots || [])
+        this._domSnapshots.add(`${entry.callId}/${entry.phase}`);
       this.videos.push(...(context.videos || []));
     }
-    this.hasDomSnapshots = this.actions.some(action => !!action.beforeSnapshot || !!action.afterSnapshot || !!action.inputSnapshot);
+    this.hasDomSnapshots = !!this._domSnapshots.size;
     this.hasAriaSnapshots = !!this._screenshots.size || !!this._ariaSnapshots.size;
     this.attachments = this.actions.flatMap(action => action.attachments?.map(attachment => ({ ...attachment, callId: action.callId, traceUri })) ?? []);
     this.visibleAttachments = this.attachments.filter(attachment => !attachment.name.startsWith('_'));
@@ -177,6 +180,10 @@ export class TraceModel {
     return this._screenshots.get(`${callId}/${phase}`);
   }
 
+  hasDomSnapshotForCall(callId: string, phase: trace.ActionPhase): boolean {
+    return this._domSnapshots.has(`${callId}/${phase}`);
+  }
+
   ariaSnapshotForCall(callId: string, phase: trace.ActionPhase): trace.AriaSnapshotTraceEvent | undefined {
     return this._ariaSnapshots.get(`${callId}/${phase}`);
   }

packages/isomorphic/trace/traceModernizer.ts

@@ -21,6 +21,7 @@ import type * as traceV5 from './versions/traceV5';
 import type * as traceV6 from './versions/traceV6';
 import type * as traceV7 from './versions/traceV7';
 import type * as traceV8 from './versions/traceV8';
+import type { FrameSnapshot } from '@trace/snapshot';
 import type { ActionEntry, ContextEntry, PageEntry } from './entries';
 import type { SnapshotStorage } from './snapshotStorage';
 
@@ -48,6 +49,7 @@ export class TraceModernizer {
   private _jsHandles = new Map<string, { preview: string }>();
   private _consoleObjects = new Map<string, { type: string, text: string, location: { url: string, lineNumber: number, columnNumber: number }, args?: { preview: string, value: string }[] }>();
   private _apiRequestRef: string | undefined;
+  private _snapshotPhases = new Map<string, trace.ActionPhase>();
 
   constructor(contextEntry: ContextEntry, snapshotStorage: SnapshotStorage) {
     this._contextEntry = contextEntry;
@@ -63,6 +65,11 @@ export class TraceModernizer {
     return [...this._actionMap.values()];
   }
 
+  private _collectSnapshotPhase(snapshotName: string | undefined, phase: trace.ActionPhase) {
+    if (snapshotName)
+      this._snapshotPhases.set(snapshotName, phase);
+  }
+
   private _pageEntry(pageId: string): PageEntry {
     let pageEntry = this._pageEntries.get(pageId);
     if (!pageEntry) {
@@ -129,7 +136,6 @@ export class TraceModernizer {
       }
       case 'input': {
         const existing = this._actionMap.get(event.callId);
-        existing!.inputSnapshot = event.inputSnapshot;
         existing!.point = event.point;
         existing!.box = event.box;
         break;
@@ -147,7 +153,6 @@ export class TraceModernizer {
       }
       case 'after': {
         const existing = this._actionMap.get(event.callId);
-        existing!.afterSnapshot = event.afterSnapshot;
         existing!.endTime = event.endTime;
         existing!.result = event.result;
         existing!.error = event.error;
@@ -162,6 +167,9 @@ export class TraceModernizer {
         break;
       }
       case 'event': {
+        // Make sure there is a page entry for each page.
+        if ((event.method === 'page' || event.method === 'pageClosed') && event.params?.pageId)
+          this._pageEntry(event.params.pageId);
         contextEntry.events.push(event);
         break;
       }
@@ -185,9 +193,13 @@ export class TraceModernizer {
         this._snapshotStorage.addResource(event.snapshot);
         contextEntry.resources.push(event.snapshot);
         break;
-      case 'frame-snapshot':
-        this._snapshotStorage.addFrameSnapshot(event.snapshot, this._pageEntry(event.snapshot.pageId).screencastFrames);
+      case 'frame-snapshot': {
+        const snapshot = event.snapshot;
+        this._snapshotStorage.addFrameSnapshot(snapshot, this._pageEntry(snapshot.pageId).screencastFrames);
+        if (snapshot.isMainFrame && snapshot.phase)
+          contextEntry.domSnapshots.push({ callId: snapshot.callId, phase: snapshot.phase });
         break;
+      }
     }
     // Make sure there is a page entry for each page, even without screencast frames,
     // to show in the metadata view.
@@ -500,6 +512,19 @@ export class TraceModernizer {
 
   _modernize_8_to_9(events: traceV8.TraceEvent[]): trace.TraceEvent[] {
     for (const event of events) {
+      // Actions used to point at their snapshots by name, now snapshots know their own phase.
+      if (event.type === 'before' || event.type === 'input' || event.type === 'after' || event.type === 'action') {
+        const action = event as traceV8.ActionTraceEvent;
+        this._collectSnapshotPhase(action.beforeSnapshot, 'before');
+        this._collectSnapshotPhase(action.inputSnapshot, 'action');
+        this._collectSnapshotPhase(action.afterSnapshot, 'after');
+        delete action.beforeSnapshot;
+        delete action.inputSnapshot;
+        delete action.afterSnapshot;
+      }
+      if (event.type === 'frame-snapshot' && event.snapshot.snapshotName)
+        (event.snapshot as FrameSnapshot).phase = this._snapshotPhases.get(event.snapshot.snapshotName);
+
       if (event.type !== 'resource-snapshot')
         continue;
       const snapshot = event.snapshot;

packages/playwright-core/src/server/dispatchers/dispatcher.ts

@@ -348,8 +348,6 @@ export class DispatcherConnection {
       internal: validMetadata.internal,
       stepId: validMetadata.stepId,
       objectId: sdkObject.guid,
-      pageId: sdkObject.attribution?.page?.guid,
-      frameId: sdkObject.attribution?.frame?.guid,
       startTime: monotonicTime(),
       endTime: 0,
       type: dispatcher._type,
@@ -440,8 +438,6 @@ export class DispatcherConnection {
         internal: validMetadata.internal,
         stepId: validMetadata.stepId,
         objectId: sdkObject.guid,
-        pageId: sdkObject.attribution?.page?.guid,
-        frameId: sdkObject.attribution?.frame?.guid,
         startTime: monotonicTime(),
         endTime: 0,
         type: dispatcher._type,

packages/playwright-core/src/server/instrumentation.ts

@@ -103,8 +103,6 @@ export type CallMetadata = {
   error?: SerializedError;
   result?: any;
   objectId?: string;
-  pageId?: string;
-  frameId?: string;
 };
 
 export interface Instrumentation {

packages/playwright-core/src/server/page.ts

@@ -259,7 +259,7 @@ export class Page extends SdkObject<PageEventMap> {
     // corresponding Close event after it is reported on the context.
     if (this.isClosed())
       this.emit(Page.Events.Close);
-    else
+    else if (!this.isStorageStatePage)
       this.instrumentation.onPageOpen(this);
 
     // Note: it is important to resolve _initializedPromise at the end,
@@ -315,7 +315,8 @@ export class Page extends SdkObject<PageEventMap> {
     this.emit(Page.Events.Close);
     this.browserContext.emit(BrowserContext.Events.PageClosed, this);
     this.closedPromise.resolve();
-    this.instrumentation.onPageClose(this);
+    if (!this.isStorageStatePage)
+      this.instrumentation.onPageClose(this);
   }
 
   _didCrash() {

packages/playwright-core/src/server/trace/recorder/snapshotter.ts

@@ -29,6 +29,7 @@ import type { RegisteredListener } from '@utils/eventsHelper';
 import type { Frame } from '../../frames';
 import type { InitScript } from '../../page';
 import type { FrameSnapshot } from '@trace/snapshot';
+import type { ActionPhase } from '@trace/trace';
 import type { Progress } from '../../progress';
 
 export type SnapshotterBlob = {
@@ -120,7 +121,7 @@ export class Snapshotter {
     }
   }
 
-  async captureSnapshot(page: Page, callId: string, snapshotName: string, resetTargets: boolean): Promise<void> {
+  async captureSnapshot(page: Page, callId: string, phase: ActionPhase, resetTargets: boolean): Promise<void> {
     // In each frame, in a non-stalling manner, capture the snapshots.
     const snapshots = page.frames().map(async frame => {
       const data = await this._captureFrameSnapshot(frame, resetTargets);
@@ -130,7 +131,7 @@ export class Snapshotter {
 
       const snapshot: FrameSnapshot = {
         callId,
-        snapshotName,
+        phase,
         pageId: page.guid,
         frameId: frame.guid,
         frameUrl: data.url,