fix(firefox): launch races (#15259)

Potential fixes to avoid startup races:
- Wait for "juggler listening" message.
- Make sure `transport.onclose` is called when connecting to the transport after the actual pipe closure.

packages/playwright-core/src/server/browserType.ts

@@ -38,6 +38,7 @@ import { helper } from './helper';
 import { RecentLogsCollector } from '../common/debugLogger';
 import type { CallMetadata } from './instrumentation';
 import { SdkObject } from './instrumentation';
+import { ManualPromise } from '../utils/manualPromise';
 
 export const kNoXServerRunningError = 'Looks like you launched a headed browser without having a XServer running.\n' +
   'Set either \'headless: true\' or use \'xvfb-run <your-playwright-app>\' before running Playwright.\n\n<3 Playwright Team';
@@ -186,9 +187,8 @@ export abstract class BrowserType extends SdkObject {
       await registryExecutable.validateHostRequirements(this._playwrightOptions.sdkLanguage);
     }
 
-    let wsEndpointCallback: ((wsEndpoint: string) => void) | undefined;
+    const waitForWSEndpoint = (options.useWebSocket || options.args?.some(a => a.startsWith('--remote-debugging-port'))) ? new ManualPromise<string>() : undefined;
-    const shouldWaitForWSListening = options.useWebSocket || options.args?.some(a => a.startsWith('--remote-debugging-port'));
+    const waitForJuggler = this._name === 'firefox' ? new ManualPromise<void>() : undefined;
-    const waitForWSEndpoint = shouldWaitForWSListening ? new Promise<string>(f => wsEndpointCallback = f) : undefined;
     // Note: it is important to define these variables before launchProcess, so that we don't get
     // "Cannot access 'browserServer' before initialization" if something went wrong.
     let transport: ConnectionTransport | undefined = undefined;
@@ -201,11 +201,13 @@ export abstract class BrowserType extends SdkObject {
       handleSIGTERM,
       handleSIGHUP,
       log: (message: string) => {
-        if (wsEndpointCallback) {
+        if (waitForWSEndpoint) {
           const match = message.match(/DevTools listening on (.*)/);
           if (match)
-            wsEndpointCallback(match[1]);
+            waitForWSEndpoint.resolve(match[1]);
         }
+        if (waitForJuggler && message.includes('Juggler listening to the pipe'))
+          waitForJuggler.resolve();
         progress.log(message);
         browserLogsCollector.log(message);
       },
@@ -220,6 +222,8 @@ export abstract class BrowserType extends SdkObject {
         this._attemptToGracefullyCloseBrowser(transport!);
       },
       onExit: (exitCode, signal) => {
+        // Unblock launch when browser prematurely exits.
+        waitForJuggler?.resolve();
         if (browserProcess && browserProcess.onclose)
           browserProcess.onclose(exitCode, signal);
       },
@@ -244,9 +248,8 @@ export abstract class BrowserType extends SdkObject {
       kill
     };
     progress.cleanupWhenAborted(() => closeOrKill(progress.timeUntilDeadline()));
-    let wsEndpoint: string | undefined;
+    const wsEndpoint = await waitForWSEndpoint;
-    if (shouldWaitForWSListening)
+    await waitForJuggler;
-      wsEndpoint = await waitForWSEndpoint;
     if (options.useWebSocket) {
       transport = await WebSocketTransport.connect(progress, wsEndpoint!);
     } else {

packages/playwright-core/src/server/chromium/crConnection.ts

@@ -49,10 +49,11 @@ export class CRConnection extends EventEmitter {
     this._transport = transport;
     this._protocolLogger = protocolLogger;
     this._browserLogsCollector = browserLogsCollector;
-    this._transport.onmessage = this._onMessage.bind(this);
-    this._transport.onclose = this._onClose.bind(this);
     this.rootSession = new CRSession(this, '', 'browser', '');
     this._sessions.set('', this.rootSession);
+    this._transport.onmessage = this._onMessage.bind(this);
+    // onclose should be set last, since it can be immediately called.
+    this._transport.onclose = this._onClose.bind(this);
   }
 
   static fromSession(session: CRSession): CRConnection {

packages/playwright-core/src/server/firefox/ffConnection.ts

@@ -58,8 +58,6 @@ export class FFConnection extends EventEmitter {
     this._lastId = 0;
     this._callbacks = new Map();
 
-    this._transport.onmessage = this._onMessage.bind(this);
-    this._transport.onclose = this._onClose.bind(this);
     this._sessions = new Map();
     this._closed = false;
 
@@ -68,6 +66,10 @@ export class FFConnection extends EventEmitter {
     this.off = super.removeListener;
     this.removeListener = super.removeListener;
     this.once = super.once;
+
+    this._transport.onmessage = this._onMessage.bind(this);
+    // onclose should be set last, since it can be immediately called.
+    this._transport.onclose = this._onClose.bind(this);
   }
 
   async send<T extends keyof Protocol.CommandParameters>(

packages/playwright-core/src/server/pipeTransport.ts

@@ -20,26 +20,37 @@ import { makeWaitForNextTask } from '../utils';
 import { debugLogger } from '../common/debugLogger';
 
 export class PipeTransport implements ConnectionTransport {
+  private _pipeRead: NodeJS.ReadableStream;
   private _pipeWrite: NodeJS.WritableStream;
   private _pendingMessage = '';
   private _waitForNextTask = makeWaitForNextTask();
   private _closed = false;
+  private _onclose?: () => void;
 
   onmessage?: (message: ProtocolResponse) => void;
-  onclose?: () => void;
 
   constructor(pipeWrite: NodeJS.WritableStream, pipeRead: NodeJS.ReadableStream) {
+    this._pipeRead = pipeRead;
     this._pipeWrite = pipeWrite;
     pipeRead.on('data', buffer => this._dispatch(buffer));
     pipeRead.on('close', () => {
       this._closed = true;
-      if (this.onclose)
+      if (this._onclose)
-        this.onclose.call(null);
+        this._onclose.call(null);
     });
     pipeRead.on('error', e => debugLogger.log('error', e));
     pipeWrite.on('error', e => debugLogger.log('error', e));
     this.onmessage = undefined;
-    this.onclose = undefined;
+  }
+
+  get onclose() {
+    return this._onclose;
+  }
+
+  set onclose(onclose: undefined | (() => void)) {
+    this._onclose = onclose;
+    if (onclose && !this._pipeRead.readable)
+      onclose();
   }
 
   send(message: ProtocolRequest) {

packages/playwright-core/src/server/webkit/wkConnection.ts

@@ -47,14 +47,15 @@ export class WKConnection {
 
   constructor(transport: ConnectionTransport, onDisconnect: () => void, protocolLogger: ProtocolLogger, browserLogsCollector: RecentLogsCollector) {
     this._transport = transport;
-    this._transport.onmessage = this._dispatchMessage.bind(this);
-    this._transport.onclose = this._onClose.bind(this);
     this._onDisconnect = onDisconnect;
     this._protocolLogger = protocolLogger;
     this._browserLogsCollector = browserLogsCollector;
     this.browserSession = new WKSession(this, '', kBrowserClosedError, (message: any) => {
       this.rawSend(message);
     });
+    this._transport.onmessage = this._dispatchMessage.bind(this);
+    // onclose should be set last, since it can be immediately called.
+    this._transport.onclose = this._onClose.bind(this);
   }
 
   nextMessageId(): number {