chore: move output parsing to concrete browsers (#32129)

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

@@ -38,12 +38,18 @@ import { helper } from './helper';
 import { RecentLogsCollector } from '../utils/debugLogger';
 import type { CallMetadata } from './instrumentation';
 import { SdkObject } from './instrumentation';
-import { ManualPromise } from '../utils/manualPromise';
 import { type ProtocolError, isProtocolError } from './protocolError';
 
 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';
 
+
+export interface BrowserReadyState {
+  onBrowserOutput(message: string): void;
+  onBrowserExit(): void;
+  waitUntilReady(): Promise<{ wsEndpoint?: string }>;
+}
+
 export abstract class BrowserType extends SdkObject {
   private _name: BrowserName;
 
@@ -190,8 +196,7 @@ export abstract class BrowserType extends SdkObject {
       await registry.validateHostRequirementsForExecutablesIfNeeded([registryExecutable], this.attribution.playwright.options.sdkLanguage);
     }
 
-    const waitForWSEndpoint = (options.useWebSocket || options.args?.some(a => a.startsWith('--remote-debugging-port'))) ? new ManualPromise<string>() : undefined;
+    const readyState = this.readyState(options);
-    const waitForJuggler = this._name === 'firefox' ? new ManualPromise<void>() : 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;
@@ -204,13 +209,7 @@ export abstract class BrowserType extends SdkObject {
       handleSIGTERM,
       handleSIGHUP,
       log: (message: string) => {
-        if (waitForWSEndpoint) {
+        readyState?.onBrowserOutput(message);
-          const match = message.match(/DevTools listening on (.*)/);
-          if (match)
-            waitForWSEndpoint.resolve(match[1]);
-        }
-        if (waitForJuggler && message.includes('Juggler listening to the pipe'))
-          waitForJuggler.resolve();
         progress.log(message);
         browserLogsCollector.log(message);
       },
@@ -226,7 +225,7 @@ export abstract class BrowserType extends SdkObject {
       },
       onExit: (exitCode, signal) => {
         // Unblock launch when browser prematurely exits.
-        waitForJuggler?.resolve();
+        readyState?.onBrowserExit();
         if (browserProcess && browserProcess.onclose)
           browserProcess.onclose(exitCode, signal);
       },
@@ -251,8 +250,7 @@ export abstract class BrowserType extends SdkObject {
       kill
     };
     progress.cleanupWhenAborted(() => closeOrKill(progress.timeUntilDeadline()));
-    const wsEndpoint = await waitForWSEndpoint;
+    const wsEndpoint = (await readyState?.waitUntilReady())?.wsEndpoint;
-    await waitForJuggler;
     if (options.useWebSocket) {
       transport = await WebSocketTransport.connect(progress, wsEndpoint!);
     } else {
@@ -308,6 +306,10 @@ export abstract class BrowserType extends SdkObject {
     return this.doRewriteStartupLog(error);
   }
 
+  readyState(options: types.LaunchOptions): BrowserReadyState|undefined {
+    return undefined;
+  }
+
   abstract defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[];
   abstract connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<Browser>;
   abstract amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env;

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

@@ -24,6 +24,7 @@ import type { Env } from '../../utils/processLauncher';
 import { gracefullyCloseSet } from '../../utils/processLauncher';
 import { kBrowserCloseMessageId } from './crConnection';
 import { BrowserType, kNoXServerRunningError } from '../browserType';
+import type { BrowserReadyState } from '../browserType';
 import type { ConnectionTransport, ProtocolRequest } from '../transport';
 import { WebSocketTransport } from '../transport';
 import { CRDevTools } from './crDevTools';
@@ -349,6 +350,29 @@ export class Chromium extends BrowserType {
     chromeArguments.push(...args);
     return chromeArguments;
   }
+
+  override readyState(options: types.LaunchOptions): BrowserReadyState | undefined {
+    if (options.useWebSocket || options.args?.some(a => a.startsWith('--remote-debugging-port')))
+      return new ChromiumReadyState();
+    return undefined;
+  }
+}
+
+class ChromiumReadyState implements BrowserReadyState {
+  private readonly _wsEndpoint = new ManualPromise<string|undefined>();
+
+  onBrowserOutput(message: string): void {
+    const match = message.match(/DevTools listening on (.*)/);
+    if (match)
+      this._wsEndpoint.resolve(match[1]);
+  }
+  onBrowserExit(): void {
+    this._wsEndpoint.resolve(undefined);
+  }
+  async waitUntilReady(): Promise<{ wsEndpoint?: string }> {
+    const wsEndpoint = await this._wsEndpoint;
+    return { wsEndpoint };
+  }
 }
 
 async function urlToWSEndpoint(progress: Progress, endpointURL: string, headers: { [key: string]: string; }) {

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

@@ -20,11 +20,12 @@ import path from 'path';
 import { FFBrowser } from './ffBrowser';
 import { kBrowserCloseMessageId } from './ffConnection';
 import { BrowserType, kNoXServerRunningError } from '../browserType';
+import type { BrowserReadyState } from '../browserType';
 import type { Env } from '../../utils/processLauncher';
 import type { ConnectionTransport } from '../transport';
 import type { BrowserOptions } from '../browser';
 import type * as types from '../types';
-import { wrapInASCIIBox } from '../../utils';
+import { ManualPromise, wrapInASCIIBox } from '../../utils';
 import type { SdkObject } from '../instrumentation';
 import type { ProtocolError } from '../protocolError';
 
@@ -88,5 +89,26 @@ export class Firefox extends BrowserType {
       firefoxArguments.push('-silent');
     return firefoxArguments;
   }
+
+  override readyState(options: types.LaunchOptions): BrowserReadyState | undefined {
+    return new JugglerReadyState();
+  }
+}
+
+class JugglerReadyState implements BrowserReadyState {
+  private readonly _jugglerPromise = new ManualPromise<void>();
+
+  onBrowserOutput(message: string): void {
+    if (message.includes('Juggler listening to the pipe'))
+      this._jugglerPromise.resolve();
+  }
+  onBrowserExit(): void {
+    // Unblock launch when browser prematurely exits.
+    this._jugglerPromise.resolve();
+  }
+  async waitUntilReady(): Promise<{ wsEndpoint?: string }> {
+    await this._jugglerPromise;
+    return { };
+  }
 }