diff --git a/docs/api.md b/docs/api.md index d21a9b6a3b..fb2c624f95 100644 --- a/docs/api.md +++ b/docs/api.md @@ -3941,8 +3941,9 @@ Emitted when the browser server closes. Closes the browser gracefully and makes sure the process is terminated. #### browserServer.kill() +- returns: <[Promise]> -Kills the browser process. +Kills the browser process and waits for the process to exit. #### browserServer.process() - returns: <[ChildProcess]> Spawned browser application process. diff --git a/src/server/browserServer.ts b/src/server/browserServer.ts index cf2f55bdde..a2e3e866fc 100644 --- a/src/server/browserServer.ts +++ b/src/server/browserServer.ts @@ -50,13 +50,15 @@ export class WebSocketWrapper { export class BrowserServer extends EventEmitter { private _process: ChildProcess; - private _gracefullyClose: (() => Promise); + private _gracefullyClose: () => Promise; + private _kill: () => Promise; _webSocketWrapper: WebSocketWrapper | null = null; - constructor(process: ChildProcess, gracefullyClose: () => Promise) { + constructor(process: ChildProcess, gracefullyClose: () => Promise, kill: () => Promise) { super(); this._process = process; this._gracefullyClose = gracefullyClose; + this._kill = kill; } process(): ChildProcess { @@ -67,8 +69,8 @@ export class BrowserServer extends EventEmitter { return this._webSocketWrapper ? this._webSocketWrapper.wsEndpoint : ''; } - kill() { - helper.killProcess(this._process); + async kill(): Promise { + await this._kill(); } async close(): Promise { @@ -84,7 +86,7 @@ export class BrowserServer extends EventEmitter { try { await helper.waitWithDeadline(this.close(), '', deadline, ''); // The error message is ignored. } catch (ignored) { - this.kill(); + await this.kill(); // Make sure to await actual process exit. } } } diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 10e690b10b..7f1a2d2894 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -230,7 +230,7 @@ export abstract class BrowserTypeBase implements BrowserType { // "Cannot access 'browserServer' before initialization" if something went wrong. let transport: ConnectionTransport | undefined = undefined; let browserServer: BrowserServer | undefined = undefined; - const { launchedProcess, gracefullyClose } = await launchProcess({ + const { launchedProcess, gracefullyClose, kill } = await launchProcess({ executablePath: executable, args: browserArguments, env: this._amendEnvironment(env, userDataDir, executable, browserArguments), @@ -248,7 +248,7 @@ export abstract class BrowserTypeBase implements BrowserType { // our connection ignores kBrowserCloseMessageId. this._attemptToGracefullyCloseBrowser(transport!); }, - onkill: (exitCode, signal) => { + onExit: (exitCode, signal) => { if (browserServer) browserServer.emit(Events.BrowserServer.Close, exitCode, signal); }, @@ -269,7 +269,7 @@ export abstract class BrowserTypeBase implements BrowserType { helper.killProcess(launchedProcess); throw e; } - browserServer = new BrowserServer(launchedProcess, gracefullyClose); + browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill); return { browserServer, downloadsPath, transport }; } diff --git a/src/server/electron.ts b/src/server/electron.ts index eabac0f8fc..aa7c4f17b9 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -173,7 +173,7 @@ export class Electron { const logger = new RootLogger(options.logger); const electronArguments = ['--inspect=0', '--remote-debugging-port=0', '--require', path.join(__dirname, 'electronLoader.js'), ...args]; - const { launchedProcess, gracefullyClose } = await launchProcess({ + const { launchedProcess, gracefullyClose, kill } = await launchProcess({ executablePath, args: electronArguments, env, @@ -185,7 +185,7 @@ export class Electron { cwd: options.cwd, tempDirectories: [], attemptToGracefullyClose: () => app!.close(), - onkill: (exitCode, signal) => { + onExit: (exitCode, signal) => { if (app) app.emit(ElectronEvents.ElectronApplication.Close, exitCode, signal); }, @@ -198,7 +198,7 @@ export class Electron { const chromeMatch = await waitForLine(launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/, helper.timeUntilDeadline(deadline), timeoutError); const chromeTransport = await WebSocketTransport.connect(chromeMatch[1], logger, deadline); - const browserServer = new BrowserServer(launchedProcess, gracefullyClose); + const browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill); const browser = await CRBrowser.connect(chromeTransport, { headful: true, logger, persistent: { viewport: null }, ownedServer: browserServer }); app = new ElectronApplication(logger, browser, nodeConnection); await app._init(); diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index b9ff33aaba..aa9577fb2c 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -56,13 +56,14 @@ export type LaunchProcessOptions = { // Note: attemptToGracefullyClose should reject if it does not close the browser. attemptToGracefullyClose: () => Promise, - onkill: (exitCode: number | null, signal: string | null) => void, + onExit: (exitCode: number | null, signal: string | null) => void, logger: RootLogger, }; type LaunchResult = { launchedProcess: childProcess.ChildProcess, gracefullyClose: () => Promise, + kill: () => Promise, }; export async function launchProcess(options: LaunchProcessOptions): Promise { @@ -110,14 +111,14 @@ export async function launchProcess(options: LaunchProcessOptions): Promise {}; - const waitForClose = new Promise(f => fulfillClose = f); + const waitForClose = new Promise(f => fulfillClose = f); let fulfillCleanup = () => {}; - const waitForCleanup = new Promise(f => fulfillCleanup = f); + const waitForCleanup = new Promise(f => fulfillCleanup = f); spawnedProcess.once('exit', (exitCode, signal) => { logger._log(browserLog, ``); processClosed = true; helper.removeEventListeners(listeners); - options.onkill(exitCode, signal); + options.onExit(exitCode, signal); fulfillClose(); // Cleanup as process exits. cleanup().then(fulfillCleanup); @@ -175,7 +176,12 @@ export async function launchProcess(options: LaunchProcessOptions): Promise { diff --git a/test/launcher.spec.js b/test/launcher.spec.js index 4746cd043c..45f94f7685 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -223,8 +223,8 @@ describe('Browser.close', function() { }); }); -describe('browserType.launch |webSocket| option', function() { - it('should support the webSocket option', async({browserType, defaultBrowserOptions}) => { +describe('browserType.launchServer', function() { + it('should work', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const browserContext = await browser.newContext(); @@ -237,7 +237,7 @@ describe('browserType.launch |webSocket| option', function() { await browserServer._checkLeaks(); await browserServer.close(); }); - it('should fire "disconnected" when closing with webSocket', async({browserType, defaultBrowserOptions}) => { + it('should fire "disconnected" when closing the server', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const disconnectedEventPromise = new Promise(resolve => browser.once('disconnected', resolve)); @@ -248,6 +248,19 @@ describe('browserType.launch |webSocket| option', function() { closedPromise, ]); }); + it('should fire "close" event during kill', async({browserType, defaultBrowserOptions}) => { + const order = []; + const browserServer = await browserType.launchServer(defaultBrowserOptions); + const closedPromise = new Promise(f => browserServer.on('close', () => { + order.push('closed'); + f(); + })); + await Promise.all([ + browserServer.kill().then(() => order.push('killed')), + closedPromise, + ]); + expect(order).toEqual(['closed', 'killed']); + }); }); describe('browserType.connect', function() {