chore: simplify trace sw

Author: pavelfeldman

packages/playwright-core/src/cli/program.ts

@@ -375,13 +375,13 @@ program
     });
 
 program
-    .command('show-trace [trace...]')
+    .command('show-trace [trace]')
     .option('-b, --browser <browserType>', 'browser to use, one of cr, chromium, ff, firefox, wk, webkit', 'chromium')
     .option('-h, --host <host>', 'Host to serve trace on; specifying this option opens trace in a browser tab')
     .option('-p, --port <port>', 'Port to serve trace on, 0 for any free port; specifying this option opens trace in a browser tab')
     .option('--stdin', 'Accept trace URLs over stdin to update the viewer')
     .description('show trace viewer')
-    .action(function(traces, options) {
+    .action(function(trace, options) {
       if (options.browser === 'cr')
         options.browser = 'chromium';
       if (options.browser === 'ff')
@@ -396,9 +396,9 @@ program
       };
 
       if (options.port !== undefined || options.host !== undefined)
-        runTraceInBrowser(traces, openOptions).catch(logErrorAndExit);
+        runTraceInBrowser(trace, openOptions).catch(logErrorAndExit);
       else
-        runTraceViewerApp(traces, options.browser, openOptions, true).catch(logErrorAndExit);
+        runTraceViewerApp(trace, options.browser, openOptions, true).catch(logErrorAndExit);
     }).addHelpText('afterAll', `
 Examples:
 

packages/playwright-core/src/server/trace/viewer/traceViewer.ts

@@ -52,16 +52,16 @@ export type TraceViewerAppOptions = {
   persistentContextOptions?: Parameters<BrowserType['launchPersistentContext']>[2];
 };
 
-function validateTraceUrls(traceUrls: string[]) {
-  for (const traceUrl of traceUrls) {
-    let traceFile = traceUrl;
-    // If .json is requested, we'll synthesize it.
-    if (traceUrl.endsWith('.json'))
-      traceFile = traceUrl.substring(0, traceUrl.length - '.json'.length);
-
-    if (!traceUrl.startsWith('http://') && !traceUrl.startsWith('https://') && !fs.existsSync(traceFile) && !fs.existsSync(traceFile + '.trace'))
-      throw new Error(`Trace file ${traceUrl} does not exist!`);
-  }
+function validateTraceUrl(traceUrl: string | undefined) {
+  if (!traceUrl)
+    return;
+  let traceFile = traceUrl;
+  // If .json is requested, we'll synthesize it.
+  if (traceUrl.endsWith('.json'))
+    traceFile = traceUrl.substring(0, traceUrl.length - '.json'.length);
+
+  if (!traceUrl.startsWith('http://') && !traceUrl.startsWith('https://') && !fs.existsSync(traceFile) && !fs.existsSync(traceFile + '.trace'))
+    throw new Error(`Trace file ${traceUrl} does not exist!`);
 }
 
 export async function startTraceViewerServer(options?: TraceViewerServerOptions): Promise<HttpServer> {
@@ -108,11 +108,11 @@ export async function startTraceViewerServer(options?: TraceViewerServerOptions)
   return server;
 }
 
-export async function installRootRedirect(server: HttpServer, traceUrls: string[], options: TraceViewerRedirectOptions) {
+export async function installRootRedirect(server: HttpServer, traceUrl: string | undefined, options: TraceViewerRedirectOptions) {
   const params = new URLSearchParams();
   if (path.sep !== path.posix.sep)
     params.set('pathSeparator', path.sep);
-  for (const traceUrl of traceUrls)
+  if (traceUrl)
     params.append('trace', traceUrl);
   if (server.wsGuid())
     params.append('ws', server.wsGuid()!);
@@ -146,20 +146,20 @@ export async function installRootRedirect(server: HttpServer, traceUrls: string[
   });
 }
 
-export async function runTraceViewerApp(traceUrls: string[], browserName: string, options: TraceViewerServerOptions & { headless?: boolean }, exitOnClose?: boolean) {
-  validateTraceUrls(traceUrls);
+export async function runTraceViewerApp(traceUrl: string | undefined, browserName: string, options: TraceViewerServerOptions & { headless?: boolean }, exitOnClose?: boolean) {
+  validateTraceUrl(traceUrl);
   const server = await startTraceViewerServer(options);
-  await installRootRedirect(server, traceUrls, options);
+  await installRootRedirect(server, traceUrl, options);
   const page = await openTraceViewerApp(server.urlPrefix('precise'), browserName, options);
   if (exitOnClose)
     page.on('close', () => gracefullyProcessExitDoNotHang(0));
   return page;
 }
 
-export async function runTraceInBrowser(traceUrls: string[], options: TraceViewerServerOptions) {
-  validateTraceUrls(traceUrls);
+export async function runTraceInBrowser(traceUrl: string | undefined, options: TraceViewerServerOptions) {
+  validateTraceUrl(traceUrl);
   const server = await startTraceViewerServer(options);
-  await installRootRedirect(server, traceUrls, options);
+  await installRootRedirect(server, traceUrl, options);
   await openTraceInBrowser(server.urlPrefix('human-readable'));
 }
 

packages/playwright/src/runner/testServer.ts

@@ -262,7 +262,7 @@ export class TestServerDispatcher implements TestServerInterface {
 export async function runUIMode(configFile: string | undefined, configCLIOverrides: ConfigCLIOverrides, options: TraceViewerServerOptions & TraceViewerRedirectOptions): Promise<reporterTypes.FullResult['status']> {
   const configLocation = resolveConfigLocation(configFile);
   return await innerRunTestServer(configLocation, configCLIOverrides, options, async (server: HttpServer, cancelPromise: ManualPromise<void>) => {
-    await installRootRedirect(server, [], { ...options, webApp: 'uiMode.html' });
+    await installRootRedirect(server, undefined, { ...options, webApp: 'uiMode.html' });
     if (options.host !== undefined || options.port !== undefined) {
       await openTraceInBrowser(server.urlPrefix('human-readable'));
     } else {

packages/trace-viewer/src/sw/main.ts

@@ -36,19 +36,19 @@ const scopePath = new URL(self.registration.scope).pathname;
 
 const loadedTraces = new Map<string, { traceModel: TraceModel, snapshotServer: SnapshotServer }>();
 
-const clientIdToTraceUrls = new Map<string, { limit: number | undefined, traceUrls: Set<string>, traceViewerServer: TraceViewerServer }>();
+type ClientData = {
+  traceUrl: string;
+  traceViewerServer: TraceViewerServer
+};
+const clientIdToTraceUrls = new Map<string, ClientData>();
 
-async function loadTrace(traceUrl: string, traceFileName: string | null, client: any | undefined, limit: number | undefined, progress: (done: number, total: number) => undefined): Promise<TraceModel> {
-  await gc();
+async function loadTrace(traceUrl: string, traceFileName: string | null, client: any | undefined, progress: (done: number, total: number) => undefined): Promise<TraceModel> {
   const clientId = client?.id ?? '';
-  let data = clientIdToTraceUrls.get(clientId);
-  if (!data) {
-    const clientURL = new URL(client?.url ?? self.registration.scope);
-    const traceViewerServerBaseUrl = new URL(clientURL.searchParams.get('server') ?? '../', clientURL);
-    data = { limit, traceUrls: new Set(), traceViewerServer: new TraceViewerServer(traceViewerServerBaseUrl) };
-    clientIdToTraceUrls.set(clientId, data);
-  }
-  data.traceUrls.add(traceUrl);
+  const clientURL = new URL(client?.url ?? self.registration.scope);
+  const traceViewerServerBaseUrl = new URL(clientURL.searchParams.get('server') ?? '../', clientURL);
+  const data: ClientData = { traceUrl, traceViewerServer: new TraceViewerServer(traceViewerServerBaseUrl) };
+  clientIdToTraceUrls.set(clientId, data);
+  await gc();
 
   const traceModel = new TraceModel();
   try {
@@ -106,8 +106,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
 
     if (relativePath === '/contexts') {
       try {
-        const limit = url.searchParams.has('limit') ? +url.searchParams.get('limit')! : undefined;
-        const traceModel = await loadTrace(traceUrl!, url.searchParams.get('traceFileName'), client, limit, (done: number, total: number) => {
+        const traceModel = await loadTrace(traceUrl!, url.searchParams.get('traceFileName'), client, (done: number, total: number) => {
           client.postMessage({ method: 'progress', params: { done, total } });
         });
         return new Response(JSON.stringify(traceModel!.contextEntries), {
@@ -209,12 +208,7 @@ async function gc() {
       clientIdToTraceUrls.delete(clientId);
       continue;
     }
-    if (data.limit !== undefined) {
-      const ordered = [...data.traceUrls];
-      // Leave the newest requested traces.
-      data.traceUrls = new Set(ordered.slice(ordered.length - data.limit));
-    }
-    data.traceUrls.forEach(url => usedTraces.add(url));
+    usedTraces.add(data.traceUrl);
   }
 
   for (const traceUrl of loadedTraces.keys()) {

packages/trace-viewer/src/sw/traceModel.ts

@@ -61,7 +61,6 @@ export class TraceModel {
     let done = 0;
     for (const ordinal of ordinals) {
       const contextEntry = createEmptyContext();
-      contextEntry.traceUrl = backend.traceURL();
       contextEntry.hasSource = hasSource;
       const modernizer = new TraceModernizer(contextEntry, this._snapshotStorage);
 
@@ -138,7 +137,6 @@ function stripEncodingFromContentType(contentType: string) {
 function createEmptyContext(): ContextEntry {
   return {
     origin: 'testRunner',
-    traceUrl: '',
     startTime: Number.MAX_SAFE_INTEGER,
     wallTime: Number.MAX_SAFE_INTEGER,
     endTime: 0,

packages/trace-viewer/src/types/entries.ts

@@ -22,7 +22,6 @@ import type * as trace from '@trace/trace';
 
 export type ContextEntry = {
   origin: 'testRunner'|'library';
-  traceUrl: string;
   startTime: number;
   endTime: number;
   browserName: string;

packages/trace-viewer/src/ui/liveWorkbenchLoader.tsx

@@ -36,7 +36,7 @@ export const LiveWorkbenchLoader: React.FC<{ traceJson: string }> = ({ traceJson
         const model = await loadSingleTraceFile(traceJson);
         setModel(model);
       } catch {
-        const model = new MultiTraceModel([]);
+        const model = new MultiTraceModel('', []);
         setModel(model);
       } finally {
         setCounter(counter + 1);
@@ -54,8 +54,7 @@ export const LiveWorkbenchLoader: React.FC<{ traceJson: string }> = ({ traceJson
 async function loadSingleTraceFile(traceJson: string): Promise<MultiTraceModel> {
   const params = new URLSearchParams();
   params.set('trace', traceJson);
-  params.set('limit', '1');
   const response = await fetch(`contexts?${params.toString()}`);
   const contextEntries = await response.json() as ContextEntry[];
-  return new MultiTraceModel(contextEntries);
+  return new MultiTraceModel(traceJson, contextEntries);
 }

packages/trace-viewer/src/ui/modelUtil.ts

@@ -85,12 +85,14 @@ export class MultiTraceModel {
   readonly sources: Map<string, SourceModel>;
   resources: ResourceSnapshot[];
   readonly actionCounters: Map<string, number>;
+  readonly traceUrl: string;
 
 
-  constructor(contexts: ContextEntry[]) {
+  constructor(traceUrl: string, contexts: ContextEntry[]) {
     contexts.forEach(contextEntry => indexModel(contextEntry));
     const libraryContext = contexts.find(context => context.origin === 'library');
 
+    this.traceUrl = traceUrl;
     this.browserName = libraryContext?.browserName || '';
     this.sdkLanguage = libraryContext?.sdkLanguage;
     this.channel = libraryContext?.channel;
@@ -110,7 +112,7 @@ export class MultiTraceModel {
     this.hasSource = contexts.some(c => c.hasSource);
     this.hasStepData = contexts.some(context => context.origin === 'testRunner');
     this.resources = [...contexts.map(c => c.resources)].flat();
-    this.attachments = this.actions.flatMap(action => action.attachments?.map(attachment => ({ ...attachment, traceUrl: action.context.traceUrl })) ?? []);
+    this.attachments = this.actions.flatMap(action => action.attachments?.map(attachment => ({ ...attachment, traceUrl })) ?? []);
     this.visibleAttachments = this.attachments.filter(attachment => !attachment.name.startsWith('_'));
 
     this.events.sort((a1, a2) => a1.time - a2.time);
@@ -179,30 +181,9 @@ function indexModel(context: ContextEntry) {
 }
 
 function mergeActionsAndUpdateTiming(contexts: ContextEntry[]) {
-  const traceFileToContexts = new Map<string, ContextEntry[]>();
-  for (const context of contexts) {
-    const traceFile = context.traceUrl;
-    let list = traceFileToContexts.get(traceFile);
-    if (!list) {
-      list = [];
-      traceFileToContexts.set(traceFile, list);
-    }
-    list.push(context);
-  }
-
   const result: ActionTraceEventInContext[] = [];
-  let traceFileId = 0;
-  for (const [, contexts] of traceFileToContexts) {
-    // Action ids are unique only within a trace file. If there are
-    // traces from more than one file we make the ids unique across the
-    // files. The code does not update snapshot ids as they are always
-    // retrieved from a particular trace file.
-    if (traceFileToContexts.size > 1)
-      makeCallIdsUniqueAcrossTraceFiles(contexts, ++traceFileId);
-    // Align action times across runner and library contexts within each trace file.
-    const actions = mergeActionsAndUpdateTimingSameTrace(contexts);
-    result.push(...actions);
-  }
+  const actions = mergeActionsAndUpdateTimingSameTrace(contexts);
+  result.push(...actions);
 
   result.sort((a1, a2) => {
     if (a2.parentId === a1.callId)
@@ -229,17 +210,6 @@ function mergeActionsAndUpdateTiming(contexts: ContextEntry[]) {
   return result;
 }
 
-function makeCallIdsUniqueAcrossTraceFiles(contexts: ContextEntry[], traceFileId: number) {
-  for (const context of contexts) {
-    for (const action of context.actions) {
-      if (action.callId)
-        action.callId = `${traceFileId}:${action.callId}`;
-      if (action.parentId)
-        action.parentId = `${traceFileId}:${action.parentId}`;
-    }
-  }
-}
-
 let lastTmpStepId = 0;
 
 function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]): ActionTraceEventInContext[] {

packages/trace-viewer/src/ui/snapshotTab.tsx

@@ -17,7 +17,7 @@
 import './snapshotTab.css';
 import * as React from 'react';
 import type { ActionTraceEvent } from '@trace/trace';
-import { context, type MultiTraceModel, nextActionByStartTime, previousActionByEndTime } from './modelUtil';
+import { type MultiTraceModel, nextActionByStartTime, previousActionByEndTime } from './modelUtil';
 import { Toolbar } from '@web/components/toolbar';
 import { ToolbarButton } from '@web/components/toolbarButton';
 import { clsx, useMeasure, useSetting } from '@web/uiUtils';
@@ -47,7 +47,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
   setIsInspecting: (isInspecting: boolean) => void,
   highlightedElement: HighlightedElement,
   setHighlightedElement: (element: HighlightedElement) => void,
-}> = ({ action, sdkLanguage, testIdAttributeName, isInspecting, setIsInspecting, highlightedElement, setHighlightedElement }) => {
+}> = ({ action, model, sdkLanguage, testIdAttributeName, isInspecting, setIsInspecting, highlightedElement, setHighlightedElement }) => {
   const [snapshotTab, setSnapshotTab] = React.useState<'action'|'before'|'after'>('action');
 
   const [shouldPopulateCanvasFromScreenshot] = useSetting('shouldPopulateCanvasFromScreenshot', false);
@@ -57,8 +57,8 @@ export const SnapshotTabsView: React.FunctionComponent<{
   }, [action]);
   const { snapshotInfoUrl, snapshotUrl, popoutUrl } = React.useMemo(() => {
     const snapshot = snapshots[snapshotTab];
-    return snapshot ? extendSnapshot(snapshot, shouldPopulateCanvasFromScreenshot) : { snapshotInfoUrl: undefined, snapshotUrl: undefined, popoutUrl: undefined };
-  }, [snapshots, snapshotTab, shouldPopulateCanvasFromScreenshot]);
+    return model && snapshot ? extendSnapshot(model.traceUrl, snapshot, shouldPopulateCanvasFromScreenshot) : { snapshotInfoUrl: undefined, snapshotUrl: undefined, popoutUrl: undefined };
+  }, [snapshots, snapshotTab, shouldPopulateCanvasFromScreenshot, model]);
 
   const snapshotUrls = React.useMemo((): SnapshotUrls | undefined => snapshotInfoUrl !== undefined ? { snapshotInfoUrl, snapshotUrl, popoutUrl } : undefined, [snapshotInfoUrl, snapshotUrl, popoutUrl]);
 
@@ -414,9 +414,9 @@ export function collectSnapshots(action: ActionTraceEvent | undefined): Snapshot
 const isUnderTest = new URLSearchParams(window.location.search).has('isUnderTest');
 const serverParam = new URLSearchParams(window.location.search).get('server');
 
-export function extendSnapshot(snapshot: Snapshot, shouldPopulateCanvasFromScreenshot: boolean): SnapshotUrls {
+export function extendSnapshot(traceUrl: string, snapshot: Snapshot, shouldPopulateCanvasFromScreenshot: boolean): SnapshotUrls {
   const params = new URLSearchParams();
-  params.set('trace', context(snapshot.action).traceUrl);
+  params.set('trace', traceUrl);
   params.set('name', snapshot.snapshotName);
   if (isUnderTest)
     params.set('isUnderTest', 'true');
@@ -436,7 +436,7 @@ export function extendSnapshot(snapshot: Snapshot, shouldPopulateCanvasFromScree
   popoutParams.set('r', snapshotUrl);
   if (serverParam)
     popoutParams.set('server', serverParam);
-  popoutParams.set('trace', context(snapshot.action).traceUrl);
+  popoutParams.set('trace', traceUrl);
   if (snapshot.point) {
     popoutParams.set('pointX', String(snapshot.point.x));
     popoutParams.set('pointY', String(snapshot.point.y));

packages/trace-viewer/src/ui/uiModeTraceView.tsx

@@ -75,7 +75,7 @@ export const TraceView: React.FC<{
         const model = await loadSingleTraceFile(traceLocation);
         setModel({ model, isLive: true });
       } catch {
-        const model = new MultiTraceModel([]);
+        const model = new MultiTraceModel('', []);
         model.errorDescriptors.push(...result.errors.flatMap(error => !!error.message ? [{ message: error.message }] : []));
         setModel({ model, isLive: false });
       } finally {
@@ -113,8 +113,7 @@ const outputDirForTestCase = (testCase: reporterTypes.TestCase): string | undefi
 async function loadSingleTraceFile(url: string): Promise<MultiTraceModel> {
   const params = new URLSearchParams();
   params.set('trace', url);
-  params.set('limit', '1');
   const response = await fetch(`contexts?${params.toString()}`);
   const contextEntries = await response.json() as ContextEntry[];
-  return new MultiTraceModel(contextEntries);
+  return new MultiTraceModel(url, contextEntries);
 }