Skip to content

chore: simplify bidi browsers handling #36363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/playwright-core/src/browserServerImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import type { WebSocketEventEmitter } from './utilsBundle';
import type { Browser } from './server/browser';

export class BrowserServerLauncherImpl implements BrowserServerLauncher {
private _browserName: 'chromium' | 'firefox' | 'webkit' | 'bidiFirefox' | 'bidiChromium';
private _browserName: 'chromium' | 'firefox' | 'webkit' | '_bidiFirefox' | '_bidiChromium';

constructor(browserName: 'chromium' | 'firefox' | 'webkit' | 'bidiFirefox' | 'bidiChromium') {
constructor(browserName: 'chromium' | 'firefox' | 'webkit' | '_bidiFirefox' | '_bidiChromium') {
this._browserName = browserName;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/client/playwright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ export class Playwright extends ChannelOwner<channels.PlaywrightChannel> {
this._android._playwright = this;
this._electron = Electron.from(initializer.electron);
this._electron._playwright = this;
this._bidiChromium = BrowserType.from(initializer.bidiChromium);
this._bidiChromium = BrowserType.from(initializer._bidiChromium);
this._bidiChromium._playwright = this;
this._bidiFirefox = BrowserType.from(initializer.bidiFirefox);
this._bidiFirefox = BrowserType.from(initializer._bidiFirefox);
this._bidiFirefox._playwright = this;
this.devices = this._connection.localUtils()?.devices ?? {};
this.selectors = new Selectors(this._connection._platform);
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/inProcessFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export function createInProcessPlaywright(): PlaywrightAPI {
playwrightAPI.firefox._serverLauncher = new BrowserServerLauncherImpl('firefox');
playwrightAPI.webkit._serverLauncher = new BrowserServerLauncherImpl('webkit');
playwrightAPI._android._serverLauncher = new AndroidServerLauncherImpl();
playwrightAPI._bidiChromium._serverLauncher = new BrowserServerLauncherImpl('bidiChromium');
playwrightAPI._bidiFirefox._serverLauncher = new BrowserServerLauncherImpl('bidiFirefox');
playwrightAPI._bidiChromium._serverLauncher = new BrowserServerLauncherImpl('_bidiChromium');
playwrightAPI._bidiFirefox._serverLauncher = new BrowserServerLauncherImpl('_bidiFirefox');

// Switch to async dispatch after we got Playwright object.
dispatcherConnection.onmessage = message => setImmediate(() => clientConnection.dispatch(message));
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ scheme.PlaywrightInitializer = tObject({
chromium: tChannel(['BrowserType']),
firefox: tChannel(['BrowserType']),
webkit: tChannel(['BrowserType']),
bidiChromium: tChannel(['BrowserType']),
bidiFirefox: tChannel(['BrowserType']),
_bidiChromium: tChannel(['BrowserType']),
_bidiFirefox: tChannel(['BrowserType']),
android: tChannel(['Android']),
electron: tChannel(['Electron']),
utils: tOptional(tChannel(['LocalUtils'])),
Expand Down
9 changes: 1 addition & 8 deletions packages/playwright-core/src/remote/playwrightConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,7 @@ export class PlaywrightConnection {
private async _initLaunchBrowserMode(scope: RootDispatcher, options: channels.RootInitializeParams) {
debugLogger.log('server', `[${this._id}] engaged launch mode for "${this._options.browserName}"`);
const ownedSocksProxy = await this._createOwnedSocksProxy();
let browserName = this._options.browserName;
if ('bidi' === browserName) {
if (this._options.launchOptions?.channel?.toLocaleLowerCase().includes('firefox'))
browserName = 'bidiFirefox';
else
browserName = 'bidiChromium';
}
const browser = await this._playwright[browserName as 'chromium'].launch(serverSideCallMetadata(), this._options.launchOptions);
const browser = await this._playwright[this._options.browserName as 'chromium'].launch(serverSideCallMetadata(), this._options.launchOptions);
browser.options.sdkLanguage = options.sdkLanguage;

this._cleanups.push(() => browser.close({ reason: 'Connection terminated' }));
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/bidi/bidiChromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import type * as types from '../types';

export class BidiChromium extends BrowserType {
constructor(parent: SdkObject) {
super(parent, 'bidi');
super(parent, '_bidiChromium');
}

override async connectToTransport(transport: ConnectionTransport, options: BrowserOptions, browserLogsCollector: RecentLogsCollector): Promise<BidiBrowser> {
Expand Down
6 changes: 5 additions & 1 deletion packages/playwright-core/src/server/bidi/bidiFirefox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import type { RecentLogsCollector } from '../utils/debugLogger';

export class BidiFirefox extends BrowserType {
constructor(parent: SdkObject) {
super(parent, 'bidi');
super(parent, '_bidiFirefox');
}

override executablePath(): string {
return '';
}

override async connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<BidiBrowser> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ export class PlaywrightDispatcher extends Dispatcher<Playwright, channels.Playwr
const chromium = new BrowserTypeDispatcher(scope, playwright.chromium, denyLaunch);
const firefox = new BrowserTypeDispatcher(scope, playwright.firefox, denyLaunch);
const webkit = new BrowserTypeDispatcher(scope, playwright.webkit, denyLaunch);
const bidiChromium = new BrowserTypeDispatcher(scope, playwright.bidiChromium, denyLaunch);
const bidiFirefox = new BrowserTypeDispatcher(scope, playwright.bidiFirefox, denyLaunch);
const _bidiChromium = new BrowserTypeDispatcher(scope, playwright._bidiChromium, denyLaunch);
const _bidiFirefox = new BrowserTypeDispatcher(scope, playwright._bidiFirefox, denyLaunch);
const android = new AndroidDispatcher(scope, playwright.android, denyLaunch);
const initializer: channels.PlaywrightInitializer = {
chromium,
firefox,
webkit,
bidiChromium,
bidiFirefox,
_bidiChromium,
_bidiFirefox,
android,
electron: new ElectronDispatcher(scope, playwright.electron, denyLaunch),
utils: playwright.options.isServer ? undefined : new LocalUtilsDispatcher(scope, playwright),
Expand All @@ -69,14 +69,7 @@ export class PlaywrightDispatcher extends Dispatcher<Playwright, channels.Playwr

let browserDispatcher: BrowserDispatcher | undefined;
if (options.preLaunchedBrowser) {
let browserTypeDispatcher: BrowserTypeDispatcher;
switch (options.preLaunchedBrowser.options.name) {
case 'chromium': browserTypeDispatcher = chromium; break;
case 'firefox': browserTypeDispatcher = firefox; break;
case 'webkit': browserTypeDispatcher = webkit; break;
case 'bidi': browserTypeDispatcher = options.preLaunchedBrowser.options.channel?.includes('firefox') ? bidiFirefox : bidiChromium; break;
default: throw new Error(`Unknown browser name: ${options.preLaunchedBrowser.options.name}`);
}
const browserTypeDispatcher = initializer[options.preLaunchedBrowser.options.name as keyof typeof initializer] as BrowserTypeDispatcher;
browserDispatcher = new BrowserDispatcher(browserTypeDispatcher, options.preLaunchedBrowser, {
ignoreStopAndKill: true,
isolateContexts: !options.sharedBrowser,
Expand Down
8 changes: 4 additions & 4 deletions packages/playwright-core/src/server/playwright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export class Playwright extends SdkObject {
readonly electron: Electron;
readonly firefox: BrowserType;
readonly webkit: BrowserType;
readonly bidiChromium: BrowserType;
readonly bidiFirefox: BrowserType;
readonly _bidiChromium: BrowserType;
readonly _bidiFirefox: BrowserType;
readonly options: PlaywrightOptions;
readonly debugController: DebugController;
private _allPages = new Set<Page>();
Expand All @@ -65,8 +65,8 @@ export class Playwright extends SdkObject {
}
}, null);
this.chromium = new Chromium(this);
this.bidiChromium = new BidiChromium(this);
this.bidiFirefox = new BidiFirefox(this);
this._bidiChromium = new BidiChromium(this);
this._bidiFirefox = new BidiFirefox(this);
this.firefox = new Firefox(this);
this.webkit = new WebKit(this);
this.electron = new Electron(this);
Expand Down
31 changes: 9 additions & 22 deletions packages/playwright-core/src/server/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,9 @@ const DOWNLOAD_PATHS: Record<BrowserName | InternalTool, DownloadPaths> = {
'win64': 'builds/android/%s/android.zip',
},
// TODO(bidi): implement downloads.
'bidi': {
'_bidiFirefox': {
} as DownloadPaths,
'_bidiChromium': {
} as DownloadPaths,
};

Expand Down Expand Up @@ -480,7 +482,7 @@ function readDescriptors(browsersJSON: BrowsersJSON): BrowsersJSONDescriptor[] {
});
}

export type BrowserName = 'chromium' | 'firefox' | 'webkit' | 'bidi';
export type BrowserName = 'chromium' | 'firefox' | 'webkit' | '_bidiFirefox' | '_bidiChromium';
type InternalTool = 'ffmpeg' | 'winldd' | 'firefox-beta' | 'chromium-tip-of-tree' | 'chromium-headless-shell' | 'chromium-tip-of-tree-headless-shell' | 'android';
type BidiChannel = 'moz-firefox' | 'moz-firefox-beta' | 'moz-firefox-nightly' | 'bidi-chrome-canary' | 'bidi-chrome-stable' | 'bidi-chromium';
type ChromiumChannel = 'chrome' | 'chrome-beta' | 'chrome-dev' | 'chrome-canary' | 'msedge' | 'msedge-beta' | 'msedge-dev' | 'msedge-canary';
Expand Down Expand Up @@ -717,8 +719,8 @@ export class Registry {
}));
this._executables.push({
type: 'browser',
name: 'bidi-chromium',
browserName: 'bidi',
name: '_bidiChromium',
browserName: '_bidiChromium',
directory: chromium.dir,
executablePath: () => chromiumExecutable,
executablePathOrDie: (sdkLanguage: string) => executablePathOrDie('chromium', chromiumExecutable, chromium.installByDefault, sdkLanguage),
Expand Down Expand Up @@ -842,21 +844,6 @@ export class Registry {
_dependencyGroup: 'tools',
_isHermeticInstallation: true,
});

this._executables.push({
type: 'browser',
name: 'bidi',
browserName: 'bidi',
directory: undefined,
executablePath: () => undefined,
executablePathOrDie: () => '',
installType: 'none',
_validateHostRequirements: () => Promise.resolve(),
downloadURLs: [],
_install: () => Promise.resolve(),
_dependencyGroup: 'tools',
_isHermeticInstallation: true,
});
}

private _createChromiumChannel(name: ChromiumChannel, lookAt: Record<'linux' | 'darwin' | 'win32', string>, install?: () => Promise<void>): ExecutableImpl {
Expand Down Expand Up @@ -931,7 +918,7 @@ export class Registry {
return {
type: 'channel',
name,
browserName: 'bidi',
browserName: '_bidiFirefox',
directory: undefined,
executablePath: (sdkLanguage: string) => executablePath(sdkLanguage, false),
executablePathOrDie: (sdkLanguage: string) => executablePath(sdkLanguage, true)!,
Expand All @@ -947,7 +934,7 @@ export class Registry {
const suffix = lookAt[process.platform as 'linux' | 'darwin' | 'win32'];
if (!suffix) {
if (shouldThrow)
throw new Error(`Firefox distribution '${name}' is not supported on ${process.platform}`);
throw new Error(`Chromium distribution '${name}' is not supported on ${process.platform}`);
return undefined;
}
const prefixes = (process.platform === 'win32' ? [
Expand All @@ -974,7 +961,7 @@ export class Registry {
return {
type: 'channel',
name,
browserName: 'bidi',
browserName: '_bidiChromium',
directory: undefined,
executablePath: (sdkLanguage: string) => executablePath(sdkLanguage, false),
executablePathOrDie: (sdkLanguage: string) => executablePath(sdkLanguage, true)!,
Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/src/channels.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,8 @@ export type PlaywrightInitializer = {
chromium: BrowserTypeChannel,
firefox: BrowserTypeChannel,
webkit: BrowserTypeChannel,
bidiChromium: BrowserTypeChannel,
bidiFirefox: BrowserTypeChannel,
_bidiChromium: BrowserTypeChannel,
_bidiFirefox: BrowserTypeChannel,
android: AndroidChannel,
electron: ElectronChannel,
utils?: LocalUtilsChannel,
Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,8 @@ Playwright:
chromium: BrowserType
firefox: BrowserType
webkit: BrowserType
bidiChromium: BrowserType
bidiFirefox: BrowserType
_bidiChromium: BrowserType
_bidiFirefox: BrowserType
android: Android
electron: Electron
utils: LocalUtils?
Expand Down
2 changes: 1 addition & 1 deletion tests/bidi/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ for (const [key, channels] of Object.entries(browserToChannels)) {
use: {
browserName,
headless: !headed,
channel,
channel: channel === 'bidi-chromium' ? undefined : channel,
video: 'off',
launchOptions: {
executablePath,
Expand Down
6 changes: 0 additions & 6 deletions tests/config/remoteServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,6 @@ export class RemoteServer implements PlaywrightServer {
launchOptions,
...remoteServerOptions,
};
if ('bidi' === browserType.name()) {
if (channel.toLocaleLowerCase().includes('firefox'))
options.browserTypeName = '_bidiFirefox';
else
options.browserTypeName = '_bidiChromium';
}
this._process = childProcess({
command: ['node', path.join(__dirname, 'remote-server-impl.js'), JSON.stringify(options)],
});
Expand Down
4 changes: 1 addition & 3 deletions tests/library/browsertype-connect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,7 @@ for (const kind of ['launchServer', 'run-server'] as const) {
}).catch(() => {})
]);
expect(request.headers['user-agent']).toBe(getUserAgent());
// _bidiFirefox and _bidiChromium are initialized with 'bidi' as browser name.
const bidiAwareBrowserName = browserName.startsWith('_bidi') ? 'bidi' : browserName;
expect(request.headers['x-playwright-browser']).toBe(bidiAwareBrowserName);
expect(request.headers['x-playwright-browser']).toBe(browserName);
expect(request.headers['foo']).toBe('bar');
});

Expand Down
8 changes: 2 additions & 6 deletions tests/library/har.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ it('should have browser', async ({ browserName, browser, contextFactory, server
await page.goto(server.EMPTY_PAGE);
const log = await getLog();

// _bidiFirefox and _bidiChromium are initialized with 'bidi' as browser name.
const harBrowserName = browserName.startsWith('_bidi') ? 'bidi' : browserName;
expect(log.browser!.name.toLowerCase()).toBe(harBrowserName);
expect(log.browser!.name.toLowerCase()).toBe(browserName);
expect(log.browser!.version).toBe(browser.version());
});

Expand Down Expand Up @@ -915,8 +913,6 @@ it('should not hang on slow chunked response', async ({ browserName, browser, co
await page.evaluate(() => (window as any).receivedFirstData);
const log = await getLog();

// _bidiFirefox and _bidiChromium are initialized with 'bidi' as browser name.
const harBrowserName = browserName.startsWith('_bidi') ? 'bidi' : browserName;
expect(log.browser!.name.toLowerCase()).toBe(harBrowserName);
expect(log.browser!.name.toLowerCase()).toBe(browserName);
expect(log.browser!.version).toBe(browser.version());
});
Loading