feat(rpc): update BrowserServer (#3112)

- Make BrowserServer a scope.
- Plumb exitCode and signal to BrowserServer.on('close').
- Use toImpl in a test.

Author: dgozman

src/rpc/channels.ts

@@ -274,7 +274,10 @@ export interface BrowserServerChannel extends Channel {
   close(params?: BrowserServerCloseParams): Promise<BrowserServerCloseResult>;
   kill(params?: BrowserServerKillParams): Promise<BrowserServerKillResult>;
 }
-export type BrowserServerCloseEvent = {};
+export type BrowserServerCloseEvent = {
+  exitCode?: number,
+  signal?: string,
+};
 export type BrowserServerCloseParams = {};
 export type BrowserServerCloseResult = void;
 export type BrowserServerKillParams = {};

src/rpc/client/browserServer.ts

@@ -25,8 +25,11 @@ export class BrowserServer extends ChannelOwner<BrowserServerChannel, BrowserSer
   }
 
   constructor(parent: ChannelOwner, type: string, guid: string, initializer: BrowserServerInitializer) {
-    super(parent, type, guid, initializer);
-    this._channel.on('close', () => this.emit(Events.BrowserServer.Close));
+    super(parent, type, guid, initializer, true);
+    this._channel.on('close', ({ exitCode, signal }) => {
+      this.emit(Events.BrowserServer.Close, exitCode === undefined ? null : exitCode, signal === undefined ? null : signal);
+      this._dispose();
+    });
   }
 
   process(): ChildProcess {

src/rpc/client/validator.ts

@@ -242,7 +242,10 @@ scheme.BrowserServerInitializer = tObject({
   wsEndpoint: tString,
   pid: tNumber,
 });
-scheme.BrowserServerCloseEvent = tObject({});
+scheme.BrowserServerCloseEvent = tObject({
+  exitCode: tOptional(tNumber),
+  signal: tOptional(tString),
+});
 scheme.BrowserServerCloseParams = tOptional(tObject({}));
 scheme.BrowserServerCloseResult = tUndefined;
 scheme.BrowserServerKillParams = tOptional(tObject({}));

src/rpc/protocol.yml

@@ -354,7 +354,9 @@ BrowserServer:
   events:
 
     close:
-
+      parameters:
+        exitCode: number?
+        signal: string?
 
 
 Browser:

src/rpc/server/browserServerDispatcher.ts

@@ -24,8 +24,14 @@ export class BrowserServerDispatcher extends Dispatcher<BrowserServer, BrowserSe
     super(scope, browserServer, 'BrowserServer', {
       wsEndpoint: browserServer.wsEndpoint(),
       pid: browserServer.process().pid
+    }, true);
+    browserServer.on(Events.BrowserServer.Close, (exitCode, signal) => {
+      this._dispatchEvent('close', {
+        exitCode: exitCode === null ? undefined : exitCode,
+        signal: signal === null ? undefined : signal,
+      });
+      this._dispose();
     });
-    browserServer.on(Events.BrowserServer.Close, () => this._dispatchEvent('close'));
   }
 
   async close(): Promise<void> {

test/defaultbrowsercontext.jest.js

@@ -327,8 +327,8 @@ describe('launchPersistentContext()', function() {
     const error = await browserType.launchPersistentContext(userDataDir, options).catch(e => e);
     expect(error.message).toContain('can not specify page');
   });
-  it('should have passed URL when launching with ignoreDefaultArgs: true', async ({browserType, defaultBrowserOptions, server, userDataDir}) => {
-    const args = require('..')[browserType.name()]._defaultArgs(defaultBrowserOptions, 'persistent', userDataDir, 0).filter(a => a !== 'about:blank');
+  it.skip(USES_HOOKS)('should have passed URL when launching with ignoreDefaultArgs: true', async ({browserType, defaultBrowserOptions, server, userDataDir, toImpl}) => {
+    const args = toImpl(browserType)._defaultArgs(defaultBrowserOptions, 'persistent', userDataDir, 0).filter(a => a !== 'about:blank');
     const options = {
       ...defaultBrowserOptions,
       args: [...args, server.EMPTY_PAGE],

test/launcher.jest.js

@@ -274,10 +274,12 @@ describe('browserType.launchServer', function() {
   });
   it('should fire close event', async ({browserType, defaultBrowserOptions}) => {
     const browserServer = await browserType.launchServer(defaultBrowserOptions);
-    await Promise.all([
-      new Promise(f => browserServer.on('close', f)),
+    const [result] = await Promise.all([
+      new Promise(f => browserServer.on('close', (exitCode, signal) => f({ exitCode, signal }))),
       browserServer.close(),
     ]);
+    expect(result.exitCode).toBe(0);
+    expect(result.signal).toBe(null);
   });
 });