diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index cd59af0ba7fe0..86a3ad3052644 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -95,6 +95,7 @@ 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 db00e29a01059..b234d7396e27f 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -85,16 +85,21 @@ export class CRBrowser extends Browser { return browser; } browser._defaultContext = new CRBrowserContext(browser, undefined, options.persistent); - await Promise.all([ - session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }).then(async () => { - // Target.setAutoAttach has a bug where it does not wait for new Targets being attached. - // However making a dummy call afterwards fixes this. - // This can be removed after https://chromium-review.googlesource.com/c/chromium/src/+/2885888 lands in stable. - await session.send('Target.getTargetInfo'); - }), - browser._defaultContext.initialize(), - ]); - await browser._waitForAllPagesToBeInitialized(); + browser._defaultContext._skipCrashedPages = true; + try { + await Promise.all([ + session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }).then(async () => { + // Target.setAutoAttach has a bug where it does not wait for new Targets being attached. + // However making a dummy call afterwards fixes this. + // This can be removed after https://chromium-review.googlesource.com/c/chromium/src/+/2885888 lands in stable. + await session.send('Target.getTargetInfo'); + }), + browser._defaultContext.initialize(), + ]); + await browser._waitForAllPagesToBeInitialized(); + } finally { + browser._defaultContext._skipCrashedPages = false; + } return browser; } diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index f3e907233b83d..ebf4e0e5a5e95 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -163,6 +163,8 @@ export class CRSession extends SdkObject } } else if (object.id && object.error?.code === -32001) { // Message to a closed session, just ignore it. + } else if (object.id && this._crashed) { + // _markAsCrashed() has already rejected all pending calls, nothing to do here. } else { assert(!object.id, object?.error?.message || undefined); Promise.resolve().then(() => { diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 17d2933f545c7..2c478516c7516 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -469,6 +469,12 @@ 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() && @@ -579,6 +585,8 @@ class FrameSession { for (const initScript of this._crPage._page.allInitScripts()) promises.push(this._evaluateOnNewDocument(initScript, 'main', true /* runImmediately */)); } + if (inspectorEnabled) + promises.push(inspectorEnabled); promises.push(this._client.send('Runtime.runIfWaitingForDebugger')); promises.push(this._firstNonInitialNavigationCommittedPromise); await Promise.all(promises); diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 0c5d10398148f..420953e43f785 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -244,6 +244,13 @@ 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 64b4c1be9d543..9b11c84acce63 100644 --- a/tests/library/chromium/connect-over-cdp.spec.ts +++ b/tests/library/chromium/connect-over-cdp.spec.ts @@ -42,6 +42,87 @@ test('should connect to an existing cdp session', async ({ browserType, mode }, } }); +test('should connect when 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({ + args: ['--remote-debugging-port=' + port] + }); + try { + // Prepare a page without a renderer, e.g. one discarded by the Memory Saver. + const cdpBrowser1 = await browserType.connectOverCDP({ + endpointURL: `http://127.0.0.1:${port}/`, + }); + const context1 = cdpBrowser1.contexts()[0]; + const healthyPage1 = await context1.newPage(); + await healthyPage1.goto(server.PREFIX + '/title.html'); + const crashedPage1 = await context1.newPage(); + await crashedPage1.goto(server.EMPTY_PAGE); + crashedPage1.goto('chrome://crash').catch(() => {}); + await crashedPage1.waitForEvent('crash'); + await cdpBrowser1.close(); + + // Connecting again should not hang on the page without a renderer, + // and should not report it at all. + 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.PREFIX + '/title.html']); + expect(await pages[0].title()).toBe('Woof-Woof'); + 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) => { + const port = 9339 + testInfo.workerIndex; + const userDataDir = await createUserDataDir(); + // chrome://discards is an internal debug page that is only available with this pref. + fs.writeFileSync(path.join(userDataDir, 'Local State'), JSON.stringify({ internal_only_uis_enabled: true })); + const context = await browserType.launchPersistentContext(userDataDir, { + headless: false, + // Emulating the viewport of a discarded tab crashes the browser in + // WebContentsImpl::SetDeviceEmulationSize, because it has no view. + viewport: null, + args: [ + '--remote-debugging-port=' + port, + // Chrome refuses to discard tabs with DevTools attached, and we attach to all of them. + '--enable-features=AllowDevtoolsConnectedDiscard', + ], + }); + try { + const victimUrl = server.PREFIX + '/title.html'; + const victim = await context.newPage(); + await victim.goto(victimUrl); + const discards = await context.newPage(); + await discards.goto('chrome://discards/'); + // Discarding replaces the tab's WebContents, but the url in the table survives. + const row = discards.getByRole('row', { name: victimUrl }); + await row.getByText('Urgent Discard').click(); + await expect(row).toContainText('discarded'); + // The renderer process is shut down asynchronously after the discard. + await discards.waitForTimeout(3000); + await testInfo.attach('discards rows', { body: (await discards.getByRole('row').allInnerTexts()).map(text => text.replace(/\s+/g, ' ')).join('\n') }); + const targets = await (await fetch(`http://127.0.0.1:${port}/json/list`)).json(); + await testInfo.attach('targets', { body: JSON.stringify(targets, null, 2), contentType: 'application/json' }); + + // Connecting should not hang on the discarded page, and the page should not be reported. + const cdpBrowser = await browserType.connectOverCDP({ + endpointURL: `http://127.0.0.1:${port}/`, + }); + const pages = cdpBrowser.contexts()[0].pages(); + expect(pages.map(page => page.url()).sort()).toEqual(['about:blank', discards.url()].sort()); + await cdpBrowser.close(); + } finally { + await context.close(); + } +}); + test('should cleanup artifacts dir after connectOverCDP disconnects due to ws close', async ({ browserType, toImpl, mode }, testInfo) => { const port = 9339 + testInfo.workerIndex; const browserServer = await browserType.launch({