From 9790ea5b5dc1f534eef7b0befa264026fa6061bf Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 18 Aug 2020 09:37:40 -0700 Subject: [PATCH] chore: align more server-side options with rpc protocol (#3506) This touches: - noDefaultViewport; - ignoreAllDefaultArgs; - env; - validateXYZ logic that was copying objects - we do not need that anymore; - shuffles some converters closer to their usage. --- src/browser.ts | 10 ++--- src/browserContext.ts | 51 +++++++------------------ src/chromium/crBrowser.ts | 9 ++--- src/chromium/crPage.ts | 4 +- src/converters.ts | 16 -------- src/firefox/ffBrowser.ts | 9 ++--- src/page.ts | 2 +- src/rpc/browserServerImpl.ts | 8 +++- src/rpc/client/browserType.ts | 3 +- src/rpc/client/clientHelper.ts | 10 +++++ src/rpc/client/electron.ts | 2 +- src/rpc/server/browserDispatcher.ts | 1 - src/rpc/server/browserTypeDispatcher.ts | 12 +----- src/rpc/server/electronDispatcher.ts | 7 +--- src/server/browserType.ts | 42 +++++++++----------- src/server/chromium.ts | 4 +- src/server/electron.ts | 9 ++--- src/server/firefox.ts | 5 ++- src/server/processLauncher.ts | 8 ++++ src/server/webkit.ts | 4 +- src/types.ts | 19 +++++---- src/webkit/wkBrowser.ts | 9 ++--- src/webkit/wkPage.ts | 2 +- 23 files changed, 105 insertions(+), 141 deletions(-) diff --git a/src/browser.ts b/src/browser.ts index 1e28152b90..04178c73ef 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -40,12 +40,10 @@ export type BrowserOptions = { proxy?: ProxySettings, }; -export type BrowserContextOptions = types.BrowserContextOptions; - export interface Browser extends EventEmitter { - newContext(options?: BrowserContextOptions): Promise; + newContext(options?: types.BrowserContextOptions): Promise; contexts(): BrowserContext[]; - newPage(options?: BrowserContextOptions): Promise; + newPage(options?: types.BrowserContextOptions): Promise; isConnected(): boolean; close(): Promise; version(): string; @@ -62,12 +60,12 @@ export abstract class BrowserBase extends EventEmitter implements Browser { this._options = options; } - abstract newContext(options?: BrowserContextOptions): Promise; + abstract newContext(options?: types.BrowserContextOptions): Promise; abstract contexts(): BrowserContext[]; abstract isConnected(): boolean; abstract version(): string; - async newPage(options?: BrowserContextOptions): Promise { + async newPage(options?: types.BrowserContextOptions): Promise { const context = await this.newContext(options); const page = await context.newPage(); page._ownedContext = context; diff --git a/src/browserContext.ts b/src/browserContext.ts index 84e516101b..f13bb9f053 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -228,47 +228,23 @@ export function assertBrowserContextIsNotOwned(context: BrowserContextBase) { } } -export function validateBrowserContextOptions(options: types.BrowserContextOptions): types.BrowserContextOptions { - // Copy all fields manually to strip any extra junk. - // Especially useful when we share context and launch options for launchPersistent. - const result: types.BrowserContextOptions = { - ignoreHTTPSErrors: options.ignoreHTTPSErrors, - bypassCSP: options.bypassCSP, - locale: options.locale, - timezoneId: options.timezoneId, - offline: options.offline, - colorScheme: options.colorScheme, - acceptDownloads: options.acceptDownloads, - viewport: options.viewport, - javaScriptEnabled: options.javaScriptEnabled, - userAgent: options.userAgent, - geolocation: options.geolocation, - permissions: options.permissions, - extraHTTPHeaders: options.extraHTTPHeaders, - httpCredentials: options.httpCredentials, - deviceScaleFactor: options.deviceScaleFactor, - isMobile: options.isMobile, - hasTouch: options.hasTouch, - }; - if (result.viewport === null && result.deviceScaleFactor !== undefined) +export function validateBrowserContextOptions(options: types.BrowserContextOptions) { + if (options.noDefaultViewport && options.deviceScaleFactor !== undefined) throw new Error(`"deviceScaleFactor" option is not supported with null "viewport"`); - if (result.viewport === null && result.isMobile !== undefined) + if (options.noDefaultViewport && options.isMobile !== undefined) throw new Error(`"isMobile" option is not supported with null "viewport"`); - if (!result.viewport && result.viewport !== null) - result.viewport = { width: 1280, height: 720 }; - if (result.viewport) - result.viewport = { ...result.viewport }; - if (result.geolocation) - result.geolocation = verifyGeolocation(result.geolocation); - if (result.extraHTTPHeaders) - result.extraHTTPHeaders = network.verifyHeaders(result.extraHTTPHeaders); - return result; + if (!options.viewport && !options.noDefaultViewport) + options.viewport = { width: 1280, height: 720 }; + verifyGeolocation(options.geolocation); + if (options.extraHTTPHeaders) + options.extraHTTPHeaders = network.verifyHeaders(options.extraHTTPHeaders); } -export function verifyGeolocation(geolocation: types.Geolocation): types.Geolocation { - const result = { ...geolocation }; - result.accuracy = result.accuracy || 0; - const { longitude, latitude, accuracy } = result; +export function verifyGeolocation(geolocation?: types.Geolocation) { + if (!geolocation) + return; + geolocation.accuracy = geolocation.accuracy || 0; + const { longitude, latitude, accuracy } = geolocation; if (!helper.isNumber(longitude)) throw new Error(`geolocation.longitude: expected number, got ${typeof longitude}`); if (longitude < -180 || longitude > 180) @@ -281,7 +257,6 @@ export function verifyGeolocation(geolocation: types.Geolocation): types.Geoloca throw new Error(`geolocation.accuracy: expected number, got ${typeof accuracy}`); if (accuracy < 0) throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`); - return result; } export function verifyProxySettings(proxy: types.ProxySettings): types.ProxySettings { diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index a5b6f83070..9ab10ea43f 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { BrowserBase, BrowserOptions, BrowserContextOptions } from '../browser'; +import { BrowserBase, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, BrowserContextBase, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; import { Events as CommonEvents } from '../events'; import { assert } from '../helper'; @@ -99,8 +99,8 @@ export class CRBrowser extends BrowserBase { this._session.on('Target.detachedFromTarget', this._onDetachedFromTarget.bind(this)); } - async newContext(options: BrowserContextOptions = {}): Promise { - options = validateBrowserContextOptions(options); + async newContext(options: types.BrowserContextOptions = {}): Promise { + validateBrowserContextOptions(options); const { browserContextId } = await this._session.send('Target.createBrowserContext', { disposeOnDetach: true }); const context = new CRBrowserContext(this, browserContextId, options); await context._initialize(); @@ -381,8 +381,7 @@ export class CRBrowserContext extends BrowserContextBase { } async setGeolocation(geolocation?: types.Geolocation): Promise { - if (geolocation) - geolocation = verifyGeolocation(geolocation); + verifyGeolocation(geolocation); this._options.geolocation = geolocation; for (const page of this.pages()) await (page._delegate as CRPage).updateGeolocation(); diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 28c447f05a..b5e26e8b6c 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -74,7 +74,7 @@ export class CRPage implements PageDelegate { this._mainFrameSession = new FrameSession(this, client, targetId, null); this._sessions.set(targetId, this._mainFrameSession); client.once(CRSessionEvents.Disconnected, () => this._page._didDisconnect()); - if (opener && browserContext._options.viewport !== null) { + if (opener && !browserContext._options.noDefaultViewport) { const features = opener._nextWindowOpenPopupFeatures.shift() || []; const viewportSize = helper.getViewportSizeFromWindowFeatures(features); if (viewportSize) @@ -371,7 +371,7 @@ class FrameSession { } async _initialize(hasUIWindow: boolean) { - if (hasUIWindow && this._crPage._browserContext._options.viewport !== null) { + if (hasUIWindow && !this._crPage._browserContext._options.noDefaultViewport) { const { windowId } = await this._client.send('Browser.getWindowForTarget'); this._windowId = windowId; } diff --git a/src/converters.ts b/src/converters.ts index 00fdbde9a1..0f54dc3706 100644 --- a/src/converters.ts +++ b/src/converters.ts @@ -105,19 +105,3 @@ export function headersArrayToObject(headers: types.HeadersArray): types.Headers result[name] = value; return result; } - -export function envObjectToArray(env: types.Env): types.EnvArray { - const result: types.EnvArray = []; - for (const name in env) { - if (!Object.is(env[name], undefined)) - result.push({ name, value: String(env[name]) }); - } - return result; -} - -export function envArrayToObject(env: types.EnvArray): types.Env { - const result: types.Env = {}; - for (const { name, value } of env) - result[name] = value; - return result; -} diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 7b094ab88f..e02b9ef8f0 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { BrowserBase, BrowserOptions, BrowserContextOptions } from '../browser'; +import { BrowserBase, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, BrowserContextBase, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; import { Events } from '../events'; import { assert, helper, RegisteredListener } from '../helper'; @@ -91,8 +91,8 @@ export class FFBrowser extends BrowserBase { return !this._connection._closed; } - async newContext(options: BrowserContextOptions = {}): Promise { - options = validateBrowserContextOptions(options); + async newContext(options: types.BrowserContextOptions = {}): Promise { + validateBrowserContextOptions(options); if (options.isMobile) throw new Error('options.isMobile is not supported in Firefox'); const { browserContextId } = await this._connection.send('Browser.createBrowserContext', { removeOnDetach: true }); @@ -289,8 +289,7 @@ export class FFBrowserContext extends BrowserContextBase { } async setGeolocation(geolocation?: types.Geolocation): Promise { - if (geolocation) - geolocation = verifyGeolocation(geolocation); + verifyGeolocation(geolocation); this._options.geolocation = geolocation; await this._browser._connection.send('Browser.setGeolocationOverride', { browserContextId: this._browserContextId || undefined, geolocation: geolocation || null }); } diff --git a/src/page.ts b/src/page.ts index ba98b81ac0..42ec606f02 100644 --- a/src/page.ts +++ b/src/page.ts @@ -127,7 +127,7 @@ export class Page extends EventEmitter { this._crashedPromise = new Promise(f => this._crashedCallback = f); this._browserContext = browserContext; this._state = { - viewportSize: browserContext._options.viewport ? { ...browserContext._options.viewport } : null, + viewportSize: browserContext._options.viewport || null, mediaType: null, colorScheme: null, extraHTTPHeaders: null, diff --git a/src/rpc/browserServerImpl.ts b/src/rpc/browserServerImpl.ts index 5d47758808..5ba7f92aee 100644 --- a/src/rpc/browserServerImpl.ts +++ b/src/rpc/browserServerImpl.ts @@ -27,6 +27,7 @@ import { BrowserDispatcher } from './server/browserDispatcher'; import { BrowserContextDispatcher } from './server/browserContextDispatcher'; import { BrowserNewContextParams, BrowserContextChannel } from './channels'; import { BrowserServerLauncher, BrowserServer } from './client/browserType'; +import { envObjectToArray } from './client/clientHelper'; export class BrowserServerLauncherImpl implements BrowserServerLauncher { private _browserType: BrowserTypeBase; @@ -36,7 +37,12 @@ export class BrowserServerLauncherImpl implements BrowserServerLauncher { } async launchServer(options: LaunchServerOptions = {}): Promise { - const browser = await this._browserType.launch(options); + const browser = await this._browserType.launch({ + ...options, + ignoreDefaultArgs: Array.isArray(options.ignoreDefaultArgs) ? options.ignoreDefaultArgs : undefined, + ignoreAllDefaultArgs: !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs), + env: options.env ? envObjectToArray(options.env) : undefined, + }); return new BrowserServerImpl(this._browserType, browser as BrowserBase, options.port); } } diff --git a/src/rpc/client/browserType.ts b/src/rpc/client/browserType.ts index f3fcb30555..522aa7c6f2 100644 --- a/src/rpc/client/browserType.ts +++ b/src/rpc/client/browserType.ts @@ -18,7 +18,7 @@ import { BrowserTypeChannel, BrowserTypeInitializer, BrowserTypeLaunchParams, Br import { Browser } from './browser'; import { BrowserContext } from './browserContext'; import { ChannelOwner } from './channelOwner'; -import { headersObjectToArray, envObjectToArray } from '../../converters'; +import { headersObjectToArray } from '../../converters'; import { assert, helper } from '../../helper'; import { LaunchOptions, LaunchServerOptions, ConnectOptions, LaunchPersistentContextOptions } from './types'; import * as WebSocket from 'ws'; @@ -27,6 +27,7 @@ import { serializeError } from '../serializers'; import { Events } from './events'; import { TimeoutSettings } from '../../timeoutSettings'; import { ChildProcess } from 'child_process'; +import { envObjectToArray } from './clientHelper'; export interface BrowserServerLauncher { launchServer(options?: LaunchServerOptions): Promise; diff --git a/src/rpc/client/clientHelper.ts b/src/rpc/client/clientHelper.ts index 53809acd01..b5b6692587 100644 --- a/src/rpc/client/clientHelper.ts +++ b/src/rpc/client/clientHelper.ts @@ -16,6 +16,7 @@ */ import { isUnderTest as commonIsUnderTest } from '../../helper'; +import * as types from './types'; const deprecatedHits = new Set(); export function deprecate(methodName: string, message: string) { @@ -28,3 +29,12 @@ export function deprecate(methodName: string, message: string) { export function isUnderTest() { return commonIsUnderTest(); } + +export function envObjectToArray(env: types.Env): { name: string, value: string }[] { + const result: { name: string, value: string }[] = []; + for (const name in env) { + if (!Object.is(env[name], undefined)) + result.push({ name, value: String(env[name]) }); + } + return result; +} diff --git a/src/rpc/client/electron.ts b/src/rpc/client/electron.ts index d63c7d7484..cb351d5a8d 100644 --- a/src/rpc/client/electron.ts +++ b/src/rpc/client/electron.ts @@ -22,8 +22,8 @@ import { serializeArgument, FuncOn, parseResult, SmartHandle, JSHandle } from '. import { TimeoutSettings } from '../../timeoutSettings'; import { Waiter } from './waiter'; import { Events } from './events'; -import { envObjectToArray } from '../../converters'; import { WaitForEventOptions, Env, LoggerSink } from './types'; +import { envObjectToArray } from './clientHelper'; type ElectronOptions = Omit & { env?: Env, diff --git a/src/rpc/server/browserDispatcher.ts b/src/rpc/server/browserDispatcher.ts index 0c7ca8acb6..26411d01e9 100644 --- a/src/rpc/server/browserDispatcher.ts +++ b/src/rpc/server/browserDispatcher.ts @@ -39,7 +39,6 @@ export class BrowserDispatcher extends Dispatcher i async newContext(params: BrowserNewContextParams): Promise<{ context: BrowserContextChannel }> { const options = { ...params, - viewport: params.viewport || (params.noDefaultViewport ? null : undefined), extraHTTPHeaders: params.extraHTTPHeaders ? headersArrayToObject(params.extraHTTPHeaders) : undefined, }; return { context: new BrowserContextDispatcher(this._scope, await this._object.newContext(options) as BrowserContextBase) }; diff --git a/src/rpc/server/browserTypeDispatcher.ts b/src/rpc/server/browserTypeDispatcher.ts index f27e389cc6..fcb0b74ff2 100644 --- a/src/rpc/server/browserTypeDispatcher.ts +++ b/src/rpc/server/browserTypeDispatcher.ts @@ -21,7 +21,7 @@ import { BrowserChannel, BrowserTypeChannel, BrowserContextChannel, BrowserTypeI import { Dispatcher, DispatcherScope } from './dispatcher'; import { BrowserContextBase } from '../../browserContext'; import { BrowserContextDispatcher } from './browserContextDispatcher'; -import { headersArrayToObject, envArrayToObject } from '../../converters'; +import { headersArrayToObject } from '../../converters'; export class BrowserTypeDispatcher extends Dispatcher implements BrowserTypeChannel { constructor(scope: DispatcherScope, browserType: BrowserTypeBase) { @@ -32,21 +32,13 @@ export class BrowserTypeDispatcher extends Dispatcher { - const options = { - ...params, - ignoreDefaultArgs: params.ignoreAllDefaultArgs ? true : params.ignoreDefaultArgs, - env: params.env ? envArrayToObject(params.env) : undefined, - }; - const browser = await this._object.launch(options); + const browser = await this._object.launch(params); return { browser: new BrowserDispatcher(this._scope, browser as BrowserBase) }; } async launchPersistentContext(params: BrowserTypeLaunchPersistentContextParams): Promise<{ context: BrowserContextChannel }> { const options = { ...params, - viewport: params.viewport || (params.noDefaultViewport ? null : undefined), - ignoreDefaultArgs: params.ignoreAllDefaultArgs ? true : params.ignoreDefaultArgs, - env: params.env ? envArrayToObject(params.env) : undefined, extraHTTPHeaders: params.extraHTTPHeaders ? headersArrayToObject(params.extraHTTPHeaders) : undefined, }; const browserContext = await this._object.launchPersistentContext(params.userDataDir, options); diff --git a/src/rpc/server/electronDispatcher.ts b/src/rpc/server/electronDispatcher.ts index b6ff0a6fdd..b50bdfff4a 100644 --- a/src/rpc/server/electronDispatcher.ts +++ b/src/rpc/server/electronDispatcher.ts @@ -22,7 +22,6 @@ import { BrowserContextBase } from '../../browserContext'; import { PageDispatcher } from './pageDispatcher'; import { parseArgument, serializeResult } from './jsHandleDispatcher'; import { createHandle } from './elementHandlerDispatcher'; -import { envArrayToObject } from '../../converters'; export class ElectronDispatcher extends Dispatcher implements ElectronChannel { constructor(scope: DispatcherScope, electron: Electron) { @@ -30,11 +29,7 @@ export class ElectronDispatcher extends Dispatcher { - const options = { - ...params, - env: params.env ? envArrayToObject(params.env) : undefined, - }; - const electronApplication = await this._object.launch(params.executablePath, options); + const electronApplication = await this._object.launch(params.executablePath, params); return { electronApplication: new ElectronApplicationDispatcher(this._scope, electronApplication) }; } } diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 018e541306..091a242dcd 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -23,24 +23,18 @@ import * as browserPaths from '../install/browserPaths'; import { ConnectionTransport, WebSocketTransport } from '../transport'; import { BrowserBase, BrowserOptions, Browser, BrowserProcess } from '../browser'; import { assert, helper } from '../helper'; -import { launchProcess, Env, waitForLine } from './processLauncher'; +import { launchProcess, Env, waitForLine, envArrayToObject } from './processLauncher'; import { PipeTransport } from './pipeTransport'; import { Progress, runAbortableTask } from '../progress'; import * as types from '../types'; import { TimeoutSettings } from '../timeoutSettings'; import { validateHostRequirements } from './validateDependencies'; -type FirefoxPrefsOptions = { firefoxUserPrefs?: { [key: string]: string | number | boolean } }; - -export type LaunchNonPersistentOptions = types.LaunchOptions & FirefoxPrefsOptions; -type LaunchPersistentOptions = types.LaunchOptions & types.BrowserContextOptions; -type LaunchServerOptions = types.LaunchServerOptions & FirefoxPrefsOptions; - export interface BrowserType { executablePath(): string; name(): string; - launch(options?: LaunchNonPersistentOptions): Promise; - launchPersistentContext(userDataDir: string, options?: LaunchPersistentOptions): Promise; + launch(options?: types.LaunchOptions): Promise; + launchPersistentContext(userDataDir: string, options?: types.LaunchPersistentOptions): Promise; } const mkdirAsync = util.promisify(fs.mkdir); @@ -76,7 +70,7 @@ export abstract class BrowserTypeBase implements BrowserType { return this._name; } - async launch(options: LaunchNonPersistentOptions = {}): Promise { + async launch(options: types.LaunchOptions = {}): Promise { 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); @@ -84,10 +78,11 @@ export abstract class BrowserTypeBase implements BrowserType { return browser; } - async launchPersistentContext(userDataDir: string, options: LaunchPersistentOptions = {}): Promise { + async launchPersistentContext(userDataDir: string, options: types.LaunchPersistentOptions = {}): Promise { assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); options = validateLaunchOptions(options); - const persistent = validateBrowserContextOptions(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); }); return browser._defaultContext!; } @@ -109,23 +104,24 @@ export abstract class BrowserTypeBase implements BrowserType { copyTestHooks(options, browserOptions); const browser = await this._connectToTransport(transport, browserOptions); // We assume no control when using custom arguments, and do not prepare the default context in that case. - const hasCustomArguments = !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs); - if (persistent && !hasCustomArguments) + if (persistent && !options.ignoreAllDefaultArgs) await browser._defaultContext!._loadDefaultContext(progress); return browser; } - private async _launchProcess(progress: Progress, options: LaunchServerOptions, isPersistent: boolean, userDataDir?: string): Promise<{ browserProcess: BrowserProcess, downloadsPath: string, transport: ConnectionTransport }> { + private async _launchProcess(progress: Progress, options: types.LaunchOptions, isPersistent: boolean, userDataDir?: string): Promise<{ browserProcess: BrowserProcess, downloadsPath: string, transport: ConnectionTransport }> { const { - ignoreDefaultArgs = false, + ignoreDefaultArgs, + ignoreAllDefaultArgs, args = [], executablePath = null, - env = process.env, handleSIGINT = true, handleSIGTERM = true, handleSIGHUP = true, } = options; + const env = options.env ? envArrayToObject(options.env) : process.env; + const tempDirectories = []; let downloadsPath: string; if (options.downloadsPath) { @@ -142,12 +138,12 @@ export abstract class BrowserTypeBase implements BrowserType { } const browserArguments = []; - if (!ignoreDefaultArgs) - browserArguments.push(...this._defaultArgs(options, isPersistent, userDataDir)); - else if (Array.isArray(ignoreDefaultArgs)) + if (ignoreAllDefaultArgs) + browserArguments.push(...args); + else if (ignoreDefaultArgs) browserArguments.push(...this._defaultArgs(options, isPersistent, userDataDir).filter(arg => ignoreDefaultArgs.indexOf(arg) === -1)); else - browserArguments.push(...args); + browserArguments.push(...this._defaultArgs(options, isPersistent, userDataDir)); const executable = executablePath || this.executablePath(); if (!executable) @@ -211,7 +207,7 @@ export abstract class BrowserTypeBase implements BrowserType { return { browserProcess, downloadsPath, transport }; } - abstract _defaultArgs(options: types.LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[]; + abstract _defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[]; abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise; abstract _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env; abstract _amendArguments(browserArguments: string[]): string[]; @@ -226,7 +222,7 @@ function copyTestHooks(from: object, to: object) { } } -function validateLaunchOptions(options: Options): Options { +function validateLaunchOptions(options: Options): Options { const { devtools = false, headless = !helper.isDebugMode() && !devtools } = options; return { ...options, devtools, headless }; } diff --git a/src/server/chromium.ts b/src/server/chromium.ts index ac93c7ff2e..28a05be574 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -27,7 +27,7 @@ import { ConnectionTransport, ProtocolRequest } from '../transport'; import { BrowserDescriptor } from '../install/browserPaths'; import { CRDevTools } from '../chromium/crDevTools'; import { BrowserOptions } from '../browser'; -import { LaunchOptionsBase } from '../types'; +import * as types from '../types'; export class Chromium extends BrowserTypeBase { private _devtools: CRDevTools | undefined; @@ -101,7 +101,7 @@ export class Chromium extends BrowserTypeBase { transport.send(message); } - _defaultArgs(options: LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[] { + _defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { const { args = [], proxy } = options; const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir')); if (userDataDirArg) diff --git a/src/server/electron.ts b/src/server/electron.ts index efa4b4d33c..7d03eee8c0 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -24,7 +24,7 @@ import { Page } from '../page'; import { TimeoutSettings } from '../timeoutSettings'; import { WebSocketTransport } from '../transport'; import * as types from '../types'; -import { launchProcess, waitForLine } from './processLauncher'; +import { launchProcess, waitForLine, envArrayToObject } from './processLauncher'; import { BrowserContext } from '../browserContext'; import type {BrowserWindow} from 'electron'; import { runAbortableTask, ProgressController } from '../progress'; @@ -35,7 +35,7 @@ import { BrowserProcess } from '../browser'; export type ElectronLaunchOptionsBase = { args?: string[], cwd?: string, - env?: types.Env, + env?: types.EnvArray, handleSIGINT?: boolean, handleSIGTERM?: boolean, handleSIGHUP?: boolean, @@ -145,7 +145,6 @@ export class Electron { async launch(executablePath: string, options: ElectronLaunchOptionsBase = {}): Promise { const { args = [], - env = process.env, handleSIGINT = true, handleSIGTERM = true, handleSIGHUP = true, @@ -156,7 +155,7 @@ export class Electron { const { launchedProcess, gracefullyClose, kill } = await launchProcess({ executablePath, args: electronArguments, - env, + env: options.env ? envArrayToObject(options.env) : process.env, handleSIGINT, handleSIGTERM, handleSIGHUP, @@ -180,7 +179,7 @@ export class Electron { close: gracefullyClose, kill }; - const browser = await CRBrowser.connect(chromeTransport, { name: 'electron', headful: true, persistent: { viewport: null }, browserProcess }); + const browser = await CRBrowser.connect(chromeTransport, { name: 'electron', headful: true, persistent: { noDefaultViewport: true }, browserProcess }); app = new ElectronApplication(browser, nodeConnection); await app._init(); return app; diff --git a/src/server/firefox.ts b/src/server/firefox.ts index fb4c4eb030..5f77dd8e57 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -20,11 +20,12 @@ import * as fs from 'fs'; import * as path from 'path'; import { FFBrowser } from '../firefox/ffBrowser'; import { kBrowserCloseMessageId } from '../firefox/ffConnection'; -import { BrowserTypeBase, LaunchNonPersistentOptions } from './browserType'; +import { BrowserTypeBase } from './browserType'; import { Env } from './processLauncher'; import { ConnectionTransport } from '../transport'; import { BrowserOptions } from '../browser'; import { BrowserDescriptor } from '../install/browserPaths'; +import * as types from '../types'; export class Firefox extends BrowserTypeBase { constructor(packagePath: string, browser: BrowserDescriptor) { @@ -57,7 +58,7 @@ export class Firefox extends BrowserTypeBase { transport.send(message); } - _defaultArgs(options: LaunchNonPersistentOptions, isPersistent: boolean, userDataDir: string): string[] { + _defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { const { args = [], devtools, headless } = options; if (devtools) console.warn('devtools parameter is not supported as a launch argument in Firefox. You can launch the devtools window manually.'); diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index 7a638a7273..436cdeffa0 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -21,6 +21,7 @@ import * as removeFolder from 'rimraf'; import * as stream from 'stream'; import { helper, isUnderTest } from '../helper'; import { Progress } from '../progress'; +import * as types from '../types'; export type Env = {[key: string]: string | number | boolean | undefined}; @@ -204,3 +205,10 @@ export function waitForLine(progress: Progress, process: childProcess.ChildProce } }); } + +export function envArrayToObject(env: types.EnvArray): Env { + const result: Env = {}; + for (const { name, value } of env) + result[name] = value; + return result; +} diff --git a/src/server/webkit.ts b/src/server/webkit.ts index 1f3d24de00..2353fa724f 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -23,7 +23,7 @@ import { BrowserTypeBase } from './browserType'; import { ConnectionTransport } from '../transport'; import { BrowserOptions } from '../browser'; import { BrowserDescriptor } from '../install/browserPaths'; -import { LaunchOptionsBase } from '../types'; +import * as types from '../types'; export class WebKit extends BrowserTypeBase { constructor(packagePath: string, browser: BrowserDescriptor) { @@ -50,7 +50,7 @@ export class WebKit extends BrowserTypeBase { transport.send({method: 'Playwright.close', params: {}, id: kBrowserCloseMessageId}); } - _defaultArgs(options: LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[] { + _defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[] { const { args = [], proxy, devtools, headless } = options; if (devtools) console.warn('devtools parameter as a launch argument in WebKit is not supported. Also starting Web Inspector manually will terminate the execution in WebKit.'); diff --git a/src/types.ts b/src/types.ts index 7563050261..e4c0f3a9b5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -265,7 +265,8 @@ export type SetNetworkCookieParam = { }; export type BrowserContextOptions = { - viewport?: Size | null, + viewport?: Size, + noDefaultViewport?: boolean, ignoreHTTPSErrors?: boolean, javaScriptEnabled?: boolean, bypassCSP?: boolean, @@ -284,27 +285,29 @@ export type BrowserContextOptions = { acceptDownloads?: boolean, }; -export type Env = {[key: string]: string | number | boolean | undefined}; export type EnvArray = { name: string, value: string }[]; -export type LaunchOptionsBase = { +type LaunchOptionsBase = { executablePath?: string, args?: string[], - ignoreDefaultArgs?: boolean | string[], + ignoreDefaultArgs?: string[], + ignoreAllDefaultArgs?: boolean, handleSIGINT?: boolean, handleSIGTERM?: boolean, handleSIGHUP?: boolean, timeout?: number, - env?: Env, + env?: EnvArray, headless?: boolean, devtools?: boolean, proxy?: ProxySettings, downloadsPath?: string, chromiumSandbox?: boolean, + slowMo?: number, }; - -export type LaunchOptions = LaunchOptionsBase & { slowMo?: number }; -export type LaunchServerOptions = LaunchOptionsBase & { port?: number }; +export type LaunchOptions = LaunchOptionsBase & { + firefoxUserPrefs?: { [key: string]: string | number | boolean }, +}; +export type LaunchPersistentOptions = LaunchOptionsBase & BrowserContextOptions; export type SerializedAXNode = { role: string, diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index ee66206bfe..a6f77da9a2 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { BrowserBase, BrowserOptions, BrowserContextOptions } from '../browser'; +import { BrowserBase, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, BrowserContextBase, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; import { Events } from '../events'; import { helper, RegisteredListener, assert } from '../helper'; @@ -72,8 +72,8 @@ export class WKBrowser extends BrowserBase { this._didClose(); } - async newContext(options: BrowserContextOptions = {}): Promise { - options = validateBrowserContextOptions(options); + async newContext(options: types.BrowserContextOptions = {}): Promise { + validateBrowserContextOptions(options); const { browserContextId } = await this._browserSession.send('Playwright.createContext'); options.userAgent = options.userAgent || DEFAULT_USER_AGENT; const context = new WKBrowserContext(this, browserContextId, options); @@ -287,8 +287,7 @@ export class WKBrowserContext extends BrowserContextBase { } async setGeolocation(geolocation?: types.Geolocation): Promise { - if (geolocation) - geolocation = verifyGeolocation(geolocation); + verifyGeolocation(geolocation); this._options.geolocation = geolocation; const payload: any = geolocation ? { ...geolocation, timestamp: Date.now() } : undefined; await this._browser._browserSession.send('Playwright.setGeolocationOverride', { browserContextId: this._browserContextId, geolocation: payload }); diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index f77671d539..a7202e0cc8 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -90,7 +90,7 @@ export class WKPage implements PageDelegate { this._firstNonInitialNavigationCommittedFulfill = f; this._firstNonInitialNavigationCommittedReject = r; }); - if (opener && browserContext._options.viewport !== null && opener._nextWindowOpenPopupFeatures) { + if (opener && !browserContext._options.noDefaultViewport && opener._nextWindowOpenPopupFeatures) { const viewportSize = helper.getViewportSizeFromWindowFeatures(opener._nextWindowOpenPopupFeatures); opener._nextWindowOpenPopupFeatures = undefined; if (viewportSize)