chore: ensure we emit Page event before resoliving pageOrError (#6012)

Internal callers of pageOrError should be able to rely on the
Page being already reported.

src/server/chromium/crBrowser.ts

@@ -136,20 +136,15 @@ export class CRBrowser extends Browser {
     assert(!this._serviceWorkers.has(targetInfo.targetId), 'Duplicate target ' + targetInfo.targetId);
 
     if (targetInfo.type === 'background_page') {
-      const backgroundPage = new CRPage(session, targetInfo.targetId, context, null, false);
+      const backgroundPage = new CRPage(session, targetInfo.targetId, context, null, false, true);
       this._backgroundPages.set(targetInfo.targetId, backgroundPage);
-      backgroundPage.pageOrError().then(pageOrError => {
-        if (pageOrError instanceof Page)
-          context!.emit(CRBrowserContext.CREvents.BackgroundPage, backgroundPage._page);
-      });
       return;
     }
 
     if (targetInfo.type === 'page') {
       const opener = targetInfo.openerId ? this._crPages.get(targetInfo.openerId) || null : null;
-      const crPage = new CRPage(session, targetInfo.targetId, context, opener, true);
+      const crPage = new CRPage(session, targetInfo.targetId, context, opener, true, false);
       this._crPages.set(targetInfo.targetId, crPage);
-      crPage._page.reportAsNew();
       return;
     }
 

src/server/chromium/crPage.ts

@@ -57,6 +57,7 @@ export class CRPage implements PageDelegate {
   readonly _browserContext: CRBrowserContext;
   private readonly _pagePromise: Promise<Page | Error>;
   _initializedPage: Page | null = null;
+  private _isBackgroundPage: boolean;
 
   // Holds window features for the next popup being opened via window.open,
   // until the popup target arrives. This could be racy if two oopifs
@@ -70,9 +71,10 @@ export class CRPage implements PageDelegate {
     return crPage._mainFrameSession;
   }
 
-  constructor(client: CRSession, targetId: string, browserContext: CRBrowserContext, opener: CRPage | null, hasUIWindow: boolean) {
+  constructor(client: CRSession, targetId: string, browserContext: CRBrowserContext, opener: CRPage | null, hasUIWindow: boolean, isBackgroundPage: boolean) {
     this._targetId = targetId;
     this._opener = opener;
+    this._isBackgroundPage = isBackgroundPage;
     this.rawKeyboard = new RawKeyboardImpl(client, browserContext._browser._isMac);
     this.rawMouse = new RawMouseImpl(client);
     this.rawTouchscreen = new RawTouchscreenImpl(client);
@@ -89,7 +91,25 @@ export class CRPage implements PageDelegate {
       if (viewportSize)
         this._page._state.emulatedSize = { viewport: viewportSize, screen: viewportSize };
     }
-    this._pagePromise = this._mainFrameSession._initialize(hasUIWindow).then(() => this._initializedPage = this._page).catch(e => e);
+    // 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._initializedPage = this._page;
+      this._reportAsNew();
+      return this._page;
+    }).catch(e => {
+      this._reportAsNew(e);
+      return e;
+    });
+  }
+
+  private _reportAsNew(error?: Error) {
+    if (this._isBackgroundPage) {
+      if (!error)
+        this._browserContext.emit(CRBrowserContext.CREvents.BackgroundPage, this._page);
+    } else {
+      this._page.reportAsNew(error);
+    }
   }
 
   private async _forAllFrameSessions(cb: (frame: FrameSession) => Promise<any>) {

src/server/firefox/ffBrowser.ts

@@ -107,7 +107,6 @@ 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();
   }
 
   _onDownloadCreated(payload: Protocol.Browser.downloadCreatedPayload) {
@@ -120,7 +119,7 @@ export class FFBrowser extends Browser {
     if (!originPage) {
       // Resume the page creation with an error. The page will automatically close right
       // after the download begins.
-      ffPage._pageCallback(new Error('Starting new page download'));
+      ffPage._markAsError(new Error('Starting new page download'));
       if (ffPage._opener)
         originPage = ffPage._opener._initializedPage;
     }

src/server/firefox/ffPage.ts

@@ -44,7 +44,7 @@ export class FFPage implements PageDelegate {
   readonly _networkManager: FFNetworkManager;
   readonly _browserContext: FFBrowserContext;
   private _pagePromise: Promise<Page | Error>;
-  _pageCallback: (pageOrError: Page | Error) => void = () => {};
+  private _pageCallback: (pageOrError: Page | Error) => void = () => {};
   _initializedPage: Page | null = null;
   readonly _opener: FFPage | null;
   private readonly _contextIdToContext: Map<string, dom.FrameExecutionContext>;
@@ -93,12 +93,22 @@ export class FFPage implements PageDelegate {
     this._pagePromise = new Promise(f => this._pageCallback = f);
     session.once(FFSessionEvents.Disconnected, () => this._page._didDisconnect());
     this._session.once('Page.ready', () => {
-      this._pageCallback(this._page);
+      // 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;
+      this._page.reportAsNew();
+      this._pageCallback(this._page);
     });
     // Ideally, we somehow ensure that utility world is created before Page.ready arrives, but currently it is racy.
     // Therefore, we can end up with an initialized page without utility world, although very unlikely.
-    this._session.send('Page.addScriptToEvaluateOnNewDocument', { script: '', worldName: UTILITY_WORLD_NAME }).catch(this._pageCallback);
+    this._session.send('Page.addScriptToEvaluateOnNewDocument', { script: '', worldName: UTILITY_WORLD_NAME }).catch(e => this._markAsError(e));
+  }
+
+  _markAsError(error: Error) {
+    if (!this._initializedPage) {
+      this._page.reportAsNew(error);
+      this._pageCallback(error);
+    }
   }
 
   async pageOrError(): Promise<Page | Error> {

src/server/page.ts

@@ -181,14 +181,13 @@ export class Page extends SdkObject {
     this.selectors = browserContext.selectors();
   }
 
-  async reportAsNew() {
+  async reportAsNew(error?: Error) {
-    const pageOrError = await this._delegate.pageOrError();
+    if (error) {
-    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(pageOrError);
+      this._setIsError(error);
     }
     this._browserContext.emit(BrowserContext.Events.Page, this);
     const openerDelegate = this._delegate.openerDelegate();

src/server/webkit/wkBrowser.ts

@@ -155,7 +155,6 @@ 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();
   }
 
   _onPageProxyDestroyed(event: Protocol.Playwright.pageProxyDestroyedPayload) {

src/server/webkit/wkPage.ts

@@ -316,7 +316,10 @@ export class WKPage implements PageDelegate {
         // Avoid rejection on disconnect.
         this._firstNonInitialNavigationCommittedPromise.catch(() => {});
       }
+      // 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;
+      this._page.reportAsNew(pageOrError instanceof Page ? undefined : pageOrError);
       this._pagePromiseCallback(pageOrError);
     } else {
       assert(targetInfo.isProvisional);