From 6f360f720727c2517e6aaeb7f0573ba8c6a894f4 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Wed, 20 Mar 2024 21:01:17 -0700
Subject: [PATCH] feat(trace): do not record events that are not used in the
 viewer (#30030)

This is especially useful for network events that are already in the
har, but also get into the trace.
---
 .../src/server/dispatchers/dispatcher.ts      | 33 +++++--------------
 .../src/server/instrumentation.ts             |  3 --
 .../src/server/trace/recorder/tracing.ts      | 26 ++++++++-------
 3 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts
index 0b250fe76e..44d68c73a2 100644
--- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts
+++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts
@@ -24,7 +24,6 @@ import { SdkObject } from '../instrumentation';
 import type { PlaywrightDispatcher } from './playwrightDispatcher';
 import { eventsHelper } from '../..//utils/eventsHelper';
 import type { RegisteredListener } from '../..//utils/eventsHelper';
-import type * as trace from '@trace/trace';
 import { isProtocolError } from '../protocolError';
 
 export const dispatcherSymbol = Symbol('dispatcher');
@@ -81,7 +80,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
     }
 
     if (this._parent)
-      this._connection.sendCreate(this._parent, type, guid, initializer, this._parent._object);
+      this._connection.sendCreate(this._parent, type, guid, initializer);
     this._connection.maybeDisposeStaleDispatchers(this._gcBucket);
   }
 
@@ -121,8 +120,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
       // Just ignore this event outside of tests.
       return;
     }
-    const sdkObject = this._object instanceof SdkObject ? this._object : undefined;
-    this._connection.sendEvent(this, method as string, params, sdkObject);
+    this._connection.sendEvent(this, method as string, params);
   }
 
   _dispose(reason?: 'gc') {
@@ -195,39 +193,24 @@ export class DispatcherConnection {
     this._isLocal = !!isLocal;
   }
 
-  sendEvent(dispatcher: DispatcherScope, event: string, params: any, sdkObject?: SdkObject) {
+  sendEvent(dispatcher: DispatcherScope, event: string, params: any) {
     const validator = findValidator(dispatcher._type, event, 'Event');
     params = validator(params, '', { tChannelImpl: this._tChannelImplToWire.bind(this), binary: this._isLocal ? 'buffer' : 'toBase64' });
-    this._sendMessageToClient(dispatcher._guid, dispatcher._type, event, params, sdkObject);
+    this.onmessage({ guid: dispatcher._guid, method: event, params });
   }
 
-  sendCreate(parent: DispatcherScope, type: string, guid: string, initializer: any, sdkObject?: SdkObject) {
+  sendCreate(parent: DispatcherScope, type: string, guid: string, initializer: any) {
     const validator = findValidator(type, '', 'Initializer');
     initializer = validator(initializer, '', { tChannelImpl: this._tChannelImplToWire.bind(this), binary: this._isLocal ? 'buffer' : 'toBase64' });
-    this._sendMessageToClient(parent._guid, type, '__create__', { type, initializer, guid }, sdkObject);
+    this.onmessage({ guid: parent._guid, method: '__create__', params: { type, initializer, guid } });
   }
 
   sendAdopt(parent: DispatcherScope, dispatcher: DispatcherScope) {
-    this._sendMessageToClient(parent._guid, dispatcher._type, '__adopt__', { guid: dispatcher._guid });
+    this.onmessage({ guid: parent._guid, method: '__adopt__', params: { guid: dispatcher._guid } });
   }
 
   sendDispose(dispatcher: DispatcherScope, reason?: 'gc') {
-    this._sendMessageToClient(dispatcher._guid, dispatcher._type, '__dispose__', { reason });
-  }
-
-  private _sendMessageToClient(guid: string, type: string, method: string, params: any, sdkObject?: SdkObject) {
-    if (sdkObject) {
-      const event: trace.EventTraceEvent = {
-        type: 'event',
-        class: type,
-        method,
-        params: params || {},
-        time: monotonicTime(),
-        pageId: sdkObject?.attribution?.page?.guid,
-      };
-      sdkObject.instrumentation?.onEvent(sdkObject, event);
-    }
-    this.onmessage({ guid, method, params });
+    this.onmessage({ guid: dispatcher._guid, method: '__dispose__', params: { reason } });
   }
 
   private _tChannelImplFromWire(names: '*' | string[], arg: any, path: string, context: ValidatorContext): any {
diff --git a/packages/playwright-core/src/server/instrumentation.ts b/packages/playwright-core/src/server/instrumentation.ts
index 7ff19202d9..5d212279e3 100644
--- a/packages/playwright-core/src/server/instrumentation.ts
+++ b/packages/playwright-core/src/server/instrumentation.ts
@@ -36,7 +36,6 @@ export type Attribution = {
 
 import type { CallMetadata } from '@protocol/callMetadata';
 export type { CallMetadata } from '@protocol/callMetadata';
-import type * as trace from '@trace/trace';
 
 export const kTestSdkObjects = new WeakSet<SdkObject>();
 
@@ -63,7 +62,6 @@ export interface Instrumentation {
   onBeforeInputAction(sdkObject: SdkObject, metadata: CallMetadata, element: ElementHandle): Promise<void>;
   onCallLog(sdkObject: SdkObject, metadata: CallMetadata, logName: string, message: string): void;
   onAfterCall(sdkObject: SdkObject, metadata: CallMetadata): Promise<void>;
-  onEvent(sdkObject: SdkObject, event: trace.EventTraceEvent): void;
   onPageOpen(page: Page): void;
   onPageClose(page: Page): void;
   onBrowserOpen(browser: Browser): void;
@@ -75,7 +73,6 @@ export interface InstrumentationListener {
   onBeforeInputAction?(sdkObject: SdkObject, metadata: CallMetadata, element: ElementHandle): Promise<void>;
   onCallLog?(sdkObject: SdkObject, metadata: CallMetadata, logName: string, message: string): void;
   onAfterCall?(sdkObject: SdkObject, metadata: CallMetadata): Promise<void>;
-  onEvent?(sdkObject: SdkObject, event: trace.EventTraceEvent): void;
   onPageOpen?(page: Page): void;
   onPageClose?(page: Page): void;
   onBrowserOpen?(browser: Browser): void;
diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts
index 52fe777134..774cd24163 100644
--- a/packages/playwright-core/src/server/trace/recorder/tracing.ts
+++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts
@@ -43,6 +43,7 @@ import { Snapshotter } from './snapshotter';
 import { yazl } from '../../../zipBundle';
 import type { ConsoleMessage } from '../../console';
 import { Dispatcher } from '../../dispatchers/dispatcher';
+import { serializeError } from '../../errors';
 
 const version: trace.VERSION = 6;
 
@@ -182,6 +183,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     this._context.instrumentation.addListener(this, this._context);
     this._eventListeners.push(
         eventsHelper.addEventListener(this._context, BrowserContext.Events.Console, this._onConsoleMessage.bind(this)),
+        eventsHelper.addEventListener(this._context, BrowserContext.Events.PageError, this._onPageError.bind(this)),
     );
     if (this._state.options.screenshots)
       this._startScreencast();
@@ -396,18 +398,6 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     return this._captureSnapshot(event.afterSnapshot, sdkObject, metadata);
   }
 
-  onEvent(sdkObject: SdkObject, event: trace.EventTraceEvent) {
-    if (!sdkObject.attribution.context)
-      return;
-    if (event.method === 'console' ||
-        (event.method === '__create__' && event.class === 'ConsoleMessage') ||
-        (event.method === '__create__' && event.class === 'JSHandle')) {
-      // Console messages are handled separately.
-      return;
-    }
-    this._appendTraceEvent(event);
-  }
-
   onEntryStarted(entry: har.Entry) {
     this._pendingHarEntries.add(entry);
   }
@@ -456,6 +446,18 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     this._appendTraceEvent(event);
   }
 
+  private _onPageError(error: Error, page: Page) {
+    const event: trace.EventTraceEvent = {
+      type: 'event',
+      time: monotonicTime(),
+      class: 'BrowserContext',
+      method: 'pageError',
+      params: { error: serializeError(error) },
+      pageId: page.guid,
+    };
+    this._appendTraceEvent(event);
+  }
+
   private _startScreencastInPage(page: Page) {
     page.setScreencastOptions(kScreencastOptions);
     const prefix = page.guid;