diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index d9f1c9da72602..b30b06cbc3f4d 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -157,7 +157,7 @@ export class Chromium extends BrowserType { noDefaults: options.noDefaults, }; validateBrowserContextOptions(persistent, browserOptions); - const browser = await progress.race(CRBrowser.connect(this.attribution.playwright, transport, browserOptions)); + const browser = await progress.race(CRBrowser.connect(this.attribution.playwright, transport, browserOptions, undefined, true /* connectToExistingBrowser */)); if (!options.isLocal) browser._isBrowserCollocatedWithServer = false; browser.on(Browser.Events.Disconnected, doCleanup); diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index ec96fb94966da..3b0fa29be618e 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -56,7 +56,7 @@ export class CRBrowser extends Browser { private _tracingClient: CRSession | undefined; private _userAgent: string = ''; - static async connect(parent: SdkObject, transport: ConnectionTransport, options: BrowserOptions, devtools?: CRDevTools): Promise { + static async connect(parent: SdkObject, transport: ConnectionTransport, options: BrowserOptions, devtools?: CRDevTools, connectToExistingBrowser?: boolean): Promise { // Make a copy in case we need to update `headful` property below. options = { ...options }; const connection = new CRConnection(parent, transport, options.protocolLogger, options.browserLogsCollector); @@ -93,7 +93,7 @@ export class CRBrowser extends Browser { }), browser._defaultContext.initialize(), ]); - await browser._waitForAllPagesToBeInitialized(); + await browser._waitForAllPagesToBeInitialized(connectToExistingBrowser); return browser; } @@ -157,8 +157,15 @@ export class CRBrowser extends Browser { return this.options.name === 'clank'; } - async _waitForAllPagesToBeInitialized() { - await Promise.all([...this._crPages.values()].map(crPage => crPage._page.waitForInitializedOrError())); + async _waitForAllPagesToBeInitialized(connectToExistingBrowser?: boolean) { + await Promise.all([...this._crPages.values()].map(crPage => { + // When connecting to an existing browser, do not block on a page that is stuck on the initial + // empty document - its first navigation may never commit. Such a page is reported later, once + // it navigates. See https://github.com/microsoft/playwright/issues/41093. + if (connectToExistingBrowser) + return Promise.race([crPage._page.waitForInitializedOrError(), crPage._initialEmptyPagePromise]); + return crPage._page.waitForInitializedOrError(); + })); } _onAttachedToTarget({ targetInfo, sessionId, waitingForDebugger }: Protocol.Target.attachedToTargetPayload) { diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 907f4a47907ca..884b07ae7729f 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -16,6 +16,7 @@ */ import { assert } from '@isomorphic/assert'; +import { ManualPromise } from '@isomorphic/manualPromise'; import { rewriteErrorMessage } from '@isomorphic/stackTrace'; import { eventsHelper } from '@utils/eventsHelper'; import * as dialog from '../dialog'; @@ -63,6 +64,7 @@ export class CRPage implements PageDelegate { private readonly _pdf: CRPDF; private readonly _coverage: CRCoverage; readonly _browserContext: CRBrowserContext; + readonly _initialEmptyPagePromise = new ManualPromise(); // Holds window features for the next popup being opened via window.open, // until the popup target arrives. This could be racy if two oopifs @@ -496,6 +498,8 @@ class FrameSession { lifecycleEventsEnabled.catch(e => {}).then(() => { this._eventListeners.push(eventsHelper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event))); }); + if (!this._crPage._initialEmptyPagePromise.isDone()) + this._crPage._initialEmptyPagePromise.resolve(); } else { this._firstNonInitialNavigationCommittedFulfill(); this._eventListeners.push(eventsHelper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event))); diff --git a/tests/library/chromium/connect-over-cdp.spec.ts b/tests/library/chromium/connect-over-cdp.spec.ts index 5ebca4c902078..91697c7d1abca 100644 --- a/tests/library/chromium/connect-over-cdp.spec.ts +++ b/tests/library/chromium/connect-over-cdp.spec.ts @@ -224,6 +224,53 @@ test('should connect to existing page with iframe and navigate', async ({ browse } }); +test('should connect to a browser with a page stuck loading its first navigation', { + annotation: { + type: 'issue', + description: 'https://github.com/microsoft/playwright/issues/41093' + } +}, async ({ browserType, server, childProcess, waitForPort, mode }, testInfo) => { + test.skip(mode !== 'default', 'Spawns the browser executable directly'); + + // Hold the response so the tab's first navigation never commits; release it later. + let releaseHang = () => {}; + server.setRoute('/hang', (req, res) => { + releaseHang = () => { + res.writeHead(200, { 'content-type': 'text/html' }); + res.end(''); + }; + }); + + const port = 9339 + testInfo.workerIndex; + // Launch the browser directly (not via Playwright) so that we are the only CDP client. + childProcess({ + command: [browserType.executablePath(), + `--remote-debugging-port=${port}`, + `--user-data-dir=${testInfo.outputPath('cdp-user-data-dir')}`, + '--headless=new', + '--no-first-run', + '--no-default-browser-check', + 'about:blank', + ], + }); + await waitForPort(port); + + // Open a tab whose very first navigation never commits. + await Promise.all([ + server.waitForRequest('/hang'), + fetch(`http://127.0.0.1:${port}/json/new?${server.PREFIX}/hang`, { method: 'PUT' }), + ]); + + // connectOverCDP must not hang waiting for the stuck page to commit a navigation. + const cdpBrowser = await browserType.connectOverCDP(`http://127.0.0.1:${port}/`); + const context = cdpBrowser.contexts()[0]; + // The stuck tab has not committed a navigation yet, so it is not reported. Releasing the + // response lets it finish navigating to /hang and show up in the context. + releaseHang(); + await expect.poll(() => context.pages().map(page => page.url())).toContain(server.PREFIX + '/hang'); + await cdpBrowser.close(); +}); + test('should connect to existing service workers', async ({ browserType, mode, server }, testInfo) => { const port = 9339 + testInfo.workerIndex; const browserServer = await browserType.launch({