diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 38134b7c5e690..b36cd6a403dcb 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -95,7 +95,6 @@ export abstract class BrowserContext extends Sdk readonly requestInterceptors: network.RouteHandler[] = []; private _isPersistentContext: boolean; private _closedStatus: 'open' | 'closing' | 'closed' = 'open'; - _skipCrashedPages = false; readonly _closePromise: Promise; private _closePromiseFulfill: ((error: Error) => void) | undefined; readonly _permissions = new Map(); diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index 5cf1363bf552e..2bf9475749369 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -49,6 +49,7 @@ export class CRBrowser extends Browser { _crPages = new Map(); _serviceWorkers = new Map(); _devtools?: CRDevTools; + _isConnecting = false; private _version = ''; private _majorVersion = 0; _revision = ''; @@ -85,7 +86,7 @@ export class CRBrowser extends Browser { return browser; } browser._defaultContext = new CRBrowserContext(browser, undefined, options.persistent); - browser._defaultContext._skipCrashedPages = true; + browser._isConnecting = true; try { await Promise.all([ session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }).then(async () => { @@ -98,7 +99,7 @@ export class CRBrowser extends Browser { ]); await browser._waitForAllPagesToBeInitialized(); } finally { - browser._defaultContext._skipCrashedPages = false; + browser._isConnecting = false; } return browser; } diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 3ba35b210442f..7e259ca1c5ed5 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -506,12 +506,6 @@ class FrameSession { } async _initialize(hasUIWindow: boolean) { - let inspectorEnabled: Promise | undefined; - if (this._isMainFrame() && this._crPage._browserContext._skipCrashedPages) { - // Get notified with Inspector.targetCrashed right away, so we can skip pages without a renderer. - inspectorEnabled = this._client._sendMayFail('Inspector.enable'); - } - const browserOptions = this._crPage._browserContext._browser.options; if (!this._page.isStorageStatePage && hasUIWindow && !this._crPage._browserContext._browser.isClank() && @@ -629,8 +623,12 @@ class FrameSession { for (const initScript of this._crPage._page.allInitScripts()) promises.push(this._evaluateOnNewDocument(initScript, 'main', true /* runImmediately */)); } - if (inspectorEnabled) - promises.push(inspectorEnabled); + if (this._isMainFrame() && this._crPage._browserContext._browser._isConnecting) { + // An existing page without a renderer, e.g. crashed or discarded, never responds to the commands above. + // Get notified with Inspector.targetCrashed right away, so that such a page is reported as closed, + // and we do not stall while connecting to the browser. + promises.push(this._client._sendMayFail('Inspector.enable')); + } promises.push(this._client.send('Runtime.runIfWaitingForDebugger')); promises.push(this._firstNonInitialNavigationCommittedPromise); await Promise.all(promises); @@ -993,9 +991,14 @@ class FrameSession { this._page.addPageError(exceptionToError(exceptionDetails), stackTraceToLocation(exceptionDetails.stackTrace)); } - async _onTargetCrashed() { + _onTargetCrashed() { this._client._markAsCrashed(); this._page._didCrash(); + if (this._crPage._browserContext._browser._isConnecting && !this._page.initializedOrUndefined()) { + // When connecting, any crashed/discarded/unloaded page is reported as closed right away. + this._crPage._browserContext._browser._crPages.delete(this._crPage._targetId); + this._crPage.didClose(); + } } _onLogEntryAdded(event: Protocol.Log.entryAddedPayload) { diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 47e06bbc2058d..b66220abd88f0 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -247,13 +247,6 @@ export class Page extends SdkObject { // context/browser closure. Just ignore the page. if (this.browserContext.isClosingOrClosed()) return; - if (this._lifecycle === 'crashed' && this.browserContext._skipCrashedPages) { - // When connecting, any crashed/discarded/unloaded page is not reported to the client at all. - // This is not a default behavior to avoid false positives when a newly created - // page is navigating slowly for whatever reason. TODO: fix in chromium upstream. - this._initializedPromise.resolve(error); - return; - } this.frameManager.createDummyMainFrameIfNeeded(); } this._initialized = error || this; diff --git a/tests/library/chromium/connect-over-cdp.spec.ts b/tests/library/chromium/connect-over-cdp.spec.ts index 9b11c84acce63..e6346a0ce277e 100644 --- a/tests/library/chromium/connect-over-cdp.spec.ts +++ b/tests/library/chromium/connect-over-cdp.spec.ts @@ -77,6 +77,43 @@ test('should connect when an existing page has no renderer', { } }); +test('should connect when the opener of an existing page has no renderer', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41714' }, +}, async ({ browserType, server }, testInfo) => { + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ + // The popup should be in a different process to survive the crash of the opener. + args: ['--remote-debugging-port=' + port, '--site-per-process'] + }); + try { + const cdpBrowser1 = await browserType.connectOverCDP({ + endpointURL: `http://127.0.0.1:${port}/`, + }); + const crashedPage1 = await cdpBrowser1.contexts()[0].newPage(); + await crashedPage1.goto(server.EMPTY_PAGE); + const [popup1] = await Promise.all([ + crashedPage1.waitForEvent('popup'), + crashedPage1.evaluate(url => { window.open(url); }, server.CROSS_PROCESS_PREFIX + '/title.html'), + ]); + await popup1.waitForLoadState(); + crashedPage1.goto('chrome://crash').catch(() => {}); + await crashedPage1.waitForEvent('crash'); + await cdpBrowser1.close(); + + // Connecting again should not report the opener, and should not wait for it. + const cdpBrowser2 = await browserType.connectOverCDP({ + endpointURL: `http://127.0.0.1:${port}/`, + }); + const pages = cdpBrowser2.contexts()[0].pages(); + expect(pages.map(page => page.url())).toEqual([server.CROSS_PROCESS_PREFIX + '/title.html']); + expect(await pages[0].title()).toBe('Woof-Woof'); + expect(await pages[0].opener()).toBe(null); + await cdpBrowser2.close(); + } finally { + await browserServer.close(); + } +}); + test('should connect when an existing page has been discarded', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41714' }, }, async ({ browserType, createUserDataDir, server }, testInfo) => { diff --git a/tests/mcp/cli-cdp.spec.ts b/tests/mcp/cli-cdp.spec.ts index 48c3a237a7683..e4232a552d635 100644 --- a/tests/mcp/cli-cdp.spec.ts +++ b/tests/mcp/cli-cdp.spec.ts @@ -141,3 +141,30 @@ test('tracing-start-stop over cdp', async ({ cdpServer, cli, server }, testInfo) expect(fs.existsSync(testInfo.outputPath('.playwright-cli', 'traces', `trace-${timestamp}.trace`))).toBeTruthy(); expect(fs.existsSync(testInfo.outputPath('.playwright-cli', 'traces', `trace-${timestamp}.network`))).toBeTruthy(); }); + +test('attach via cdp with a crashed page', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41714' }, +}, async ({ cdpServer, cli, server }) => { + const browserContext = await cdpServer.start(); + const [page] = browserContext.pages(); + await page.goto(server.HELLO_WORLD); + const crashedPage = await browserContext.newPage(); + await crashedPage.goto(server.PREFIX + '/title.html'); + crashedPage.goto('chrome://crash').catch(() => {}); + await crashedPage.waitForEvent('crash'); + + // Attaching should not hang on the crashed page, and should not list it. + const { output: attachOutput, exitCode } = await cli('attach', `--cdp=${cdpServer.endpoint}`); + expect(exitCode).toBe(0); + expect(attachOutput).toContain(`### Page +- Page URL: ${server.HELLO_WORLD} +- Page Title: Title`); + expect(attachOutput).not.toContain('### Open tabs'); + + const { output: listOutput } = await cli('tab-list'); + expect(listOutput).toBe(`### Result +- 0: (current) [Title](${server.HELLO_WORLD})`); + + const { inlineSnapshot } = await cli('snapshot'); + expect(inlineSnapshot).toContain(`- generic [active] [ref=e1]: Hello, world!`); +});