From f0e13164d72494e161a37d2f6d6b05fddab5f4b1 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 5 Sep 2024 18:31:56 -0700 Subject: [PATCH] chore: split firefox and chromium bidi implementations (#32478) --- .../playwright-core/src/client/playwright.ts | 9 +- .../playwright-core/src/protocol/validator.ts | 3 +- .../playwright-core/src/server/bidi/DEPS.list | 2 +- .../{bidiBrowserType.ts => bidiChromium.ts} | 101 +++++------------- .../src/server/bidi/bidiFirefox.ts | 101 ++++++++++++++++++ .../dispatchers/playwrightDispatcher.ts | 3 +- .../playwright-core/src/server/playwright.ts | 9 +- packages/playwright-core/types/types.d.ts | 3 +- packages/playwright/src/index.ts | 6 +- packages/protocol/src/channels.ts | 3 +- packages/protocol/src/protocol.yml | 3 +- tests/bidi/playwright.config.ts | 60 ++++++----- tests/library/channels.spec.ts | 11 ++ utils/generate_types/overrides.d.ts | 3 +- 14 files changed, 201 insertions(+), 116 deletions(-) rename packages/playwright-core/src/server/bidi/{bidiBrowserType.ts => bidiChromium.ts} (61%) create mode 100644 packages/playwright-core/src/server/bidi/bidiFirefox.ts diff --git a/packages/playwright-core/src/client/playwright.ts b/packages/playwright-core/src/client/playwright.ts index 9415125be4..9933ce15de 100644 --- a/packages/playwright-core/src/client/playwright.ts +++ b/packages/playwright-core/src/client/playwright.ts @@ -26,7 +26,8 @@ import { Selectors, SelectorsOwner } from './selectors'; export class Playwright extends ChannelOwner { readonly _android: Android; readonly _electron: Electron; - readonly _experimentalBidi: BrowserType; + readonly _bidiChromium: BrowserType; + readonly _bidiFirefox: BrowserType; readonly chromium: BrowserType; readonly firefox: BrowserType; readonly webkit: BrowserType; @@ -46,8 +47,10 @@ export class Playwright extends ChannelOwner { this.webkit._playwright = this; this._android = Android.from(initializer.android); this._electron = Electron.from(initializer.electron); - this._experimentalBidi = BrowserType.from(initializer.bidi); - this._experimentalBidi._playwright = this; + this._bidiChromium = BrowserType.from(initializer.bidiChromium); + this._bidiChromium._playwright = this; + this._bidiFirefox = BrowserType.from(initializer.bidiFirefox); + this._bidiFirefox._playwright = this; this.devices = this._connection.localUtils()?.devices ?? {}; this.selectors = new Selectors(); this.errors = { TimeoutError }; diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 0ec06e23b3..b55303810e 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -321,9 +321,10 @@ scheme.RootInitializeResult = tObject({ }); scheme.PlaywrightInitializer = tObject({ chromium: tChannel(['BrowserType']), - bidi: tChannel(['BrowserType']), firefox: tChannel(['BrowserType']), webkit: tChannel(['BrowserType']), + bidiChromium: tChannel(['BrowserType']), + bidiFirefox: tChannel(['BrowserType']), android: tChannel(['Android']), electron: tChannel(['Electron']), utils: tOptional(tChannel(['LocalUtils'])), diff --git a/packages/playwright-core/src/server/bidi/DEPS.list b/packages/playwright-core/src/server/bidi/DEPS.list index a97787788d..9be31302c5 100644 --- a/packages/playwright-core/src/server/bidi/DEPS.list +++ b/packages/playwright-core/src/server/bidi/DEPS.list @@ -7,5 +7,5 @@ [bidiOverCdp.ts] *** -[bidiBrowserType.ts] +[bidiChromium.ts] ../chromium/chromiumSwitches.ts diff --git a/packages/playwright-core/src/server/bidi/bidiBrowserType.ts b/packages/playwright-core/src/server/bidi/bidiChromium.ts similarity index 61% rename from packages/playwright-core/src/server/bidi/bidiBrowserType.ts rename to packages/playwright-core/src/server/bidi/bidiChromium.ts index 841beb71ad..e94dabf072 100644 --- a/packages/playwright-core/src/server/bidi/bidiBrowserType.ts +++ b/packages/playwright-core/src/server/bidi/bidiChromium.ts @@ -15,58 +15,55 @@ */ import os from 'os'; -import path from 'path'; import { assert, wrapInASCIIBox } from '../../utils'; import type { Env } from '../../utils/processLauncher'; import type { BrowserOptions } from '../browser'; -import { BrowserReadyState } from '../browserType'; -import { BrowserType, kNoXServerRunningError } from '../browserType'; +import { BrowserReadyState, BrowserType, kNoXServerRunningError } from '../browserType'; +import { chromiumSwitches } from '../chromium/chromiumSwitches'; import type { SdkObject } from '../instrumentation'; import type { ProtocolError } from '../protocolError'; import type { ConnectionTransport } from '../transport'; import type * as types from '../types'; import { BidiBrowser } from './bidiBrowser'; import { kBrowserCloseMessageId } from './bidiConnection'; -import { chromiumSwitches } from '../chromium/chromiumSwitches'; -export class BidiBrowserType extends BrowserType { +export class BidiChromium extends BrowserType { constructor(parent: SdkObject) { super(parent, 'bidi'); this._useBidi = true; } override async connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise { - if (options.channel?.includes('chrome')) { - // Chrome doesn't support Bidi, we create Bidi over CDP which is used by Chrome driver. - // bidiOverCdp depends on chromium-bidi which we only have in devDependencies, so - // we load bidiOverCdp dynamically. - const bidiTransport = await require('./bidiOverCdp').connectBidiOverCdp(transport); - (transport as any)[kBidiOverCdpWrapper] = bidiTransport; - transport = bidiTransport; - } - return BidiBrowser.connect(this.attribution.playwright, transport, options); + // Chrome doesn't support Bidi, we create Bidi over CDP which is used by Chrome driver. + // bidiOverCdp depends on chromium-bidi which we only have in devDependencies, so + // we load bidiOverCdp dynamically. + const bidiTransport = await require('./bidiOverCdp').connectBidiOverCdp(transport); + (transport as any)[kBidiOverCdpWrapper] = bidiTransport; + return BidiBrowser.connect(this.attribution.playwright, bidiTransport, options); } override doRewriteStartupLog(error: ProtocolError): ProtocolError { if (!error.logs) return error; - // https://github.com/microsoft/playwright/issues/6500 - if (error.logs.includes(`as root in a regular user's session is not supported.`)) - error.logs = '\n' + wrapInASCIIBox(`Firefox is unable to launch if the $HOME folder isn't owned by the current user.\nWorkaround: Set the HOME=/root environment variable${process.env.GITHUB_ACTION ? ' in your GitHub Actions workflow file' : ''} when running Playwright.`, 1); - if (error.logs.includes('no DISPLAY environment variable specified')) + if (error.logs.includes('Missing X server')) error.logs = '\n' + wrapInASCIIBox(kNoXServerRunningError, 1); + // These error messages are taken from Chromium source code as of July, 2020: + // https://github.com/chromium/chromium/blob/70565f67e79f79e17663ad1337dc6e63ee207ce9/content/browser/zygote_host/zygote_host_impl_linux.cc + if (!error.logs.includes('crbug.com/357670') && !error.logs.includes('No usable sandbox!') && !error.logs.includes('crbug.com/638180')) + return error; + error.logs = [ + `Chromium sandboxing failed!`, + `================================`, + `To avoid the sandboxing issue, do either of the following:`, + ` - (preferred): Configure your environment to support sandboxing`, + ` - (alternative): Launch Chromium without sandbox using 'chromiumSandbox: false' option`, + `================================`, + ``, + ].join('\n'); return error; } override amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env { - if (!path.isAbsolute(os.homedir())) - throw new Error(`Cannot launch Firefox with relative home directory. Did you set ${os.platform() === 'win32' ? 'USERPROFILE' : 'HOME'} to a relative path?`); - if (os.platform() === 'linux') { - // Always remove SNAP_NAME and SNAP_INSTANCE_NAME env variables since they - // confuse Firefox: in our case, builds never come from SNAP. - // See https://github.com/microsoft/playwright/issues/20555 - return { ...env, SNAP_NAME: undefined, SNAP_INSTANCE_NAME: undefined }; - } return env; } @@ -78,44 +75,6 @@ export class BidiBrowserType extends BrowserType { } override defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { - if (options.channel === 'bidi-firefox-stable') - return this._defaultFirefoxArgs(options, isPersistent, userDataDir); - else if (options.channel === 'bidi-chrome-canary') - return this._defaultChromiumArgs(options, isPersistent, userDataDir); - throw new Error(`Unknown Bidi channel "${options.channel}"`); - } - - override readyState(options: types.LaunchOptions): BrowserReadyState | undefined { - assert(options.useWebSocket); - if (options.channel?.includes('firefox')) - return new FirefoxReadyState(); - if (options.channel?.includes('chrome')) - return new ChromiumReadyState(); - return undefined; - } - - private _defaultFirefoxArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { - const { args = [], headless } = options; - const userDataDirArg = args.find(arg => arg.startsWith('-profile') || arg.startsWith('--profile')); - if (userDataDirArg) - throw this._createUserDataDirArgMisuseError('--profile'); - const firefoxArguments = ['--remote-debugging-port=0']; - if (headless) - firefoxArguments.push('--headless'); - else - firefoxArguments.push('--foreground'); - firefoxArguments.push(`--profile`, userDataDir); - firefoxArguments.push(...args); - // TODO: make ephemeral context work without this argument. - firefoxArguments.push('about:blank'); - // if (isPersistent) - // firefoxArguments.push('about:blank'); - // else - // firefoxArguments.push('-silent'); - return firefoxArguments; - } - - private _defaultChromiumArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { const chromeArguments = this._innerDefaultArgs(options); chromeArguments.push(`--user-data-dir=${userDataDir}`); chromeArguments.push('--remote-debugging-port=0'); @@ -126,6 +85,11 @@ export class BidiBrowserType extends BrowserType { return chromeArguments; } + override readyState(options: types.LaunchOptions): BrowserReadyState | undefined { + assert(options.useWebSocket); + return new ChromiumReadyState(); + } + private _innerDefaultArgs(options: types.LaunchOptions): string[] { const { args = [], proxy } = options; const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir')); @@ -186,15 +150,6 @@ export class BidiBrowserType extends BrowserType { } } -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'); - } -} - class ChromiumReadyState extends BrowserReadyState { override onBrowserOutput(message: string): void { const match = message.match(/DevTools listening on (.*)/); diff --git a/packages/playwright-core/src/server/bidi/bidiFirefox.ts b/packages/playwright-core/src/server/bidi/bidiFirefox.ts new file mode 100644 index 0000000000..3a2da48c25 --- /dev/null +++ b/packages/playwright-core/src/server/bidi/bidiFirefox.ts @@ -0,0 +1,101 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the 'License'); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an 'AS IS' BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import os from 'os'; +import path from 'path'; +import { assert, wrapInASCIIBox } from '../../utils'; +import type { Env } from '../../utils/processLauncher'; +import type { BrowserOptions } from '../browser'; +import { BrowserReadyState, BrowserType, kNoXServerRunningError } from '../browserType'; +import type { SdkObject } from '../instrumentation'; +import type { ProtocolError } from '../protocolError'; +import type { ConnectionTransport } from '../transport'; +import type * as types from '../types'; +import { BidiBrowser } from './bidiBrowser'; +import { kBrowserCloseMessageId } from './bidiConnection'; + +export class BidiFirefox extends BrowserType { + constructor(parent: SdkObject) { + super(parent, 'bidi'); + this._useBidi = true; + } + + override async connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise { + return BidiBrowser.connect(this.attribution.playwright, transport, options); + } + + override doRewriteStartupLog(error: ProtocolError): ProtocolError { + if (!error.logs) + return error; + // https://github.com/microsoft/playwright/issues/6500 + if (error.logs.includes(`as root in a regular user's session is not supported.`)) + error.logs = '\n' + wrapInASCIIBox(`Firefox is unable to launch if the $HOME folder isn't owned by the current user.\nWorkaround: Set the HOME=/root environment variable${process.env.GITHUB_ACTION ? ' in your GitHub Actions workflow file' : ''} when running Playwright.`, 1); + if (error.logs.includes('no DISPLAY environment variable specified')) + error.logs = '\n' + wrapInASCIIBox(kNoXServerRunningError, 1); + return error; + } + + override amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env { + if (!path.isAbsolute(os.homedir())) + throw new Error(`Cannot launch Firefox with relative home directory. Did you set ${os.platform() === 'win32' ? 'USERPROFILE' : 'HOME'} to a relative path?`); + if (os.platform() === 'linux') { + // Always remove SNAP_NAME and SNAP_INSTANCE_NAME env variables since they + // confuse Firefox: in our case, builds never come from SNAP. + // See https://github.com/microsoft/playwright/issues/20555 + return { ...env, SNAP_NAME: undefined, SNAP_INSTANCE_NAME: undefined }; + } + return env; + } + + override attemptToGracefullyCloseBrowser(transport: ConnectionTransport): void { + transport.send({ method: 'browser.close', params: {}, id: kBrowserCloseMessageId }); + } + + override defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { + const { args = [], headless } = options; + const userDataDirArg = args.find(arg => arg.startsWith('-profile') || arg.startsWith('--profile')); + if (userDataDirArg) + throw this._createUserDataDirArgMisuseError('--profile'); + const firefoxArguments = ['--remote-debugging-port=0']; + if (headless) + firefoxArguments.push('--headless'); + else + firefoxArguments.push('--foreground'); + firefoxArguments.push(`--profile`, userDataDir); + firefoxArguments.push(...args); + // TODO: make ephemeral context work without this argument. + firefoxArguments.push('about:blank'); + // if (isPersistent) + // firefoxArguments.push('about:blank'); + // else + // firefoxArguments.push('-silent'); + return firefoxArguments; + } + + override readyState(options: types.LaunchOptions): BrowserReadyState | undefined { + assert(options.useWebSocket); + 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'); + } +} diff --git a/packages/playwright-core/src/server/dispatchers/playwrightDispatcher.ts b/packages/playwright-core/src/server/dispatchers/playwrightDispatcher.ts index cc0a259d75..c411f93198 100644 --- a/packages/playwright-core/src/server/dispatchers/playwrightDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/playwrightDispatcher.ts @@ -44,9 +44,10 @@ export class PlaywrightDispatcher extends Dispatcher(); @@ -64,7 +66,8 @@ export class Playwright extends SdkObject { } }, null); this.chromium = new Chromium(this); - this.bidi = new BidiBrowserType(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); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 8970d336cc..765c84c36e 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -15135,7 +15135,8 @@ export type AndroidKey = export const _electron: Electron; export const _android: Android; -export const _experimentalBidi: BrowserType; +export const _bidiChromium: BrowserType; +export const _bidiFirefox: BrowserType; // This is required to not export everything by default. See https://github.com/Microsoft/TypeScript/issues/19545#issuecomment-340490459 export {}; diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 8273c5ef76..add1661502 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -83,15 +83,15 @@ const playwrightFixtures: Fixtures = ({ options.channel = channel; options.tracesDir = tracing().tracesDir(); - for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit, playwright._experimentalBidi]) + for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit, playwright._bidiChromium, playwright._bidiFirefox]) (browserType as any)._defaultLaunchOptions = options; await use(options); - for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit, playwright._experimentalBidi]) + for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit, playwright._bidiChromium, playwright._bidiFirefox]) (browserType as any)._defaultLaunchOptions = undefined; }, { scope: 'worker', auto: true, box: true }], browser: [async ({ playwright, browserName, _browserOptions, connectOptions, _reuseContext }, use, testInfo) => { - if (!['chromium', 'firefox', 'webkit', '_experimentalBidi'].includes(browserName)) + if (!['chromium', 'firefox', 'webkit', '_bidiChromium', '_bidiFirefox'].includes(browserName)) throw new Error(`Unexpected browserName "${browserName}", must be one of "chromium", "firefox" or "webkit"`); if (connectOptions) { diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index 0aead445ab..c2896d169e 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -560,9 +560,10 @@ export interface RootEvents { // ----------- Playwright ----------- export type PlaywrightInitializer = { chromium: BrowserTypeChannel, - bidi: BrowserTypeChannel, firefox: BrowserTypeChannel, webkit: BrowserTypeChannel, + bidiChromium: BrowserTypeChannel, + bidiFirefox: BrowserTypeChannel, android: AndroidChannel, electron: ElectronChannel, utils?: LocalUtilsChannel, diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index 71fd44255d..212fd7d1d9 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -668,9 +668,10 @@ Playwright: initializer: chromium: BrowserType - bidi: BrowserType firefox: BrowserType webkit: BrowserType + bidiChromium: BrowserType + bidiFirefox: BrowserType android: Android electron: Electron utils: LocalUtils? diff --git a/tests/bidi/playwright.config.ts b/tests/bidi/playwright.config.ts index 3bde84756a..7aa17f35ba 100644 --- a/tests/bidi/playwright.config.ts +++ b/tests/bidi/playwright.config.ts @@ -58,38 +58,44 @@ const config: Config { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [] }, + { _guid: 'browser-type', objects: [] }, { _guid: 'electron', objects: [] }, { _guid: 'localUtils', objects: [] }, { _guid: 'Playwright', objects: [] }, @@ -169,6 +174,7 @@ it('should scope browser handles', async ({ browserType, expectScopeState }) => { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [] }, + { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [ { _guid: 'browser', objects: [ @@ -206,6 +212,7 @@ it('should not generate dispatchers for subresources w/o listeners', async ({ pa { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [] }, + { _guid: 'browser-type', objects: [] }, { _guid: 'browser-type', objects: [ { _guid: 'browser', objects: [ @@ -289,6 +296,10 @@ it('exposeFunction should not leak', async ({ page, expectScopeState, server }) '_guid': 'browser-type', 'objects': [], }, + { + '_guid': 'browser-type', + 'objects': [], + }, { '_guid': 'browser-type', 'objects': [ diff --git a/utils/generate_types/overrides.d.ts b/utils/generate_types/overrides.d.ts index f2cb83997a..be8ca9d71a 100644 --- a/utils/generate_types/overrides.d.ts +++ b/utils/generate_types/overrides.d.ts @@ -377,7 +377,8 @@ export type AndroidKey = export const _electron: Electron; export const _android: Android; -export const _experimentalBidi: BrowserType; +export const _bidiChromium: BrowserType; +export const _bidiFirefox: BrowserType; // This is required to not export everything by default. See https://github.com/Microsoft/TypeScript/issues/19545#issuecomment-340490459 export {};