chore: refactor CRBrowserServer (#408)

docs/api.md

@@ -577,7 +577,7 @@ Connects to the browser server and returns a <[Browser]> object.
 - returns: <?[ChildProcess]> Spawned browser server process.
 
 #### browserServer.wsEndpoint()
-- returns: <[string]> Browser websocket url.
+- returns: <?[string]> Browser websocket url.
 
 Browser websocket endpoint which can be used as an argument to `playwright.connect`.
 

src/chromium/crPlaywright.ts

@@ -22,7 +22,6 @@ import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
 import * as util from 'util';
-import { BrowserServer } from '../browser';
 import { BrowserFetcher, BrowserFetcherOptions, BrowserFetcherRevisionInfo, OnProgressCallback } from '../browserFetcher';
 import { DeviceDescriptors } from '../deviceDescriptors';
 import * as Errors from '../errors';
@@ -33,15 +32,21 @@ import { CRBrowser } from './crBrowser';
 import * as platform from '../platform';
 import { TimeoutError } from '../errors';
 import { launchProcess, waitForLine } from '../processLauncher';
+import { ChildProcess } from 'child_process';
+import { CRConnection } from './crConnection';
 
-export type LauncherChromeArgOptions = {
+export type SlowMoOptions = {
+  slowMo?: number,
+};
+
+export type ChromeArgOptions = {
   headless?: boolean,
   args?: string[],
   userDataDir?: string,
   devtools?: boolean,
 };
 
-export type LauncherLaunchOptions = {
+export type LaunchOptions = ChromeArgOptions & SlowMoOptions & {
   executablePath?: string,
   ignoreDefaultArgs?: boolean|string[],
   handleSIGINT?: boolean,
@@ -53,10 +58,46 @@ export type LauncherLaunchOptions = {
   pipe?: boolean,
 };
 
-export type ConnectionOptions = {
+export type ConnectOptions = SlowMoOptions & {
-  slowMo?: number,
+  browserWSEndpoint?: string;
+  browserURL?: string;
+  transport?: ConnectionTransport;
 };
 
+export class CRBrowserServer {
+  private _process: ChildProcess;
+  private _connectOptions: ConnectOptions;
+
+  constructor(process: ChildProcess, connectOptions: ConnectOptions) {
+    this._process = process;
+    this._connectOptions = connectOptions;
+  }
+
+  async connect(): Promise<CRBrowser> {
+    const transport = await createTransport(this._connectOptions);
+    return CRBrowser.create(transport);
+  }
+
+  process(): ChildProcess {
+    return this._process;
+  }
+
+  wsEndpoint(): string | null {
+    return this._connectOptions.browserWSEndpoint || null;
+  }
+
+  connectOptions(): ConnectOptions {
+    return this._connectOptions;
+  }
+
+  async close(): Promise<void> {
+    const transport = await createTransport(this._connectOptions);
+    const connection = new CRConnection(transport);
+    await connection.rootSession.send('Browser.close');
+    connection.dispose();
+  }
+}
+
 export class CRPlaywright {
   private _projectRoot: string;
   readonly _revision: string;
@@ -73,12 +114,12 @@ export class CRPlaywright {
     return revisionInfo;
   }
 
-  async launch(options?: (LauncherLaunchOptions & LauncherChromeArgOptions & ConnectionOptions) | undefined): Promise<CRBrowser> {
+  async launch(options?: LaunchOptions): Promise<CRBrowser> {
     const server = await this.launchServer(options);
     return server.connect();
   }
 
-  async launchServer(options: (LauncherLaunchOptions & LauncherChromeArgOptions & ConnectionOptions) = {}): Promise<BrowserServer<CRBrowser>> {
+  async launchServer(options: LaunchOptions = {}): Promise<CRBrowserServer> {
     const {
       ignoreDefaultArgs = false,
       args = [],
@@ -131,51 +172,36 @@ export class CRPlaywright {
       pipe: usePipe,
       tempDir: temporaryUserDataDir
     }, () => {
-      if (temporaryUserDataDir || !browser)
+      if (temporaryUserDataDir || !server)
         return Promise.reject();
-      return browser.close();
+      return server.close();
     });
 
-    let browser: CRBrowser | undefined;
+    let server: CRBrowserServer | undefined;
     try {
-      let transport: ConnectionTransport | null = null;
+      let connectOptions: ConnectOptions | undefined;
       let browserWSEndpoint: string = '';
       if (!usePipe) {
         const timeoutError = new TimeoutError(`Timed out after ${timeout} ms while trying to connect to Chrome! The only Chrome revision guaranteed to work is r${this._revision}`);
         const match = await waitForLine(launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/, timeout, timeoutError);
         browserWSEndpoint = match[1];
-        transport = await WebSocketTransport.create(browserWSEndpoint);
+        connectOptions = { browserWSEndpoint, slowMo };
       } else {
-        transport = new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream);
+        const transport = new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream);
+        connectOptions = { slowMo, transport };
       }
-
+      server = new CRBrowserServer(launchedProcess, connectOptions);
-      browser = await CRBrowser.create(SlowMoTransport.wrap(transport, slowMo));
+      return server;
-      return new BrowserServer(browser, launchedProcess, browserWSEndpoint);
     } catch (e) {
-      if (browser)
+      if (server)
-        await browser.close();
+        await server.close();
       throw e;
     }
   }
 
-  async connect(options: (ConnectionOptions & {
+  async connect(options: ConnectOptions): Promise<CRBrowser> {
-      browserWSEndpoint?: string;
+    const transport = await createTransport(options);
-      browserURL?: string;
+    return CRBrowser.create(transport);
-      transport?: ConnectionTransport; })): Promise<CRBrowser> {
-    assert(Number(!!options.browserWSEndpoint) + Number(!!options.browserURL) + Number(!!options.transport) === 1, 'Exactly one of browserWSEndpoint, browserURL or transport must be passed to playwright.connect');
-
-    let transport: ConnectionTransport | undefined;
-    let connectionURL: string = '';
-    if (options.transport) {
-      transport = options.transport;
-    } else if (options.browserWSEndpoint) {
-      connectionURL = options.browserWSEndpoint;
-      transport = await WebSocketTransport.create(options.browserWSEndpoint);
-    } else if (options.browserURL) {
-      connectionURL = await getWSEndpoint(options.browserURL);
-      transport = await WebSocketTransport.create(connectionURL);
-    }
-    return CRBrowser.create(SlowMoTransport.wrap(transport, options.slowMo));
   }
 
   executablePath(): string {
@@ -190,7 +216,7 @@ export class CRPlaywright {
     return Errors;
   }
 
-  defaultArgs(options: LauncherChromeArgOptions = {}): string[] {
+  defaultArgs(options: ChromeArgOptions = {}): string[] {
     const {
       devtools = false,
       headless = !devtools,
@@ -332,3 +358,19 @@ function getWSEndpoint(browserURL: string): Promise<string> {
     throw e;
   });
 }
+
+async function createTransport(options: ConnectOptions): Promise<ConnectionTransport> {
+  assert(Number(!!options.browserWSEndpoint) + Number(!!options.browserURL) + Number(!!options.transport) === 1, 'Exactly one of browserWSEndpoint, browserURL or transport must be passed to playwright.connect');
+  let transport: ConnectionTransport | undefined;
+  let connectionURL: string = '';
+  if (options.transport) {
+    transport = options.transport;
+  } else if (options.browserWSEndpoint) {
+    connectionURL = options.browserWSEndpoint;
+    transport = await WebSocketTransport.create(options.browserWSEndpoint);
+  } else if (options.browserURL) {
+    connectionURL = await getWSEndpoint(options.browserURL);
+    transport = await WebSocketTransport.create(connectionURL);
+  }
+  return SlowMoTransport.wrap(transport, options.slowMo);
+}

test/chromium/launcher.spec.js

@@ -163,7 +163,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
         const browserServer = await playwright.launchServer(options);
         const browser = await browserServer.connect();
         expect((await browser.defaultContext().pages()).length).toBe(1);
-        expect(browserServer.wsEndpoint()).toBe('');
+        expect(browserServer.wsEndpoint()).toBe(null);
         const page = await browser.defaultContext().newPage();
         expect(await page.evaluate('11 * 11')).toBe(121);
         await page.close();
@@ -174,7 +174,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
         options.args = ['--remote-debugging-pipe'].concat(options.args || []);
         const browserServer = await playwright.launchServer(options);
         const browser = await browserServer.connect();
-        expect(browserServer.wsEndpoint()).toBe('');
+        expect(browserServer.wsEndpoint()).toBe(null);
         const page = await browser.defaultContext().newPage();
         expect(await page.evaluate('11 * 11')).toBe(121);
         await page.close();