From e8e761f77f65f469a5a04628b52849132cfafcfe Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 14 May 2020 13:22:33 -0700 Subject: [PATCH] chore: use internal BrowserOptions to unify browsers (#2230) --- src/browser.ts | 29 +++++++++++++-------- src/chromium/crBrowser.ts | 22 ++++++++-------- src/firefox/ffBrowser.ts | 19 +++++++------- src/server/browserType.ts | 5 ++++ src/server/chromium.ts | 41 +++++++++++++++++------------ src/server/electron.ts | 3 +-- src/server/firefox.ts | 43 +++++++++++++++++-------------- src/server/webkit.ts | 47 ++++++++++++++++++---------------- src/webkit/wkBrowser.ts | 17 ++++++------ test/electron/electron.spec.js | 2 +- test/test.js | 5 +--- 11 files changed, 128 insertions(+), 105 deletions(-) diff --git a/src/browser.ts b/src/browser.ts index 2d5894032720d..ad685ef83903a 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -21,6 +21,17 @@ import { Download } from './download'; import type { BrowserServer } from './server/browserServer'; import { Events } from './events'; import { InnerLogger, Log } from './logger'; +import * as types from './types'; + +export type BrowserOptions = { + logger: InnerLogger, + downloadsPath: string, + headful?: boolean, + persistent?: boolean, + slowMo?: number, + viewport?: types.Size | null, + ownedServer?: BrowserServer, +}; export interface Browser extends EventEmitter { newContext(options?: BrowserContextOptions): Promise; @@ -31,14 +42,12 @@ export interface Browser extends EventEmitter { } export abstract class BrowserBase extends EventEmitter implements Browser, InnerLogger { - _downloadsPath: string = ''; + readonly _options: BrowserOptions; private _downloads = new Map(); - _ownedServer: BrowserServer | null = null; - readonly _logger: InnerLogger; - constructor(logger: InnerLogger) { + constructor(options: BrowserOptions) { super(); - this._logger = logger; + this._options = options; } abstract newContext(options?: BrowserContextOptions): Promise; @@ -54,7 +63,7 @@ export abstract class BrowserBase extends EventEmitter implements Browser, Inner } _downloadCreated(page: Page, uuid: string, url: string, suggestedFilename?: string) { - const download = new Download(page, this._downloadsPath, uuid, url, suggestedFilename); + const download = new Download(page, this._options.downloadsPath, uuid, url, suggestedFilename); this._downloads.set(uuid, download); } @@ -74,8 +83,8 @@ export abstract class BrowserBase extends EventEmitter implements Browser, Inner } async close() { - if (this._ownedServer) { - await this._ownedServer.close(); + if (this._options.ownedServer) { + await this._options.ownedServer.close(); } else { await Promise.all(this.contexts().map(context => context.close())); this._disconnect(); @@ -85,11 +94,11 @@ export abstract class BrowserBase extends EventEmitter implements Browser, Inner } _isLogEnabled(log: Log): boolean { - return this._logger._isLogEnabled(log); + return this._options.logger._isLogEnabled(log); } _log(log: Log, message: string | Error, ...args: any[]) { - return this._logger._log(log, message, ...args); + return this._options.logger._log(log, message, ...args); } } diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index 01cac268ddccd..f2f12397f4855 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { BrowserBase } from '../browser'; +import { BrowserBase, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, BrowserContextBase, BrowserContextOptions, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; import { Events as CommonEvents } from '../events'; import { assert, helper } from '../helper'; @@ -29,7 +29,7 @@ import { readProtocolStream } from './crProtocolHelper'; import { Events } from './events'; import { Protocol } from './protocol'; import { CRExecutionContext } from './crExecutionContext'; -import { InnerLogger, logError } from '../logger'; +import { logError } from '../logger'; export class CRBrowser extends BrowserBase { readonly _connection: CRConnection; @@ -44,13 +44,12 @@ export class CRBrowser extends BrowserBase { private _tracingRecording = false; private _tracingPath: string | null = ''; private _tracingClient: CRSession | undefined; - readonly _isHeadful: boolean; - static async connect(transport: ConnectionTransport, isPersistent: boolean, logger: InnerLogger, options: { slowMo?: number, headless?: boolean, viewport?: types.Size | null } = {}): Promise { - const connection = new CRConnection(SlowMoTransport.wrap(transport, options.slowMo), logger); - const browser = new CRBrowser(connection, logger, { persistent: isPersistent, headful: !options.headless, viewport: options.viewport }); + static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise { + const connection = new CRConnection(SlowMoTransport.wrap(transport, options.slowMo), options.logger); + const browser = new CRBrowser(connection, options); const session = connection.rootSession; - if (!isPersistent) { + if (!options.persistent) { await session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }); return browser; } @@ -83,14 +82,13 @@ export class CRBrowser extends BrowserBase { return browser; } - constructor(connection: CRConnection, logger: InnerLogger, options: { headful?: boolean, persistent?: boolean, viewport?: types.Size | null } = {}) { - super(logger); + constructor(connection: CRConnection, options: BrowserOptions) { + super(options); this._connection = connection; this._session = this._connection.rootSession; if (options.persistent) this._defaultContext = new CRBrowserContext(this, null, validateBrowserContextOptions({ viewport: options.viewport })); - this._isHeadful = !!options.headful; this._connection.on(ConnectionEvents.Disconnected, () => { for (const context of this._contexts.values()) context._browserClosed(); @@ -150,7 +148,7 @@ export class CRBrowser extends BrowserBase { if (targetInfo.type === 'page') { const opener = targetInfo.openerId ? this._crPages.get(targetInfo.openerId) || null : null; - const crPage = new CRPage(session, targetInfo.targetId, context, opener, this._isHeadful); + const crPage = new CRPage(session, targetInfo.targetId, context, opener, !!this._options.headful); this._crPages.set(targetInfo.targetId, crPage); crPage.pageOrError().then(() => { context!.emit(CommonEvents.BrowserContext.Page, crPage._page); @@ -289,7 +287,7 @@ export class CRBrowserContext extends BrowserContextBase { this._browser._session.send('Browser.setDownloadBehavior', { behavior: this._options.acceptDownloads ? 'allowAndName' : 'deny', browserContextId: this._browserContextId || undefined, - downloadPath: this._browser._downloadsPath + downloadPath: this._browser._options.downloadsPath }) ]; if (this._options.permissions) diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 565551e3c5676..a9559be4108a7 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { BrowserBase } from '../browser'; +import { BrowserBase, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, BrowserContextBase, BrowserContextOptions, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; import { Events } from '../events'; import { assert, helper, RegisteredListener } from '../helper'; @@ -27,7 +27,6 @@ import { ConnectionEvents, FFConnection } from './ffConnection'; import { headersArray } from './ffNetworkManager'; import { FFPage } from './ffPage'; import { Protocol } from './protocol'; -import { InnerLogger } from '../logger'; export class FFBrowser extends BrowserBase { _connection: FFConnection; @@ -36,19 +35,19 @@ export class FFBrowser extends BrowserBase { readonly _contexts: Map; private _eventListeners: RegisteredListener[]; - static async connect(transport: ConnectionTransport, logger: InnerLogger, attachToDefaultContext: boolean, slowMo?: number): Promise { - const connection = new FFConnection(SlowMoTransport.wrap(transport, slowMo), logger); - const browser = new FFBrowser(connection, logger, attachToDefaultContext); - await connection.send('Browser.enable', { attachToDefaultContext }); + static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise { + const connection = new FFConnection(SlowMoTransport.wrap(transport, options.slowMo), options.logger); + const browser = new FFBrowser(connection, options); + await connection.send('Browser.enable', { attachToDefaultContext: !!options.persistent }); return browser; } - constructor(connection: FFConnection, logger: InnerLogger, isPersistent: boolean) { - super(logger); + constructor(connection: FFConnection, options: BrowserOptions) { + super(options); this._connection = connection; this._ffPages = new Map(); - if (isPersistent) + if (options.persistent) this._defaultContext = new FFBrowserContext(this, null, validateBrowserContextOptions({})); this._contexts = new Map(); this._connection.on(ConnectionEvents.Disconnected, () => { @@ -159,7 +158,7 @@ export class FFBrowserContext extends BrowserContextBase { browserContextId, downloadOptions: { behavior: this._options.acceptDownloads ? 'saveToDisk' : 'cancel', - downloadsDir: this._browser._downloadsPath, + downloadsDir: this._browser._options.downloadsPath, }, }), ]; diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 5f897b5e7cc39..07bb981041fc9 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -36,6 +36,11 @@ type LaunchOptionsBase = BrowserArgOptions & { env?: {[key: string]: string|number|boolean} }; +export function processBrowserArgOptions(options: LaunchOptionsBase): { devtools: boolean, headless: boolean } { + const { devtools = false, headless = !devtools } = options; + return { devtools, headless }; +} + export type ConnectOptions = { wsEndpoint: string, slowMo?: number, diff --git a/src/server/chromium.ts b/src/server/chromium.ts index 2cb6d881fbb20..6c397aa17dc53 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -25,7 +25,7 @@ import * as ws from 'ws'; import { launchProcess } from './processLauncher'; import { kBrowserCloseMessageId } from '../chromium/crConnection'; import { PipeTransport } from './pipeTransport'; -import { LaunchOptions, BrowserArgOptions, ConnectOptions, LaunchServerOptions, AbstractBrowserType } from './browserType'; +import { LaunchOptions, BrowserArgOptions, ConnectOptions, LaunchServerOptions, AbstractBrowserType, processBrowserArgOptions } from './browserType'; import { LaunchType } from '../browser'; import { BrowserServer, WebSocketWrapper } from './browserServer'; import { Events } from '../events'; @@ -48,10 +48,13 @@ export class Chromium extends AbstractBrowserType { return await browserServer._initializeOrClose(deadline, async () => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - const browser = await CRBrowser.connect(transport!, false, logger, options); - browser._ownedServer = browserServer; - browser._downloadsPath = downloadsPath; - return browser; + return await CRBrowser.connect(transport!, { + slowMo: options.slowMo, + headful: !processBrowserArgOptions(options).headless, + logger, + downloadsPath, + ownedServer: browserServer + }); }); } @@ -62,12 +65,18 @@ export class Chromium extends AbstractBrowserType { async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { const { timeout = 30000 } = options; const deadline = TimeoutSettings.computeDeadline(timeout); - const { transport, browserServer, logger } = await this._launchServer(options, 'persistent', userDataDir); + const { transport, browserServer, downloadsPath, logger } = await this._launchServer(options, 'persistent', userDataDir); return await browserServer._initializeOrClose(deadline, async () => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - const browser = await CRBrowser.connect(transport!, true, logger, options); - browser._ownedServer = browserServer; + const browser = await CRBrowser.connect(transport!, { + slowMo: options.slowMo, + persistent: true, + logger, + downloadsPath, + headful: !processBrowserArgOptions(options).headless, + ownedServer: browserServer + }); const context = browser._defaultContext!; if (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs)) await context._loadDefaultContext(); @@ -109,6 +118,11 @@ export class Chromium extends AbstractBrowserType { const chromeExecutable = executablePath || this.executablePath(); if (!chromeExecutable) throw new Error(`No executable path is specified. Pass "executablePath" option directly.`); + + // Note: it is important to define these variables before launchProcess, so that we don't get + // "Cannot access 'browserServer' before initialization" if something went wrong. + let transport: PipeTransport | undefined = undefined; + let browserServer: BrowserServer | undefined = undefined; const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({ executablePath: chromeExecutable, args: chromeArguments, @@ -134,8 +148,6 @@ export class Chromium extends AbstractBrowserType { }, }); - let transport: PipeTransport | undefined = undefined; - let browserServer: BrowserServer | undefined = undefined; const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream]; transport = new PipeTransport(stdio[3], stdio[4], logger); browserServer = new BrowserServer(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null); @@ -146,16 +158,13 @@ export class Chromium extends AbstractBrowserType { return await WebSocketTransport.connect(options.wsEndpoint, async transport => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - return CRBrowser.connect(transport, false, new RootLogger(options.logger), options); + return CRBrowser.connect(transport, { slowMo: options.slowMo, logger: new RootLogger(options.logger), downloadsPath: '' }); }); } private _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string): string[] { - const { - devtools = false, - headless = !devtools, - args = [], - } = options; + const { devtools, headless } = processBrowserArgOptions(options); + const { args = [] } = options; const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir')); if (userDataDirArg) throw new Error('Pass userDataDir parameter instead of specifying --user-data-dir argument'); diff --git a/src/server/electron.ts b/src/server/electron.ts index a9172d87c946e..69adbadddafbd 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -204,8 +204,7 @@ export class Electron { return transport; }); const browserServer = new BrowserServer(launchedProcess, gracefullyClose, null); - const browser = await CRBrowser.connect(chromeTransport, true, logger, { ...options, viewport: null }); - browser._ownedServer = browserServer; + const browser = await CRBrowser.connect(chromeTransport, { headful: true, logger, persistent: true, viewport: null, ownedServer: browserServer, downloadsPath: '' }); app = new ElectronApplication(logger, browser, nodeConnection); await app._init(); return app; diff --git a/src/server/firefox.ts b/src/server/firefox.ts index bac23e3ae640d..cdb80061a8789 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -28,7 +28,7 @@ import { FFBrowser } from '../firefox/ffBrowser'; import { kBrowserCloseMessageId } from '../firefox/ffConnection'; import { helper, assert } from '../helper'; import { BrowserServer, WebSocketWrapper } from './browserServer'; -import { BrowserArgOptions, LaunchOptions, LaunchServerOptions, ConnectOptions, AbstractBrowserType } from './browserType'; +import { BrowserArgOptions, LaunchOptions, LaunchServerOptions, ConnectOptions, AbstractBrowserType, processBrowserArgOptions } from './browserType'; import { launchProcess, waitForLine } from './processLauncher'; import { ConnectionTransport, SequenceNumberMixer, WebSocketTransport } from '../transport'; import { RootLogger, InnerLogger, logError } from '../logger'; @@ -51,10 +51,14 @@ export class Firefox extends AbstractBrowserType { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); const browser = await WebSocketTransport.connect(browserServer.wsEndpoint()!, transport => { - return FFBrowser.connect(transport, logger, false, options.slowMo); + return FFBrowser.connect(transport, { + slowMo: options.slowMo, + logger, + downloadsPath, + headful: !processBrowserArgOptions(options).headless, + ownedServer: browserServer, + }); }); - browser._ownedServer = browserServer; - browser._downloadsPath = downloadsPath; return browser; }); } @@ -64,20 +68,22 @@ export class Firefox extends AbstractBrowserType { } async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { - const { - timeout = 30000, - slowMo = 0, - } = options; + const { timeout = 30000 } = options; const deadline = TimeoutSettings.computeDeadline(timeout); const { browserServer, downloadsPath, logger } = await this._launchServer(options, 'persistent', userDataDir); return await browserServer._initializeOrClose(deadline, async () => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); const browser = await WebSocketTransport.connect(browserServer.wsEndpoint()!, transport => { - return FFBrowser.connect(transport, logger, true, slowMo); + return FFBrowser.connect(transport, { + slowMo: options.slowMo, + logger, + persistent: true, + downloadsPath, + ownedServer: browserServer, + headful: !processBrowserArgOptions(options).headless, + }); }); - browser._ownedServer = browserServer; - browser._downloadsPath = downloadsPath; const context = browser._defaultContext!; if (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs)) await context._loadDefaultContext(); @@ -118,6 +124,10 @@ export class Firefox extends AbstractBrowserType { if (!firefoxExecutable) throw new Error(`No executable path is specified. Pass "executablePath" option directly.`); + // Note: it is important to define these variables before launchProcess, so that we don't get + // "Cannot access 'browserServer' before initialization" if something went wrong. + let browserServer: BrowserServer | undefined = undefined; + let browserWSEndpoint: string | undefined = undefined; const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({ executablePath: firefoxExecutable, args: firefoxArguments, @@ -149,8 +159,6 @@ export class Firefox extends AbstractBrowserType { const match = await waitForLine(launchedProcess, launchedProcess.stdout, /^Juggler listening on (ws:\/\/.*)$/, timeout, timeoutError); const innerEndpoint = match[1]; - let browserServer: BrowserServer | undefined = undefined; - let browserWSEndpoint: string | undefined = undefined; const webSocketWrapper = launchType === 'server' ? (await WebSocketTransport.connect(innerEndpoint, t => wrapTransportWithWebSocket(t, logger, port))) : new WebSocketWrapper(innerEndpoint, []); browserWSEndpoint = webSocketWrapper.wsEndpoint; browserServer = new BrowserServer(launchedProcess, gracefullyClose, webSocketWrapper); @@ -161,16 +169,13 @@ export class Firefox extends AbstractBrowserType { return await WebSocketTransport.connect(options.wsEndpoint, async transport => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - return FFBrowser.connect(transport, new RootLogger(options.logger), false, options.slowMo); + return FFBrowser.connect(transport, { slowMo: options.slowMo, logger: new RootLogger(options.logger), downloadsPath: '' }); }); } private _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string, port: number): string[] { - const { - devtools = false, - headless = !devtools, - args = [], - } = options; + const { devtools, headless } = processBrowserArgOptions(options); + const { args = [] } = options; if (devtools) console.warn('devtools parameter is not supported as a launch argument in Firefox. You can launch the devtools window manually.'); const userDataDirArg = args.find(arg => arg.startsWith('-profile') || arg.startsWith('--profile')); diff --git a/src/server/webkit.ts b/src/server/webkit.ts index 2d4fe68cf3d85..e78d3ff37b6c8 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -24,7 +24,7 @@ import * as os from 'os'; import * as util from 'util'; import { helper, assert } from '../helper'; import { kBrowserCloseMessageId } from '../webkit/wkConnection'; -import { LaunchOptions, BrowserArgOptions, LaunchServerOptions, ConnectOptions, AbstractBrowserType } from './browserType'; +import { LaunchOptions, BrowserArgOptions, LaunchServerOptions, ConnectOptions, AbstractBrowserType, processBrowserArgOptions } from './browserType'; import { ConnectionTransport, SequenceNumberMixer, WebSocketTransport } from '../transport'; import * as ws from 'ws'; import { LaunchType } from '../browser'; @@ -48,10 +48,13 @@ export class WebKit extends AbstractBrowserType { return await browserServer._initializeOrClose(deadline, async () => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - const browser = await WKBrowser.connect(transport!, logger, options.slowMo, false); - browser._ownedServer = browserServer; - browser._downloadsPath = downloadsPath; - return browser; + return await WKBrowser.connect(transport!, { + slowMo: options.slowMo, + headful: !processBrowserArgOptions(options).headless, + logger, + downloadsPath, + ownedServer: browserServer + }); }); } @@ -60,17 +63,20 @@ export class WebKit extends AbstractBrowserType { } async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { - const { - timeout = 30000, - slowMo = 0, - } = options; + const { timeout = 30000 } = options; const deadline = TimeoutSettings.computeDeadline(timeout); - const { transport, browserServer, logger } = await this._launchServer(options, 'persistent', userDataDir); + const { transport, browserServer, logger, downloadsPath } = await this._launchServer(options, 'persistent', userDataDir); return await browserServer._initializeOrClose(deadline, async () => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - const browser = await WKBrowser.connect(transport!, logger, slowMo, true); - browser._ownedServer = browserServer; + const browser = await WKBrowser.connect(transport!, { + slowMo: options.slowMo, + headful: !processBrowserArgOptions(options).headless, + logger, + persistent: true, + downloadsPath, + ownedServer: browserServer + }); const context = browser._defaultContext!; if (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs)) await context._loadDefaultContext(); @@ -110,6 +116,10 @@ export class WebKit extends AbstractBrowserType { if (!webkitExecutable) throw new Error(`No executable path is specified.`); + // Note: it is important to define these variables before launchProcess, so that we don't get + // "Cannot access 'browserServer' before initialization" if something went wrong. + let transport: ConnectionTransport | undefined = undefined; + let browserServer: BrowserServer | undefined = undefined; const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({ executablePath: webkitExecutable, args: webkitArguments, @@ -133,10 +143,6 @@ export class WebKit extends AbstractBrowserType { }, }); - // Note: it is important to define these variables before launchProcess, so that we don't get - // "Cannot access 'browserServer' before initialization" if something went wrong. - let transport: ConnectionTransport | undefined = undefined; - let browserServer: BrowserServer | undefined = undefined; const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream]; transport = new PipeTransport(stdio[3], stdio[4], logger); browserServer = new BrowserServer(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port || 0) : null); @@ -147,16 +153,13 @@ export class WebKit extends AbstractBrowserType { return await WebSocketTransport.connect(options.wsEndpoint, async transport => { if ((options as any).__testHookBeforeCreateBrowser) await (options as any).__testHookBeforeCreateBrowser(); - return WKBrowser.connect(transport, new RootLogger(options.logger), options.slowMo); + return WKBrowser.connect(transport, { slowMo: options.slowMo, logger: new RootLogger(options.logger), downloadsPath: '' }); }); } _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string, port: number): string[] { - const { - devtools = false, - headless = !devtools, - args = [], - } = options; + const { devtools, headless } = processBrowserArgOptions(options); + const { args = [] } = 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.'); const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir=')); diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index b712c287f9e9e..9214306fde451 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { BrowserBase } from '../browser'; +import { BrowserBase, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, BrowserContextBase, BrowserContextOptions, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; import { Events } from '../events'; import { helper, RegisteredListener } from '../helper'; @@ -26,7 +26,6 @@ import * as types from '../types'; import { Protocol } from './protocol'; import { kPageProxyMessageReceived, PageProxyMessageReceivedPayload, WKConnection, WKSession } from './wkConnection'; import { WKPage } from './wkPage'; -import { InnerLogger } from '../logger'; const DEFAULT_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_2) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.4 Safari/605.1.15'; @@ -38,17 +37,17 @@ export class WKBrowser extends BrowserBase { readonly _wkPages = new Map(); private readonly _eventListeners: RegisteredListener[]; - static async connect(transport: ConnectionTransport, logger: InnerLogger, slowMo: number = 0, attachToDefaultContext: boolean = false): Promise { - const browser = new WKBrowser(SlowMoTransport.wrap(transport, slowMo), logger, attachToDefaultContext); + static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise { + const browser = new WKBrowser(SlowMoTransport.wrap(transport, options.slowMo), options); return browser; } - constructor(transport: ConnectionTransport, logger: InnerLogger, attachToDefaultContext: boolean) { - super(logger); - this._connection = new WKConnection(transport, logger, this._onDisconnect.bind(this)); + constructor(transport: ConnectionTransport, options: BrowserOptions) { + super(options); + this._connection = new WKConnection(transport, options.logger, this._onDisconnect.bind(this)); this._browserSession = this._connection.browserSession; - if (attachToDefaultContext) + if (options.persistent) this._defaultContext = new WKBrowserContext(this, undefined, validateBrowserContextOptions({})); this._eventListeners = [ @@ -205,7 +204,7 @@ export class WKBrowserContext extends BrowserContextBase { const promises: Promise[] = [ this._browser._browserSession.send('Playwright.setDownloadBehavior', { behavior: this._options.acceptDownloads ? 'allow' : 'deny', - downloadPath: this._browser._downloadsPath, + downloadPath: this._browser._options.downloadsPath, browserContextId }) ]; diff --git a/test/electron/electron.spec.js b/test/electron/electron.spec.js index fc8257aeb1f01..2b48522838256 100644 --- a/test/electron/electron.spec.js +++ b/test/electron/electron.spec.js @@ -146,7 +146,7 @@ describe('Electron per window', function() { afterEach(async state => { await state.page.close(); }); - + it('should click the button', async({page, server}) => { await page.goto(server.PREFIX + '/input/button.html'); await page.click('button'); diff --git a/test/test.js b/test/test.js index 33a85a58c1a76..b09bebde7d074 100644 --- a/test/test.js +++ b/test/test.js @@ -80,17 +80,16 @@ function collect(browserNames) { const playwrightEnvironment = new Environment('Playwright'); playwrightEnvironment.beforeAll(async state => { state.playwright = playwright; - global.playwright = playwright; }); playwrightEnvironment.afterAll(async state => { delete state.playwright; - delete global.playwright; }); testRunner.collector().useEnvironment(playwrightEnvironment); for (const e of config.globalEnvironments || []) testRunner.collector().useEnvironment(e); + global.playwright = playwright; for (const browserName of browserNames) { const browserType = playwright[browserName]; const browserTypeEnvironment = new Environment('BrowserType'); @@ -172,7 +171,6 @@ function collect(browserNames) { const suiteName = { 'chromium': 'Chromium', 'firefox': 'Firefox', 'webkit': 'WebKit' }[browserName]; describe(suiteName, () => { // In addition to state, expose these two on global so that describes can access them. - global.playwright = playwright; global.browserType = browserType; global.HEADLESS = !!launchOptions.headless; @@ -200,7 +198,6 @@ function collect(browserNames) { delete global.HEADLESS; delete global.browserType; - delete global.playwright; }); } for (const [key, value] of Object.entries(testRunner.api())) {