diff --git a/src/server/chromium/crBrowser.ts b/src/server/chromium/crBrowser.ts index 081e06440e..269b3c0a80 100644 --- a/src/server/chromium/crBrowser.ts +++ b/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; } diff --git a/src/server/chromium/crPage.ts b/src/server/chromium/crPage.ts index 8b81be9929..9f71b0404a 100644 --- a/src/server/chromium/crPage.ts +++ b/src/server/chromium/crPage.ts @@ -57,6 +57,7 @@ export class CRPage implements PageDelegate { readonly _browserContext: CRBrowserContext; private readonly _pagePromise: Promise; _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) { diff --git a/src/server/firefox/ffBrowser.ts b/src/server/firefox/ffBrowser.ts index e3314d0075..1419571a4a 100644 --- a/src/server/firefox/ffBrowser.ts +++ b/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; } diff --git a/src/server/firefox/ffPage.ts b/src/server/firefox/ffPage.ts index a1f802459e..1653536551 100644 --- a/src/server/firefox/ffPage.ts +++ b/src/server/firefox/ffPage.ts @@ -44,7 +44,7 @@ export class FFPage implements PageDelegate { readonly _networkManager: FFNetworkManager; readonly _browserContext: FFBrowserContext; private _pagePromise: Promise; - _pageCallback: (pageOrError: Page | Error) => void = () => {}; + private _pageCallback: (pageOrError: Page | Error) => void = () => {}; _initializedPage: Page | null = null; readonly _opener: FFPage | null; private readonly _contextIdToContext: Map; @@ -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 { diff --git a/src/server/page.ts b/src/server/page.ts index 336f4208c7..3877148502 100644 --- a/src/server/page.ts +++ b/src/server/page.ts @@ -181,14 +181,13 @@ export class Page extends SdkObject { this.selectors = browserContext.selectors(); } - async reportAsNew() { - const pageOrError = await this._delegate.pageOrError(); - if (pageOrError instanceof Error) { + async reportAsNew(error?: Error) { + if (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(); diff --git a/src/server/webkit/wkBrowser.ts b/src/server/webkit/wkBrowser.ts index 90f6f6920f..9a4f52cdda 100644 --- a/src/server/webkit/wkBrowser.ts +++ b/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) { diff --git a/src/server/webkit/wkPage.ts b/src/server/webkit/wkPage.ts index 3ee8db0321..5afb17e540 100644 --- a/src/server/webkit/wkPage.ts +++ b/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);