fix(persistent): close browser instead of pages (#18485)

Previously, we closed pages one by one before closing the browser when
shutting down the persistent context. This logic was introduced in
https://github.com/microsoft/playwright/pull/4040 to properly finish
video recordings in persistent context.

Such a process makes it unnecessary brittle to close the persistent
context. For example, Chromium headless is sometimes unable to close the
last persistent page for unknown reasons.

Instead, we can just stop video recordings manually and close the
browser right away.

Fixes #18229.

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

@@ -404,10 +404,6 @@ export abstract class BrowserContext extends SdkObject {
 
       if (this._customCloseHandler) {
         await this._customCloseHandler();
-      } else if (this._isPersistentContext) {
-        // Close all the pages instead of the context,
-        // because we cannot close the default context.
-        await Promise.all(this.pages().map(page => page.close(metadata)));
       } else {
         // Close the context.
         await this.doClose();
@@ -420,15 +416,8 @@ export abstract class BrowserContext extends SdkObject {
       await Promise.all(promises);
 
       // Custom handler should trigger didCloseInternal itself.
-      if (this._customCloseHandler)
-        return;
-
-      // Persistent context should also close the browser.
-      if (this._isPersistentContext)
-        await this._browser.close();
-
-      // Bookkeeping.
-      this._didCloseInternal();
+      if (!this._customCloseHandler)
+        this._didCloseInternal();
     }
     await this._closePromise;
   }

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

@@ -334,13 +334,12 @@ export class CRBrowserContext extends BrowserContext {
     await Promise.all(promises);
   }
 
+  private _crPages() {
+    return [...this._browser._crPages.values()].filter(crPage => crPage._browserContext === this);
+  }
+
   pages(): Page[] {
-    const result: Page[] = [];
-    for (const crPage of this._browser._crPages.values()) {
-      if (crPage._browserContext === this && crPage._initializedPage)
-        result.push(crPage._initializedPage);
-    }
-    return result;
+    return this._crPages().map(crPage => crPage._initializedPage).filter(Boolean) as Page[];
   }
 
   async newPageDelegate(): Promise<PageDelegate> {
@@ -489,19 +488,24 @@ export class CRBrowserContext extends BrowserContext {
   }
 
   async doClose() {
-    assert(this._browserContextId);
     // Headful chrome cannot dispose browser context with opened 'beforeunload'
     // dialogs, so we should close all that are currently opened.
     // We also won't get new ones since `Target.disposeBrowserContext` does not trigger
     // beforeunload.
     const openedBeforeUnloadDialogs: Dialog[] = [];
-    for (const crPage of this._browser._crPages.values()) {
-      if (crPage._browserContext !== this)
-        continue;
+    for (const crPage of this._crPages()) {
       const dialogs = [...crPage._page._frameManager._openedDialogs].filter(dialog => dialog.type() === 'beforeunload');
       openedBeforeUnloadDialogs.push(...dialogs);
     }
     await Promise.all(openedBeforeUnloadDialogs.map(dialog => dialog.dismiss()));
+
+    if (!this._browserContextId) {
+      await Promise.all(this._crPages().map(crPage => crPage._mainFrameSession._stopVideoRecording()));
+      // Closing persistent context should close the browser.
+      await this._browser.close();
+      return;
+    }
+
     await this._browser._session.send('Target.disposeBrowserContext', { browserContextId: this._browserContextId });
     this._browser._contexts.delete(this._browserContextId);
     for (const [targetId, serviceWorker] of this._browser._serviceWorkers) {

packages/playwright-core/src/server/firefox/ffBrowser.ts

@@ -359,9 +359,19 @@ export class FFBrowserContext extends BrowserContext {
   onClosePersistent() {}
 
   async doClose() {
-    assert(this._browserContextId);
-    await this._browser._connection.send('Browser.removeBrowserContext', { browserContextId: this._browserContextId });
-    this._browser._contexts.delete(this._browserContextId);
+    if (!this._browserContextId) {
+      if (this._options.recordVideo) {
+        await this._browser._connection.send('Browser.setVideoRecordingOptions', {
+          options: undefined,
+          browserContextId: this._browserContextId
+        });
+      }
+      // Closing persistent context should close the browser.
+      await this._browser.close();
+    } else {
+      await this._browser._connection.send('Browser.removeBrowserContext', { browserContextId: this._browserContextId });
+      this._browser._contexts.delete(this._browserContextId);
+    }
   }
 
   async cancelDownload(uuid: string) {

packages/playwright-core/src/server/webkit/wkBrowser.ts

@@ -338,9 +338,14 @@ export class WKBrowserContext extends BrowserContext {
   onClosePersistent() {}
 
   async doClose() {
-    assert(this._browserContextId);
-    await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId });
-    this._browser._contexts.delete(this._browserContextId);
+    if (!this._browserContextId) {
+      await Promise.all(this._wkPages().map(wkPage => wkPage._stopVideo()));
+      // Closing persistent context should close the browser.
+      await this._browser.close();
+    } else {
+      await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId });
+      this._browser._contexts.delete(this._browserContextId);
+    }
   }
 
   async cancelDownload(uuid: string) {

packages/playwright-core/src/server/webkit/wkPage.ts

@@ -825,7 +825,7 @@ export class WKPage implements PageDelegate {
     this._browserContext._browser._videoStarted(this._browserContext, screencastId, options.outputFile, this.pageOrError());
   }
 
-  private async _stopVideo(): Promise<void> {
+  async _stopVideo(): Promise<void> {
     if (!this._recordingVideoFile)
       return;
     await this._pageProxySession.sendMayFail('Screencast.stopVideo');