fix: dispatch popup event on the client end (#6044)

src/client/browserContext.ts

@@ -88,6 +88,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
   private _onPage(page: Page): void {
     this._pages.add(page);
     this.emit(Events.BrowserContext.Page, page);
+    if (page._opener && !page._opener.isClosed())
+      page._opener.emit(Events.Page.Popup, page);
   }
 
   _onRoute(route: network.Route, request: network.Request) {

src/client/page.ts

@@ -86,6 +86,7 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
   readonly _timeoutSettings: TimeoutSettings;
   _isPageCall = false;
   private _video: Video | null = null;
+  readonly _opener: Page | null;
 
   static from(page: channels.PageChannel): Page {
     return (page as any)._object;
@@ -110,6 +111,7 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
     this._frames.add(this._mainFrame);
     this._viewportSize = initializer.viewportSize || null;
     this._closed = initializer.isClosed;
+    this._opener = Page.fromNullable(initializer.opener);
 
     this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding)));
     this._channel.on('close', () => this._onClose());
@@ -130,7 +132,6 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
     this._channel.on('frameDetached', ({ frame }) => this._onFrameDetached(Frame.from(frame)));
     this._channel.on('load', () => this.emit(Events.Page.Load, this));
     this._channel.on('pageError', ({ error }) => this.emit(Events.Page.PageError, parseError(error)));
-    this._channel.on('popup', ({ page }) => this.emit(Events.Page.Popup, Page.from(page)));
     this._channel.on('request', ({ request }) => this.emit(Events.Page.Request, Request.from(request)));
     this._channel.on('requestFailed', ({ request, failureText, responseEndTiming }) => this._onRequestFailed(Request.from(request), responseEndTiming, failureText));
     this._channel.on('requestFinished', ({ request, responseEndTiming }) => this._onRequestFinished(Request.from(request), responseEndTiming));
@@ -220,9 +221,9 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
   }
 
   async opener(): Promise<Page | null> {
-    return this._wrapApiCall('page.opener', async (channel: channels.PageChannel) => {
+    if (!this._opener || this._opener.isClosed())
-      return Page.fromNullable((await channel.opener()).page);
+      return null;
-    });
+    return this._opener;
   }
 
   mainFrame(): Frame {

src/dispatchers/pageDispatcher.ts

@@ -38,13 +38,21 @@ import { Download } from '../server/download';
 export class PageDispatcher extends Dispatcher<Page, channels.PageInitializer> implements channels.PageChannel {
   private _page: Page;
 
+  private static fromNullable(scope: DispatcherScope, page: Page | undefined): PageDispatcher | undefined {
+    if (!page)
+      return undefined;
+    const result = existingDispatcher<PageDispatcher>(page);
+    return result || new PageDispatcher(scope, page);
+  }
+
   constructor(scope: DispatcherScope, page: Page) {
     // TODO: theoretically, there could be more than one frame already.
     // If we split pageCreated and pageReady, there should be no main frame during pageCreated.
     super(scope, page, 'Page', {
       mainFrame: FrameDispatcher.from(scope, page.mainFrame()),
       viewportSize: page.viewportSize() || undefined,
-      isClosed: page.isClosed()
+      isClosed: page.isClosed(),
+      opener: PageDispatcher.fromNullable(scope, page.opener())
     }, true);
     this._page = page;
     page.on(Page.Events.Close, () => {
@@ -66,7 +74,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageInitializer> i
     page.on(Page.Events.FrameDetached, frame => this._onFrameDetached(frame));
     page.on(Page.Events.Load, () => this._dispatchEvent('load'));
     page.on(Page.Events.PageError, error => this._dispatchEvent('pageError', { error: serializeError(error) }));
-    page.on(Page.Events.Popup, page => this._dispatchEvent('popup', { page: lookupDispatcher<PageDispatcher>(page) }));
     page.on(Page.Events.Request, request => this._dispatchEvent('request', { request: RequestDispatcher.from(this._scope, request) }));
     page.on(Page.Events.RequestFailed, (request: Request) => this._dispatchEvent('requestFailed', {
       request: RequestDispatcher.from(this._scope, request),
@@ -93,10 +100,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageInitializer> i
     this._page.setDefaultTimeout(params.timeout);
   }
 
-  async opener(params: channels.PageOpenerParams, metadata: CallMetadata): Promise<channels.PageOpenerResult> {
-    return { page: lookupNullableDispatcher<PageDispatcher>(await this._page.opener()) };
-  }
-
   async exposeBinding(params: channels.PageExposeBindingParams, metadata: CallMetadata): Promise<void> {
     await this._page.exposeBinding(params.name, !!params.needsHandle, (source, ...args) => {
       const binding = new BindingCallDispatcher(this._scope, params.name, !!params.needsHandle, source, args);

src/protocol/channels.ts

@@ -797,6 +797,7 @@ export type PageInitializer = {
     height: number,
   },
   isClosed: boolean,
+  opener?: PageChannel,
 };
 export interface PageChannel extends Channel {
   on(event: 'bindingCall', callback: (params: PageBindingCallEvent) => void): this;
@@ -811,7 +812,6 @@ export interface PageChannel extends Channel {
   on(event: 'frameDetached', callback: (params: PageFrameDetachedEvent) => void): this;
   on(event: 'load', callback: (params: PageLoadEvent) => void): this;
   on(event: 'pageError', callback: (params: PagePageErrorEvent) => void): this;
-  on(event: 'popup', callback: (params: PagePopupEvent) => void): this;
   on(event: 'request', callback: (params: PageRequestEvent) => void): this;
   on(event: 'requestFailed', callback: (params: PageRequestFailedEvent) => void): this;
   on(event: 'requestFinished', callback: (params: PageRequestFinishedEvent) => void): this;
@@ -829,7 +829,6 @@ export interface PageChannel extends Channel {
   exposeBinding(params: PageExposeBindingParams, metadata?: Metadata): Promise<PageExposeBindingResult>;
   goBack(params: PageGoBackParams, metadata?: Metadata): Promise<PageGoBackResult>;
   goForward(params: PageGoForwardParams, metadata?: Metadata): Promise<PageGoForwardResult>;
-  opener(params?: PageOpenerParams, metadata?: Metadata): Promise<PageOpenerResult>;
   reload(params: PageReloadParams, metadata?: Metadata): Promise<PageReloadResult>;
   screenshot(params: PageScreenshotParams, metadata?: Metadata): Promise<PageScreenshotResult>;
   setExtraHTTPHeaders(params: PageSetExtraHTTPHeadersParams, metadata?: Metadata): Promise<PageSetExtraHTTPHeadersResult>;
@@ -884,9 +883,6 @@ export type PageLoadEvent = {};
 export type PagePageErrorEvent = {
   error: SerializedError,
 };
-export type PagePopupEvent = {
-  page: PageChannel,
-};
 export type PageRequestEvent = {
   request: RequestChannel,
 };
@@ -989,11 +985,6 @@ export type PageGoForwardOptions = {
 export type PageGoForwardResult = {
   response?: ResponseChannel,
 };
-export type PageOpenerParams = {};
-export type PageOpenerOptions = {};
-export type PageOpenerResult = {
-  page?: PageChannel,
-};
 export type PageReloadParams = {
   timeout?: number,
   waitUntil?: 'load' | 'domcontentloaded' | 'networkidle',

src/protocol/protocol.yml

@@ -644,6 +644,7 @@ Page:
         width: number
         height: number
     isClosed: boolean
+    opener: Page?
 
   commands:
 
@@ -714,10 +715,6 @@ Page:
       returns:
         response: Response?
 
-    opener:
-      returns:
-        page: Page?
-
     reload:
       parameters:
         timeout: number?
@@ -958,10 +955,6 @@ Page:
       parameters:
         error: SerializedError
 
-    popup:
-      parameters:
-        page: Page
-
     request:
       parameters:
         request: Request

src/protocol/validator.ts

@@ -415,7 +415,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
     timeout: tOptional(tNumber),
     waitUntil: tOptional(tEnum(['load', 'domcontentloaded', 'networkidle'])),
   });
-  scheme.PageOpenerParams = tOptional(tObject({}));
   scheme.PageReloadParams = tObject({
     timeout: tOptional(tNumber),
     waitUntil: tOptional(tEnum(['load', 'domcontentloaded', 'networkidle'])),

src/server/chromium/crPage.ts

@@ -93,7 +93,13 @@ export class CRPage implements PageDelegate {
     }
     // Note: it is important to call |reportAsNew| before resolving pageOrError promise,
     // so that anyone who awaits pageOrError got a ready and reported page.
-    this._pagePromise = this._mainFrameSession._initialize(hasUIWindow).then(() => {
+    this._pagePromise = this._mainFrameSession._initialize(hasUIWindow).then(async r => {
+      await this._page.initOpener(this._opener);
+      return r;
+    }).catch(async e => {
+      await this._page.initOpener(this._opener);
+      throw e;
+    }).then(() => {
       this._initializedPage = this._page;
       this._reportAsNew();
       return this._page;
@@ -146,10 +152,6 @@ export class CRPage implements PageDelegate {
     return this._pagePromise;
   }
 
-  openerDelegate(): PageDelegate | null {
-    return this._opener;
-  }
-
   didClose() {
     for (const session of this._sessions.values())
       session.dispose();
@@ -203,15 +205,6 @@ export class CRPage implements PageDelegate {
     await this._forAllFrameSessions(frame => frame._setFileChooserIntercepted(enabled));
   }
 
-  async opener(): Promise<Page | null> {
-    if (!this._opener)
-      return null;
-    const openerPage = await this._opener.pageOrError();
-    if (openerPage instanceof Page && !openerPage.isClosed())
-      return openerPage;
-    return null;
-  }
-
   async reload(): Promise<void> {
     await this._mainFrameSession._client.send('Page.reload');
   }

src/server/firefox/ffPage.ts

@@ -46,6 +46,7 @@ export class FFPage implements PageDelegate {
   private _pagePromise: Promise<Page | Error>;
   private _pageCallback: (pageOrError: Page | Error) => void = () => {};
   _initializedPage: Page | null = null;
+  private _initializationFailed = false;
   readonly _opener: FFPage | null;
   private readonly _contextIdToContext: Map<string, dom.FrameExecutionContext>;
   private _eventListeners: RegisteredListener[];
@@ -91,8 +92,12 @@ export class FFPage implements PageDelegate {
       helper.addEventListener(this._session, 'Page.webSocketFrameSent', this._onWebSocketFrameSent.bind(this)),
     ];
     this._pagePromise = new Promise(f => this._pageCallback = f);
-    session.once(FFSessionEvents.Disconnected, () => this._page._didDisconnect());
+    session.once(FFSessionEvents.Disconnected, () => {
-    this._session.once('Page.ready', () => {
+      this._markAsError(new Error('Page closed'));
+      this._page._didDisconnect();
+    });
+    this._session.once('Page.ready', async () => {
+      await this._page.initOpener(this._opener);
       // Note: it is important to call |reportAsNew| before resolving pageOrError promise,
       // so that anyone who awaits pageOrError got a ready and reported page.
       this._initializedPage = this._page;
@@ -104,8 +109,14 @@ export class FFPage implements PageDelegate {
     this._session.send('Page.addScriptToEvaluateOnNewDocument', { script: '', worldName: UTILITY_WORLD_NAME }).catch(e => this._markAsError(e));
   }
 
-  _markAsError(error: Error) {
+  async _markAsError(error: Error) {
+    // Same error may be report twice: channer disconnected and session.send fails.
+    if (this._initializationFailed)
+      return;
+    this._initializationFailed = true;
+
     if (!this._initializedPage) {
+      await this._page.initOpener(this._opener);
       this._page.reportAsNew(error);
       this._pageCallback(error);
     }
@@ -115,10 +126,6 @@ export class FFPage implements PageDelegate {
     return this._pagePromise;
   }
 
-  openerDelegate(): PageDelegate | null {
-    return this._opener;
-  }
-
   _onWebSocketCreated(event: Protocol.Page.webSocketCreatedPayload) {
     this._page._frameManager.onWebSocketCreated(webSocketId(event.frameId, event.wsid), event.requestURL);
     this._page._frameManager.onWebSocketRequest(webSocketId(event.frameId, event.wsid));
@@ -310,8 +317,6 @@ export class FFPage implements PageDelegate {
   }
 
   didClose() {
-    if (!this._initializedPage)
-      this._markAsError(new Error('Page has been closed'));
     this._session.dispose();
     helper.removeEventListeners(this._eventListeners);
     this._networkManager.dispose();
@@ -358,15 +363,6 @@ export class FFPage implements PageDelegate {
     await this._session.send('Page.setInterceptFileChooserDialog', { enabled }).catch(e => {}); // target can be closed.
   }
 
-  async opener(): Promise<Page | null> {
-    if (!this._opener)
-      return null;
-    const result = await this._opener.pageOrError();
-    if (result instanceof Page && !result.isClosed())
-      return result;
-    return null;
-  }
-
   async reload(): Promise<void> {
     await this._session.send('Page.reload', { frameId: this._page.mainFrame()._id });
   }

src/server/page.ts

@@ -39,8 +39,6 @@ export interface PageDelegate {
   readonly rawKeyboard: input.RawKeyboard;
   readonly rawTouchscreen: input.RawTouchscreen;
 
-  opener(): Promise<Page | null>;
-
   reload(): Promise<void>;
   goBack(): Promise<boolean>;
   goForward(): Promise<boolean>;
@@ -48,7 +46,6 @@ export interface PageDelegate {
   evaluateOnNewDocument(source: string): Promise<void>;
   closePage(runBeforeUnload: boolean): Promise<void>;
   pageOrError(): Promise<Page | Error>;
-  openerDelegate(): PageDelegate | null;
 
   navigateFrame(frame: frames.Frame, url: string, referrer: string | undefined): Promise<frames.GotoResult>;
 
@@ -114,7 +111,6 @@ export class Page extends SdkObject {
     FrameDetached: 'framedetached',
     InternalFrameNavigatedToNewDocument: 'internalframenavigatedtonewdocument',
     Load: 'load',
-    Popup: 'popup',
     Video: 'video',
     WebSocket: 'websocket',
     Worker: 'worker',
@@ -150,6 +146,7 @@ export class Page extends SdkObject {
   readonly uniqueId: string;
   _pageIsError: Error | undefined;
   _video: Artifact | null = null;
+  _opener: Page | undefined;
 
   constructor(delegate: PageDelegate, browserContext: BrowserContext) {
     super(browserContext);
@@ -182,6 +179,14 @@ export class Page extends SdkObject {
     this.selectors = browserContext.selectors();
   }
 
+  async initOpener(opener: PageDelegate | null) {
+    if (!opener)
+      return;
+    const openerPage = await opener.pageOrError();
+    if (openerPage instanceof Page && !openerPage.isClosed())
+      this._opener = openerPage;
+  }
+
   reportAsNew(error?: Error) {
     if (error) {
       // Initialization error could have happened because of
@@ -191,13 +196,11 @@ export class Page extends SdkObject {
       this._setIsError(error);
     }
     this._browserContext.emit(BrowserContext.Events.Page, this);
-    const openerDelegate = this._delegate.openerDelegate();
+    // I may happen that page iniatialization finishes after Close event has already been sent,
-    if (openerDelegate) {
+    // in that case we fire another Close event to ensure that each reported Page will have
-      openerDelegate.pageOrError().then(openerPage => {
+    // corresponding Close event after it is reported on the context.
-        if (openerPage instanceof Page && !openerPage.isClosed())
+    if (this.isClosed())
-          openerPage.emit(Page.Events.Popup, this);
+      this.emit(Page.Events.Close);
-      });
-    }
   }
 
   async _doSlowMo() {
@@ -248,8 +251,8 @@ export class Page extends SdkObject {
     return this._browserContext;
   }
 
-  async opener(): Promise<Page | null> {
+  opener(): Page | undefined {
-    return await this._delegate.opener();
+    return this._opener;
   }
 
   mainFrame(): frames.Frame {

src/server/supplements/recorderSupplement.ts

@@ -304,17 +304,14 @@ export class RecorderSupplement {
     });
     frame.on(Frame.Events.Navigation, () => this._onFrameNavigated(frame, page));
     page.on(Page.Events.Download, () => this._onDownload(page));
-    page.on(Page.Events.Popup, popup => this._onPopup(page, popup));
     page.on(Page.Events.Dialog, () => this._onDialog(page));
     const suffix = this._pageAliases.size ? String(++this._lastPopupOrdinal) : '';
     const pageAlias = 'page' + suffix;
     this._pageAliases.set(page, pageAlias);
 
-    const isPopup = !!await page.opener();
+    if (page.opener()) {
-    // Could happen due to the await above.
+      this._onPopup(page.opener()!, page);
-    if (page.isClosed())
+    } else {
-      return;
-    if (!isPopup) {
       this._generator.addAction({
         pageAlias,
         ...describeFrame(page.mainFrame()),

src/server/webkit/wkPage.ts

@@ -273,10 +273,6 @@ export class WKPage implements PageDelegate {
     return this._pagePromise;
   }
 
-  openerDelegate(): PageDelegate | null {
-    return this._opener;
-  }
-
   private async _onTargetCreated(event: Protocol.Target.targetCreatedPayload) {
     const { targetInfo } = event;
     const session = new WKSession(this._pageProxySession.connection, targetInfo.targetId, `The ${targetInfo.type} has been closed.`, (message: any) => {
@@ -316,6 +312,7 @@ export class WKPage implements PageDelegate {
         // Avoid rejection on disconnect.
         this._firstNonInitialNavigationCommittedPromise.catch(() => {});
       }
+      await this._page.initOpener(this._opener);
       // Note: it is important to call |reportAsNew| before resolving pageOrError promise,
       // so that anyone who awaits pageOrError got a ready and reported page.
       this._initializedPage = pageOrError instanceof Page ? pageOrError : null;
@@ -664,15 +661,6 @@ export class WKPage implements PageDelegate {
     await this._session.send('Page.setInterceptFileChooserDialog', { enabled }).catch(e => {}); // target can be closed.
   }
 
-  async opener(): Promise<Page | null> {
-    if (!this._opener)
-      return null;
-    const openerPage = await this._opener.pageOrError();
-    if (openerPage instanceof Page && !openerPage.isClosed())
-      return openerPage;
-    return null;
-  }
-
   async reload(): Promise<void> {
     await this._session.send('Page.reload');
   }