Revert "feat(firefox): migrate to the pipe channel (#4068)" (#4073)

Mac sporadically hangs on browser close.

Author: pavelfeldman

browsers.json

@@ -8,7 +8,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1184",
+      "revision": "1182",
       "download": true
     },
     {

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 } from './transport';
+import { ConnectionTransport, WebSocketTransport } from './transport';
 import { BrowserOptions, Browser, BrowserProcess } from './browser';
-import { launchProcess, Env, envArrayToObject } from './processLauncher';
+import { launchProcess, Env, waitForLine, envArrayToObject } from './processLauncher';
 import { PipeTransport } from './pipeTransport';
 import { Progress, ProgressController } from './progress';
 import * as types from './types';
@@ -35,18 +35,22 @@ 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) {
+  constructor(packagePath: string, browser: browserPaths.BrowserDescriptor, webSocketOrPipe: WebSocketNotPipe | null) {
     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 {
@@ -171,6 +175,7 @@ export abstract class BrowserType {
       handleSIGTERM,
       handleSIGHUP,
       progress,
+      pipe: !this._webSocketNotPipe,
       tempDirectories,
       attemptToGracefullyClose: async () => {
         if ((options as any).__testHookGracefullyClose)
@@ -193,8 +198,14 @@ export abstract class BrowserType {
     };
     progress.cleanupWhenAborted(() => browserProcess && closeOrKill(browserProcess, progress.timeUntilDeadline()));
 
-    const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
-    transport = new PipeTransport(stdio[3], stdio[4]);
+    if (this._webSocketNotPipe) {
+      const match = await waitForLine(progress, launchedProcess, this._webSocketNotPipe.stream === 'stdout' ? launchedProcess.stdout : launchedProcess.stderr, this._webSocketNotPipe.webSocketRegex);
+      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

@@ -31,9 +31,18 @@ import { isDebugMode, getFromENV } from '../../utils/utils';
 
 export class Chromium extends BrowserType {
   private _devtools: CRDevTools | undefined;
+  private _debugPort: number | undefined;
 
   constructor(packagePath: string, browser: BrowserDescriptor) {
-    super(packagePath, browser);
+    const debugPortStr = getFromENV('PLAYWRIGHT_CHROMIUM_DEBUG_PORT');
+    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();
   }
@@ -101,16 +110,10 @@ 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}`);
-
-    const debugPortStr = getFromENV('PLAYWRIGHT_CHROMIUM_DEBUG_PORT');
-    if (debugPortStr) {
-      const debugPort = +debugPortStr;
-      if (Number.isNaN(debugPort))
-        throw new Error(`PLAYWRIGHT_CHROMIUM_DEBUG_PORT must be a number, but is set to "${debugPortStr}"`);
-      chromeArguments.push('--remote-debugging-port=' + debugPort);
-    }
-
-    chromeArguments.push('--remote-debugging-pipe');
+    if (this._debugPort !== undefined)
+      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/electron/electron.ts

@@ -163,6 +163,7 @@ export class Electron  {
         handleSIGTERM,
         handleSIGHUP,
         progress,
+        pipe: true,
         cwd: options.cwd,
         tempDirectories: [],
         attemptToGracefullyClose: () => app!.close(),

src/server/firefox/firefox.ts

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

src/server/processLauncher.ts

@@ -35,6 +35,7 @@ export type LaunchProcessOptions = {
   handleSIGINT?: boolean,
   handleSIGTERM?: boolean,
   handleSIGHUP?: boolean,
+  pipe?: boolean,
   pipeStdin?: boolean,
   tempDirectories: string[],
 
@@ -82,7 +83,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
   const cleanup = () => helper.removeFolders(options.tempDirectories);
 
   const progress = options.progress;
-  const stdio: ('ignore' | 'pipe')[] = ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'];
+  const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe'];
   if (options.pipeStdin)
     stdio[0] = 'pipe';
   progress.log(`<launching> ${options.executablePath} ${options.args.join(' ')}`);

src/server/webkit/webkit.ts

@@ -27,7 +27,7 @@ import * as types from '../types';
 
 export class WebKit extends BrowserType {
   constructor(packagePath: string, browser: BrowserDescriptor) {
-    super(packagePath, browser);
+    super(packagePath, browser, null /* use pipe not websocket */);
   }
 
   _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<WKBrowser> {