feat(browserApp): kill and onclose (#641)

src/events.ts

@@ -20,6 +20,10 @@ export const Events = {
     Disconnected: 'disconnected'
   },
 
+  BrowserApp: {
+    Close: 'close',
+  },
+
   Page: {
     Close: 'close',
     Console: 'console',

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<void>;
   private _connectOptions: ConnectOptions;
 
   constructor(process: ChildProcess, gracefullyClose: () => Promise<void>, 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<void> {
     await this._gracefullyClose();
   }

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<CRBrowser> {

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<FFBrowser> {

src/server/processLauncher.ts

@@ -40,6 +40,7 @@ export type LaunchProcessOptions = {
 
   // Note: attemptToGracefullyClose should reject if it does not close the browser.
   attemptToGracefullyClose: () => Promise<any>,
+  onkill: () => void,
 };
 
 type LaunchResult = { launchedProcess: childProcess.ChildProcess, gracefullyClose: () => Promise<void> };
@@ -97,6 +98,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
       } else {
         fulfill();
       }
+      options.onkill();
     });
   });
 

src/server/webkit.ts

@@ -36,6 +36,7 @@ import * as ws from 'ws';
 import * as uuidv4 from 'uuid/v4';
 import { ConnectOptions } from '../browser';
 import { BrowserApp } from './browserApp';
+import { Events } from '../events';
 
 export class WebKit implements BrowserType {
   private _projectRoot: string;
@@ -94,8 +95,9 @@ export class WebKit implements BrowserType {
         throw new Error(missingText);
       webkitExecutable = executablePath;
     }
-    let transport: PipeTransport | undefined = undefined;
 
+    let transport: PipeTransport | undefined = undefined;
+    let browserApp: BrowserApp | undefined = undefined;
     const { launchedProcess, gracefullyClose } = await launchProcess({
       executablePath: webkitExecutable!,
       args: webkitArguments,
@@ -115,6 +117,10 @@ export class WebKit implements BrowserType {
         const message = JSON.stringify({method: 'Browser.close', params: {}, id: kBrowserCloseMessageId});
         transport.send(message);
       },
+      onkill: () => {
+        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<WKBrowser> {

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,17 +252,15 @@ 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() => {
       const options = Object.assign({webSocket: true}, defaultBrowserOptions);
       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;
     });
   });