feat(firefox): switch to use pipe instead of websocket (#3279)

src/server/browserType.ts

@@ -20,9 +20,9 @@ import * as path from 'path';
 import * as util from 'util';
 import { BrowserContext, normalizeProxySettings, validateBrowserContextOptions } from './browserContext';
 import * as browserPaths from '../utils/browserPaths';
-import { ConnectionTransport, WebSocketTransport } from './transport';
+import { ConnectionTransport } from './transport';
 import { BrowserOptions, Browser, BrowserProcess } from './browser';
-import { launchProcess, Env, waitForLine, envArrayToObject } from './processLauncher';
+import { launchProcess, Env, envArrayToObject } from './processLauncher';
 import { PipeTransport } from './pipeTransport';
 import { Progress, ProgressController } from './progress';
 import * as types from './types';
@@ -35,22 +35,18 @@ const mkdtempAsync = util.promisify(fs.mkdtemp);
 const existsAsync = (path: string): Promise<boolean> => new Promise(resolve => fs.stat(path, err => resolve(!err)));
 const DOWNLOADS_FOLDER = path.join(os.tmpdir(), 'playwright_downloads-');
 
-type WebSocketNotPipe = { webSocketRegex: RegExp, stream: 'stdout' | 'stderr' };
-
 export abstract class BrowserType {
   private _name: string;
   private _executablePath: string;
-  private _webSocketNotPipe: WebSocketNotPipe | null;
   private _browserDescriptor: browserPaths.BrowserDescriptor;
   readonly _browserPath: string;
 
-  constructor(packagePath: string, browser: browserPaths.BrowserDescriptor, webSocketOrPipe: WebSocketNotPipe | null) {
+  constructor(packagePath: string, browser: browserPaths.BrowserDescriptor) {
     this._name = browser.name;
     const browsersPath = browserPaths.browsersPath(packagePath);
     this._browserDescriptor = browser;
     this._browserPath = browserPaths.browserDirectory(browsersPath, browser);
     this._executablePath = browserPaths.executablePath(this._browserPath, browser) || '';
-    this._webSocketNotPipe = webSocketOrPipe;
   }
 
   executablePath(): string {
@@ -175,7 +171,7 @@ export abstract class BrowserType {
       handleSIGTERM,
       handleSIGHUP,
       progress,
-      pipe: !this._webSocketNotPipe,
+      stdio: 'pipe',
       tempDirectories,
       attemptToGracefullyClose: async () => {
         if ((options as any).__testHookGracefullyClose)
@@ -198,14 +194,8 @@ export abstract class BrowserType {
     };
     progress.cleanupWhenAborted(() => browserProcess && closeOrKill(browserProcess, progress.timeUntilDeadline()));
 
-    if (this._webSocketNotPipe) {
+    const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
-      const match = await waitForLine(progress, launchedProcess, this._webSocketNotPipe.stream === 'stdout' ? launchedProcess.stdout : launchedProcess.stderr, this._webSocketNotPipe.webSocketRegex);
+    transport = new PipeTransport(stdio[3], stdio[4]);
-      const innerEndpoint = match[1];
-      transport = await WebSocketTransport.connect(progress, innerEndpoint);
-    } else {
-      const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
-      transport = new PipeTransport(stdio[3], stdio[4]);
-    }
     return { browserProcess, downloadsPath, transport };
   }
 

src/server/chromium/chromium.ts

@@ -26,22 +26,13 @@ import type { BrowserDescriptor } from '../../utils/browserPaths';
 import { CRDevTools } from './crDevTools';
 import { BrowserOptions } from '../browser';
 import * as types from '../types';
-import { isDebugMode, getFromENV } from '../../utils/utils';
+import { isDebugMode } from '../../utils/utils';
 
 export class Chromium extends BrowserType {
   private _devtools: CRDevTools | undefined;
-  private _debugPort: number | undefined;
 
   constructor(packagePath: string, browser: BrowserDescriptor) {
-    const debugPortStr = getFromENV('PLAYWRIGHT_CHROMIUM_DEBUG_PORT');
+    super(packagePath, browser);
-    const debugPort: number | undefined = debugPortStr ? +debugPortStr : undefined;
-    if (debugPort !== undefined) {
-      if (Number.isNaN(debugPort))
-        throw new Error(`PLAYWRIGHT_CHROMIUM_DEBUG_PORT must be a number, but is set to "${debugPortStr}"`);
-    }
-
-    super(packagePath, browser, debugPort ? { webSocketRegex: /^DevTools listening on (ws:\/\/.*)$/, stream: 'stderr' } : null);
-    this._debugPort = debugPort;
     if (isDebugMode())
       this._devtools = this._createDevTools();
   }
@@ -95,10 +86,7 @@ export class Chromium extends BrowserType {
       throw new Error('Arguments can not specify page to be opened');
     const chromeArguments = [...DEFAULT_ARGS];
     chromeArguments.push(`--user-data-dir=${userDataDir}`);
-    if (this._debugPort !== undefined)
+    chromeArguments.push('--remote-debugging-pipe');
-      chromeArguments.push('--remote-debugging-port=' + this._debugPort);
-    else
-      chromeArguments.push('--remote-debugging-pipe');
     if (options.devtools)
       chromeArguments.push('--auto-open-devtools-for-tabs');
     if (options.headless) {

src/server/chromium/videoRecorder.ts

@@ -64,7 +64,7 @@ export class VideoRecorder {
     const { launchedProcess, gracefullyClose } = await launchProcess({
       executablePath,
       args,
-      pipeStdin: true,
+      stdio: 'stdin',
       progress,
       tempDirectories: [],
       attemptToGracefullyClose: async () => {

src/server/electron/electron.ts

@@ -23,13 +23,15 @@ import { Page } from '../page';
 import { TimeoutSettings } from '../../utils/timeoutSettings';
 import { WebSocketTransport } from '../transport';
 import * as types from '../types';
-import { launchProcess, waitForLine, envArrayToObject } from '../processLauncher';
+import { launchProcess, envArrayToObject } from '../processLauncher';
 import { BrowserContext } from '../browserContext';
 import type {BrowserWindow} from 'electron';
-import { ProgressController, runAbortableTask } from '../progress';
+import { Progress, ProgressController, runAbortableTask } from '../progress';
 import { EventEmitter } from 'events';
 import { helper } from '../helper';
 import { BrowserProcess } from '../browser';
+import * as childProcess from 'child_process';
+import * as readline from 'readline';
 
 export type ElectronLaunchOptionsBase = {
   args?: string[],
@@ -163,18 +165,18 @@ export class Electron  {
         handleSIGTERM,
         handleSIGHUP,
         progress,
-        pipe: true,
+        stdio: 'pipe',
         cwd: options.cwd,
         tempDirectories: [],
         attemptToGracefullyClose: () => app!.close(),
         onExit: () => {},
       });
 
-      const nodeMatch = await waitForLine(progress, launchedProcess, launchedProcess.stderr, /^Debugger listening on (ws:\/\/.*)$/);
+      const nodeMatch = await waitForLine(progress, launchedProcess, /^Debugger listening on (ws:\/\/.*)$/);
       const nodeTransport = await WebSocketTransport.connect(progress, nodeMatch[1]);
       const nodeConnection = new CRConnection(nodeTransport);
 
-      const chromeMatch = await waitForLine(progress, launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/);
+      const chromeMatch = await waitForLine(progress, launchedProcess, /^DevTools listening on (ws:\/\/.*)$/);
       const chromeTransport = await WebSocketTransport.connect(progress, chromeMatch[1]);
       const browserProcess: BrowserProcess = {
         onclose: undefined,
@@ -189,3 +191,31 @@ export class Electron  {
     }, TimeoutSettings.timeout(options));
   }
 }
+
+function waitForLine(progress: Progress, process: childProcess.ChildProcess, regex: RegExp): Promise<RegExpMatchArray> {
+  return new Promise((resolve, reject) => {
+    const rl = readline.createInterface({ input: process.stderr });
+    const failError = new Error('Process failed to launch!');
+    const listeners = [
+      helper.addEventListener(rl, 'line', onLine),
+      helper.addEventListener(rl, 'close', reject.bind(null, failError)),
+      helper.addEventListener(process, 'exit', reject.bind(null, failError)),
+      // It is Ok to remove error handler because we did not create process and there is another listener.
+      helper.addEventListener(process, 'error', reject.bind(null, failError))
+    ];
+
+    progress.cleanupWhenAborted(cleanup);
+
+    function onLine(line: string) {
+      const match = line.match(regex);
+      if (!match)
+        return;
+      cleanup();
+      resolve(match);
+    }
+
+    function cleanup() {
+      helper.removeEventListeners(listeners);
+    }
+  });
+}

src/server/firefox/firefox.ts

@@ -24,15 +24,9 @@ import { BrowserType } from '../browserType';
 import { Env } from '../processLauncher';
 import { ConnectionTransport } from '../transport';
 import { BrowserOptions } from '../browser';
-import { BrowserDescriptor } from '../../utils/browserPaths';
 import * as types from '../types';
 
 export class Firefox extends BrowserType {
-  constructor(packagePath: string, browser: BrowserDescriptor) {
-    const webSocketRegex = /^Juggler listening on (ws:\/\/.*)$/;
-    super(packagePath, browser, { webSocketRegex, stream: 'stdout' });
-  }
-
   _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<FFBrowser> {
     return FFBrowser.connect(transport, options);
   }
@@ -78,7 +72,7 @@ export class Firefox extends BrowserType {
       firefoxArguments.push('-foreground');
     }
     firefoxArguments.push(`-profile`, userDataDir);
-    firefoxArguments.push('-juggler', '0');
+    firefoxArguments.push('-juggler-pipe');
     firefoxArguments.push(...args);
     if (isPersistent)
       firefoxArguments.push('about:blank');

src/server/processLauncher.ts

@@ -18,7 +18,6 @@
 import * as childProcess from 'child_process';
 import * as readline from 'readline';
 import * as removeFolder from 'rimraf';
-import * as stream from 'stream';
 import { helper } from './helper';
 import { Progress } from './progress';
 import * as types from './types';
@@ -34,8 +33,7 @@ export type LaunchProcessOptions = {
   handleSIGINT?: boolean,
   handleSIGTERM?: boolean,
   handleSIGHUP?: boolean,
-  pipe?: boolean,
+  stdio: 'pipe' | 'stdin',
-  pipeStdin?: boolean,
   tempDirectories: string[],
 
   cwd?: string,
@@ -68,9 +66,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
   const cleanup = () => helper.removeFolders(options.tempDirectories);
 
   const progress = options.progress;
-  const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe'];
+  const stdio: ('ignore' | 'pipe')[] = options.stdio === 'pipe' ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['pipe', 'pipe', 'pipe'];
-  if (options.pipeStdin)
-    stdio[0] = 'pipe';
   progress.log(`<launching> ${options.executablePath} ${options.args.join(' ')}`);
   const spawnedProcess = childProcess.spawn(
       options.executablePath,
@@ -193,34 +189,6 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
   return { launchedProcess: spawnedProcess, gracefullyClose, kill: killAndWait };
 }
 
-export function waitForLine(progress: Progress, process: childProcess.ChildProcess, inputStream: stream.Readable, regex: RegExp): Promise<RegExpMatchArray> {
-  return new Promise((resolve, reject) => {
-    const rl = readline.createInterface({ input: inputStream });
-    const failError = new Error('Process failed to launch!');
-    const listeners = [
-      helper.addEventListener(rl, 'line', onLine),
-      helper.addEventListener(rl, 'close', reject.bind(null, failError)),
-      helper.addEventListener(process, 'exit', reject.bind(null, failError)),
-      // It is Ok to remove error handler because we did not create process and there is another listener.
-      helper.addEventListener(process, 'error', reject.bind(null, failError))
-    ];
-
-    progress.cleanupWhenAborted(cleanup);
-
-    function onLine(line: string) {
-      const match = line.match(regex);
-      if (!match)
-        return;
-      cleanup();
-      resolve(match);
-    }
-
-    function cleanup() {
-      helper.removeEventListeners(listeners);
-    }
-  });
-}
-
 export function envArrayToObject(env: types.EnvArray): Env {
   const result: Env = {};
   for (const { name, value } of env)

src/server/webkit/webkit.ts

@@ -22,14 +22,9 @@ import { kBrowserCloseMessageId } from './wkConnection';
 import { BrowserType } from '../browserType';
 import { ConnectionTransport } from '../transport';
 import { BrowserOptions } from '../browser';
-import { BrowserDescriptor } from '../../utils/browserPaths';
 import * as types from '../types';
 
 export class WebKit extends BrowserType {
-  constructor(packagePath: string, browser: BrowserDescriptor) {
-    super(packagePath, browser, null /* use pipe not websocket */);
-  }
-
   _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<WKBrowser> {
     return WKBrowser.connect(transport, options);
   }