chore: merge BrowserType and BrowserTypeBase, remove logName (#3837)

- We do not need the public BrowserType different from BrowserTypeBase anymore.
- Removing 'logName' parameter from runAbortableTask - it will
be used for metadata instead.

src/browserServerImpl.ts

@@ -15,7 +15,7 @@
  */
 
 import { LaunchServerOptions } from './client/types';
-import { BrowserTypeBase } from './server/browserType';
+import { BrowserType } from './server/browserType';
 import * as ws from 'ws';
 import { Browser } from './server/browser';
 import { ChildProcess } from 'child_process';
@@ -31,9 +31,9 @@ import { SelectorsDispatcher } from './dispatchers/selectorsDispatcher';
 import { Selectors } from './server/selectors';
 
 export class BrowserServerLauncherImpl implements BrowserServerLauncher {
-  private _browserType: BrowserTypeBase;
+  private _browserType: BrowserType;
 
-  constructor(browserType: BrowserTypeBase) {
+  constructor(browserType: BrowserType) {
     this._browserType = browserType;
   }
 
@@ -50,12 +50,12 @@ export class BrowserServerLauncherImpl implements BrowserServerLauncher {
 
 export class BrowserServerImpl extends EventEmitter implements BrowserServer {
   private _server: ws.Server;
-  private _browserType: BrowserTypeBase;
+  private _browserType: BrowserType;
   private _browser: Browser;
   private _wsEndpoint: string;
   private _process: ChildProcess;
 
-  constructor(browserType: BrowserTypeBase, browser: Browser, port: number = 0) {
+  constructor(browserType: BrowserType, browser: Browser, port: number = 0) {
     super();
 
     this._browserType = browserType;

src/dispatchers/browserTypeDispatcher.ts

@@ -14,14 +14,14 @@
  * limitations under the License.
  */
 
-import { BrowserTypeBase, BrowserType } from '../server/browserType';
+import { BrowserType } from '../server/browserType';
 import { BrowserDispatcher } from './browserDispatcher';
 import * as channels from '../protocol/channels';
 import { Dispatcher, DispatcherScope } from './dispatcher';
 import { BrowserContextDispatcher } from './browserContextDispatcher';
 
 export class BrowserTypeDispatcher extends Dispatcher<BrowserType, channels.BrowserTypeInitializer> implements channels.BrowserTypeChannel {
-  constructor(scope: DispatcherScope, browserType: BrowserTypeBase) {
+  constructor(scope: DispatcherScope, browserType: BrowserType) {
     super(scope, browserType, 'BrowserType', {
       executablePath: browserType.executablePath(),
       name: browserType.name()

src/server/browserType.ts

@@ -24,19 +24,12 @@ import { ConnectionTransport, WebSocketTransport } from './transport';
 import { BrowserOptions, Browser, BrowserProcess } from './browser';
 import { launchProcess, Env, waitForLine, envArrayToObject } from './processLauncher';
 import { PipeTransport } from './pipeTransport';
-import { Progress, runAbortableTask } from './progress';
+import { Progress, ProgressController } from './progress';
 import * as types from './types';
 import { TimeoutSettings } from '../utils/timeoutSettings';
 import { validateHostRequirements } from './validateDependencies';
 import { assert, isDebugMode } from '../utils/utils';
 
-export interface BrowserType {
-  executablePath(): string;
-  name(): string;
-  launch(options?: types.LaunchOptions): Promise<Browser>;
-  launchPersistentContext(userDataDir: string, options?: types.LaunchPersistentOptions): Promise<BrowserContext>;
-}
-
 const mkdirAsync = util.promisify(fs.mkdir);
 const mkdtempAsync = util.promisify(fs.mkdtemp);
 const existsAsync = (path: string): Promise<boolean> => new Promise(resolve => fs.stat(path, err => resolve(!err)));
@@ -45,7 +38,7 @@ const VIDEOS_FOLDER = path.join(os.tmpdir(), 'playwright_videos-');
 
 type WebSocketNotPipe = { webSocketRegex: RegExp, stream: 'stdout' | 'stderr' };
 
-export abstract class BrowserTypeBase implements BrowserType {
+export abstract class BrowserType {
   private _name: string;
   private _executablePath: string | undefined;
   private _webSocketNotPipe: WebSocketNotPipe | null;
@@ -75,7 +68,9 @@ export abstract class BrowserTypeBase implements BrowserType {
     assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead');
     assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
     options = validateLaunchOptions(options);
-    const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, undefined), TimeoutSettings.timeout(options), 'browser').catch(e => { throw this._rewriteStartupError(e); });
+    const controller = new ProgressController(TimeoutSettings.timeout(options));
+    controller.setLogName('browser');
+    const browser = await controller.run(progress => this._innerLaunch(progress, options, undefined).catch(e => { throw this._rewriteStartupError(e); }));
     return browser;
   }
 
@@ -84,7 +79,9 @@ export abstract class BrowserTypeBase implements BrowserType {
     options = validateLaunchOptions(options);
     const persistent: types.BrowserContextOptions = options;
     validateBrowserContextOptions(persistent);
-    const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, persistent, userDataDir), TimeoutSettings.timeout(options), 'browser').catch(e => { throw this._rewriteStartupError(e); });
+    const controller = new ProgressController(TimeoutSettings.timeout(options));
+    controller.setLogName('browser');
+    const browser = await controller.run(progress => this._innerLaunch(progress, options, persistent, userDataDir).catch(e => { throw this._rewriteStartupError(e); }));
     return browser._defaultContext!;
   }
 

src/server/chromium/chromium.ts

@@ -21,7 +21,7 @@ import { CRBrowser } from './crBrowser';
 import { Env } from '../processLauncher';
 import { kBrowserCloseMessageId } from './crConnection';
 import { rewriteErrorMessage } from '../../utils/stackTrace';
-import { BrowserTypeBase } from '../browserType';
+import { BrowserType } from '../browserType';
 import { ConnectionTransport, ProtocolRequest } from '../transport';
 import type { BrowserDescriptor } from '../../utils/browserPaths';
 import { CRDevTools } from './crDevTools';
@@ -29,7 +29,7 @@ import { BrowserOptions } from '../browser';
 import * as types from '../types';
 import { isDebugMode, getFromENV } from '../../utils/utils';
 
-export class Chromium extends BrowserTypeBase {
+export class Chromium extends BrowserType {
   private _devtools: CRDevTools | undefined;
   private _debugPort: number | undefined;
 

src/server/chromium/videoRecorder.ts

@@ -16,7 +16,7 @@
 
 import { launchProcess } from '../processLauncher';
 import { ChildProcess } from 'child_process';
-import { Progress, runAbortableTask } from '../progress';
+import { Progress, ProgressController } from '../progress';
 import * as types from '../types';
 import * as path from 'path';
 import * as os from 'os';
@@ -37,11 +37,13 @@ export class VideoRecorder {
     if (!options.outputFile.endsWith('.webm'))
       throw new Error('File must have .webm extension');
 
-    return await runAbortableTask(async progress => {
+    const controller = new ProgressController(0);
+    controller.setLogName('browser');
+    return await controller.run(async progress => {
       const recorder = new VideoRecorder(progress);
       await recorder._launch(options);
       return recorder;
-    }, 0, 'browser');
+    });
   }
 
   private constructor(progress: Progress) {

src/server/electron/electron.ts

@@ -27,7 +27,7 @@ import * as types from '../types';
 import { launchProcess, waitForLine, envArrayToObject } from '../processLauncher';
 import { BrowserContext } from '../browserContext';
 import type {BrowserWindow} from 'electron';
-import { runAbortableTask, ProgressController } from '../progress';
+import { ProgressController } from '../progress';
 import { EventEmitter } from 'events';
 import { helper } from '../helper';
 import { BrowserProcess } from '../browser';
@@ -147,7 +147,9 @@ export class Electron  {
       handleSIGTERM = true,
       handleSIGHUP = true,
     } = options;
-    return runAbortableTask(async progress => {
+    const controller = new ProgressController(TimeoutSettings.timeout(options));
+    controller.setLogName('browser');
+    return controller.run(async progress => {
       let app: ElectronApplication | undefined = undefined;
       const electronArguments = ['--inspect=0', '--remote-debugging-port=0', '--require', path.join(__dirname, 'electronLoader.js'), ...args];
 
@@ -188,6 +190,6 @@ export class Electron  {
       app = new ElectronApplication(browser, nodeConnection);
       await app._init();
       return app;
-    }, TimeoutSettings.timeout(options), 'browser');
+    });
   }
 }

src/server/firefox/firefox.ts

@@ -20,14 +20,14 @@ import * as fs from 'fs';
 import * as path from 'path';
 import { FFBrowser } from './ffBrowser';
 import { kBrowserCloseMessageId } from './ffConnection';
-import { BrowserTypeBase } from '../browserType';
+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 BrowserTypeBase {
+export class Firefox extends BrowserType {
   constructor(packagePath: string, browser: BrowserDescriptor) {
     const webSocketRegex = /^Juggler listening on (ws:\/\/.*)$/;
     super(packagePath, browser, { webSocketRegex, stream: 'stdout' });

src/server/progress.ts

@@ -28,8 +28,8 @@ export interface Progress {
   throwIfAborted(): void;
 }
 
-export async function runAbortableTask<T>(task: (progress: Progress) => Promise<T>, timeout: number, logName: LogName = 'api'): Promise<T> {
+export async function runAbortableTask<T>(task: (progress: Progress) => Promise<T>, timeout: number): Promise<T> {
-  const controller = new ProgressController(timeout, logName);
+  const controller = new ProgressController(timeout);
   return controller.run(task);
 }
 
@@ -47,14 +47,13 @@ export class ProgressController {
   // Cleanups to be run only in the case of abort.
   private _cleanups: (() => any)[] = [];
 
-  private _logName: LogName;
+  private _logName: LogName = 'api';
   private _state: 'before' | 'running' | 'aborted' | 'finished' = 'before';
   private _deadline: number;
   private _timeout: number;
   private _logRecordring: string[] = [];
 
-  constructor(timeout: number, logName: LogName = 'api') {
+  constructor(timeout: number) {
-    this._logName = logName;
     this._timeout = timeout;
     this._deadline = timeout ? monotonicTime() + timeout : 0;
 
@@ -63,6 +62,10 @@ export class ProgressController {
     this._abortedPromise = new Promise(resolve => this._aborted = resolve);
   }
 
+  setLogName(logName: LogName) {
+    this._logName = logName;
+  }
+
   async run<T>(task: (progress: Progress) => Promise<T>): Promise<T> {
     assert(this._state === 'before');
     this._state = 'running';

src/server/webkit/webkit.ts

@@ -19,13 +19,13 @@ import { WKBrowser } from '../webkit/wkBrowser';
 import { Env } from '../processLauncher';
 import * as path from 'path';
 import { kBrowserCloseMessageId } from './wkConnection';
-import { BrowserTypeBase } from '../browserType';
+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 BrowserTypeBase {
+export class WebKit extends BrowserType {
   constructor(packagePath: string, browser: BrowserDescriptor) {
     super(packagePath, browser, null /* use pipe not websocket */);
   }