fix(electron): close pages from the main process to avoid Target.closeTarget hang

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

@@ -25,7 +25,7 @@ import { eventsHelper } from '@utils/eventsHelper';
 import { envArrayToObject, launchProcess } from '@utils/processLauncher';
 import { ManualPromise } from '@isomorphic/manualPromise';
 import { libPath } from '../../package';
-import { validateBrowserContextOptions } from '../browserContext';
+import { BrowserContext, validateBrowserContextOptions } from '../browserContext';
 import { CRBrowser } from '../chromium/crBrowser';
 import { CRConnection } from '../chromium/crConnection';
 import { createHandle, CRExecutionContext } from '../chromium/crExecutionContext';
@@ -38,7 +38,6 @@ import { WebSocketTransport } from '../transport';
 import { nullProgress } from '../progress';
 
 import type { BrowserOptions, BrowserProcess } from '../browser';
-import type { BrowserContext } from '../browserContext';
 import type { CRBrowserContext } from '../chromium/crBrowser';
 import type { CRSession } from '../chromium/crConnection';
 import type { CRPage } from '../chromium/crPage';
@@ -86,6 +85,7 @@ export class ElectronApplication extends SdkObject {
       this._nodeElectronHandlePromise.resolve(new js.JSHandle(this._nodeExecutionContext!, 'object', 'ElectronModule', remoteObject.objectId!));
     });
     this._nodeSession.on('Runtime.consoleAPICalled', event => this._onConsoleAPI(event));
+    this._browserContext.on(BrowserContext.Events.Page, page => this._onPage(page));
     const appClosePromise = new Promise(f => this.once(ElectronApplication.Events.Close, f));
     this._browserContext.setCustomCloseHandler(async () => {
       const electronHandle = await this._nodeElectronHandlePromise;
@@ -138,6 +138,28 @@ export class ElectronApplication extends SdkObject {
     await this._browserContext.close(progress, { reason: 'Application exited' });
   }
 
+  private _onPage(page: Page) {
+    if (process.env.PLAYWRIGHT_ELECTRON_LEGACY_PAGE_CLOSE)
+      return;
+    // Target.closeTarget can hang on Electron when the close races a committing
+    // navigation. Close from the main process instead. We close the webContents
+    // rather than the BrowserWindow because BrowserWindow.close() always runs the
+    // beforeunload handler, while webContents.close() lets us opt in or out of it.
+    page.setCustomCloseHandler(async runBeforeUnload => {
+      const electronHandle = await this._nodeElectronHandlePromise;
+      const closed = await electronHandle.evaluate(({ webContents }, { targetId, runBeforeUnload }) => {
+        const wc = webContents.fromDevToolsTargetId(targetId);
+        if (!wc || wc.isDestroyed())
+          return false;
+        wc.close({ waitForBeforeUnload: runBeforeUnload });
+        return true;
+      }, { targetId: (page.delegate as CRPage)._targetId, runBeforeUnload }).catch(() => false);
+      // Fall back to the default close if the webContents could not be found.
+      if (!closed)
+        await page.delegate.closePage(runBeforeUnload);
+    });
+  }
+
   async browserWindow(progress: Progress, page: Page): Promise<js.JSHandle<BrowserWindow>> {
     // Assume CRPage as Electron is always Chromium.
     const targetId = (page.delegate as CRPage)._targetId;

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

@@ -202,6 +202,7 @@ export class Page extends SdkObject<PageEventMap> {
   readonly overlay: Overlay;
   readonly screencast: Screencast;
   _closeReason: string | undefined;
+  private _customCloseHandler?: (runBeforeUnload: boolean) => Promise<void>;
 
   constructor(delegate: PageDelegate, browserContext: BrowserContext) {
     super(browserContext, 'page');
@@ -833,19 +834,25 @@ export class Page extends SdkObject<PageEventMap> {
       this._lifecycle = 'closing';
       // This might throw if the browser context containing the page closes
       // while we are trying to close the page.
-      await this.delegate.closePage(false).catch(e => debugLogger.log('error', e));
+      const closePage = this._customCloseHandler ?? (runBeforeUnload => this.delegate.closePage(runBeforeUnload));
+      await closePage(false).catch(e => debugLogger.log('error', e));
     }
     await this.closedPromise;
   }
 
+  setCustomCloseHandler(handler: ((runBeforeUnload: boolean) => Promise<void>) | undefined) {
+    this._customCloseHandler = handler;
+  }
+
   async runBeforeUnload(progress: Progress) {
     await progress.race(this._runBeforeUnload());
   }
 
   private async _runBeforeUnload() {
     // This might throw if the browser context containing the page closes
     // while we are trying to close the page.
-    await this.delegate.closePage(true).catch(e => debugLogger.log('error', e));
+    const closePage = this._customCloseHandler ?? (runBeforeUnload => this.delegate.closePage(runBeforeUnload));
+    await closePage(true).catch(e => debugLogger.log('error', e));
   }
 
   isClosed(): boolean {