diff --git a/src/events.ts b/src/events.ts index ecbfa4a457..7985d06d7e 100644 --- a/src/events.ts +++ b/src/events.ts @@ -20,6 +20,10 @@ export const Events = { Disconnected: 'disconnected' }, + BrowserApp: { + Close: 'close', + }, + Page: { Close: 'close', Console: 'console', diff --git a/src/server/browserApp.ts b/src/server/browserApp.ts index 60a3c6cfb4..b2fe486fa5 100644 --- a/src/server/browserApp.ts +++ b/src/server/browserApp.ts @@ -14,15 +14,17 @@ * limitations under the License. */ -import { ChildProcess } from 'child_process'; +import { ChildProcess, execSync } from 'child_process'; import { ConnectOptions } from '../browser'; +import * as platform from '../platform'; -export class BrowserApp { +export class BrowserApp extends platform.EventEmitter { private _process: ChildProcess; private _gracefullyClose: () => Promise; private _connectOptions: ConnectOptions; constructor(process: ChildProcess, gracefullyClose: () => Promise, connectOptions: ConnectOptions) { + super(); this._process = process; this._gracefullyClose = gracefullyClose; this._connectOptions = connectOptions; @@ -40,6 +42,19 @@ export class BrowserApp { return this._connectOptions; } + kill() { + if (this._process.pid && !this._process.killed) { + try { + if (process.platform === 'win32') + execSync(`taskkill /pid ${this._process.pid} /T /F`); + else + process.kill(-this._process.pid, 'SIGKILL'); + } catch (e) { + // the process might have already stopped + } + } + } + async close(): Promise { await this._gracefullyClose(); } diff --git a/src/server/chromium.ts b/src/server/chromium.ts index cd794798be..8b0736e4ae 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -32,6 +32,7 @@ import { PipeTransport } from './pipeTransport'; import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType'; import { createTransport, ConnectOptions } from '../browser'; import { BrowserApp } from './browserApp'; +import { Events } from '../events'; export class Chromium implements BrowserType { private _projectRoot: string; @@ -94,6 +95,7 @@ export class Chromium implements BrowserType { if (usePipe && webSocket) throw new Error(`Argument "--remote-debugging-pipe" is not compatible with "webSocket" launch option.`); + let browserApp: BrowserApp | undefined = undefined; const { launchedProcess, gracefullyClose } = await launchProcess({ executablePath: chromeExecutable!, args: chromeArguments, @@ -105,19 +107,22 @@ export class Chromium implements BrowserType { pipe: usePipe, tempDir: temporaryUserDataDir || undefined, attemptToGracefullyClose: async () => { - if (!connectOptions) + if (!browserApp) return Promise.reject(); - // We try to gracefully close to prevent crash reporting and core dumps. // Note that it's fine to reuse the pipe transport, since // our connection ignores kBrowserCloseMessageId. - const transport = await createTransport(connectOptions); + const transport = await createTransport(browserApp.connectOptions()); const message = { method: 'Browser.close', id: kBrowserCloseMessageId }; transport.send(JSON.stringify(message)); }, + onkill: () => { + if (browserApp) + browserApp.emit(Events.BrowserApp.Close); + }, }); - let connectOptions: ConnectOptions | undefined; + let connectOptions: ConnectOptions; if (!usePipe) { const timeoutError = new TimeoutError(`Timed out after ${timeout} ms while trying to connect to Chromium! The only Chromium revision guaranteed to work is r${this._revision}`); const match = await waitForLine(launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/, timeout, timeoutError); @@ -127,7 +132,8 @@ export class Chromium implements BrowserType { const transport = new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream); connectOptions = { slowMo, transport }; } - return new BrowserApp(launchedProcess, gracefullyClose, connectOptions); + browserApp = new BrowserApp(launchedProcess, gracefullyClose, connectOptions); + return browserApp; } async connect(options: ConnectOptions & { browserURL?: string }): Promise { diff --git a/src/server/firefox.ts b/src/server/firefox.ts index 80db28f035..fa553dd1ca 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -31,6 +31,7 @@ import { assert } from '../helper'; import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType'; import { createTransport, ConnectOptions } from '../browser'; import { BrowserApp } from './browserApp'; +import { Events } from '../events'; export class Firefox implements BrowserType { private _projectRoot: string; @@ -89,8 +90,7 @@ export class Firefox implements BrowserType { firefoxExecutable = executablePath; } - let connectOptions: ConnectOptions | undefined = undefined; - + let browserApp: BrowserApp | undefined = undefined; const { launchedProcess, gracefullyClose } = await launchProcess({ executablePath: firefoxExecutable, args: firefoxArguments, @@ -106,27 +106,33 @@ export class Firefox implements BrowserType { pipe: false, tempDir: temporaryProfileDir || undefined, attemptToGracefullyClose: async () => { - if (!connectOptions) + if (!browserApp) return Promise.reject(); // We try to gracefully close to prevent crash reporting and core dumps. // Note that it's fine to reuse the pipe transport, since // our connection ignores kBrowserCloseMessageId. - const transport = await createTransport(connectOptions); + const transport = await createTransport(browserApp.connectOptions()); const message = { method: 'Browser.close', params: {}, id: kBrowserCloseMessageId }; transport.send(JSON.stringify(message)); }, + onkill: () => { + if (browserApp) + browserApp.emit(Events.BrowserApp.Close); + }, }); const timeoutError = new TimeoutError(`Timed out after ${timeout} ms while trying to connect to Firefox!`); const match = await waitForLine(launchedProcess, launchedProcess.stdout, /^Juggler listening on (ws:\/\/.*)$/, timeout, timeoutError); const browserWSEndpoint = match[1]; + let connectOptions: ConnectOptions; if (webSocket) { connectOptions = { browserWSEndpoint, slowMo }; } else { const transport = await platform.createWebSocketTransport(browserWSEndpoint); connectOptions = { transport, slowMo }; } - return new BrowserApp(launchedProcess, gracefullyClose, connectOptions); + browserApp = new BrowserApp(launchedProcess, gracefullyClose, connectOptions); + return browserApp; } async connect(options: ConnectOptions & { browserURL?: string }): Promise { diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index db5e4bad2c..f9526d9f08 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -40,6 +40,7 @@ export type LaunchProcessOptions = { // Note: attemptToGracefullyClose should reject if it does not close the browser. attemptToGracefullyClose: () => Promise, + onkill: () => void, }; type LaunchResult = { launchedProcess: childProcess.ChildProcess, gracefullyClose: () => Promise }; @@ -97,6 +98,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise { + if (browserApp) + browserApp.emit(Events.BrowserApp.Close); + }, }); transport = new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream); @@ -126,7 +132,8 @@ export class WebKit implements BrowserType { } else { connectOptions = { transport, slowMo }; } - return new BrowserApp(launchedProcess, gracefullyClose, connectOptions); + browserApp = new BrowserApp(launchedProcess, gracefullyClose, connectOptions); + return browserApp; } async connect(options: ConnectOptions & { browserURL?: string }): Promise { diff --git a/test/launcher.spec.js b/test/launcher.spec.js index b34fd27fb3..709825dac7 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -216,6 +216,14 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p expect(message).not.toContain('Timeout'); } }); + it('should be able to close remote browser', async({server}) => { + const browserApp = await playwright.launchBrowserApp({...defaultBrowserOptions, webSocket: true}); + const remote = await playwright.connect(browserApp.connectOptions()); + await Promise.all([ + new Promise(f => browserApp.once('close', f)), + remote.close(), + ]); + }); }); describe('Playwright.launch |webSocket| option', function() { @@ -244,8 +252,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p const browserApp = await playwright.launchBrowserApp(defaultBrowserOptions); const browser = await playwright.connect(browserApp.connectOptions()); const disconnectedEventPromise = new Promise(resolve => browser.once('disconnected', resolve)); - // Emulate user exiting browser. - process.kill(-browserApp.process().pid, 'SIGKILL'); + browserApp.kill(); await disconnectedEventPromise; }); it('should fire "disconnected" when closing with webSocket', async() => { @@ -253,8 +260,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p const browserApp = await playwright.launchBrowserApp(options); const browser = await playwright.connect(browserApp.connectOptions()); const disconnectedEventPromise = new Promise(resolve => browser.once('disconnected', resolve)); - // Emulate user exiting browser. - process.kill(-browserApp.process().pid, 'SIGKILL'); + browserApp.kill(); await disconnectedEventPromise; }); });