Skip to content

Commit 4b3b628

Browse files
committed
cherry-pick(#40736): Revert "feat(electron): add timeout option to electronApp.close() for force-kill escalation"
1 parent f869f96 commit 4b3b628

6 files changed

Lines changed: 5 additions & 90 deletions

File tree

docs/src/electron-api/class-electronapplication.md

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,6 @@ Page to retrieve the window for.
8585

8686
Closes Electron application.
8787

88-
### option: ElectronApplication.close.timeout
89-
* since: v1.60
90-
- `timeout` <[float]>
91-
92-
Maximum time in milliseconds to wait for the Electron application to gracefully close before forcefully terminating it.
93-
By default, [`method: ElectronApplication.close`] waits indefinitely for the application to exit, which can hang if the
94-
app has `before-quit` handlers that prevent shutdown, leaky IPC handlers, or child processes that keep it alive. When
95-
specified, the underlying process is force-killed (SIGKILL on POSIX, `taskkill /T /F` on Windows) if it does not exit
96-
within the given timeout.
97-
9888
## method: ElectronApplication.context
9989
* since: v1.9
10090
- returns: <[BrowserContext]>

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17045,19 +17045,8 @@ export interface ElectronApplication {
1704517045

1704617046
/**
1704717047
* Closes Electron application.
17048-
* @param options
1704917048
*/
17050-
close(options?: {
17051-
/**
17052-
* Maximum time in milliseconds to wait for the Electron application to gracefully close before forcefully terminating
17053-
* it. By default,
17054-
* [electronApplication.close([options])](https://playwright.dev/docs/api/class-electronapplication#electron-application-close)
17055-
* waits indefinitely for the application to exit, which can hang if the app has `before-quit` handlers that prevent
17056-
* shutdown, leaky IPC handlers, or child processes that keep it alive. When specified, the underlying process is
17057-
* force-killed (SIGKILL on POSIX, `taskkill /T /F` on Windows) if it does not exit within the given timeout.
17058-
*/
17059-
timeout?: number;
17060-
}): Promise<void>;
17049+
close(): Promise<void>;
1706117050

1706217051
/**
1706317052
* This method returns browser context that can be used for setting up context-wide routing, etc.

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export class Electron implements api.Electron {
180180
const chromeMatch = await Promise.race([chromeMatchPromise, waitForXserverError]);
181181
const browser = await chromium.connectOverCDP(chromeMatch[1], { timeout: progress.timeUntilDeadline(), isLocal: true });
182182

183-
app = new ElectronApplication(worker, browser, launchedProcess, kill);
183+
app = new ElectronApplication(worker, browser, launchedProcess);
184184
await progress.race(app._initialize());
185185
return app;
186186
} catch (error) {
@@ -194,13 +194,12 @@ export class ElectronApplication extends EventEmitter implements api.ElectronApp
194194
private _worker: Worker;
195195
private _browser: Browser;
196196
private _process: childProcess.ChildProcess;
197-
private _kill: () => Promise<void>;
198197
private _context: BrowserContext;
199198
private _windows = new Map<Page, JSHandle<BrowserWindow> | undefined>();
200199
private _appHandlePromise = new ManualPromise<JSHandle<ElectronAppType>>();
201200
private _closedPromise: Promise<void> | undefined;
202201

203-
constructor(worker: Worker, browser: Browser, process: childProcess.ChildProcess, kill: () => Promise<void>) {
202+
constructor(worker: Worker, browser: Browser, process: childProcess.ChildProcess) {
204203
super();
205204

206205
this._worker = worker;
@@ -215,7 +214,6 @@ export class ElectronApplication extends EventEmitter implements api.ElectronApp
215214
this._context.close = () => this.close();
216215

217216
this._process = process;
218-
this._kill = kill;
219217
}
220218

221219
_onClose() {
@@ -251,25 +249,14 @@ export class ElectronApplication extends EventEmitter implements api.ElectronApp
251249
await this.close();
252250
}
253251

254-
async close(options: { timeout?: number } = {}) {
252+
async close() {
255253
if (!this._closedPromise) {
256254
this._closedPromise = new Promise<void>(f => this.once(Events.ElectronApplication.Close, f));
257255
await this._browser.close();
258256
const appHandle = await this._appHandlePromise;
259257
await appHandle.evaluate(({ app }) => app.quit()).catch(() => {});
260258
await this._worker._disconnect();
261259
}
262-
if (options.timeout) {
263-
let timer: NodeJS.Timeout | undefined;
264-
const timedOut = new Promise<boolean>(resolve => {
265-
timer = setTimeout(() => resolve(true), options.timeout);
266-
});
267-
const exited = this._closedPromise.then(() => false);
268-
const didTimeout = await Promise.race([exited, timedOut]);
269-
clearTimeout(timer);
270-
if (didTimeout)
271-
await this._kill();
272-
}
273260
await this._closedPromise;
274261
}
275262

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17045,19 +17045,8 @@ export interface ElectronApplication {
1704517045

1704617046
/**
1704717047
* Closes Electron application.
17048-
* @param options
1704917048
*/
17050-
close(options?: {
17051-
/**
17052-
* Maximum time in milliseconds to wait for the Electron application to gracefully close before forcefully terminating
17053-
* it. By default,
17054-
* [electronApplication.close([options])](https://playwright.dev/docs/api/class-electronapplication#electron-application-close)
17055-
* waits indefinitely for the application to exit, which can hang if the app has `before-quit` handlers that prevent
17056-
* shutdown, leaky IPC handlers, or child processes that keep it alive. When specified, the underlying process is
17057-
* force-killed (SIGKILL on POSIX, `taskkill /T /F` on Windows) if it does not exit within the given timeout.
17058-
*/
17059-
timeout?: number;
17060-
}): Promise<void>;
17049+
close(): Promise<void>;
1706117050

1706217051
/**
1706317052
* This method returns browser context that can be used for setting up context-wide routing, etc.

tests/electron/electron-app-hang-on-close.js

Lines changed: 0 additions & 14 deletions
This file was deleted.

tests/electron/electron-app.spec.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -69,32 +69,6 @@ test('should fire close event when the app quits itself', async ({ launchElectro
6969
expect([...events].sort()).toEqual(['application(close)', 'context(close)', 'process(exit)']);
7070
});
7171

72-
test('should force-kill the app when close timeout fires', async ({ launchElectronApp }) => {
73-
const electronApp = await launchElectronApp('electron-app-hang-on-close.js');
74-
const events = [];
75-
electronApp.on('close', () => events.push('application(close)'));
76-
electronApp.process().on('exit', () => events.push('process(exit)'));
77-
const start = Date.now();
78-
await electronApp.close({ timeout: 1000 });
79-
const elapsed = Date.now() - start;
80-
// Should not wait much longer than the timeout itself.
81-
expect(elapsed).toBeLessThan(10000);
82-
expect([...events].sort()).toEqual(['application(close)', 'process(exit)']);
83-
// A second call should be a noop and not throw.
84-
await electronApp.close({ timeout: 1000 });
85-
});
86-
87-
test('should not force-kill the app when close completes within timeout', async ({ launchElectronApp }) => {
88-
const electronApp = await launchElectronApp('electron-app.js');
89-
const events = [];
90-
electronApp.on('close', () => events.push('application(close)'));
91-
electronApp.process().on('exit', code => events.push(`process(exit:${code})`));
92-
await electronApp.close({ timeout: 30000 });
93-
expect(events).toContain('application(close)');
94-
// Exit code 0 indicates a graceful shutdown (force-kill would yield null).
95-
expect(events).toContain('process(exit:0)');
96-
});
97-
9872
test('should fire console events', async ({ launchElectronApp }) => {
9973
const electronApp = await launchElectronApp('electron-app.js');
10074
const messages = [];

0 commit comments

Comments
 (0)