From f72b098a040e6b383f5727ba21686e496d74ed60 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 24 Feb 2021 19:29:16 -0800 Subject: [PATCH] chore: encapsulate parsed snapshot id in the trace viewer (#5607) --- src/server/frames.ts | 2 + src/server/page.ts | 4 +- src/server/trace/recorder/snapshotter.ts | 8 +-- src/server/trace/recorder/tracer.ts | 20 ++---- src/server/trace/viewer/snapshotServer.ts | 76 ++++++++--------------- src/server/trace/viewer/traceViewer.ts | 39 ++++++++++-- 6 files changed, 74 insertions(+), 75 deletions(-) diff --git a/src/server/frames.ts b/src/server/frames.ts index 18e59323d9..006916588c 100644 --- a/src/server/frames.ts +++ b/src/server/frames.ts @@ -411,9 +411,11 @@ export class Frame extends SdkObject { private _setContentCounter = 0; readonly _detachedPromise: Promise; 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; diff --git a/src/server/page.ts b/src/server/page.ts index dd7fa21512..fe205978a7 100644 --- a/src/server/page.ts +++ b/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 = () => {}; diff --git a/src/server/trace/recorder/snapshotter.ts b/src/server/trace/recorder/snapshotter.ts index 1099cf42ca..14b4a961df 100644 --- a/src/server/trace/recorder/snapshotter.ts +++ b/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), - frameId: this._delegate.frameId(response.frame()), + pageId: page.traceId, + frameId: response.frame().traceId, url, contentType, responseHeaders: response.headers(), diff --git a/src/server/trace/recorder/tracer.ts b/src/server/trace/recorder/tracer.ts index d2b71ebd2e..5e97c54469 100644 --- a/src/server/trace/recorder/tracer.ts +++ b/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; private _appendEventChain: Promise; @@ -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), - frameId: this.frameId(frame), + pageId: frame._page.traceId, + 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 { 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(); - (page as any)[pageIdSymbol] = pageId; + const pageId = page.traceId; const event: trace.PageCreatedTraceEvent = { timestamp: monotonicTime(), diff --git a/src/server/trace/viewer/snapshotServer.ts b/src/server/trace/viewer/snapshotServer.ts index 979d971c1c..bf32c21537 100644 --- a/src/server/trace/viewer/snapshotServer.ts +++ b/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 _traceModel: TraceModel; + private _urlPrefix: string; + 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//snapshotId// - // - /snapshot/pageId//timestamp// - 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 { - 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 || ''}×tamp=${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 ? - this._traceModel.findSnapshotById(parsed.pageId, parsed.frameId, parsed.snapshotId) : - this._traceModel.findSnapshotByTime(parsed.pageId, parsed.frameId, parsed.timestamp!); + const snapshot = this._snapshotStorage.snapshotByName(parsed.snapshotName); 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); - if (!resource) - return false; + const resource = this._snapshotStorage.resourceById(resourceId); const sha1 = overrideSha1 || resource.responseSha1; try { const content = fs.readFileSync(path.join(this._resourcesDir, sha1)); diff --git a/src/server/trace/viewer/traceViewer.ts b/src/server/trace/viewer/traceViewer.ts index fb7bddccb0..121814d6e8 100644 --- a/src/server/trace/viewer/traceViewer.ts +++ b/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 { SnapshotServer } from './snapshotServer'; +import { NetworkResourceTraceEvent, TraceEvent } from '../common/traceEvents'; +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/". 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//snapshotId// + // - /snapshot/pageId//timestamp// + 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), + }; +}