From 76150f1bcbc2c446462477895aedfcf019e56cd6 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 10 Aug 2021 17:06:14 -0700 Subject: [PATCH] chore(tracing): remove proactive snapshotSizes caching (#8126) --- src/server/snapshot/snapshotRenderer.ts | 4 ++ src/server/snapshot/snapshotServer.ts | 25 ++++++++--- src/server/trace/viewer/traceModel.ts | 4 -- src/web/traceViewer/ui/snapshotTab.tsx | 57 ++++++++++++++----------- src/web/traceViewer/ui/workbench.tsx | 3 +- 5 files changed, 56 insertions(+), 37 deletions(-) diff --git a/src/server/snapshot/snapshotRenderer.ts b/src/server/snapshot/snapshotRenderer.ts index 794a4a747a..8979745740 100644 --- a/src/server/snapshot/snapshotRenderer.ts +++ b/src/server/snapshot/snapshotRenderer.ts @@ -33,6 +33,10 @@ export class SnapshotRenderer { return this._snapshots[this._index]; } + viewport(): { width: number, height: number } { + return this._snapshots[this._index].viewport; + } + render(): RenderedFrameSnapshot { const visit = (n: NodeSnapshot, snapshotIndex: number): string => { // Text node. diff --git a/src/server/snapshot/snapshotServer.ts b/src/server/snapshot/snapshotServer.ts index 9475919590..8982405025 100644 --- a/src/server/snapshot/snapshotServer.ts +++ b/src/server/snapshot/snapshotServer.ts @@ -28,6 +28,7 @@ export class SnapshotServer { this._snapshotStorage = snapshotStorage; server.routePrefix('/snapshot/', this._serveSnapshot.bind(this)); + server.routePrefix('/snapshotSize/', this._serveSnapshotSize.bind(this)); server.routePrefix('/resources/', this._serveResource.bind(this)); } @@ -152,16 +153,28 @@ export class SnapshotServer { return this._serveSnapshotRoot(request, response); if (request.url!.endsWith('/snapshot/service-worker.js')) return this._serveServiceWorker(request, response); + const snapshot = this._snapshot(request.url!.substring('/snapshot/'.length)); + this._respondWithJson(response, snapshot ? snapshot.render() : { html: '' }); + return true; + } + private _serveSnapshotSize(request: http.IncomingMessage, response: http.ServerResponse): boolean { + const snapshot = this._snapshot(request.url!.substring('/snapshotSize/'.length)); + this._respondWithJson(response, snapshot ? snapshot.viewport() : {}); + return true; + } + + private _snapshot(uri: string) { + const [ pageOrFrameId, query ] = uri.split('?'); + const parsed: any = querystring.parse(query); + return this._snapshotStorage.snapshotByName(pageOrFrameId, parsed.name); + } + + private _respondWithJson(response: http.ServerResponse, object: any) { response.statusCode = 200; response.setHeader('Cache-Control', 'public, max-age=31536000'); response.setHeader('Content-Type', 'application/json'); - const [ pageOrFrameId, query ] = request.url!.substring('/snapshot/'.length).split('?'); - const parsed: any = querystring.parse(query); - const snapshot = this._snapshotStorage.snapshotByName(pageOrFrameId, parsed.name); - const snapshotData: any = snapshot ? snapshot.render() : { html: '' }; - response.end(JSON.stringify(snapshotData)); - return true; + response.end(JSON.stringify(object)); } private _serveResource(request: http.IncomingMessage, response: http.ServerResponse): boolean { diff --git a/src/server/trace/viewer/traceModel.ts b/src/server/trace/viewer/traceModel.ts index 4e7f56fae1..4098cc2fb8 100644 --- a/src/server/trace/viewer/traceModel.ts +++ b/src/server/trace/viewer/traceModel.ts @@ -38,7 +38,6 @@ export class TraceModel { options: { sdkLanguage: '' }, pages: [], resources: [], - snapshotSizes: {}, }; } @@ -98,8 +97,6 @@ export class TraceModel { break; case 'frame-snapshot': this._snapshotStorage.addFrameSnapshot(event.snapshot); - if (event.snapshot.snapshotName && event.snapshot.isMainFrame) - this.contextEntry.snapshotSizes[event.snapshot.snapshotName] = event.snapshot.viewport; break; } if (event.type === 'action' || event.type === 'event') { @@ -142,7 +139,6 @@ export type ContextEntry = { options: BrowserContextOptions; pages: PageEntry[]; resources: ResourceSnapshot[]; - snapshotSizes: { [snapshotName: string]: { width: number, height: number } }; }; export type PageEntry = { diff --git a/src/web/traceViewer/ui/snapshotTab.tsx b/src/web/traceViewer/ui/snapshotTab.tsx index 960fb83535..e9ea79280d 100644 --- a/src/web/traceViewer/ui/snapshotTab.tsx +++ b/src/web/traceViewer/ui/snapshotTab.tsx @@ -19,14 +19,12 @@ import './snapshotTab.css'; import './tabbedPane.css'; import * as React from 'react'; import { useMeasure } from './helpers'; -import type { Point } from '../../../common/types'; import { ActionTraceEvent } from '../../../server/trace/common/traceEvents'; export const SnapshotTab: React.FunctionComponent<{ action: ActionTraceEvent | undefined, - snapshotSizes: { [snapshotName: string]: Size }, defaultSnapshotSize: Size, -}> = ({ action, snapshotSizes, defaultSnapshotSize }) => { +}> = ({ action, defaultSnapshotSize }) => { const [measure, ref] = useMeasure(); const [snapshotIndex, setSnapshotIndex] = React.useState(0); @@ -36,35 +34,44 @@ export const SnapshotTab: React.FunctionComponent<{ const actionSnapshot = snapshotMap.get('action') || snapshotMap.get('after'); const snapshots = [actionSnapshot ? { ...actionSnapshot, title: 'action' } : undefined, snapshotMap.get('before'), snapshotMap.get('after')].filter(Boolean) as { title: string, snapshotName: string }[]; + let snapshotUrl = 'data:text/html,'; + let snapshotSizeUrl: string | undefined; + let pointX: number | undefined; + let pointY: number | undefined; + if (action) { + const snapshot = snapshots[snapshotIndex]; + if (snapshot && snapshot.snapshotName) { + snapshotUrl = `${window.location.origin}/snapshot/${action.metadata.pageId}?name=${snapshot.snapshotName}`; + snapshotSizeUrl = `${window.location.origin}/snapshotSize/${action.metadata.pageId}?name=${snapshot.snapshotName}`; + if (snapshot.snapshotName.includes('action')) { + pointX = action.metadata.point?.x; + pointY = action.metadata.point?.y; + } + } + } + React.useEffect(() => { if (snapshots.length >= 1 && snapshotIndex >= snapshots.length) setSnapshotIndex(snapshots.length - 1); }, [snapshotIndex, snapshots]); - const iframeRef = React.createRef(); + const iframeRef = React.useRef(null); + const [snapshotSize, setSnapshotSize] = React.useState(defaultSnapshotSize); React.useEffect(() => { - if (!iframeRef.current) - return; - let snapshotUri = undefined; - let point: Point | undefined = undefined; - if (action) { - const snapshot = snapshots[snapshotIndex]; - if (snapshot && snapshot.snapshotName) { - snapshotUri = `${action.metadata.pageId}?name=${snapshot.snapshotName}`; - if (snapshot.snapshotName.includes('action')) - point = action.metadata.point; + (async () => { + if (snapshotSizeUrl) { + const response = await fetch(snapshotSizeUrl); + setSnapshotSize(await response.json()); } - } - const snapshotUrl = snapshotUri ? `${window.location.origin}/snapshot/${snapshotUri}` : 'data:text/html,'; - try { - (iframeRef.current.contentWindow as any).showSnapshot(snapshotUrl, { point }); - } catch (e) { - } - }, [action, snapshotIndex, iframeRef, snapshots]); - - let snapshotSize = defaultSnapshotSize; - if (snapshots[snapshotIndex] && snapshots[snapshotIndex].snapshotName) - snapshotSize = snapshotSizes[snapshots[snapshotIndex].snapshotName] || defaultSnapshotSize; + if (!iframeRef.current) + return; + try { + const point = pointX === undefined ? undefined : { x: pointX, y: pointY }; + (iframeRef.current.contentWindow as any).showSnapshot(snapshotUrl, { point }); + } catch (e) { + } + })(); + }, [iframeRef, snapshotUrl, snapshotSizeUrl, pointX, pointY]); const scale = Math.min(measure.width / snapshotSize.width, measure.height / snapshotSize.height); const scaledSize = { diff --git a/src/web/traceViewer/ui/workbench.tsx b/src/web/traceViewer/ui/workbench.tsx index 69b61b9ad1..50b7fdbab0 100644 --- a/src/web/traceViewer/ui/workbench.tsx +++ b/src/web/traceViewer/ui/workbench.tsx @@ -89,7 +89,7 @@ export const Workbench: React.FunctionComponent<{ - + }, { id: 'console', title: 'Console', count: consoleCount, render: () => }, @@ -125,5 +125,4 @@ const emptyContext: ContextEntry = { }, pages: [], resources: [], - snapshotSizes: {}, };