Skip to content

Commit a51ea32

Browse files
dgozmandgozman
committed
chore: simplify bidi browsers handling
1 parent 5f65f32 commit a51ea32

File tree

16 files changed

+41
-76
lines changed

16 files changed

+41
-76
lines changed

‎packages/playwright-core/src/browserServerImpl.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ import type { WebSocketEventEmitter } from './utilsBundle';
3232
import type { Browser } from './server/browser';
3333

3434
export class BrowserServerLauncherImpl implements BrowserServerLauncher {
35-
private _browserName: 'chromium' | 'firefox' | 'webkit' | 'bidiFirefox' | 'bidiChromium';
35+
private _browserName: 'chromium' | 'firefox' | 'webkit' | '_bidiFirefox' | '_bidiChromium';
3636

37-
constructor(browserName: 'chromium' | 'firefox' | 'webkit' | 'bidiFirefox' | 'bidiChromium') {
37+
constructor(browserName: 'chromium' | 'firefox' | 'webkit' | '_bidiFirefox' | '_bidiChromium') {
3838
this._browserName = browserName;
3939
}
4040

‎packages/playwright-core/src/client/playwright.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ export class Playwright extends ChannelOwner<channels.PlaywrightChannel> {
5858
this._android._playwright = this;
5959
this._electron = Electron.from(initializer.electron);
6060
this._electron._playwright = this;
61-
this._bidiChromium = BrowserType.from(initializer.bidiChromium);
61+
this._bidiChromium = BrowserType.from(initializer._bidiChromium);
6262
this._bidiChromium._playwright = this;
63-
this._bidiFirefox = BrowserType.from(initializer.bidiFirefox);
63+
this._bidiFirefox = BrowserType.from(initializer._bidiFirefox);
6464
this._bidiFirefox._playwright = this;
6565
this.devices = this._connection.localUtils()?.devices ?? {};
6666
this.selectors = new Selectors(this._connection._platform);

‎packages/playwright-core/src/inProcessFactory.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ export function createInProcessPlaywright(): PlaywrightAPI {
4242
playwrightAPI.firefox._serverLauncher = new BrowserServerLauncherImpl('firefox');
4343
playwrightAPI.webkit._serverLauncher = new BrowserServerLauncherImpl('webkit');
4444
playwrightAPI._android._serverLauncher = new AndroidServerLauncherImpl();
45-
playwrightAPI._bidiChromium._serverLauncher = new BrowserServerLauncherImpl('bidiChromium');
46-
playwrightAPI._bidiFirefox._serverLauncher = new BrowserServerLauncherImpl('bidiFirefox');
45+
playwrightAPI._bidiChromium._serverLauncher = new BrowserServerLauncherImpl('_bidiChromium');
46+
playwrightAPI._bidiFirefox._serverLauncher = new BrowserServerLauncherImpl('_bidiFirefox');
4747

4848
// Switch to async dispatch after we got Playwright object.
4949
dispatcherConnection.onmessage = message => setImmediate(() => clientConnection.dispatch(message));

‎packages/playwright-core/src/protocol/validator.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,8 @@ scheme.PlaywrightInitializer = tObject({
376376
chromium: tChannel(['BrowserType']),
377377
firefox: tChannel(['BrowserType']),
378378
webkit: tChannel(['BrowserType']),
379-
bidiChromium: tChannel(['BrowserType']),
380-
bidiFirefox: tChannel(['BrowserType']),
379+
_bidiChromium: tChannel(['BrowserType']),
380+
_bidiFirefox: tChannel(['BrowserType']),
381381
android: tChannel(['Android']),
382382
electron: tChannel(['Electron']),
383383
utils: tOptional(tChannel(['LocalUtils'])),

‎packages/playwright-core/src/remote/playwrightConnection.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,7 @@ export class PlaywrightConnection {
118118
private async _initLaunchBrowserMode(scope: RootDispatcher, options: channels.RootInitializeParams) {
119119
debugLogger.log('server', `[${this._id}] engaged launch mode for "${this._options.browserName}"`);
120120
const ownedSocksProxy = await this._createOwnedSocksProxy();
121-
let browserName = this._options.browserName;
122-
if ('bidi' === browserName) {
123-
if (this._options.launchOptions?.channel?.toLocaleLowerCase().includes('firefox'))
124-
browserName = 'bidiFirefox';
125-
else
126-
browserName = 'bidiChromium';
127-
}
128-
const browser = await this._playwright[browserName as 'chromium'].launch(serverSideCallMetadata(), this._options.launchOptions);
121+
const browser = await this._playwright[this._options.browserName as 'chromium'].launch(serverSideCallMetadata(), this._options.launchOptions);
129122
browser.options.sdkLanguage = options.sdkLanguage;
130123

131124
this._cleanups.push(() => browser.close({ reason: 'Connection terminated' }));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import type * as types from '../types';
3434

3535
export class BidiChromium extends BrowserType {
3636
constructor(parent: SdkObject) {
37-
super(parent, 'bidi');
37+
super(parent, '_bidiChromium');
3838
}
3939

4040
override async connectToTransport(transport: ConnectionTransport, options: BrowserOptions, browserLogsCollector: RecentLogsCollector): Promise<BidiBrowser> {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ import type { RecentLogsCollector } from '../utils/debugLogger';
3535

3636
export class BidiFirefox extends BrowserType {
3737
constructor(parent: SdkObject) {
38-
super(parent, 'bidi');
38+
super(parent, '_bidiFirefox');
39+
}
40+
41+
override executablePath(): string {
42+
return '';
3943
}
4044

4145
override async connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<BidiBrowser> {

‎packages/playwright-core/src/server/dispatchers/playwrightDispatcher.ts‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,15 @@ export class PlaywrightDispatcher extends Dispatcher<Playwright, channels.Playwr
5252
const chromium = new BrowserTypeDispatcher(scope, playwright.chromium, denyLaunch);
5353
const firefox = new BrowserTypeDispatcher(scope, playwright.firefox, denyLaunch);
5454
const webkit = new BrowserTypeDispatcher(scope, playwright.webkit, denyLaunch);
55-
const bidiChromium = new BrowserTypeDispatcher(scope, playwright.bidiChromium, denyLaunch);
56-
const bidiFirefox = new BrowserTypeDispatcher(scope, playwright.bidiFirefox, denyLaunch);
55+
const _bidiChromium = new BrowserTypeDispatcher(scope, playwright._bidiChromium, denyLaunch);
56+
const _bidiFirefox = new BrowserTypeDispatcher(scope, playwright._bidiFirefox, denyLaunch);
5757
const android = new AndroidDispatcher(scope, playwright.android, denyLaunch);
5858
const initializer: channels.PlaywrightInitializer = {
5959
chromium,
6060
firefox,
6161
webkit,
62-
bidiChromium,
63-
bidiFirefox,
62+
_bidiChromium,
63+
_bidiFirefox,
6464
android,
6565
electron: new ElectronDispatcher(scope, playwright.electron, denyLaunch),
6666
utils: playwright.options.isServer ? undefined : new LocalUtilsDispatcher(scope, playwright),
@@ -69,14 +69,7 @@ export class PlaywrightDispatcher extends Dispatcher<Playwright, channels.Playwr
6969

7070
let browserDispatcher: BrowserDispatcher | undefined;
7171
if (options.preLaunchedBrowser) {
72-
let browserTypeDispatcher: BrowserTypeDispatcher;
73-
switch (options.preLaunchedBrowser.options.name) {
74-
case 'chromium': browserTypeDispatcher = chromium; break;
75-
case 'firefox': browserTypeDispatcher = firefox; break;
76-
case 'webkit': browserTypeDispatcher = webkit; break;
77-
case 'bidi': browserTypeDispatcher = options.preLaunchedBrowser.options.channel?.includes('firefox') ? bidiFirefox : bidiChromium; break;
78-
default: throw new Error(`Unknown browser name: ${options.preLaunchedBrowser.options.name}`);
79-
}
72+
const browserTypeDispatcher = initializer[options.preLaunchedBrowser.options.name as keyof typeof initializer] as BrowserTypeDispatcher;
8073
browserDispatcher = new BrowserDispatcher(browserTypeDispatcher, options.preLaunchedBrowser, {
8174
ignoreStopAndKill: true,
8275
isolateContexts: !options.sharedBrowser,

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ export class Playwright extends SdkObject {
4444
readonly electron: Electron;
4545
readonly firefox: BrowserType;
4646
readonly webkit: BrowserType;
47-
readonly bidiChromium: BrowserType;
48-
readonly bidiFirefox: BrowserType;
47+
readonly _bidiChromium: BrowserType;
48+
readonly _bidiFirefox: BrowserType;
4949
readonly options: PlaywrightOptions;
5050
readonly debugController: DebugController;
5151
private _allPages = new Set<Page>();
@@ -65,8 +65,8 @@ export class Playwright extends SdkObject {
6565
}
6666
}, null);
6767
this.chromium = new Chromium(this);
68-
this.bidiChromium = new BidiChromium(this);
69-
this.bidiFirefox = new BidiFirefox(this);
68+
this._bidiChromium = new BidiChromium(this);
69+
this._bidiFirefox = new BidiFirefox(this);
7070
this.firefox = new Firefox(this);
7171
this.webkit = new WebKit(this);
7272
this.electron = new Electron(this);

‎packages/playwright-core/src/server/registry/index.ts‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,9 @@ const DOWNLOAD_PATHS: Record<BrowserName | InternalTool, DownloadPaths> = {
384384
'win64': 'builds/android/%s/android.zip',
385385
},
386386
// TODO(bidi): implement downloads.
387-
'bidi': {
387+
'_bidiFirefox': {
388+
} as DownloadPaths,
389+
'_bidiChromium': {
388390
} as DownloadPaths,
389391
};
390392

@@ -480,7 +482,7 @@ function readDescriptors(browsersJSON: BrowsersJSON): BrowsersJSONDescriptor[] {
480482
});
481483
}
482484

483-
export type BrowserName = 'chromium' | 'firefox' | 'webkit' | 'bidi';
485+
export type BrowserName = 'chromium' | 'firefox' | 'webkit' | '_bidiFirefox' | '_bidiChromium';
484486
type InternalTool = 'ffmpeg' | 'winldd' | 'firefox-beta' | 'chromium-tip-of-tree' | 'chromium-headless-shell' | 'chromium-tip-of-tree-headless-shell' | 'android';
485487
type BidiChannel = 'moz-firefox' | 'moz-firefox-beta' | 'moz-firefox-nightly' | 'bidi-chrome-canary' | 'bidi-chrome-stable' | 'bidi-chromium';
486488
type ChromiumChannel = 'chrome' | 'chrome-beta' | 'chrome-dev' | 'chrome-canary' | 'msedge' | 'msedge-beta' | 'msedge-dev' | 'msedge-canary';
@@ -717,8 +719,8 @@ export class Registry {
717719
}));
718720
this._executables.push({
719721
type: 'browser',
720-
name: 'bidi-chromium',
721-
browserName: 'bidi',
722+
name: '_bidiChromium',
723+
browserName: '_bidiChromium',
722724
directory: chromium.dir,
723725
executablePath: () => chromiumExecutable,
724726
executablePathOrDie: (sdkLanguage: string) => executablePathOrDie('chromium', chromiumExecutable, chromium.installByDefault, sdkLanguage),
@@ -842,21 +844,6 @@ export class Registry {
842844
_dependencyGroup: 'tools',
843845
_isHermeticInstallation: true,
844846
});
845-
846-
this._executables.push({
847-
type: 'browser',
848-
name: 'bidi',
849-
browserName: 'bidi',
850-
directory: undefined,
851-
executablePath: () => undefined,
852-
executablePathOrDie: () => '',
853-
installType: 'none',
854-
_validateHostRequirements: () => Promise.resolve(),
855-
downloadURLs: [],
856-
_install: () => Promise.resolve(),
857-
_dependencyGroup: 'tools',
858-
_isHermeticInstallation: true,
859-
});
860847
}
861848

862849
private _createChromiumChannel(name: ChromiumChannel, lookAt: Record<'linux' | 'darwin' | 'win32', string>, install?: () => Promise<void>): ExecutableImpl {
@@ -931,7 +918,7 @@ export class Registry {
931918
return {
932919
type: 'channel',
933920
name,
934-
browserName: 'bidi',
921+
browserName: '_bidiFirefox',
935922
directory: undefined,
936923
executablePath: (sdkLanguage: string) => executablePath(sdkLanguage, false),
937924
executablePathOrDie: (sdkLanguage: string) => executablePath(sdkLanguage, true)!,
@@ -947,7 +934,7 @@ export class Registry {
947934
const suffix = lookAt[process.platform as 'linux' | 'darwin' | 'win32'];
948935
if (!suffix) {
949936
if (shouldThrow)
950-
throw new Error(`Firefox distribution '${name}' is not supported on ${process.platform}`);
937+
throw new Error(`Chromium distribution '${name}' is not supported on ${process.platform}`);
951938
return undefined;
952939
}
953940
const prefixes = (process.platform === 'win32' ? [
@@ -974,7 +961,7 @@ export class Registry {
974961
return {
975962
type: 'channel',
976963
name,
977-
browserName: 'bidi',
964+
browserName: '_bidiChromium',
978965
directory: undefined,
979966
executablePath: (sdkLanguage: string) => executablePath(sdkLanguage, false),
980967
executablePathOrDie: (sdkLanguage: string) => executablePath(sdkLanguage, true)!,

0 commit comments

Comments
 (0)