From fc8112f904c24a463a8701627425178ee2acb0a0 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 13 Dec 2024 15:19:01 +0000 Subject: [PATCH] more cleanup --- .../src/server/bidi/bidiPage.ts | 10 +++----- .../src/server/browserContext.ts | 2 +- .../src/server/chromium/crBrowser.ts | 8 +++---- .../src/server/chromium/crPage.ts | 8 +++---- .../dispatchers/browserContextDispatcher.ts | 10 ++++---- .../server/dispatchers/dialogDispatcher.ts | 2 +- .../src/server/firefox/ffBrowser.ts | 4 ++-- .../src/server/firefox/ffPage.ts | 24 +++++++------------ packages/playwright-core/src/server/page.ts | 17 ++++++------- .../src/server/webkit/wkBrowser.ts | 4 ++-- .../src/server/webkit/wkPage.ts | 11 +++------ 11 files changed, 42 insertions(+), 58 deletions(-) diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index 95462a0335..9b501c5484 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/packages/playwright-core/src/server/bidi/bidiPage.ts @@ -80,9 +80,9 @@ export class BidiPage implements PageDelegate { // Initialize main frame. // TODO: Wait for first execution context to be created and maybe about:blank navigated. - this._initialize() - .finally(() => this._page.initOpener(this._opener?._page)) - .then(() => this._page.reportAsNew(), error => this._page.reportAsNew(error)); + this._initialize().then( + () => this._page.reportAsNew(this._opener?._page), + error => this._page.reportAsNew(this._opener?._page, error)); } private async _initialize() { @@ -101,10 +101,6 @@ export class BidiPage implements PageDelegate { return Promise.all(this._page.allInitScripts().map(initScript => this.addInitScript(initScript))); } - potentiallyUninitializedPage(): Page { - return this._page; - } - didClose() { this._session.dispose(); eventsHelper.removeEventListeners(this._sessionListeners); diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 484d792e3c..fc20c52bb5 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -257,7 +257,7 @@ export abstract class BrowserContext extends SdkObject { } pages(): Page[] { - return this.possiblyUninitializedPages().filter(page => page.initialized()); + return this.possiblyUninitializedPages().filter(page => page.initializedOrUndefined()); } // BrowserContext methods. diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index 5e62bf9470..9f03803dcb 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -259,10 +259,10 @@ export class CRBrowser extends Browser { } page.willBeginDownload(); - let originPage = page._page.initialized(); + let originPage = page._page.initializedOrUndefined(); // If it's a new window download, report it on the opener page. if (!originPage && page._opener) - originPage = page._opener._page.initialized(); + originPage = page._opener._page.initializedOrUndefined(); if (!originPage) return; this._downloadCreated(originPage, payload.guid, payload.url, payload.suggestedFilename); @@ -544,7 +544,7 @@ export class CRBrowserContext extends BrowserContext { // When persistent context is closed, we do not necessary get Target.detachedFromTarget // for all the background pages. for (const [targetId, backgroundPage] of this._browser._backgroundPages.entries()) { - if (backgroundPage._browserContext === this && backgroundPage._page.initialized()) { + if (backgroundPage._browserContext === this && backgroundPage._page.initializedOrUndefined()) { backgroundPage.didClose(); this._browser._backgroundPages.delete(targetId); } @@ -569,7 +569,7 @@ export class CRBrowserContext extends BrowserContext { backgroundPages(): Page[] { const result: Page[] = []; for (const backgroundPage of this._browser._backgroundPages.values()) { - if (backgroundPage._browserContext === this && backgroundPage._page.initialized()) + if (backgroundPage._browserContext === this && backgroundPage._page.initializedOrUndefined()) result.push(backgroundPage._page); } return result; diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 0cdd721a90..0dc4703547 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -108,9 +108,9 @@ export class CRPage implements PageDelegate { } const createdEvent = this._isBackgroundPage ? CRBrowserContext.CREvents.BackgroundPage : BrowserContext.Events.Page; - this._mainFrameSession._initialize(bits.hasUIWindow) - .finally(() => this._page.initOpener(this._opener?._page)) - .then(() => this._page.reportAsNew(undefined, createdEvent), error => this._page.reportAsNew(error, createdEvent)); + this._mainFrameSession._initialize(bits.hasUIWindow).then( + () => this._page.reportAsNew(this._opener?._page, undefined, createdEvent), + error => this._page.reportAsNew(this._opener?._page, error, createdEvent)); } private async _forAllFrameSessions(cb: (frame: FrameSession) => Promise) { @@ -873,7 +873,7 @@ class FrameSession { } _willBeginDownload() { - if (!this._crPage._page.initialized()) { + if (!this._crPage._page.initializedOrUndefined()) { // Resume the page creation with an error. The page will automatically close right // after the download begins. this._firstNonInitialNavigationCommittedReject(new Error('Starting new page download')); diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index 7f24358ac0..c6ffce49f7 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -133,7 +133,7 @@ export class BrowserContextDispatcher extends Dispatcher { @@ -142,7 +142,7 @@ export class BrowserContextDispatcher extends Dispatcher { @@ -153,7 +153,7 @@ export class BrowserContextDispatcher extends Dispatcher { @@ -164,13 +164,13 @@ export class BrowserContextDispatcher extends Dispatcher; private _eventListeners: RegisteredListener[]; @@ -99,31 +99,23 @@ export class FFPage implements PageDelegate { eventsHelper.addEventListener(this._session, 'Page.screencastFrame', this._onScreencastFrame.bind(this)), ]; - this._session.once('Page.ready', async () => { - await this._page.initOpener(this._opener?._page); - if (this._initializationFailed) + this._session.once('Page.ready', () => { + if (this._reportedAsNew) return; - this._page.reportAsNew(); + this._reportedAsNew = true; + this._page.reportAsNew(this._opener?._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.addInitScript(new InitScript('', true), UTILITY_WORLD_NAME).catch(e => this._markAsError(e)); } - potentiallyUninitializedPage(): Page { - return this._page; - } - async _markAsError(error: Error) { // Same error may be reported twice: channel disconnected and session.send fails. - if (this._initializationFailed) + if (this._reportedAsNew) return; - this._initializationFailed = true; - - if (!this._page.initialized()) { - await this._page.initOpener(this._opener?._page); - this._page.reportAsNew(error); - } + this._reportedAsNew = true; + this._page.reportAsNew(this._opener?._page, error); } _onWebSocketCreated(event: Protocol.Page.webSocketCreatedPayload) { diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index da350aa0ab..fe483b8347 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -192,15 +192,16 @@ export class Page extends SdkObject { this.coverage = delegate.coverage ? delegate.coverage() : null; } - async initOpener(opener: Page | undefined) { - if (!opener) - return; - const openerPageOrError = await opener.waitForInitializedOrError(); - if (openerPageOrError instanceof Page && !openerPageOrError.isClosed()) - this._opener = openerPageOrError; + async reportAsNew(opener: Page | undefined, error: Error | undefined = undefined, contextEvent: string = BrowserContext.Events.Page) { + if (opener) { + const openerPageOrError = await opener.waitForInitializedOrError(); + if (openerPageOrError instanceof Page && !openerPageOrError.isClosed()) + this._opener = openerPageOrError; + } + this._markInitialized(error, contextEvent); } - reportAsNew(error: Error | undefined = undefined, contextEvent: string = BrowserContext.Events.Page) { + private _markInitialized(error: Error | undefined = undefined, contextEvent: string = BrowserContext.Events.Page) { if (error) { // Initialization error could have happened because of // context/browser closure. Just ignore the page. @@ -228,7 +229,7 @@ export class Page extends SdkObject { this._initializedPromise.resolve(this._initialized); } - initialized(): Page | undefined { + initializedOrUndefined(): Page | undefined { return this._initialized ? this : undefined; } diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index 4ce375e283..86833088d8 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -121,14 +121,14 @@ export class WKBrowser extends Browser { // abort navigation that is still running. We should be able to fix this by // instrumenting policy decision start/proceed/cancel. page._page._frameManager.frameAbortedNavigation(payload.frameId, 'Download is starting'); - let originPage = page._page.initialized(); + let originPage = page._page.initializedOrUndefined(); // If it's a new window download, report it on the opener page. if (!originPage) { // Resume the page creation with an error. The page will automatically close right // after the download begins. page._firstNonInitialNavigationCommittedReject(new Error('Starting new page download')); if (page._opener) - originPage = page._opener._page.initialized(); + originPage = page._opener._page.initializedOrUndefined(); } if (!originPage) return; diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 60af3f8f6e..15005f589b 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -108,10 +108,6 @@ export class WKPage implements PageDelegate { } } - potentiallyUninitializedPage(): Page { - return this._page; - } - private async _initializePageProxySession() { if (this._page._browserContext.isSettingStorageState()) return; @@ -280,7 +276,7 @@ export class WKPage implements PageDelegate { } handleProvisionalLoadFailed(event: Protocol.Playwright.provisionalLoadFailedPayload) { - if (!this._page.initialized()) { + if (!this._page.initializedOrUndefined()) { this._firstNonInitialNavigationCommittedReject(new Error('Initial load failed')); return; } @@ -309,7 +305,7 @@ export class WKPage implements PageDelegate { assert(targetInfo.type === 'page', 'Only page targets are expected in WebKit, received: ' + targetInfo.type); if (!targetInfo.isProvisional) { - assert(!this._page.initialized()); + assert(!this._page.initializedOrUndefined()); let pageOrError: Page | Error; try { this._setSession(session); @@ -336,8 +332,7 @@ export class WKPage implements PageDelegate { // Avoid rejection on disconnect. this._firstNonInitialNavigationCommittedPromise.catch(() => {}); } - await this._page.initOpener(this._opener?._page); - this._page.reportAsNew(pageOrError instanceof Page ? undefined : pageOrError); + this._page.reportAsNew(this._opener?._page, pageOrError instanceof Page ? undefined : pageOrError); } else { assert(targetInfo.isProvisional); assert(!this._provisionalPage);