From b0dc3efc2d1bb9ee0318d1b8b1cea8756c21e967 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 13 Dec 2024 14:25:04 +0000 Subject: [PATCH] chore: simplify page initialization logic across browser types --- .../src/server/bidi/bidiBrowser.ts | 18 +++----- .../src/server/bidi/bidiPage.ts | 23 +++------- .../src/server/browserContext.ts | 44 +++++++++---------- .../src/server/chromium/crBrowser.ts | 26 +++++------ .../src/server/chromium/crPage.ts | 42 ++++-------------- .../dispatchers/browserContextDispatcher.ts | 10 ++--- .../server/dispatchers/dialogDispatcher.ts | 2 +- .../src/server/firefox/ffBrowser.ts | 18 +++----- .../src/server/firefox/ffPage.ts | 24 +++------- packages/playwright-core/src/server/page.ts | 25 +++++++---- .../src/server/webkit/wkBrowser.ts | 18 +++----- .../src/server/webkit/wkPage.ts | 21 +++------ 12 files changed, 99 insertions(+), 172 deletions(-) diff --git a/packages/playwright-core/src/server/bidi/bidiBrowser.ts b/packages/playwright-core/src/server/bidi/bidiBrowser.ts index acefc08fb2..955f6274a3 100644 --- a/packages/playwright-core/src/server/bidi/bidiBrowser.ts +++ b/packages/playwright-core/src/server/bidi/bidiBrowser.ts @@ -22,7 +22,7 @@ import { Browser } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext } from '../browserContext'; import type { SdkObject } from '../instrumentation'; import * as network from '../network'; -import type { InitScript, Page, PageDelegate } from '../page'; +import type { InitScript, Page } from '../page'; import type { ConnectionTransport } from '../transport'; import type * as types from '../types'; import type { BidiSession } from './bidiConnection'; @@ -99,8 +99,8 @@ export class BidiBrowser extends Browser { browser._defaultContext = new BidiBrowserContext(browser, undefined, options.persistent); await (browser._defaultContext as BidiBrowserContext)._initialize(); // Create default page as we cannot get access to the existing one. - const pageDelegate = await browser._defaultContext.newPageDelegate(); - await pageDelegate.pageOrError(); + const page = await browser._defaultContext.doCreateNewPage(); + await page.waitForInitializedOrError(); } return browser; } @@ -207,21 +207,17 @@ export class BidiBrowserContext extends BrowserContext { return [...this._browser._bidiPages.values()].filter(bidiPage => bidiPage._browserContext === this); } - pages(): Page[] { - return this._bidiPages().map(bidiPage => bidiPage._initializedPage).filter(Boolean) as Page[]; + override possiblyUninitializedPages(): Page[] { + return this._bidiPages().map(bidiPage => bidiPage._page); } - pagesOrErrors() { - return this._bidiPages().map(bidiPage => bidiPage.pageOrError()); - } - - async newPageDelegate(): Promise { + override async doCreateNewPage(): Promise { assertBrowserContextIsNotOwned(this); const { context } = await this._browser._browserSession.send('browsingContext.create', { type: bidi.BrowsingContext.CreateType.Window, userContext: this._browserContextId, }); - return this._browser._bidiPages.get(context)!; + return this._browser._bidiPages.get(context)!._page; } async doGetCookies(urls: string[]): Promise { diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index 56bb43cb1a..95462a0335 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/packages/playwright-core/src/server/bidi/bidiPage.ts @@ -43,7 +43,6 @@ export class BidiPage implements PageDelegate { readonly rawKeyboard: RawKeyboardImpl; readonly rawTouchscreen: RawTouchscreenImpl; readonly _page: Page; - private readonly _pagePromise: Promise; readonly _session: BidiSession; readonly _opener: BidiPage | null; private readonly _realmToContext: Map; @@ -51,7 +50,6 @@ export class BidiPage implements PageDelegate { readonly _browserContext: BidiBrowserContext; readonly _networkManager: BidiNetworkManager; private readonly _pdf: BidiPDF; - _initializedPage: Page | null = null; private _initScriptIds: string[] = []; constructor(browserContext: BidiBrowserContext, bidiSession: BidiSession, opener: BidiPage | null) { @@ -81,16 +79,10 @@ export class BidiPage implements PageDelegate { ]; // Initialize main frame. - this._pagePromise = this._initialize().finally(async () => { - await this._page.initOpener(this._opener); - }).then(() => { - this._initializedPage = this._page; - this._page.reportAsNew(); - return this._page; - }).catch(e => { - this._page.reportAsNew(e); - return e; - }); + // 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)); } private async _initialize() { @@ -119,11 +111,6 @@ export class BidiPage implements PageDelegate { this._page._didClose(); } - async pageOrError(): Promise { - // TODO: Wait for first execution context to be created and maybe about:blank navigated. - return this._pagePromise; - } - private _onFrameAttached(frameId: string, parentFrameId: string | null): frames.Frame { return this._page._frameManager.frameAttached(frameId, parentFrameId); } @@ -372,7 +359,7 @@ export class BidiPage implements PageDelegate { private async _onScriptMessage(event: bidi.Script.MessageParameters) { if (event.channel !== kPlaywrightBindingChannel) return; - const pageOrError = await this.pageOrError(); + const pageOrError = await this._page.waitForInitializedOrError(); if (pageOrError instanceof Error) return; const context = this._realmToContext.get(event.source.realm); diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 6d9227c0a7..484d792e3c 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -24,7 +24,6 @@ import type * as frames from './frames'; import { helper } from './helper'; import * as network from './network'; import { InitScript } from './page'; -import type { PageDelegate } from './page'; import { Page, PageBinding } from './page'; import type { Progress, ProgressController } from './progress'; import type { Selectors } from './selectors'; @@ -257,10 +256,13 @@ export abstract class BrowserContext extends SdkObject { this.emit(BrowserContext.Events.Close); } + pages(): Page[] { + return this.possiblyUninitializedPages().filter(page => page.initialized()); + } + // BrowserContext methods. - abstract pages(): Page[]; - abstract pagesOrErrors(): Promise[]; - abstract newPageDelegate(): Promise; + abstract possiblyUninitializedPages(): Page[]; + abstract doCreateNewPage(): Promise; abstract addCookies(cookies: channels.SetNetworkCookie[]): Promise; abstract setGeolocation(geolocation?: types.Geolocation): Promise; abstract setExtraHTTPHeaders(headers: types.HeadersArray): Promise; @@ -359,38 +361,34 @@ export abstract class BrowserContext extends SdkObject { this._timeoutSettings.setDefaultTimeout(timeout); } - async _loadDefaultContextAsIs(progress: Progress): Promise { - let pageOrError; - if (!this.pagesOrErrors().length) { + async _loadDefaultContextAsIs(progress: Progress): Promise { + if (!this.possiblyUninitializedPages().length) { const waitForEvent = helper.waitForEvent(progress, this, BrowserContext.Events.Page); progress.cleanupWhenAborted(() => waitForEvent.dispose); // Race against BrowserContext.close - pageOrError = await Promise.race([ - waitForEvent.promise as Promise, - this._closePromise, - ]); - // Consider Page initialization errors - if (pageOrError instanceof Page) - pageOrError = await pageOrError._delegate.pageOrError(); - } else { - pageOrError = await this.pagesOrErrors()[0]; + await Promise.race([waitForEvent.promise, this._closePromise]); } + const page = this.possiblyUninitializedPages()[0]; + if (!page) + return; + const pageOrError = await page.waitForInitializedOrError(); if (pageOrError instanceof Error) throw pageOrError; - await pageOrError.mainFrame()._waitForLoadState(progress, 'load'); - return pageOrError; + await page.mainFrame()._waitForLoadState(progress, 'load'); + return page; } async _loadDefaultContext(progress: Progress) { const defaultPage = await this._loadDefaultContextAsIs(progress); + if (!defaultPage) + return; const browserName = this._browser.options.name; if ((this._options.isMobile && browserName === 'chromium') || (this._options.locale && browserName === 'webkit')) { // Workaround for: // - chromium fails to change isMobile for existing page; // - webkit fails to change locale for existing page. - const oldPage = defaultPage; await this.newPage(progress.metadata); - await oldPage.close(progress.metadata); + await defaultPage.close(progress.metadata); } } @@ -488,10 +486,10 @@ export abstract class BrowserContext extends SdkObject { } async newPage(metadata: CallMetadata): Promise { - const pageDelegate = await this.newPageDelegate(); + const page = await this.doCreateNewPage(); if (metadata.isServerSide) - pageDelegate.potentiallyUninitializedPage().markAsServerSideOnly(); - const pageOrError = await pageDelegate.pageOrError(); + page.markAsServerSideOnly(); + const pageOrError = await page.waitForInitializedOrError(); if (pageOrError instanceof Page) { if (pageOrError.isClosed()) throw new Error('Page has been closed.'); diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index 1aba1ed8cb..5e62bf9470 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -21,7 +21,7 @@ import { Browser } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, verifyGeolocation } from '../browserContext'; import { assert, createGuid } from '../../utils'; import * as network from '../network'; -import type { InitScript, PageDelegate, Worker } from '../page'; +import type { InitScript, Worker } from '../page'; import { Page } from '../page'; import { Frame } from '../frames'; import type { Dialog } from '../dialog'; @@ -146,7 +146,7 @@ export class CRBrowser extends Browser { } async _waitForAllPagesToBeInitialized() { - await Promise.all([...this._crPages.values()].map(page => page.pageOrError())); + await Promise.all([...this._crPages.values()].map(crPage => crPage._page.waitForInitializedOrError())); } _onAttachedToTarget({ targetInfo, sessionId, waitingForDebugger }: Protocol.Target.attachedToTargetPayload) { @@ -259,10 +259,10 @@ export class CRBrowser extends Browser { } page.willBeginDownload(); - let originPage = page._initializedPage; + let originPage = page._page.initialized(); // If it's a new window download, report it on the opener page. if (!originPage && page._opener) - originPage = page._opener._initializedPage; + originPage = page._opener._page.initialized(); if (!originPage) return; this._downloadCreated(originPage, payload.guid, payload.url, payload.suggestedFilename); @@ -364,15 +364,11 @@ export class CRBrowserContext extends BrowserContext { return [...this._browser._crPages.values()].filter(crPage => crPage._browserContext === this); } - pages(): Page[] { - return this._crPages().map(crPage => crPage._initializedPage).filter(Boolean) as Page[]; + override possiblyUninitializedPages(): Page[] { + return this._crPages().map(crPage => crPage._page); } - pagesOrErrors() { - return this._crPages().map(crPage => crPage.pageOrError()); - } - - async newPageDelegate(): Promise { + override async doCreateNewPage(): Promise { assertBrowserContextIsNotOwned(this); const oldKeys = this._browser.isClank() ? new Set(this._browser._crPages.keys()) : undefined; @@ -395,7 +391,7 @@ export class CRBrowserContext extends BrowserContext { assert(newKeys.size === 1); [targetId] = [...newKeys]; } - return this._browser._crPages.get(targetId)!; + return this._browser._crPages.get(targetId)!._page; } async doGetCookies(urls: string[]): Promise { @@ -548,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._initializedPage) { + if (backgroundPage._browserContext === this && backgroundPage._page.initialized()) { backgroundPage.didClose(); this._browser._backgroundPages.delete(targetId); } @@ -573,8 +569,8 @@ export class CRBrowserContext extends BrowserContext { backgroundPages(): Page[] { const result: Page[] = []; for (const backgroundPage of this._browser._backgroundPages.values()) { - if (backgroundPage._browserContext === this && backgroundPage._initializedPage) - result.push(backgroundPage._initializedPage); + if (backgroundPage._browserContext === this && backgroundPage._page.initialized()) + 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 a7ee34d2c5..0cdd721a90 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -65,8 +65,6 @@ export class CRPage implements PageDelegate { private readonly _pdf: CRPDF; private readonly _coverage: CRCoverage; 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, @@ -108,30 +106,11 @@ export class CRPage implements PageDelegate { if (viewportSize) this._page._emulatedSize = { viewport: viewportSize, screen: viewportSize }; } - // 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(bits.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; - }).catch(e => { - this._reportAsNew(e); - return e; - }); - } - potentiallyUninitializedPage(): Page { - return this._page; - } - - private _reportAsNew(error?: Error) { - this._page.reportAsNew(error, this._isBackgroundPage ? CRBrowserContext.CREvents.BackgroundPage : BrowserContext.Events.Page); + 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)); } private async _forAllFrameSessions(cb: (frame: FrameSession) => Promise) { @@ -168,10 +147,6 @@ export class CRPage implements PageDelegate { this._mainFrameSession._willBeginDownload(); } - async pageOrError(): Promise { - return this._pagePromise; - } - didClose() { for (const session of this._sessions.values()) session.dispose(); @@ -492,7 +467,7 @@ class FrameSession { // Note: it is important to start video recorder before sending Page.startScreencast, // and it is equally important to send Page.startScreencast before sending Runtime.runIfWaitingForDebugger. await this._createVideoRecorder(screencastId, screencastOptions); - this._crPage.pageOrError().then(p => { + this._crPage._page.waitForInitializedOrError().then(p => { if (p instanceof Error) this._stopVideoRecording().catch(() => {}); }); @@ -833,7 +808,7 @@ class FrameSession { } async _onBindingCalled(event: Protocol.Runtime.bindingCalledPayload) { - const pageOrError = await this._crPage.pageOrError(); + const pageOrError = await this._crPage._page.waitForInitializedOrError(); if (!(pageOrError instanceof Error)) { const context = this._contextIdToContext.get(event.executionContextId); if (context) @@ -898,8 +873,7 @@ class FrameSession { } _willBeginDownload() { - const originPage = this._crPage._initializedPage; - if (!originPage) { + if (!this._crPage._page.initialized()) { // 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')); @@ -939,7 +913,7 @@ class FrameSession { }); // Wait for the first frame before reporting video to the client. gotFirstFrame.then(() => { - this._crPage._browserContext._browser._videoStarted(this._crPage._browserContext, screencastId, options.outputFile, this._crPage.pageOrError()); + this._crPage._browserContext._browser._videoStarted(this._crPage._browserContext, screencastId, options.outputFile, this._crPage._page.waitForInitializedOrError()); }); } diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index c6ffce49f7..7f24358ac0 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 ffPage._browserContext === this); } - pages(): Page[] { - return this._ffPages().map(ffPage => ffPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[]; + override possiblyUninitializedPages(): Page[] { + return this._ffPages().map(ffPage => ffPage._page); } - pagesOrErrors() { - return this._ffPages().map(ffPage => ffPage.pageOrError()); - } - - async newPageDelegate(): Promise { + override async doCreateNewPage(): Promise { assertBrowserContextIsNotOwned(this); const { targetId } = await this._browser.session.send('Browser.newPage', { browserContextId: this._browserContextId @@ -284,7 +280,7 @@ export class FFBrowserContext extends BrowserContext { throw new Error(`Invalid timezone ID: ${this._options.timezoneId}`); throw e; }); - return this._browser._ffPages.get(targetId)!; + return this._browser._ffPages.get(targetId)!._page; } async doGetCookies(urls: string[]): Promise { diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts index 61790feae4..08e3b33688 100644 --- a/packages/playwright-core/src/server/firefox/ffPage.ts +++ b/packages/playwright-core/src/server/firefox/ffPage.ts @@ -34,7 +34,6 @@ import type { Protocol } from './protocol'; import type { Progress } from '../progress'; import { splitErrorMessage } from '../../utils/stackTrace'; import { debugLogger } from '../../utils/debugLogger'; -import { ManualPromise } from '../../utils/manualPromise'; import { BrowserContext } from '../browserContext'; import { TargetClosedError } from '../errors'; @@ -49,8 +48,6 @@ export class FFPage implements PageDelegate { readonly _page: Page; readonly _networkManager: FFNetworkManager; readonly _browserContext: FFBrowserContext; - private _pagePromise = new ManualPromise(); - _initializedPage: Page | null = null; private _initializationFailed = false; readonly _opener: FFPage | null; private readonly _contextIdToContext: Map; @@ -103,14 +100,10 @@ export class FFPage implements PageDelegate { ]; this._session.once('Page.ready', async () => { - await this._page.initOpener(this._opener); + await this._page.initOpener(this._opener?._page); if (this._initializationFailed) return; - // 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._pagePromise.resolve(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. @@ -122,22 +115,17 @@ export class FFPage implements PageDelegate { } async _markAsError(error: Error) { - // Same error may be report twice: channer disconnected and session.send fails. + // Same error may be reported twice: channel disconnected and session.send fails. if (this._initializationFailed) return; this._initializationFailed = true; - if (!this._initializedPage) { - await this._page.initOpener(this._opener); + if (!this._page.initialized()) { + await this._page.initOpener(this._opener?._page); this._page.reportAsNew(error); - this._pagePromise.resolve(error); } } - async pageOrError(): Promise { - return this._pagePromise; - } - _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)); @@ -268,7 +256,7 @@ export class FFPage implements PageDelegate { } async _onBindingCalled(event: Protocol.Page.bindingCalledPayload) { - const pageOrError = await this.pageOrError(); + const pageOrError = await this._page.waitForInitializedOrError(); if (!(pageOrError instanceof Error)) { const context = this._contextIdToContext.get(event.executionContextId); if (context) @@ -333,7 +321,7 @@ export class FFPage implements PageDelegate { } _onVideoRecordingStarted(event: Protocol.Page.videoRecordingStartedPayload) { - this._browserContext._browser._videoStarted(this._browserContext, event.screencastId, event.file, this.pageOrError()); + this._browserContext._browser._videoStarted(this._browserContext, event.screencastId, event.file, this._page.waitForInitializedOrError()); } didClose() { diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 48c0827c08..da350aa0ab 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -59,8 +59,6 @@ export interface PageDelegate { addInitScript(initScript: InitScript): Promise; removeNonInternalInitScripts(): Promise; closePage(runBeforeUnload: boolean): Promise; - potentiallyUninitializedPage(): Page; - pageOrError(): Promise; navigateFrame(frame: frames.Frame, url: string, referrer: string | undefined): Promise; @@ -139,7 +137,8 @@ export class Page extends SdkObject { private _closedState: 'open' | 'closing' | 'closed' = 'open'; private _closedPromise = new ManualPromise(); - private _initialized = false; + private _initialized: Page | Error | undefined; + private _initializedPromise = new ManualPromise(); private _eventsToEmitAfterInitialized: { event: string | symbol, args: any[] }[] = []; private _crashed = false; readonly openScope = new LongStandingScope(); @@ -193,12 +192,12 @@ export class Page extends SdkObject { this.coverage = delegate.coverage ? delegate.coverage() : null; } - async initOpener(opener: PageDelegate | null) { + async initOpener(opener: Page | undefined) { if (!opener) return; - const openerPage = await opener.pageOrError(); - if (openerPage instanceof Page && !openerPage.isClosed()) - this._opener = openerPage; + const openerPageOrError = await opener.waitForInitializedOrError(); + if (openerPageOrError instanceof Page && !openerPageOrError.isClosed()) + this._opener = openerPageOrError; } reportAsNew(error: Error | undefined = undefined, contextEvent: string = BrowserContext.Events.Page) { @@ -209,7 +208,7 @@ export class Page extends SdkObject { return; this._frameManager.createDummyMainFrameIfNeeded(); } - this._initialized = true; + this._initialized = error || this; this.emitOnContext(contextEvent, this); for (const { event, args } of this._eventsToEmitAfterInitialized) @@ -223,12 +222,20 @@ export class Page extends SdkObject { this.emit(Page.Events.Close); else this.instrumentation.onPageOpen(this); + + // Note: it is important to resolve _initializedPromise at the end, + // so that anyone who awaits waitForInitializedOrError got a ready and reported page. + this._initializedPromise.resolve(this._initialized); } - initializedOrUndefined() { + initialized(): Page | undefined { return this._initialized ? this : undefined; } + waitForInitializedOrError(): Promise { + return this._initializedPromise; + } + emitOnContext(event: string | symbol, ...args: any[]) { if (this._isServerSideOnly) return; diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index 4e5467fd17..4ce375e283 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -20,7 +20,7 @@ import { Browser } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, verifyGeolocation } from '../browserContext'; import { assert } from '../../utils'; import * as network from '../network'; -import type { InitScript, Page, PageDelegate } from '../page'; +import type { InitScript, Page } from '../page'; import type { ConnectionTransport } from '../transport'; import type * as types from '../types'; import type * as channels from '@protocol/channels'; @@ -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._initializedPage; + let originPage = page._page.initialized(); // 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._initializedPage; + originPage = page._opener._page.initialized(); } if (!originPage) return; @@ -239,18 +239,14 @@ export class WKBrowserContext extends BrowserContext { return Array.from(this._browser._wkPages.values()).filter(wkPage => wkPage._browserContext === this); } - pages(): Page[] { - return this._wkPages().map(wkPage => wkPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[]; + override possiblyUninitializedPages(): Page[] { + return this._wkPages().map(wkPage => wkPage._page); } - pagesOrErrors() { - return this._wkPages().map(wkPage => wkPage.pageOrError()); - } - - async newPageDelegate(): Promise { + override async doCreateNewPage(): Promise { assertBrowserContextIsNotOwned(this); const { pageProxyId } = await this._browser._browserSession.send('Playwright.createPage', { browserContextId: this._browserContextId }); - return this._browser._wkPages.get(pageProxyId)!; + return this._browser._wkPages.get(pageProxyId)!._page; } async doGetCookies(urls: string[]): Promise { diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index a1e69e1267..60af3f8f6e 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -43,7 +43,6 @@ import { WKInterceptableRequest, WKRouteImpl } from './wkInterceptableRequest'; import { WKProvisionalPage } from './wkProvisionalPage'; import { WKWorkers } from './wkWorkers'; import { debugLogger } from '../../utils/debugLogger'; -import { ManualPromise } from '../../utils/manualPromise'; import { BrowserContext } from '../browserContext'; import { TargetClosedError } from '../errors'; @@ -56,7 +55,6 @@ export class WKPage implements PageDelegate { _session: WKSession; private _provisionalPage: WKProvisionalPage | null = null; readonly _page: Page; - private readonly _pagePromise = new ManualPromise(); private readonly _pageProxySession: WKSession; readonly _opener: WKPage | null; private readonly _requestIdToRequest = new Map(); @@ -66,7 +64,6 @@ export class WKPage implements PageDelegate { private _sessionListeners: RegisteredListener[] = []; private _eventListeners: RegisteredListener[]; readonly _browserContext: WKBrowserContext; - _initializedPage: Page | null = null; private _firstNonInitialNavigationCommittedPromise: Promise; private _firstNonInitialNavigationCommittedFulfill = () => {}; _firstNonInitialNavigationCommittedReject = (e: Error) => {}; @@ -283,7 +280,7 @@ export class WKPage implements PageDelegate { } handleProvisionalLoadFailed(event: Protocol.Playwright.provisionalLoadFailedPayload) { - if (!this._initializedPage) { + if (!this._page.initialized()) { this._firstNonInitialNavigationCommittedReject(new Error('Initial load failed')); return; } @@ -300,10 +297,6 @@ export class WKPage implements PageDelegate { this._nextWindowOpenPopupFeatures = event.windowFeatures; } - async pageOrError(): Promise { - return this._pagePromise; - } - private async _onTargetCreated(event: Protocol.Target.targetCreatedPayload) { const { targetInfo } = event; const session = new WKSession(this._pageProxySession.connection, targetInfo.targetId, (message: any) => { @@ -316,7 +309,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._initializedPage); + assert(!this._page.initialized()); let pageOrError: Page | Error; try { this._setSession(session); @@ -343,12 +336,8 @@ 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; + await this._page.initOpener(this._opener?._page); this._page.reportAsNew(pageOrError instanceof Page ? undefined : pageOrError); - this._pagePromise.resolve(pageOrError); } else { assert(targetInfo.isProvisional); assert(!this._provisionalPage); @@ -515,7 +504,7 @@ export class WKPage implements PageDelegate { } private async _onBindingCalled(contextId: Protocol.Runtime.ExecutionContextId, argument: string) { - const pageOrError = await this.pageOrError(); + const pageOrError = await this._page.waitForInitializedOrError(); if (!(pageOrError instanceof Error)) { const context = this._contextIdToContext.get(contextId); if (context) @@ -821,7 +810,7 @@ export class WKPage implements PageDelegate { toolbarHeight: this._toolbarHeight() }); this._recordingVideoFile = options.outputFile; - this._browserContext._browser._videoStarted(this._browserContext, screencastId, options.outputFile, this.pageOrError()); + this._browserContext._browser._videoStarted(this._browserContext, screencastId, options.outputFile, this._page.waitForInitializedOrError()); } async _stopVideo(): Promise {