feat(tracing): introduce context.tracing, allow exporting trace (#6313)

package-lock.json

@@ -3630,6 +3630,15 @@
         "@types/node": "*"
       }
     },
+    "@types/yazl": {
+      "version": "2.4.2",
+      "resolved": "https://registry.npmjs.org/@types/yazl/-/yazl-2.4.2.tgz",
+      "integrity": "sha512-T+9JH8O2guEjXNxqmybzQ92mJUh2oCwDDMSSimZSe1P+pceZiFROZLYmcbqkzV5EUwz6VwcKXCO2S2yUpra6XQ==",
+      "dev": true,
+      "requires": {
+        "@types/node": "*"
+      }
+    },
     "@typescript-eslint/eslint-plugin": {
       "version": "3.10.1",
       "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-3.10.1.tgz",
@@ -16285,6 +16294,14 @@
         "fd-slicer": "~1.1.0"
       }
     },
+    "yazl": {
+      "version": "2.5.1",
+      "resolved": "https://registry.npmjs.org/yazl/-/yazl-2.5.1.tgz",
+      "integrity": "sha512-phENi2PLiHnHb6QBVot+dJnaAZ0xosj7p3fWl+znIjBDlnMI2PsZCJZ306BPTFOaHf5qdDEI8x5qFrSOBN5vrw==",
+      "requires": {
+        "buffer-crc32": "~0.2.3"
+      }
+    },
     "yocto-queue": {
       "version": "0.1.0",
       "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz",

package.json

@@ -51,9 +51,15 @@
     "proxy-from-env": "^1.1.0",
     "rimraf": "^3.0.2",
     "stack-utils": "^2.0.3",
-    "ws": "^7.3.1"
+    "ws": "^7.3.1",
+    "yazl": "^2.5.1"
   },
   "devDependencies": {
+    "@storybook/addon-actions": "^6.1.20",
+    "@storybook/addon-essentials": "^6.1.20",
+    "@storybook/addon-links": "^6.1.20",
+    "@storybook/node-logger": "^6.1.20",
+    "@storybook/react": "^6.1.20",
     "@types/debug": "^4.1.5",
     "@types/extract-zip": "^1.6.2",
     "@types/mime": "^2.0.3",
@@ -68,6 +74,7 @@
     "@types/rimraf": "^3.0.0",
     "@types/webpack": "^4.41.25",
     "@types/ws": "7.2.6",
+    "@types/yazl": "^2.4.2",
     "@typescript-eslint/eslint-plugin": "^3.10.1",
     "@typescript-eslint/parser": "^3.10.1",
     "chokidar": "^3.5.0",
@@ -88,11 +95,6 @@
     "react": "^17.0.1",
     "react-dom": "^17.0.1",
     "socksv5": "0.0.6",
-    "@storybook/addon-actions": "^6.1.20",
-    "@storybook/addon-essentials": "^6.1.20",
-    "@storybook/addon-links": "^6.1.20",
-    "@storybook/node-logger": "^6.1.20",
-    "@storybook/react": "^6.1.20",
     "style-loader": "^1.2.1",
     "ts-loader": "^8.0.3",
     "typescript": "^4.0.2",

src/client/browserContext.ts

@@ -33,6 +33,7 @@ import { isSafeCloseError } from '../utils/errors';
 import * as api from '../../types/types';
 import * as structs from '../../types/structs';
 import { CDPSession } from './cdpSession';
+import { Tracing } from './tracing';
 
 const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs));
 const fsReadFileAsync = util.promisify(fs.readFile.bind(fs));
@@ -49,6 +50,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     sdkLanguage: 'javascript'
   };
 
+  readonly _tracing: Tracing;
+
   readonly _backgroundPages = new Set<Page>();
   readonly _serviceWorkers = new Set<Worker>();
   readonly _isChromium: boolean;
@@ -66,6 +69,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     if (parent instanceof Browser)
       this._browser = parent;
     this._isChromium = this._browser?._name === 'chromium';
+    this._tracing = new Tracing(this);
 
     this._channel.on('bindingCall', ({binding}) => this._onBinding(BindingCall.from(binding)));
     this._channel.on('close', () => this._onClose());
@@ -279,18 +283,6 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     this.emit(Events.BrowserContext.Close, this);
   }
 
-  async _startTracing() {
-    return await this._wrapApiCall('browserContext.startTracing', async (channel: channels.BrowserContextChannel) => {
-      await channel.startTracing();
-    });
-  }
-
-  async _stopTracing() {
-    return await this._wrapApiCall('browserContext.stopTracing', async (channel: channels.BrowserContextChannel) => {
-      await channel.stopTracing();
-    });
-  }
-
   async close(): Promise<void> {
     try {
       await this._wrapApiCall('browserContext.close', async (channel: channels.BrowserContextChannel) => {

src/client/tracing.ts

@@ -0,0 +1,50 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as channels from '../protocol/channels';
+import { Artifact } from './artifact';
+import { BrowserContext } from './browserContext';
+
+export class Tracing {
+  private _context: BrowserContext;
+
+  constructor(channel: BrowserContext) {
+    this._context = channel;
+  }
+
+  async start(options: { snapshots?: boolean, screenshots?: boolean } = {}) {
+    await this._context._wrapApiCall('tracing.start', async (channel: channels.BrowserContextChannel) => {
+      return await channel.tracingStart(options);
+    });
+  }
+
+  async stop() {
+    await this._context._wrapApiCall('tracing.stop', async (channel: channels.BrowserContextChannel) => {
+      await channel.tracingStop();
+    });
+  }
+
+  async export(path: string): Promise<void> {
+    const result = await this._context._wrapApiCall('tracing.export', async (channel: channels.BrowserContextChannel) => {
+      return await channel.tracingExport();
+    });
+    const artifact = Artifact.from(result.artifact);
+    if (this._context.browser()?._isRemote)
+      artifact._isRemote = true;
+    await artifact.saveAs(path);
+    await artifact.delete();
+  }
+}

src/dispatchers/artifactDispatcher.ts

@@ -94,5 +94,6 @@ export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactIn
 
   async delete(): Promise<void> {
     await this._object.delete();
+    this._dispose();
   }
 }

src/dispatchers/browserContextDispatcher.ts

@@ -158,11 +158,16 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
     return { session: new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession((params.page as PageDispatcher)._object)) };
   }
 
-  async startTracing(params: channels.BrowserContextStartTracingParams): Promise<void> {
-    await this._context.startTracing();
+  async tracingStart(params: channels.BrowserContextTracingStartParams): Promise<channels.BrowserContextTracingStartResult> {
+    await this._context.tracing.start(params);
   }
 
-  async stopTracing(): Promise<channels.BrowserContextStopTracingResult> {
-    await this._context.stopTracing();
+  async tracingStop(params: channels.BrowserContextTracingStopParams): Promise<channels.BrowserContextTracingStopResult> {
+    await this._context.tracing.stop();
+  }
+
+  async tracingExport(params: channels.BrowserContextTracingExportParams): Promise<channels.BrowserContextTracingExportResult> {
+    const artifact = await this._context.tracing.export();
+    return { artifact: new ArtifactDispatcher(this._scope, artifact) };
   }
 }

src/protocol/channels.ts

@@ -610,8 +610,9 @@ export interface BrowserContextChannel extends Channel {
   pause(params?: BrowserContextPauseParams, metadata?: Metadata): Promise<BrowserContextPauseResult>;
   recorderSupplementEnable(params: BrowserContextRecorderSupplementEnableParams, metadata?: Metadata): Promise<BrowserContextRecorderSupplementEnableResult>;
   newCDPSession(params: BrowserContextNewCDPSessionParams, metadata?: Metadata): Promise<BrowserContextNewCDPSessionResult>;
-  startTracing(params?: BrowserContextStartTracingParams, metadata?: Metadata): Promise<BrowserContextStartTracingResult>;
-  stopTracing(params?: BrowserContextStopTracingParams, metadata?: Metadata): Promise<BrowserContextStopTracingResult>;
+  tracingStart(params: BrowserContextTracingStartParams, metadata?: Metadata): Promise<BrowserContextTracingStartResult>;
+  tracingStop(params?: BrowserContextTracingStopParams, metadata?: Metadata): Promise<BrowserContextTracingStopResult>;
+  tracingExport(params?: BrowserContextTracingExportParams, metadata?: Metadata): Promise<BrowserContextTracingExportResult>;
 }
 export type BrowserContextBindingCallEvent = {
   binding: BindingCallChannel,
@@ -788,12 +789,25 @@ export type BrowserContextNewCDPSessionOptions = {
 export type BrowserContextNewCDPSessionResult = {
   session: CDPSessionChannel,
 };
-export type BrowserContextStartTracingParams = {};
-export type BrowserContextStartTracingOptions = {};
-export type BrowserContextStartTracingResult = void;
-export type BrowserContextStopTracingParams = {};
-export type BrowserContextStopTracingOptions = {};
-export type BrowserContextStopTracingResult = void;
+export type BrowserContextTracingStartParams = {
+  name?: string,
+  snapshots?: boolean,
+  screenshots?: boolean,
+};
+export type BrowserContextTracingStartOptions = {
+  name?: string,
+  snapshots?: boolean,
+  screenshots?: boolean,
+};
+export type BrowserContextTracingStartResult = void;
+export type BrowserContextTracingStopParams = {};
+export type BrowserContextTracingStopOptions = {};
+export type BrowserContextTracingStopResult = void;
+export type BrowserContextTracingExportParams = {};
+export type BrowserContextTracingExportOptions = {};
+export type BrowserContextTracingExportResult = {
+  artifact: ArtifactChannel,
+};
 
 // ----------- Page -----------
 export type PageInitializer = {

src/protocol/protocol.yml

@@ -601,9 +601,17 @@ BrowserContext:
       returns:
         session: CDPSession
 
-    startTracing:
+    tracingStart:
+      parameters:
+        name: string?
+        snapshots: boolean?
+        screenshots: boolean?
 
-    stopTracing:
+    tracingStop:
+
+    tracingExport:
+      returns:
+        artifact: Artifact
 
   events:
 

src/protocol/validator.ts

@@ -385,8 +385,13 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
   scheme.BrowserContextNewCDPSessionParams = tObject({
     page: tChannel('Page'),
   });
-  scheme.BrowserContextStartTracingParams = tOptional(tObject({}));
-  scheme.BrowserContextStopTracingParams = tOptional(tObject({}));
+  scheme.BrowserContextTracingStartParams = tObject({
+    name: tOptional(tString),
+    snapshots: tOptional(tBoolean),
+    screenshots: tOptional(tBoolean),
+  });
+  scheme.BrowserContextTracingStopParams = tOptional(tObject({}));
+  scheme.BrowserContextTracingExportParams = tOptional(tObject({}));
   scheme.PageSetDefaultNavigationTimeoutNoReplyParams = tObject({
     timeout: tNumber,
   });

src/server/browserContext.ts

@@ -57,7 +57,7 @@ export abstract class BrowserContext extends SdkObject {
   private _selectors?: Selectors;
   private _origins = new Set<string>();
   private _harTracer: HarTracer | undefined;
-  private _tracer: Tracer | null = null;
+  readonly tracing: Tracer;
 
   constructor(browser: Browser, options: types.BrowserContextOptions, browserContextId: string | undefined) {
     super(browser, 'browser-context');
@@ -70,6 +70,7 @@ export abstract class BrowserContext extends SdkObject {
 
     if (this._options.recordHar)
       this._harTracer = new HarTracer(this, this._options.recordHar);
+    this.tracing = new Tracer(this);
   }
 
   _setSelectors(selectors: Selectors) {
@@ -263,7 +264,7 @@ export abstract class BrowserContext extends SdkObject {
       this._closedStatus = 'closing';
 
       await this._harTracer?.flush();
-      await this._tracer?.stop();
+      await this.tracing.stop();
 
       // Cleanup.
       const promises: Promise<void>[] = [];
@@ -370,21 +371,6 @@ export abstract class BrowserContext extends SdkObject {
     this.on(BrowserContext.Events.Page, installInPage);
     return Promise.all(this.pages().map(installInPage));
   }
-
-  async startTracing() {
-    if (this._tracer)
-      throw new Error('Tracing has already been started');
-    const traceDir = this._browser.options.traceDir;
-    if (!traceDir)
-      throw new Error('Tracing directory is not specified when launching the browser');
-    this._tracer = new Tracer(this, traceDir);
-    await this._tracer.start();
-  }
-
-  async stopTracing() {
-    await this._tracer?.stop();
-    this._tracer = null;
-  }
 }
 
 export function assertBrowserContextIsNotOwned(context: BrowserContext) {

src/server/snapshot/snapshotter.ts

@@ -140,7 +140,6 @@ export class Snapshotter {
     this._eventListeners.push(helper.addEventListener(page, Page.Events.Response, (response: network.Response) => {
       this._saveResource(page, response).catch(e => debugLogger.log('error', e));
     }));
-    page.setScreencastEnabled(true);
   }
 
   private async _saveResource(page: Page, response: network.Response) {
@@ -163,10 +162,10 @@ export class Snapshotter {
     const method = original.method();
     const status = response.status();
     const requestBody = original.postDataBuffer();
-    const requestSha1 = requestBody ? calculateSha1(requestBody) : 'none';
+    const requestSha1 = requestBody ? calculateSha1(requestBody) : '';
     const requestHeaders = original.headers();
     const body = await response.body().catch(e => debugLogger.log('error', e));
-    const responseSha1 = body ? calculateSha1(body) : 'none';
+    const responseSha1 = body ? calculateSha1(body) : '';
     const resource: ResourceSnapshot = {
       pageId: page.guid,
       frameId: response.frame().guid,

src/server/trace/common/traceEvents.ts

@@ -41,7 +41,7 @@ export type PageDestroyedTraceEvent = {
 
 export type ScreencastFrameTraceEvent = {
   timestamp: number,
-  type: 'page-screencast-frame',
+  type: 'screencast-frame',
   pageId: string,
   pageTimestamp: number,
   sha1: string,

src/server/trace/recorder/traceSnapshotter.ts

@@ -27,22 +27,19 @@ import { TraceEvent } from '../common/traceEvents';
 import { monotonicTime } from '../../../utils/utils';
 
 const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs));
-const fsMkdirAsync = util.promisify(fs.mkdir.bind(fs));
 
 export class TraceSnapshotter extends EventEmitter implements SnapshotterDelegate {
   private _snapshotter: Snapshotter;
   private _resourcesDir: string;
   private _writeArtifactChain = Promise.resolve();
   private _appendTraceEvent: (traceEvent: TraceEvent) => void;
-  private _context: BrowserContext;
 
-  constructor(context: BrowserContext, resourcesDir: string, appendTraceEvent: (traceEvent: TraceEvent) => void) {
+  constructor(context: BrowserContext, resourcesDir: string, appendTraceEvent: (traceEvent: TraceEvent, sha1?: string) => void) {
     super();
-    this._context = context;
     this._resourcesDir = resourcesDir;
     this._snapshotter = new Snapshotter(context, this);
     this._appendTraceEvent = appendTraceEvent;
-    this._writeArtifactChain = fsMkdirAsync(resourcesDir, { recursive: true });
+    this._writeArtifactChain = Promise.resolve();
   }
 
   async start(): Promise<void> {

src/server/trace/recorder/tracer.ts

@@ -16,8 +16,10 @@
 
 import fs from 'fs';
 import path from 'path';
-import * as util from 'util';
-import { calculateSha1, getFromENV, mkdirIfNeeded, monotonicTime } from '../../../utils/utils';
+import util from 'util';
+import yazl from 'yazl';
+import { createGuid, mkdirIfNeeded, monotonicTime } from '../../../utils/utils';
+import { Artifact } from '../../artifact';
 import { BrowserContext } from '../../browserContext';
 import { Dialog } from '../../dialog';
 import { ElementHandle } from '../../dom';
@@ -29,27 +31,45 @@ import * as trace from '../common/traceEvents';
 import { TraceSnapshotter } from './traceSnapshotter';
 
 const fsAppendFileAsync = util.promisify(fs.appendFile.bind(fs));
-const envTrace = getFromENV('PWTRACE_RESOURCE_DIR');
+const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs));
+const fsMkdirAsync = util.promisify(fs.mkdir.bind(fs));
+
+export type TracerOptions = {
+  name?: string;
+  snapshots?: boolean;
+  screenshots?: boolean;
+};
 
 export class Tracer implements InstrumentationListener {
-  private _appendEventChain: Promise<string>;
-  private _snapshotter: TraceSnapshotter;
+  private _appendEventChain = Promise.resolve();
+  private _snapshotter: TraceSnapshotter | undefined;
   private _eventListeners: RegisteredListener[] = [];
-  private _disposed = false;
   private _pendingCalls = new Map<string, { sdkObject: SdkObject, metadata: CallMetadata }>();
   private _context: BrowserContext;
+  private _traceFile: string | undefined;
+  private _resourcesDir: string | undefined;
+  private _sha1s: string[] = [];
+  private _started = false;
+  private _traceDir: string | undefined;
 
-  constructor(context: BrowserContext, traceDir: string) {
+  constructor(context: BrowserContext) {
     this._context = context;
-    this._context.instrumentation.addListener(this);
-    const resourcesDir = envTrace || path.join(traceDir, 'resources');
-    const tracePrefix = path.join(traceDir, context._options._debugName!);
-    const traceFile = tracePrefix + '.trace';
-    this._appendEventChain = mkdirIfNeeded(traceFile).then(() => traceFile);
-    this._snapshotter = new TraceSnapshotter(context, resourcesDir, traceEvent => this._appendTraceEvent(traceEvent));
+    this._traceDir = context._browser.options.traceDir;
   }
 
-  async start(): Promise<void> {
+  async start(options: TracerOptions): Promise<void> {
+    if (!this._traceDir)
+      throw new Error('Tracing directory is not specified when launching the browser');
+    if (this._started)
+      throw new Error('Tracing has already been started');
+    this._started = true;
+    this._traceFile = path.join(this._traceDir, (options.name || createGuid()) + '.trace');
+    if (options.screenshots || options.snapshots) {
+      this._resourcesDir = path.join(this._traceDir, 'resources');
+      await fsMkdirAsync(this._resourcesDir, { recursive: true });
+    }
+
+    this._appendEventChain = mkdirIfNeeded(this._traceFile);
     const event: trace.ContextCreatedTraceEvent = {
       timestamp: monotonicTime(),
       type: 'context-metadata',
@@ -60,30 +80,58 @@ export class Tracer implements InstrumentationListener {
       debugName: this._context._options._debugName,
     };
     this._appendTraceEvent(event);
-    this._eventListeners = [
-      helper.addEventListener(this._context, BrowserContext.Events.Page, this._onPage.bind(this)),
-    ];
-    await this._snapshotter.start();
+    this._eventListeners.push(
+        helper.addEventListener(this._context, BrowserContext.Events.Page, this._onPage.bind(this, options.screenshots)),
+    );
+    this._context.instrumentation.addListener(this);
+    if (options.snapshots)
+      this._snapshotter = new TraceSnapshotter(this._context, this._resourcesDir!, traceEvent => this._appendTraceEvent(traceEvent));
+    await this._snapshotter?.start();
   }
 
-  async stop() {
-    this._disposed = true;
+  async stop(): Promise<void> {
+    if (!this._started)
+      return;
+    this._started = false;
     this._context.instrumentation.removeListener(this);
     helper.removeEventListeners(this._eventListeners);
-    await this._snapshotter.dispose();
+    await this._snapshotter?.dispose();
+    this._snapshotter = undefined;
     for (const { sdkObject, metadata } of this._pendingCalls.values())
       this.onAfterCall(sdkObject, metadata);
+    for (const page of this._context.pages())
+      page.setScreencastEnabled(false);
 
     // Ensure all writes are finished.
     await this._appendEventChain;
   }
 
+  async export(): Promise<Artifact> {
+    if (!this._traceFile)
+      throw new Error('Tracing directory is not specified when launching the browser');
+    const zipFile = new yazl.ZipFile();
+    zipFile.addFile(this._traceFile, 'trace.trace');
+    const zipFileName = this._traceFile + '.zip';
+    for (const sha1 of this._sha1s)
+      zipFile.addFile(path.join(this._resourcesDir!, sha1), path.join('resources', sha1));
+    const zipPromise = new Promise(f => {
+      zipFile.outputStream.pipe(fs.createWriteStream(zipFileName)).on('close', f);
+    });
+    zipFile.end();
+    await zipPromise;
+    const artifact = new Artifact(this._context, zipFileName);
+    artifact.reportFinished();
+    return artifact;
+  }
+
   _captureSnapshot(name: 'before' | 'after' | 'action' | 'event', sdkObject: SdkObject, metadata: CallMetadata, element?: ElementHandle) {
     if (!sdkObject.attribution.page)
       return;
+    if (!this._snapshotter)
+      return;
     const snapshotName = `${name}@${metadata.id}`;
     metadata.snapshots.push({ title: name, snapshotName });
-    this._snapshotter.captureSnapshot(sdkObject.attribution.page, snapshotName, element);
+    this._snapshotter!.captureSnapshot(sdkObject.attribution.page, snapshotName, element);
   }
 
   async onBeforeCall(sdkObject: SdkObject, metadata: CallMetadata) {
@@ -121,7 +169,7 @@ export class Tracer implements InstrumentationListener {
     this._appendTraceEvent(event);
   }
 
-  private _onPage(page: Page) {
+  private _onPage(screenshots: boolean | undefined, page: Page) {
     const pageId = page.guid;
 
     const event: trace.PageCreatedTraceEvent = {
@@ -130,88 +178,106 @@ export class Tracer implements InstrumentationListener {
       pageId,
     };
     this._appendTraceEvent(event);
+    if (screenshots)
+      page.setScreencastEnabled(true);
 
-    page.on(Page.Events.Dialog, (dialog: Dialog) => {
-      if (this._disposed)
-        return;
-      const event: trace.DialogOpenedEvent = {
-        timestamp: monotonicTime(),
-        type: 'dialog-opened',
-        pageId,
-        dialogType: dialog.type(),
-        message: dialog.message(),
-      };
-      this._appendTraceEvent(event);
-    });
+    this._eventListeners.push(
+        helper.addEventListener(page, Page.Events.Dialog, (dialog: Dialog) => {
+          const event: trace.DialogOpenedEvent = {
+            timestamp: monotonicTime(),
+            type: 'dialog-opened',
+            pageId,
+            dialogType: dialog.type(),
+            message: dialog.message(),
+          };
+          this._appendTraceEvent(event);
+        }),
 
-    page.on(Page.Events.InternalDialogClosed, (dialog: Dialog) => {
-      if (this._disposed)
-        return;
-      const event: trace.DialogClosedEvent = {
-        timestamp: monotonicTime(),
-        type: 'dialog-closed',
-        pageId,
-        dialogType: dialog.type(),
-      };
-      this._appendTraceEvent(event);
-    });
+        helper.addEventListener(page, Page.Events.InternalDialogClosed, (dialog: Dialog) => {
+          const event: trace.DialogClosedEvent = {
+            timestamp: monotonicTime(),
+            type: 'dialog-closed',
+            pageId,
+            dialogType: dialog.type(),
+          };
+          this._appendTraceEvent(event);
+        }),
 
-    page.mainFrame().on(Frame.Events.Navigation, (navigationEvent: NavigationEvent) => {
-      if (this._disposed || page.mainFrame().url() === 'about:blank')
-        return;
-      const event: trace.NavigationEvent = {
-        timestamp: monotonicTime(),
-        type: 'navigation',
-        pageId,
-        url: navigationEvent.url,
-        sameDocument: !navigationEvent.newDocument,
-      };
-      this._appendTraceEvent(event);
-    });
+        helper.addEventListener(page.mainFrame(), Frame.Events.Navigation, (navigationEvent: NavigationEvent) => {
+          if (page.mainFrame().url() === 'about:blank')
+            return;
+          const event: trace.NavigationEvent = {
+            timestamp: monotonicTime(),
+            type: 'navigation',
+            pageId,
+            url: navigationEvent.url,
+            sameDocument: !navigationEvent.newDocument,
+          };
+          this._appendTraceEvent(event);
+        }),
 
-    page.on(Page.Events.Load, () => {
-      if (this._disposed || page.mainFrame().url() === 'about:blank')
-        return;
-      const event: trace.LoadEvent = {
-        timestamp: monotonicTime(),
-        type: 'load',
-        pageId,
-      };
-      this._appendTraceEvent(event);
-    });
+        helper.addEventListener(page, Page.Events.Load, () => {
+          if (page.mainFrame().url() === 'about:blank')
+            return;
+          const event: trace.LoadEvent = {
+            timestamp: monotonicTime(),
+            type: 'load',
+            pageId,
+          };
+          this._appendTraceEvent(event);
+        }),
 
-    page.on(Page.Events.ScreencastFrame, params => {
-      const sha1 = calculateSha1(params.buffer);
-      const event: trace.ScreencastFrameTraceEvent = {
-        type: 'page-screencast-frame',
-        pageId: page.guid,
-        sha1,
-        pageTimestamp: params.timestamp,
-        width: params.width,
-        height: params.height,
-        timestamp: monotonicTime()
-      };
-      this._appendTraceEvent(event);
-      this._snapshotter.onBlob({ sha1, buffer: params.buffer });
-    });
+        helper.addEventListener(page, Page.Events.ScreencastFrame, params => {
+          const guid = createGuid();
+          const event: trace.ScreencastFrameTraceEvent = {
+            type: 'screencast-frame',
+            pageId: page.guid,
+            sha1: guid,  // no need to compute sha1 for screenshots
+            pageTimestamp: params.timestamp,
+            width: params.width,
+            height: params.height,
+            timestamp: monotonicTime()
+          };
+          this._appendTraceEvent(event);
+          this._appendEventChain = this._appendEventChain.then(async () => {
+            await fsWriteFileAsync(path.join(this._resourcesDir!, guid), params.buffer).catch(() => {});
+          });
+        }),
 
-    page.once(Page.Events.Close, () => {
-      if (this._disposed)
-        return;
-      const event: trace.PageDestroyedTraceEvent = {
-        timestamp: monotonicTime(),
-        type: 'page-destroyed',
-        pageId,
-      };
-      this._appendTraceEvent(event);
-    });
+        helper.addEventListener(page, Page.Events.Close, () => {
+          const event: trace.PageDestroyedTraceEvent = {
+            timestamp: monotonicTime(),
+            type: 'page-destroyed',
+            pageId,
+          };
+          this._appendTraceEvent(event);
+        })
+    );
   }
 
   private _appendTraceEvent(event: any) {
+    const visit = (object: any) => {
+      if (Array.isArray(object)) {
+        object.forEach(visit);
+        return;
+      }
+      if (typeof object === 'object') {
+        for (const key in object) {
+          if (key === 'sha1' || key.endsWith('Sha1')) {
+            const sha1 = object[key];
+            if (sha1)
+              this._sha1s.push(sha1);
+          }
+          visit(object[key]);
+        }
+        return;
+      }
+    };
+    visit(event);
+
     // Serialize all writes to the trace file.
-    this._appendEventChain = this._appendEventChain.then(async traceFile => {
-      await fsAppendFileAsync(traceFile, JSON.stringify(event) + '\n');
-      return traceFile;
+    this._appendEventChain = this._appendEventChain.then(async () => {
+      await fsAppendFileAsync(this._traceFile!, JSON.stringify(event) + '\n');
     });
   }
 }

src/server/trace/viewer/traceModel.ts

@@ -75,7 +75,7 @@ export class TraceModel {
         this.pageEntries.get(event.pageId)!.destroyed = event;
         break;
       }
-      case 'page-screencast-frame': {
+      case 'screencast-frame': {
         this.pageEntries.get(event.pageId)!.screencastFrames.push(event);
         break;
       }

src/web/traceViewer/ui/networkResourceDetails.tsx

@@ -36,13 +36,13 @@ export const NetworkResourceDetails: React.FunctionComponent<{
 
   React.useEffect(() => {
     const readResources = async  () => {
-      if (resource.requestSha1 !== 'none') {
+      if (resource.requestSha1) {
         const response = await fetch(`/sha1/${resource.requestSha1}`);
         const requestResource = await response.text();
         setRequestBody(requestResource);
       }
 
-      if (resource.responseSha1 !== 'none') {
+      if (resource.responseSha1) {
         const useBase64 = resource.contentType.includes('image');
         const response = await fetch(`/sha1/${resource.responseSha1}`);
         if (useBase64) {
@@ -113,10 +113,10 @@ export const NetworkResourceDetails: React.FunctionComponent<{
         <div className='network-request-headers'>{resource.requestHeaders.map(pair => `${pair.name}: ${pair.value}`).join('\n')}</div>
         <h4>Response Headers</h4>
         <div className='network-request-headers'>{resource.responseHeaders.map(pair => `${pair.name}: ${pair.value}`).join('\n')}</div>
-        {resource.requestSha1 !== 'none' ? <h4>Request Body</h4> : ''}
-        {resource.requestSha1 !== 'none' ? <div className='network-request-body'>{formatBody(requestBody, requestContentType)}</div> : ''}
+        {resource.requestSha1 ? <h4>Request Body</h4> : ''}
+        {resource.requestSha1 ? <div className='network-request-body'>{formatBody(requestBody, requestContentType)}</div> : ''}
         <h4>Response Body</h4>
-        {resource.responseSha1 === 'none' ? <div className='network-request-response-body'>Response body is not available for this request.</div> : ''}
+        {!resource.responseSha1 ? <div className='network-request-response-body'>Response body is not available for this request.</div> : ''}
         {responseBody !== null && responseBody.dataUrl ? <img src={responseBody.dataUrl} /> : ''}
         {responseBody !== null && responseBody.text ? <div className='network-request-response-body'>{formatBody(responseBody.text, resource.contentType)}</div> : ''}
       </div>

tests/browsertype-connect.spec.ts

@@ -350,7 +350,7 @@ test('should error when saving download after deletion', async ({server, browser
   const userPath = testInfo.outputPath('download.txt');
   await download.delete();
   const { message } = await download.saveAs(userPath).catch(e => e);
-  expect(message).toContain('File already deleted. Save before deleting.');
+  expect(message).toContain('Target page, context or browser has been closed');
   await browser.close();
 });
 

tests/download.spec.ts

@@ -196,7 +196,7 @@ it.describe('download event', () => {
     const userPath = testInfo.outputPath('download.txt');
     await download.delete();
     const { message } = await download.saveAs(userPath).catch(e => e);
-    expect(message).toContain('File already deleted. Save before deleting.');
+    expect(message).toContain('Target page, context or browser has been closed');
     await page.close();
   });