chore(chromium): close pages that crashed before connecting over CDP

packages/playwright-core/src/server/browserContext.ts

@@ -95,7 +95,6 @@ export abstract class BrowserContext<EM extends EventMap = EventMap> extends Sdk
   readonly requestInterceptors: network.RouteHandler[] = [];
   private _isPersistentContext: boolean;
   private _closedStatus: 'open' | 'closing' | 'closed' = 'open';
-  _skipCrashedPages = false;
   readonly _closePromise: Promise<Error>;
   private _closePromiseFulfill: ((error: Error) => void) | undefined;
   readonly _permissions = new Map<string, string[]>();

packages/playwright-core/src/server/chromium/crBrowser.ts

@@ -49,6 +49,7 @@ export class CRBrowser extends Browser {
   _crPages = new Map<string, CRPage>();
   _serviceWorkers = new Map<string, CRServiceWorker>();
   _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;
   }

packages/playwright-core/src/server/chromium/crPage.ts

@@ -506,12 +506,6 @@ class FrameSession {
   }
 
   async _initialize(hasUIWindow: boolean) {
-    let inspectorEnabled: Promise<any> | 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) {

packages/playwright-core/src/server/page.ts

@@ -247,13 +247,6 @@ export class Page extends SdkObject<PageEventMap> {
       // 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;

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) => {

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!`);
+});