chore: unify new page handling across vendors (#4411)

src/server/browserContext.ts

@@ -24,7 +24,7 @@ import { Download } from './download';
 import * as frames from './frames';
 import { helper } from './helper';
 import * as network from './network';
-import { Page, PageBinding } from './page';
+import { Page, PageBinding, PageDelegate } from './page';
 import { Progress, ProgressController, ProgressResult } from './progress';
 import { Selectors, serverSelectors } from './selectors';
 import * as types from './types';
@@ -157,7 +157,7 @@ export abstract class BrowserContext extends EventEmitter {
 
   // BrowserContext methods.
   abstract pages(): Page[];
-  abstract newPage(): Promise<Page>;
+  abstract newPageDelegate(): Promise<PageDelegate>;
   abstract _doCookies(urls: string[]): Promise<types.NetworkCookie[]>;
   abstract addCookies(cookies: types.SetNetworkCookieParam[]): Promise<void>;
   abstract clearCookies(): Promise<void>;
@@ -310,6 +310,17 @@ export abstract class BrowserContext extends EventEmitter {
     }
     await this._closePromise;
   }
+
+  async newPage(): Promise<Page> {
+    const pageDelegate = await this.newPageDelegate();
+    const pageOrError = await pageDelegate.pageOrError();
+    if (pageOrError instanceof Page) {
+      if (pageOrError.isClosed())
+        throw new Error('Page has been closed.');
+      return pageOrError;
+    }
+    throw pageOrError;
+  }
 }
 
 export function assertBrowserContextIsNotOwned(context: BrowserContext) {

src/server/chromium/crBrowser.ts

@@ -19,7 +19,7 @@ import { Browser, BrowserOptions } from '../browser';
 import { assertBrowserContextIsNotOwned, BrowserContext, validateBrowserContextOptions, verifyGeolocation } from '../browserContext';
 import { assert } from '../../utils/utils';
 import * as network from '../network';
-import { Page, PageBinding, Worker } from '../page';
+import { Page, PageBinding, PageDelegate, Worker } from '../page';
 import { ConnectionTransport } from '../transport';
 import * as types from '../types';
 import { ConnectionEvents, CRConnection, CRSession } from './crConnection';
@@ -166,23 +166,7 @@ export class CRBrowser extends Browser {
       const opener = targetInfo.openerId ? this._crPages.get(targetInfo.openerId) || null : null;
       const crPage = new CRPage(session, targetInfo.targetId, context, opener, true);
       this._crPages.set(targetInfo.targetId, crPage);
-      crPage.pageOrError().then(pageOrError => {
+      crPage._page.reportAsNew();
-        const page = crPage._page;
-        if (pageOrError instanceof Error) {
-          // Initialization error could have happened because of
-          // context/browser closure. Just ignore the page.
-          if (context!.isClosingOrClosed())
-            return;
-          page._setIsError();
-        }
-        context!.emit(BrowserContext.Events.Page, page);
-        if (opener) {
-          opener.pageOrError().then(openerPage => {
-            if (openerPage instanceof Page && !openerPage.isClosed())
-              openerPage.emit(Page.Events.Popup, page);
-          });
-        }
-      });
       return;
     }
 
@@ -330,17 +314,10 @@ export class CRBrowserContext extends BrowserContext {
     return result;
   }
 
-  async newPage(): Promise<Page> {
+  async newPageDelegate(): Promise<PageDelegate> {
     assertBrowserContextIsNotOwned(this);
     const { targetId } = await this._browser._session.send('Target.createTarget', { url: 'about:blank', browserContextId: this._browserContextId });
-    const crPage = this._browser._crPages.get(targetId)!;
+    return this._browser._crPages.get(targetId)!;
-    const result = await crPage.pageOrError();
-    if (result instanceof Page) {
-      if (result.isClosed())
-        throw new Error('Page has been closed.');
-      return result;
-    }
-    throw result;
   }
 
   async _doCookies(urls: string[]): Promise<types.NetworkCookie[]> {

src/server/chromium/crPage.ts

@@ -113,6 +113,10 @@ export class CRPage implements PageDelegate {
     return this._pagePromise;
   }
 
+  openerDelegate(): PageDelegate | null {
+    return this._opener;
+  }
+
   didClose() {
     for (const session of this._sessions.values())
       session.dispose();

src/server/firefox/ffBrowser.ts

@@ -19,7 +19,7 @@ import { assert } from '../../utils/utils';
 import { Browser, BrowserOptions } from '../browser';
 import { assertBrowserContextIsNotOwned, BrowserContext, validateBrowserContextOptions, verifyGeolocation } from '../browserContext';
 import * as network from '../network';
-import { Page, PageBinding } from '../page';
+import { Page, PageBinding, PageDelegate } from '../page';
 import { ConnectionTransport } from '../transport';
 import * as types from '../types';
 import { ConnectionEvents, FFConnection } from './ffConnection';
@@ -105,23 +105,7 @@ export class FFBrowser extends Browser {
     const opener = openerId ? this._ffPages.get(openerId)! : null;
     const ffPage = new FFPage(session, context, opener);
     this._ffPages.set(targetId, ffPage);
-
+    ffPage._page.reportAsNew();
-    ffPage.pageOrError().then(async pageOrError => {
-      const page = ffPage._page;
-      if (pageOrError instanceof Error) {
-        // Initialization error could have happened because of
-        // context/browser closure. Just ignore the page.
-        if (context.isClosingOrClosed())
-          return;
-        page._setIsError();
-      }
-      context.emit(BrowserContext.Events.Page, page);
-      if (!opener)
-        return;
-      const openerPage = await opener.pageOrError();
-      if (openerPage instanceof Page && !openerPage.isClosed())
-        openerPage.emit(Page.Events.Popup, page);
-    });
   }
 
   _onDownloadCreated(payload: Protocol.Browser.downloadCreatedPayload) {
@@ -235,7 +219,7 @@ export class FFBrowserContext extends BrowserContext {
     return this._ffPages().map(ffPage => ffPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
-  async newPage(): Promise<Page> {
+  async newPageDelegate(): Promise<PageDelegate> {
     assertBrowserContextIsNotOwned(this);
     const { targetId } = await this._browser._connection.send('Browser.newPage', {
       browserContextId: this._browserContextId
@@ -244,14 +228,7 @@ export class FFBrowserContext extends BrowserContext {
         throw new Error(`Invalid timezone ID: ${this._options.timezoneId}`);
       throw e;
     });
-    const ffPage = this._browser._ffPages.get(targetId)!;
+    return this._browser._ffPages.get(targetId)!;
-    const pageOrError = await ffPage.pageOrError();
-    if (pageOrError instanceof Page) {
-      if (pageOrError.isClosed())
-        throw new Error('Page has been closed.');
-      return pageOrError;
-    }
-    throw pageOrError;
   }
 
   async _doCookies(urls: string[]): Promise<types.NetworkCookie[]> {

src/server/firefox/ffPage.ts

@@ -105,6 +105,10 @@ 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));

src/server/page.ts

@@ -46,6 +46,8 @@ export interface PageDelegate {
   exposeBinding(binding: PageBinding): Promise<void>;
   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>;
 
@@ -176,6 +178,25 @@ export class Page extends EventEmitter {
     this.selectors = browserContext.selectors();
   }
 
+  async reportAsNew() {
+    const pageOrError = await this._delegate.pageOrError();
+    if (pageOrError instanceof Error) {
+      // Initialization error could have happened because of
+      // context/browser closure. Just ignore the page.
+      if (this._browserContext.isClosingOrClosed())
+        return;
+      this._setIsError();
+    }
+    this._browserContext.emit(BrowserContext.Events.Page, this);
+    const openerDelegate = this._delegate.openerDelegate();
+    if (openerDelegate) {
+      openerDelegate.pageOrError().then(openerPage => {
+        if (openerPage instanceof Page && !openerPage.isClosed())
+          openerPage.emit(Page.Events.Popup, this);
+      });
+    }
+  }
+
   async _doSlowMo() {
     const slowMo = this._browserContext._browser._options.slowMo;
     if (!slowMo)
@@ -387,7 +408,7 @@ export class Page extends EventEmitter {
       await this._ownedContext.close();
   }
 
-  _setIsError() {
+  private _setIsError() {
     if (!this._frameManager.mainFrame())
       this._frameManager.frameAttached('<dummy>', null);
   }

src/server/webkit/wkBrowser.ts

@@ -20,7 +20,7 @@ import { assertBrowserContextIsNotOwned, BrowserContext, validateBrowserContextO
 import { helper, RegisteredListener } from '../helper';
 import { assert } from '../../utils/utils';
 import * as network from '../network';
-import { Page, PageBinding } from '../page';
+import { Page, PageBinding, PageDelegate } from '../page';
 import { ConnectionTransport } from '../transport';
 import * as types from '../types';
 import { Protocol } from './protocol';
@@ -153,24 +153,7 @@ export class WKBrowser extends Browser {
     const opener = event.openerId ? this._wkPages.get(event.openerId) : undefined;
     const wkPage = new WKPage(context, pageProxySession, opener || null);
     this._wkPages.set(pageProxyId, wkPage);
-
+    wkPage._page.reportAsNew();
-    wkPage.pageOrError().then(async pageOrError => {
-      const page = wkPage._page;
-      if (pageOrError instanceof Error) {
-        // Initialization error could have happened because of
-        // context/browser closure. Just ignore the page.
-        if (context!.isClosingOrClosed())
-          return;
-        page._setIsError();
-      }
-      context!.emit(BrowserContext.Events.Page, page);
-      if (!opener)
-        return;
-      await opener.pageOrError();
-      const openerPage = opener._page;
-      if (!openerPage.isClosed())
-        openerPage.emit(Page.Events.Popup, page);
-    });
   }
 
   _onPageProxyDestroyed(event: Protocol.Playwright.pageProxyDestroyedPayload) {
@@ -255,16 +238,10 @@ export class WKBrowserContext extends BrowserContext {
     return this._wkPages().map(wkPage => wkPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
-  async newPage(): Promise<Page> {
+  async newPageDelegate(): Promise<PageDelegate> {
     assertBrowserContextIsNotOwned(this);
     const { pageProxyId } = await this._browser._browserSession.send('Playwright.createPage', { browserContextId: this._browserContextId });
-    const wkPage = this._browser._wkPages.get(pageProxyId)!;
+    return this._browser._wkPages.get(pageProxyId)!;
-    const result = await wkPage.pageOrError();
-    if (!(result instanceof Page))
-      throw result;
-    if (result.isClosed())
-      throw new Error('Page has been closed.');
-    return result;
   }
 
   async _doCookies(urls: string[]): Promise<types.NetworkCookie[]> {

src/server/webkit/wkPage.ts

@@ -273,6 +273,10 @@ 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) => {