feat: make console/dialog events based on subscription (#22835)

This way we do not send events from the server unless the client is
interested.

Fixes #22621.

packages/playwright-core/src/client/browserContext.ts

@@ -107,6 +107,10 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
       if (page)
         hasListeners = page.emit(Events.Page.Dialog, dialogObject) || hasListeners;
       if (!hasListeners) {
+        // Although we do similar handling on the server side, we still need this logic
+        // on the client side due to a possible race condition between two async calls:
+        // a) removing "dialog" listener subscription (client->server)
+        // b) actual "dialog" event (server->client)
         if (dialogObject.type() === 'beforeunload')
           dialog.accept({}).catch(() => {});
         else
@@ -120,6 +124,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     this._closedPromise = new Promise(f => this.once(Events.BrowserContext.Close, f));
 
     this._setEventToSubscriptionMapping(new Map<string, channels.BrowserContextUpdateSubscriptionParams['event']>([
+      [Events.BrowserContext.Console, 'console'],
+      [Events.BrowserContext.Dialog, 'dialog'],
       [Events.BrowserContext.Request, 'request'],
       [Events.BrowserContext.Response, 'response'],
       [Events.BrowserContext.RequestFinished, 'requestFinished'],

packages/playwright-core/src/client/channelOwner.ts

@@ -67,8 +67,11 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
 
   private _updateSubscription(event: string | symbol, enabled: boolean) {
     const protocolEvent = this._eventToSubscriptionMapping.get(String(event));
-    if (protocolEvent)
-      (this._channel as any).updateSubscription({ event: protocolEvent, enabled }).catch(() => {});
+    if (protocolEvent) {
+      this._wrapApiCall(async () => {
+        await (this._channel as any).updateSubscription({ event: protocolEvent, enabled });
+      }, true).catch(() => {});
+    }
   }
 
   override on(event: string | symbol, listener: Listener): this {

packages/playwright-core/src/client/page.ts

@@ -145,6 +145,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     this.once(Events.Page.Crash, () => this._closedOrCrashedRace.scopeClosed(new Error(kBrowserOrContextClosedError)));
 
     this._setEventToSubscriptionMapping(new Map<string, channels.PageUpdateSubscriptionParams['event']>([
+      [Events.Page.Console, 'console'],
+      [Events.Page.Dialog, 'dialog'],
       [Events.Page.Request, 'request'],
       [Events.Page.Response, 'response'],
       [Events.Page.RequestFinished, 'requestFinished'],

packages/playwright-core/src/protocol/validator.ts

@@ -924,7 +924,7 @@ scheme.BrowserContextCreateTempFileResult = tObject({
   writableStream: tChannel(['WritableStream']),
 });
 scheme.BrowserContextUpdateSubscriptionParams = tObject({
-  event: tEnum(['request', 'response', 'requestFinished', 'requestFailed']),
+  event: tEnum(['console', 'dialog', 'request', 'response', 'requestFinished', 'requestFailed']),
   enabled: tBoolean,
 });
 scheme.BrowserContextUpdateSubscriptionResult = tOptional(tObject({}));
@@ -1217,7 +1217,7 @@ scheme.PageStopCSSCoverageResult = tObject({
 scheme.PageBringToFrontParams = tOptional(tObject({}));
 scheme.PageBringToFrontResult = tOptional(tObject({}));
 scheme.PageUpdateSubscriptionParams = tObject({
-  event: tEnum(['fileChooser', 'request', 'response', 'requestFinished', 'requestFailed']),
+  event: tEnum(['console', 'dialog', 'fileChooser', 'request', 'response', 'requestFinished', 'requestFailed']),
   enabled: tBoolean,
 });
 scheme.PageUpdateSubscriptionResult = tOptional(tObject({}));

packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts

@@ -35,6 +35,9 @@ import { createGuid, urlMatches } from '../../utils';
 import { WritableStreamDispatcher } from './writableStreamDispatcher';
 import { ConsoleMessageDispatcher } from './consoleMessageDispatcher';
 import { DialogDispatcher } from './dialogDispatcher';
+import type { Page } from '../page';
+import type { Dialog } from '../dialog';
+import type { ConsoleMessage } from '../console';
 
 export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channels.BrowserContextChannel, DispatcherScope> implements channels.BrowserContextChannel {
   _type_EventTarget = true;
@@ -81,8 +84,16 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
       this._dispatchEvent('close');
       this._dispose();
     });
-    this.addObjectListener(BrowserContext.Events.Console, message => this._dispatchEvent('console', { message: new ConsoleMessageDispatcher(PageDispatcher.from(this, message.page()), message) }));
-    this.addObjectListener(BrowserContext.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this, dialog) }));
+    this.addObjectListener(BrowserContext.Events.Console, (message: ConsoleMessage) => {
+      if (this._shouldDispatchEvent(message.page(), 'console'))
+        this._dispatchEvent('console', { message: new ConsoleMessageDispatcher(PageDispatcher.from(this, message.page()), message) });
+    });
+    this.addObjectListener(BrowserContext.Events.Dialog, (dialog: Dialog) => {
+      if (this._shouldDispatchEvent(dialog.page(), 'dialog'))
+        this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this, dialog) });
+      else
+        dialog.close().catch(() => {});
+    });
 
     if (context._browser.options.name === 'chromium') {
       for (const page of (context as CRBrowserContext).backgroundPages())
@@ -141,9 +152,12 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   }
 
   private _shouldDispatchNetworkEvent(request: Request, event: channels.BrowserContextUpdateSubscriptionParams['event'] & channels.PageUpdateSubscriptionParams['event']): boolean {
+    return this._shouldDispatchEvent(request.frame()?._page?.initializedOrUndefined(), event);
+  }
+
+  private _shouldDispatchEvent(page: Page | undefined, event: channels.BrowserContextUpdateSubscriptionParams['event'] & channels.PageUpdateSubscriptionParams['event']): boolean {
     if (this._subscriptions.has(event))
       return true;
-    const page = request.frame()?._page?.initializedOrUndefined();
     const pageDispatcher = page ? existingDispatcher<PageDispatcher>(page) : undefined;
     if (pageDispatcher?._subscriptions.has(event))
       return true;

packages/playwright-core/src/server/trace/recorder/tracing.ts

@@ -38,12 +38,12 @@ import type { HarTracerDelegate } from '../../har/harTracer';
 import { HarTracer } from '../../har/harTracer';
 import type { FrameSnapshot } from '@trace/snapshot';
 import type * as trace from '@trace/trace';
-import type { VERSION } from '@trace/trace';
 import type { SnapshotterBlob, SnapshotterDelegate } from './snapshotter';
 import { Snapshotter } from './snapshotter';
 import { yazl } from '../../../zipBundle';
+import type { ConsoleMessage } from '../../console';
 
-const version: VERSION = 4;
+const version: trace.VERSION = 4;
 
 export type TracerOptions = {
   name?: string;
@@ -71,6 +71,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
   private _snapshotter?: Snapshotter;
   private _harTracer: HarTracer;
   private _screencastListeners: RegisteredListener[] = [];
+  private _eventListeners: RegisteredListener[] = [];
   private _context: BrowserContext | APIRequestContext;
   private _state: RecordingState | undefined;
   private _isStopping = false;
@@ -168,6 +169,9 @@ 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)),
+    );
     if (state.options.screenshots)
       this._startScreencast();
     if (state.options.snapshots)
@@ -248,6 +252,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
 
     const state = this._state!;
     this._context.instrumentation.removeListener(this);
+    eventsHelper.removeEventListeners(this._eventListeners);
     if (this._state?.options.screenshots)
       this._stopScreencast();
 
@@ -354,14 +359,8 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
   onEvent(sdkObject: SdkObject, event: trace.EventTraceEvent) {
     if (!sdkObject.attribution.context)
       return;
-    if (event.method === '__create__' && event.class === 'ConsoleMessage') {
-      const object: trace.ObjectTraceEvent = {
-        type: 'object',
-        class: event.class,
-        guid: event.params.guid,
-        initializer: event.params.initializer,
-      };
-      this._appendTraceEvent(object);
+    if (event.method === 'console' || (event.method === '__create__' && event.class === 'ConsoleMessage')) {
+      // Console messages are handled separately.
       return;
     }
     this._appendTraceEvent(event);
@@ -390,6 +389,30 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     this._appendTraceEvent({ type: 'frame-snapshot', snapshot });
   }
 
+  private _onConsoleMessage(message: ConsoleMessage) {
+    const object: trace.ObjectTraceEvent = {
+      type: 'object',
+      class: 'ConsoleMessage',
+      guid: message.guid,
+      initializer: {
+        type: message.type(),
+        text: message.text(),
+        location: message.location(),
+      },
+    };
+    this._appendTraceEvent(object);
+
+    const event: trace.EventTraceEvent = {
+      type: 'event',
+      class: 'BrowserContext',
+      method: 'console',
+      params: { message: { guid: message.guid } },
+      time: monotonicTime(),
+      pageId: message.page().guid,
+    };
+    this._appendTraceEvent(event);
+  }
+
   private _startScreencastInPage(page: Page) {
     page.setScreencastOptions(kScreencastOptions);
     const prefix = page.guid;

packages/protocol/src/channels.ts

@@ -1684,7 +1684,7 @@ export type BrowserContextCreateTempFileResult = {
   writableStream: WritableStreamChannel,
 };
 export type BrowserContextUpdateSubscriptionParams = {
-  event: 'request' | 'response' | 'requestFinished' | 'requestFailed',
+  event: 'console' | 'dialog' | 'request' | 'response' | 'requestFinished' | 'requestFailed',
   enabled: boolean,
 };
 export type BrowserContextUpdateSubscriptionOptions = {
@@ -2201,7 +2201,7 @@ export type PageBringToFrontParams = {};
 export type PageBringToFrontOptions = {};
 export type PageBringToFrontResult = void;
 export type PageUpdateSubscriptionParams = {
-  event: 'fileChooser' | 'request' | 'response' | 'requestFinished' | 'requestFailed',
+  event: 'console' | 'dialog' | 'fileChooser' | 'request' | 'response' | 'requestFinished' | 'requestFailed',
   enabled: boolean,
 };
 export type PageUpdateSubscriptionOptions = {

packages/protocol/src/protocol.yml

@@ -1153,6 +1153,8 @@ BrowserContext:
         event:
           type: enum
           literals:
+          - console
+          - dialog
           - request
           - response
           - requestFinished
@@ -1588,6 +1590,8 @@ Page:
         event:
           type: enum
           literals:
+          - console
+          - dialog
           - fileChooser
           - request
           - response

tests/page/page-event-popup.spec.ts

@@ -63,9 +63,12 @@ it('should be able to capture alert', async ({ page }) => {
     const win = window.open('');
     win.alert('hello');
   });
-  const popup = await page.waitForEvent('popup');
-  const dialog = await popup.waitForEvent('dialog');
+  const [popup, dialog] = await Promise.all([
+    page.waitForEvent('popup'),
+    page.context().waitForEvent('dialog'),
+  ]);
   expect(dialog.message()).toBe('hello');
+  expect(dialog.page()).toBe(popup);
   await dialog.dismiss();
   await evaluatePromise;
 });