chore: encapsulate parsed snapshot id in the trace viewer (#5607)

src/server/frames.ts

@@ -411,9 +411,11 @@ export class Frame extends SdkObject {
   private _setContentCounter = 0;
   readonly _detachedPromise: Promise<void>;
   private _detachedCallback = () => {};
+  readonly traceId: string;
 
   constructor(page: Page, id: string, parentFrame: Frame | null) {
     super(page);
+    this.traceId = parentFrame ? `frame@${id}` : page.traceId;
     this.attribution.frame = this;
     this._id = id;
     this._page = page;

src/server/page.ts

@@ -28,7 +28,7 @@ import { ConsoleMessage } from './console';
 import * as accessibility from './accessibility';
 import { FileChooser } from './fileChooser';
 import { ProgressController } from './progress';
-import { assert, isError } from '../utils/utils';
+import { assert, createGuid, isError } from '../utils/utils';
 import { debugLogger } from '../utils/debugLogger';
 import { Selectors } from './selectors';
 import { CallMetadata, SdkObject } from './instrumentation';
@@ -147,9 +147,11 @@ export class Page extends SdkObject {
   _ownedContext: BrowserContext | undefined;
   readonly selectors: Selectors;
   _video: Video | null = null;
+  readonly traceId: string;
 
   constructor(delegate: PageDelegate, browserContext: BrowserContext) {
     super(browserContext);
+    this.traceId = 'page@' + createGuid();
     this.attribution.page = this;
     this._delegate = delegate;
     this._closedCallback = () => {};

src/server/trace/recorder/snapshotter.ts

@@ -46,8 +46,6 @@ export interface SnapshotterDelegate {
   onBlob(blob: SnapshotterBlob): void;
   onResource(resource: SnapshotterResource): void;
   onFrameSnapshot(frame: Frame, frameUrl: string, snapshot: FrameSnapshot, snapshotId?: string): void;
-  pageId(page: Page): string;
-  frameId(frame: Frame): string;
 }
 
 export class Snapshotter {
@@ -116,7 +114,7 @@ export class Snapshotter {
         const context = await parent._mainContext();
         await context.evaluateInternal(({ kSnapshotStreamer, frameElement, frameId }) => {
           (window as any)[kSnapshotStreamer].markIframe(frameElement, frameId);
-        }, { kSnapshotStreamer, frameElement, frameId: this._delegate.frameId(frame) });
+        }, { kSnapshotStreamer, frameElement, frameId: frame.traceId });
         frameElement.dispose();
       } catch (e) {
         // Ignore
@@ -149,8 +147,8 @@ export class Snapshotter {
     const body = await response.body().catch(e => debugLogger.log('error', e));
     const responseSha1 = body ? calculateSha1(body) : 'none';
     const resource: SnapshotterResource = {
-      pageId: this._delegate.pageId(page),
+      pageId: page.traceId,
-      frameId: this._delegate.frameId(response.frame()),
+      frameId: response.frame().traceId,
       url,
       contentType,
       responseHeaders: response.headers(),

src/server/trace/recorder/tracer.ts

@@ -68,7 +68,6 @@ export class Tracer implements InstrumentationListener {
   }
 }
 
-const pageIdSymbol = Symbol('pageId');
 const snapshotsSymbol = Symbol('snapshots');
 
 // This is an official way to pass snapshots between onBefore/AfterInputAction and onAfterCall.
@@ -79,7 +78,6 @@ function snapshotsForMetadata(metadata: CallMetadata): { name: string, snapshotI
 }
 
 class ContextTracer implements SnapshotterDelegate {
-  private _context: BrowserContext;
   private _contextId: string;
   private _traceStoragePromise: Promise<string>;
   private _appendEventChain: Promise<string>;
@@ -90,7 +88,6 @@ class ContextTracer implements SnapshotterDelegate {
   private _traceFile: string;
 
   constructor(context: BrowserContext, traceStorageDir: string, traceFile: string) {
-    this._context = context;
     this._contextId = 'context@' + createGuid();
     this._traceFile = traceFile;
     this._traceStoragePromise = mkdirIfNeeded(path.join(traceStorageDir, 'sha1')).then(() => traceStorageDir);
@@ -143,8 +140,8 @@ class ContextTracer implements SnapshotterDelegate {
       timestamp: monotonicTime(),
       type: 'snapshot',
       contextId: this._contextId,
-      pageId: this.pageId(frame._page),
+      pageId: frame._page.traceId,
-      frameId: this.frameId(frame),
+      frameId: frame.traceId,
       snapshot: snapshot,
       frameUrl,
       snapshotId,
@@ -152,14 +149,6 @@ class ContextTracer implements SnapshotterDelegate {
     this._appendTraceEvent(event);
   }
 
-  pageId(page: Page): string {
-    return (page as any)[pageIdSymbol];
-  }
-
-  frameId(frame: Frame): string {
-    return frame._page.mainFrame() === frame ? this.pageId(frame._page) : frame._id;
-  }
-
   async onActionCheckpoint(name: string, sdkObject: SdkObject, metadata: CallMetadata): Promise<void> {
     if (!sdkObject.attribution.page)
       return;
@@ -175,7 +164,7 @@ class ContextTracer implements SnapshotterDelegate {
       timestamp: monotonicTime(),
       type: 'action',
       contextId: this._contextId,
-      pageId: this.pageId(sdkObject.attribution.page),
+      pageId: sdkObject.attribution.page.traceId,
       objectType: metadata.type,
       method: metadata.method,
       // FIXME: filter out evaluation snippets, binary
@@ -191,8 +180,7 @@ class ContextTracer implements SnapshotterDelegate {
   }
 
   private _onPage(page: Page) {
-    const pageId = 'page@' + createGuid();
+    const pageId = page.traceId;
-    (page as any)[pageIdSymbol] = pageId;
 
     const event: trace.PageCreatedTraceEvent = {
       timestamp: monotonicTime(),

src/server/trace/viewer/snapshotServer.ts

@@ -18,20 +18,25 @@ import * as http from 'http';
 import fs from 'fs';
 import path from 'path';
 import querystring from 'querystring';
-import type { TraceModel } from './traceModel';
 import { TraceServer } from './traceServer';
-import type { SerializedFrameSnapshot } from './frameSnapshot';
+import type { FrameSnapshot, SerializedFrameSnapshot } from './frameSnapshot';
+import type { NetworkResourceTraceEvent } from '../common/traceEvents';
+
+export interface SnapshotStorage {
+  resourceById(resourceId: string): NetworkResourceTraceEvent;
+  snapshotByName(snapshotName: string): FrameSnapshot | undefined;
+}
 
 export class SnapshotServer {
   private _resourcesDir: string | undefined;
-  private _server: TraceServer;
+  private _urlPrefix: string;
-  private _traceModel: TraceModel;
+  private _snapshotStorage: SnapshotStorage;
 
-  constructor(server: TraceServer, traceModel: TraceModel, resourcesDir: string | undefined) {
+  constructor(server: TraceServer, snapshotStorage: SnapshotStorage, resourcesDir: string | undefined) {
     this._resourcesDir = resourcesDir;
-    this._server = server;
+    this._urlPrefix = server.urlPrefix();
+    this._snapshotStorage = snapshotStorage;
 
-    this._traceModel = traceModel;
     server.routePath('/snapshot/', this._serveSnapshotRoot.bind(this), true);
     server.routePath('/snapshot/service-worker.js', this._serveServiceWorker.bind(this));
     server.routePath('/snapshot-data', this._serveSnapshot.bind(this));
@@ -39,15 +44,15 @@ export class SnapshotServer {
   }
 
   snapshotRootUrl() {
-    return this._server.urlPrefix() + '/snapshot/';
+    return this._urlPrefix + '/snapshot/';
   }
 
   snapshotUrl(pageId: string, snapshotId?: string, timestamp?: number) {
     // Prefer snapshotId over timestamp.
     if (snapshotId)
-      return this._server.urlPrefix() + `/snapshot/pageId/${pageId}/snapshotId/${snapshotId}/main`;
+      return this._urlPrefix + `/snapshot/pageId/${pageId}/snapshotId/${snapshotId}/main`;
     if (timestamp)
-      return this._server.urlPrefix() + `/snapshot/pageId/${pageId}/timestamp/${timestamp}/main`;
+      return this._urlPrefix + `/snapshot/pageId/${pageId}/timestamp/${timestamp}/main`;
     return 'data:text/html,Snapshot is not available';
   }
 
@@ -114,25 +119,6 @@ export class SnapshotServer {
         event.waitUntil(self.clients.claim());
       });
 
-      function parseUrl(urlString: string): { pageId: string, frameId: string, timestamp?: number, snapshotId?: string } {
-        const url = new URL(urlString);
-        const parts = url.pathname.split('/');
-        if (!parts[0])
-          parts.shift();
-        if (!parts[parts.length - 1])
-          parts.pop();
-        // - /snapshot/pageId/<pageId>/snapshotId/<snapshotId>/<frameId>
-        // - /snapshot/pageId/<pageId>/timestamp/<timestamp>/<frameId>
-        if (parts.length !== 6 || parts[0] !== 'snapshot' || parts[1] !== 'pageId' || (parts[3] !== 'snapshotId' && parts[3] !== 'timestamp'))
-          throw new Error(`Unexpected url "${urlString}"`);
-        return {
-          pageId: parts[2],
-          frameId: parts[5] === 'main' ? parts[2] : parts[5],
-          snapshotId: (parts[3] === 'snapshotId' ? parts[4] : undefined),
-          timestamp: (parts[3] === 'timestamp' ? +parts[4] : undefined),
-        };
-      }
-
       function respond404(): Response {
         return new Response(null, { status: 404 });
       }
@@ -152,33 +138,29 @@ export class SnapshotServer {
       }
 
       async function doFetch(event: any /* FetchEvent */): Promise<Response> {
-        try {
-          const pathname = new URL(event.request.url).pathname;
-          if (pathname === '/snapshot/service-worker.js' || pathname === '/snapshot/')
-            return fetch(event.request);
-        } catch (e) {
-        }
-
         const request = event.request;
-        let parsed: { pageId: string, frameId: string, timestamp?: number, snapshotId?: string };
+        const pathname = new URL(request.url).pathname;
+        if (pathname === '/snapshot/service-worker.js' || pathname === '/snapshot/')
+          return fetch(event.request);
+
+        let snapshotId: string;
         if (request.mode === 'navigate') {
-          parsed = parseUrl(request.url);
+          snapshotId = pathname;
         } else {
           const client = (await self.clients.get(event.clientId))!;
-          parsed = parseUrl(client.url);
+          snapshotId = new URL(client.url).pathname;
         }
-
         if (request.mode === 'navigate') {
-          const htmlResponse = await fetch(`/snapshot-data?pageId=${parsed.pageId}&snapshotId=${parsed.snapshotId || ''}&timestamp=${parsed.timestamp || ''}&frameId=${parsed.frameId || ''}`);
+          const htmlResponse = await fetch(`/snapshot-data?snapshotName=${snapshotId}`);
           const { html, resources }: SerializedFrameSnapshot  = await htmlResponse.json();
           if (!html)
             return respondNotAvailable();
-          snapshotResources.set(parsed.snapshotId + '@' + parsed.timestamp, resources);
+          snapshotResources.set(snapshotId, resources);
           const response = new Response(html, { status: 200, headers: { 'Content-Type': 'text/html' } });
           return response;
         }
 
-        const resources = snapshotResources.get(parsed.snapshotId + '@' + parsed.timestamp)!;
+        const resources = snapshotResources.get(snapshotId)!;
         const urlWithoutHash = removeHash(request.url);
         const resource = resources[urlWithoutHash];
         if (!resource)
@@ -223,9 +205,7 @@ export class SnapshotServer {
     response.setHeader('Cache-Control', 'public, max-age=31536000');
     response.setHeader('Content-Type', 'application/json');
     const parsed: any = querystring.parse(request.url!.substring(request.url!.indexOf('?') + 1));
-    const snapshot = parsed.snapshotId ?
+    const snapshot = this._snapshotStorage.snapshotByName(parsed.snapshotName);
-      this._traceModel.findSnapshotById(parsed.pageId, parsed.frameId, parsed.snapshotId) :
-      this._traceModel.findSnapshotByTime(parsed.pageId, parsed.frameId, parsed.timestamp!);
     const snapshotData: any = snapshot ? snapshot.serialize() : { html: '' };
     response.end(JSON.stringify(snapshotData));
     return true;
@@ -256,9 +236,7 @@ export class SnapshotServer {
       return false;
     }
 
-    const resource = this._traceModel.resourceById.get(resourceId);
+    const resource = this._snapshotStorage.resourceById(resourceId);
-    if (!resource)
-      return false;
     const sha1 = overrideSha1 || resource.responseSha1;
     try {
       const content = fs.readFileSync(path.join(this._resourcesDir, sha1));

src/server/trace/viewer/traceViewer.ts

@@ -20,9 +20,10 @@ import * as playwright from '../../../..';
 import * as util from 'util';
 import { ScreenshotGenerator } from './screenshotGenerator';
 import { TraceModel } from './traceModel';
-import type { TraceEvent } from '../common/traceEvents';
+import { NetworkResourceTraceEvent, TraceEvent } from '../common/traceEvents';
-import { SnapshotServer } from './snapshotServer';
+import { SnapshotServer, SnapshotStorage } from './snapshotServer';
 import { ServerRouteHandler, TraceServer } from './traceServer';
+import { FrameSnapshot } from './frameSnapshot';
 
 const fsReadFileAsync = util.promisify(fs.readFile.bind(fs));
 
@@ -33,7 +34,7 @@ type TraceViewerDocument = {
 
 const emptyModel: TraceModel = new TraceModel();
 
-class TraceViewer {
+class TraceViewer implements SnapshotStorage {
   private _document: TraceViewerDocument | undefined;
 
   async load(traceDir: string) {
@@ -74,7 +75,7 @@ class TraceViewer {
     //   and translates them into "/resources/<resourceId>".
 
     const server = new TraceServer(this._document ? this._document.model : emptyModel);
-    const snapshotServer = new SnapshotServer(server, this._document ? this._document.model : emptyModel, this._document ? this._document.resourcesDir : undefined);
+    const snapshotServer = new SnapshotServer(server, this, this._document ? this._document.resourcesDir : undefined);
     const screenshotGenerator = this._document ? new ScreenshotGenerator(snapshotServer, this._document.resourcesDir, this._document.model) : undefined;
 
     const traceViewerHandler: ServerRouteHandler = (request, response) => {
@@ -132,6 +133,18 @@ class TraceViewer {
     uiPage.on('close', () => process.exit(0));
     await uiPage.goto(urlPrefix + '/traceviewer/traceViewer/index.html');
   }
+
+  resourceById(resourceId: string): NetworkResourceTraceEvent {
+    const traceModel = this._document!.model;
+    return traceModel.resourceById.get(resourceId)!;
+  }
+
+  snapshotByName(snapshotName: string): FrameSnapshot | undefined {
+    const traceModel = this._document!.model;
+    const parsed = parseSnapshotName(snapshotName);
+    const snapshot = parsed.snapshotId ? traceModel.findSnapshotById(parsed.pageId, parsed.frameId, parsed.snapshotId) : traceModel.findSnapshotByTime(parsed.pageId, parsed.frameId, parsed.timestamp!);
+    return snapshot;
+  }
 }
 
 export async function showTraceViewer(traceDir: string) {
@@ -140,3 +153,21 @@ export async function showTraceViewer(traceDir: string) {
     await traceViewer.load(traceDir);
   await traceViewer.show();
 }
+
+function parseSnapshotName(pathname: string): { pageId: string, frameId: string, timestamp?: number, snapshotId?: string } {
+  const parts = pathname.split('/');
+  if (!parts[0])
+    parts.shift();
+  if (!parts[parts.length - 1])
+    parts.pop();
+  // - /snapshot/pageId/<pageId>/snapshotId/<snapshotId>/<frameId>
+  // - /snapshot/pageId/<pageId>/timestamp/<timestamp>/<frameId>
+  if (parts.length !== 6 || parts[0] !== 'snapshot' || parts[1] !== 'pageId' || (parts[3] !== 'snapshotId' && parts[3] !== 'timestamp'))
+    throw new Error(`Unexpected path "${pathname}"`);
+  return {
+    pageId: parts[2],
+    frameId: parts[5] === 'main' ? parts[2] : parts[5],
+    snapshotId: (parts[3] === 'snapshotId' ? parts[4] : undefined),
+    timestamp: (parts[3] === 'timestamp' ? +parts[4] : undefined),
+  };
+}