fix(electron): stalling on delayed process close (#29431)

docs/src/api/class-electronapplication.md

@@ -39,7 +39,7 @@ const { _electron: electron } = require('playwright');
 ## event: ElectronApplication.close
 * since: v1.9
 
-This event is issued when the application closes.
+This event is issued when the application process has been terminated.
 
 ## event: ElectronApplication.console
 * since: v1.42

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

@@ -30,7 +30,8 @@ import { TimeoutSettings } from '../../common/timeoutSettings';
 import { ManualPromise, wrapInASCIIBox } from '../../utils';
 import { WebSocketTransport } from '../transport';
 import { launchProcess, envArrayToObject } from '../../utils/processLauncher';
-import { BrowserContext, validateBrowserContextOptions } from '../browserContext';
+import type { BrowserContext } from '../browserContext';
+import { validateBrowserContextOptions } from '../browserContext';
 import type { BrowserWindow } from 'electron';
 import type { Progress } from '../progress';
 import { ProgressController } from '../progress';
@@ -66,10 +67,6 @@ export class ElectronApplication extends SdkObject {
     super(parent, 'electron-app');
     this._process = process;
     this._browserContext = browser._defaultContext as CRBrowserContext;
-    this._browserContext.on(BrowserContext.Events.Close, () => {
-      // Emit application closed after context closed.
-      Promise.resolve().then(() => this.emit(ElectronApplication.Events.Close));
-    });
     this._nodeConnection = nodeConnection;
     this._nodeSession = nodeConnection.rootSession;
     this._nodeSession.on('Runtime.executionContextCreated', async (event: Protocol.Runtime.executionContextCreatedPayload) => {
@@ -86,10 +83,13 @@ 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));
+    const appClosePromise = new Promise(f => this.once(ElectronApplication.Events.Close, f));
     this._browserContext.setCustomCloseHandler(async () => {
       await this._browserContext.stopVideoRecording();
       const electronHandle = await this._nodeElectronHandlePromise;
       await electronHandle.evaluate(({ app }) => app.quit()).catch(() => {});
+      this._nodeConnection.close();
+      await appClosePromise;
     });
   }
 
@@ -132,11 +132,8 @@ export class ElectronApplication extends SdkObject {
   }
 
   async close() {
-    const progressController = new ProgressController(serverSideCallMetadata(), this);
-    const closed = progressController.run(progress => helper.waitForEvent(progress, this, ElectronApplication.Events.Close).promise);
+    // This will call BrowserContext.setCustomCloseHandler.
     await this._browserContext.close({ reason: 'Application exited' });
-    this._nodeConnection.close();
-    await closed;
   }
 
   async browserWindow(page: Page): Promise<js.JSHandle<BrowserWindow>> {
@@ -218,7 +215,7 @@ export class Electron extends SdkObject {
         handleSIGINT: true,
         handleSIGTERM: true,
         handleSIGHUP: true,
-        onExit: () => {},
+        onExit: () => app?.emit(ElectronApplication.Events.Close),
       });
 
       const waitForXserverError = new Promise(async (resolve, reject) => {

packages/playwright-core/types/types.d.ts

@@ -13939,7 +13939,7 @@ export interface ElectronApplication {
    */
   evaluateHandle<R>(pageFunction: PageFunctionOn<ElectronType, void, R>, arg?: any): Promise<SmartHandle<R>>;
   /**
-   * This event is issued when the application closes.
+   * This event is issued when the application process has been terminated.
    */
   on(event: 'close', listener: () => void): this;
 
@@ -13986,7 +13986,7 @@ export interface ElectronApplication {
   once(event: 'window', listener: (page: Page) => void): this;
 
   /**
-   * This event is issued when the application closes.
+   * This event is issued when the application process has been terminated.
    */
   addListener(event: 'close', listener: () => void): this;
 
@@ -14048,7 +14048,7 @@ export interface ElectronApplication {
   off(event: 'window', listener: (page: Page) => void): this;
 
   /**
-   * This event is issued when the application closes.
+   * This event is issued when the application process has been terminated.
    */
   prependListener(event: 'close', listener: () => void): this;
 
@@ -14125,7 +14125,7 @@ export interface ElectronApplication {
   process(): ChildProcess;
 
   /**
-   * This event is issued when the application closes.
+   * This event is issued when the application process has been terminated.
    */
   waitForEvent(event: 'close', optionsOrPredicate?: { predicate?: () => boolean | Promise<boolean>, timeout?: number } | (() => boolean | Promise<boolean>)): Promise<void>;
 

tests/electron/electron-app.spec.ts

@@ -20,16 +20,51 @@ import fs from 'fs';
 import { electronTest as test, expect } from './electronTest';
 import type { ConsoleMessage } from 'playwright';
 
-test('should fire close event', async ({ launchElectronApp }) => {
+test('should fire close event via ElectronApplication.close();', async ({ launchElectronApp }) => {
   const electronApp = await launchElectronApp('electron-app.js');
   const events = [];
-  electronApp.on('close', () => events.push('application'));
-  electronApp.context().on('close', () => events.push('context'));
+  electronApp.on('close', () => events.push('application(close)'));
+  electronApp.context().on('close', () => events.push('context(close)'));
+  electronApp.process().on('exit', () => events.push('process(exit)'));
   await electronApp.close();
-  expect(events.join('|')).toBe('context|application');
+  // Close one more time - this should be a noop.
+  await electronApp.close();
+  expect(events.join('|')).toBe('process(exit)|context(close)|application(close)');
   // Give it some time to fire more events - there should not be any.
   await new Promise(f => setTimeout(f, 1000));
-  expect(events.join('|')).toBe('context|application');
+  expect(events.join('|')).toBe('process(exit)|context(close)|application(close)');
+});
+
+test('should fire close event via BrowserContext.close()', async ({ launchElectronApp }) => {
+  const electronApp = await launchElectronApp('electron-app.js');
+  const events = [];
+  electronApp.on('close', () => events.push('application(close)'));
+  electronApp.context().on('close', () => events.push('context(close)'));
+  electronApp.process().on('exit', () => events.push('process(exit)'));
+  await electronApp.context().close();
+  // Close one more time - this should be a noop.
+  await electronApp.context().close();
+  expect(events.join('|')).toBe('process(exit)|context(close)|application(close)');
+  // Give it some time to fire more events - there should not be any.
+  await new Promise(f => setTimeout(f, 1000));
+  expect(events.join('|')).toBe('process(exit)|context(close)|application(close)');
+});
+
+test('should fire close event when the app quits itself', async ({ launchElectronApp }) => {
+  const electronApp = await launchElectronApp('electron-app.js');
+  const events = [];
+  electronApp.on('close', () => events.push('application(close)'));
+  electronApp.context().on('close', () => events.push('context(close)'));
+  electronApp.process().on('exit', () => events.push('process(exit)'));
+  {
+    const waitForAppClose = new Promise<void>(f => electronApp.on('close', f));
+    await electronApp.evaluate(({ app }) => app.quit());
+    await waitForAppClose;
+  }
+  expect(events.join('|')).toBe('process(exit)|context(close)|application(close)');
+  // Give it some time to fire more events - there should not be any.
+  await new Promise(f => setTimeout(f, 1000));
+  expect(events.join('|')).toBe('process(exit)|context(close)|application(close)');
 });
 
 test('should fire console events', async ({ launchElectronApp }) => {