chore: get rid of ready state type (#36177)

packages/playwright-core/src/server/bidi/DEPS.list

@@ -11,3 +11,4 @@
 
 [bidiChromium.ts]
 ../chromium/chromiumSwitches.ts
+../chromium/chromium.ts

packages/playwright-core/src/server/bidi/bidiChromium.ts

@@ -17,11 +17,12 @@
 import os from 'os';
 
 import { wrapInASCIIBox } from '../utils/ascii';
-import { BrowserReadyState, BrowserType, kNoXServerRunningError } from '../browserType';
+import { BrowserType, kNoXServerRunningError } from '../browserType';
 import { BidiBrowser } from './bidiBrowser';
 import { kBrowserCloseMessageId } from './bidiConnection';
 import { chromiumSwitches } from '../chromium/chromiumSwitches';
 import { RecentLogsCollector } from '../utils/debugLogger';
+import { waitForReadyState } from '../chromium/chromium';
 
 import type { BrowserOptions } from '../browser';
 import type { SdkObject } from '../instrumentation';
@@ -102,8 +103,8 @@ export class BidiChromium extends BrowserType {
     return chromeArguments;
   }
 
-  override readyState(options: types.LaunchOptions): BrowserReadyState | undefined {
-    return new ChromiumReadyState();
+  override async waitForReadyState(options: types.LaunchOptions, browserLogsCollector: RecentLogsCollector): Promise<{ wsEndpoint?: string }> {
+    return waitForReadyState({ ...options, cdpPort: 0 }, browserLogsCollector);
   }
 
   private _innerDefaultArgs(options: types.LaunchOptions): string[] {
@@ -164,16 +165,4 @@ export class BidiChromium extends BrowserType {
   }
 }
 
-class ChromiumReadyState extends BrowserReadyState {
-  override onBrowserOutput(message: string): void {
-    if (message.includes('Failed to create a ProcessSingleton for your profile directory.')) {
-      this._wsEndpoint.reject(new Error('Failed to create a ProcessSingleton for your profile directory. ' +
-        'This usually means that the profile is already in use by another instance of Chromium.'));
-    }
-    const match = message.match(/DevTools listening on (.*)/);
-    if (match)
-      this._wsEndpoint.resolve(match[1]);
-  }
-}
-
 const kBidiOverCdpWrapper = Symbol('kBidiConnectionWrapper');

packages/playwright-core/src/server/bidi/bidiFirefox.ts

@@ -18,10 +18,11 @@ import os from 'os';
 import path from 'path';
 
 import { wrapInASCIIBox } from '../utils/ascii';
-import { BrowserReadyState, BrowserType, kNoXServerRunningError } from '../browserType';
+import { BrowserType, kNoXServerRunningError } from '../browserType';
 import { BidiBrowser } from './bidiBrowser';
 import { kBrowserCloseMessageId } from './bidiConnection';
 import { createProfile } from './third_party/firefoxPrefs';
+import { ManualPromise } from '../../utils/isomorphic/manualPromise';
 
 import type { BrowserOptions } from '../browser';
 import type { SdkObject } from '../instrumentation';
@@ -29,6 +30,7 @@ import type { Env } from '../utils/processLauncher';
 import type { ProtocolError } from '../protocolError';
 import type { ConnectionTransport } from '../transport';
 import type * as types from '../types';
+import type { RecentLogsCollector } from '../utils/debugLogger';
 
 
 export class BidiFirefox extends BrowserType {
@@ -101,16 +103,13 @@ export class BidiFirefox extends BrowserType {
     return firefoxArguments;
   }
 
-  override readyState(options: types.LaunchOptions): BrowserReadyState | undefined {
-    return new FirefoxReadyState();
-  }
-}
-
-class FirefoxReadyState extends BrowserReadyState {
-  override onBrowserOutput(message: string): void {
-    // Bidi WebSocket in Firefox.
-    const match = message.match(/WebDriver BiDi listening on (ws:\/\/.*)$/);
-    if (match)
-      this._wsEndpoint.resolve(match[1] + '/session');
+  override async waitForReadyState(options: types.LaunchOptions, browserLogsCollector: RecentLogsCollector): Promise<{ wsEndpoint?: string }> {
+    const result = new ManualPromise<{ wsEndpoint?: string }>();
+    browserLogsCollector.onMessage(message => {
+      const match = message.match(/WebDriver BiDi listening on (ws:\/\/.*)$/);
+      if (match)
+        result.resolve({ wsEndpoint: match[1] + '/session' });
+    });
+    return result;
   }
 }

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

@@ -49,22 +49,6 @@ import type * as channels from '@protocol/channels';
 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 abstract class BrowserReadyState {
-  protected readonly _wsEndpoint = new ManualPromise<string|undefined>();
-
-  onBrowserExit(): void {
-    // Unblock launch when browser prematurely exits.
-    this._wsEndpoint.resolve(undefined);
-  }
-  async waitUntilReady(): Promise<{ wsEndpoint?: string }> {
-    const wsEndpoint = await this._wsEndpoint;
-    return { wsEndpoint };
-  }
-
-  abstract onBrowserOutput(message: string): void;
-}
-
 export abstract class BrowserType extends SdkObject {
   private _name: BrowserName;
 
@@ -216,11 +200,11 @@ export abstract class BrowserType extends SdkObject {
       await registry.validateHostRequirementsForExecutablesIfNeeded([registryExecutable], this.attribution.playwright.options.sdkLanguage);
     }
 
-    const readyState = this.readyState(options);
     // 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;
     let browserProcess: BrowserProcess | undefined = undefined;
+    const exitPromise = new ManualPromise();
     const { launchedProcess, gracefullyClose, kill } = await launchProcess({
       command: executable,
       args: browserArguments,
@@ -229,7 +213,6 @@ export abstract class BrowserType extends SdkObject {
       handleSIGTERM,
       handleSIGHUP,
       log: (message: string) => {
-        readyState?.onBrowserOutput(message);
         progress.log(message);
         browserLogsCollector.log(message);
       },
@@ -245,11 +228,12 @@ export abstract class BrowserType extends SdkObject {
       },
       onExit: (exitCode, signal) => {
         // Unblock launch when browser prematurely exits.
-        readyState?.onBrowserExit();
+        exitPromise.resolve();
         if (browserProcess && browserProcess.onclose)
           browserProcess.onclose(exitCode, signal);
       },
     });
+
     async function closeOrKill(timeout: number): Promise<void> {
       let timer: NodeJS.Timeout;
       try {
@@ -270,7 +254,10 @@ export abstract class BrowserType extends SdkObject {
       kill
     };
     progress.cleanupWhenAborted(() => closeOrKill(progress.timeUntilDeadline()));
-    const wsEndpoint = (await readyState?.waitUntilReady())?.wsEndpoint;
+    const { wsEndpoint } = await Promise.race([
+      this.waitForReadyState(options, browserLogsCollector),
+      exitPromise.then(() => ({ wsEndpoint: undefined })),
+    ]);
     if (options.cdpPort !== undefined || !this.supportsPipeTransport()) {
       transport = await WebSocketTransport.connect(progress, wsEndpoint!);
     } else {
@@ -326,8 +313,8 @@ export abstract class BrowserType extends SdkObject {
     return this.doRewriteStartupLog(error);
   }
 
-  readyState(options: types.LaunchOptions): BrowserReadyState|undefined {
-    return undefined;
+  async waitForReadyState(options: types.LaunchOptions, browserLogsCollector: RecentLogsCollector): Promise<{ wsEndpoint?: string }> {
+    return {};
   }
 
   async prepareUserDataDir(options: types.LaunchOptions, userDataDir: string): Promise<void> {

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

@@ -30,7 +30,6 @@ import { fetchData } from '../utils/network';
 import { getUserAgent } from '../utils/userAgent';
 import { validateBrowserContextOptions } from '../browserContext';
 import { BrowserType, kNoXServerRunningError } from '../browserType';
-import { BrowserReadyState } from '../browserType';
 import { helper } from '../helper';
 import { registry } from '../registry';
 import { WebSocketTransport } from '../transport';
@@ -353,10 +352,8 @@ export class Chromium extends BrowserType {
     return chromeArguments;
   }
 
-  override readyState(options: types.LaunchOptions): BrowserReadyState | undefined {
-    if (options.cdpPort !== undefined || options.args?.some(a => a.startsWith('--remote-debugging-port')))
-      return new ChromiumReadyState();
-    return undefined;
+  override async waitForReadyState(options: types.LaunchOptions, browserLogsCollector: RecentLogsCollector): Promise<{ wsEndpoint?: string }> {
+    return waitForReadyState(options, browserLogsCollector);
   }
 
   override getExecutableName(options: types.LaunchOptions): string {
@@ -366,16 +363,21 @@ export class Chromium extends BrowserType {
   }
 }
 
-class ChromiumReadyState extends BrowserReadyState {
-  override onBrowserOutput(message: string): void {
+export async function waitForReadyState(options: types.LaunchOptions, browserLogsCollector: RecentLogsCollector): Promise<{ wsEndpoint?: string }> {
+  if (options.cdpPort === undefined && !options.args?.some(a => a.startsWith('--remote-debugging-port')))
+    return {};
+
+  const result = new ManualPromise<{ wsEndpoint?: string }>();
+  browserLogsCollector.onMessage(message => {
     if (message.includes('Failed to create a ProcessSingleton for your profile directory.')) {
-      this._wsEndpoint.reject(new Error('Failed to create a ProcessSingleton for your profile directory. ' +
+      result.reject(new Error('Failed to create a ProcessSingleton for your profile directory. ' +
         'This usually means that the profile is already in use by another instance of Chromium.'));
     }
     const match = message.match(/DevTools listening on (.*)/);
     if (match)
-      this._wsEndpoint.resolve(match[1]);
-  }
+      result.resolve({ wsEndpoint: match[1] });
+  });
+  return result;
 }
 
 async function urlToWSEndpoint(progress: Progress, endpointURL: string, headers: { [key: string]: string; }) {

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

@@ -22,7 +22,7 @@ import { FFBrowser } from './ffBrowser';
 import { kBrowserCloseMessageId } from './ffConnection';
 import { wrapInASCIIBox } from '../utils/ascii';
 import { BrowserType, kNoXServerRunningError } from '../browserType';
-import { BrowserReadyState } from '../browserType';
+import { ManualPromise } from '../../utils/isomorphic/manualPromise';
 
 import type { BrowserOptions } from '../browser';
 import type { SdkObject } from '../instrumentation';
@@ -30,6 +30,7 @@ import type { Env } from '../utils/processLauncher';
 import type { ProtocolError } from '../protocolError';
 import type { ConnectionTransport } from '../transport';
 import type * as types from '../types';
+import type { RecentLogsCollector } from '../utils/debugLogger';
 
 export class Firefox extends BrowserType {
   constructor(parent: SdkObject) {
@@ -92,14 +93,12 @@ export class Firefox extends BrowserType {
     return firefoxArguments;
   }
 
-  override readyState(options: types.LaunchOptions): BrowserReadyState | undefined {
-    return new JugglerReadyState();
-  }
-}
-
-class JugglerReadyState extends BrowserReadyState {
-  override onBrowserOutput(message: string): void {
-    if (message.includes('Juggler listening to the pipe'))
-      this._wsEndpoint.resolve(undefined);
+  override waitForReadyState(options: types.LaunchOptions, browserLogsCollector: RecentLogsCollector): Promise<{ wsEndpoint?: string }> {
+    const result = new ManualPromise<{ wsEndpoint?: string }>();
+    browserLogsCollector.onMessage(message => {
+      if (message.includes('Juggler listening to the pipe'))
+        result.resolve({});
+    });
+    return result;
   }
 }

packages/playwright-core/src/server/utils/debugLogger.ts

@@ -72,11 +72,14 @@ export const debugLogger = new DebugLogger();
 const kLogCount = 150;
 export class RecentLogsCollector {
   private _logs: string[] = [];
+  private _listeners: ((log: string) => void)[] = [];
 
   log(message: string) {
     this._logs.push(message);
     if (this._logs.length === kLogCount * 2)
       this._logs.splice(0, kLogCount);
+    for (const listener of this._listeners)
+      listener(message);
   }
 
   recentLogs(): string[] {
@@ -84,4 +87,10 @@ export class RecentLogsCollector {
       return this._logs.slice(-kLogCount);
     return this._logs;
   }
+
+  onMessage(listener: (message: string) => void) {
+    for (const message of this._logs)
+      listener(message);
+    this._listeners.push(listener);
+  }
 }