diff --git a/src/server/browserServer.ts b/src/server/browserServer.ts index 6624d9ee16..cf2f55bdde 100644 --- a/src/server/browserServer.ts +++ b/src/server/browserServer.ts @@ -17,13 +17,11 @@ import { ChildProcess } from 'child_process'; import { EventEmitter } from 'events'; import { helper } from '../helper'; -import { RootLogger } from '../logger'; -import { LaunchOptions, processBrowserArgOptions } from './browserType'; -import { ConnectionTransport } from '../transport'; export class WebSocketWrapper { readonly wsEndpoint: string; private _bindings: (Map | Set)[]; + constructor(wsEndpoint: string, bindings: (Map|Set)[]) { this.wsEndpoint = wsEndpoint; this._bindings = bindings; @@ -53,24 +51,12 @@ export class WebSocketWrapper { export class BrowserServer extends EventEmitter { private _process: ChildProcess; private _gracefullyClose: (() => Promise); - private _webSocketWrapper: WebSocketWrapper | null = null; - readonly _launchOptions: LaunchOptions; - readonly _logger: RootLogger; - readonly _downloadsPath: string | undefined; - readonly _transport: ConnectionTransport; - readonly _headful: boolean; + _webSocketWrapper: WebSocketWrapper | null = null; - constructor(options: LaunchOptions, process: ChildProcess, gracefullyClose: () => Promise, transport: ConnectionTransport, downloadsPath: string | undefined, webSocketWrapper: WebSocketWrapper | null) { + constructor(process: ChildProcess, gracefullyClose: () => Promise) { super(); - this._launchOptions = options; - this._headful = !processBrowserArgOptions(options).headless; - this._logger = new RootLogger(options.logger); - this._process = process; this._gracefullyClose = gracefullyClose; - this._transport = transport; - this._downloadsPath = downloadsPath; - this._webSocketWrapper = webSocketWrapper; } process(): ChildProcess { diff --git a/src/server/browserType.ts b/src/server/browserType.ts index a7c63a29fb..60391af04d 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -59,7 +59,6 @@ type ConnectOptions = { logger?: Logger, timeout?: number, }; -export type LaunchType = 'local' | 'server' | 'persistent'; export type LaunchOptions = LaunchOptionsBase & { slowMo?: number }; type LaunchServerOptions = LaunchOptionsBase & { port?: number }; @@ -73,6 +72,7 @@ export interface BrowserType { } const mkdtempAsync = util.promisify(fs.mkdtemp); +const DOWNLOADS_FOLDER = path.join(os.tmpdir(), 'playwright_downloads-'); export abstract class BrowserTypeBase implements BrowserType { private _name: string; @@ -100,24 +100,37 @@ export abstract class BrowserTypeBase implements BrowserType { async launch(options: LaunchOptions = {}): Promise { assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); - return this._innerLaunch('local', options, undefined); + assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); + return this._innerLaunch(options, undefined); } async launchPersistentContext(userDataDir: string, options: LaunchOptions & PersistentContextOptions = {}): Promise { + assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); const persistent = validatePersistentContextOptions(options); - const browser = await this._innerLaunch('persistent', options, persistent, userDataDir); + const browser = await this._innerLaunch(options, persistent, userDataDir); return browser._defaultContext!; } - async _innerLaunch(launchType: LaunchType, options: LaunchOptions, persistent: PersistentContextOptions | undefined, userDataDir?: string): Promise { + async _innerLaunch(options: LaunchOptions, persistent: PersistentContextOptions | undefined, userDataDir?: string): Promise { const deadline = TimeoutSettings.computeDeadline(options.timeout, 30000); const logger = new RootLogger(options.logger); logger.startLaunchRecording(); let browserServer: BrowserServer | undefined; try { - browserServer = await this._launchServer(options, launchType, logger, deadline, userDataDir); - const promise = this._innerLaunchPromise(browserServer, options, persistent); + const launched = await this._launchServer(options, !!persistent, logger, deadline, userDataDir); + browserServer = launched.browserServer; + const browserOptions: BrowserOptions = { + slowMo: options.slowMo, + persistent, + headful: !processBrowserArgOptions(options).headless, + logger, + downloadsPath: launched.downloadsPath, + ownedServer: browserServer, + }; + copyTestHooks(options, browserOptions); + const hasCustomArguments = !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs); + const promise = this._innerCreateBrowser(launched.transport, browserOptions, hasCustomArguments); const browser = await helper.waitWithDeadline(promise, 'the browser to launch', deadline, 'pw:browser*'); return browser; } catch (e) { @@ -132,34 +145,23 @@ export abstract class BrowserTypeBase implements BrowserType { } } - async _innerLaunchPromise(browserServer: BrowserServer, options: LaunchOptions, persistent: PersistentContextOptions | undefined): Promise { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); - - const browserOptions: BrowserOptions = { - slowMo: options.slowMo, - persistent, - headful: browserServer._headful, - logger: browserServer._logger, - downloadsPath: browserServer._downloadsPath, - ownedServer: browserServer, - }; - for (const [key, value] of Object.entries(options)) { - if (key.startsWith('__testHook')) - (browserOptions as any)[key] = value; - } - - const browser = await this._connectToTransport(browserServer._transport, browserOptions); - if (persistent && (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs))) { - const context = browser._defaultContext!; - await context._loadDefaultContext(); - } + async _innerCreateBrowser(transport: ConnectionTransport, browserOptions: BrowserOptions, hasCustomArguments: boolean): Promise { + if ((browserOptions as any).__testHookBeforeCreateBrowser) + await (browserOptions as any).__testHookBeforeCreateBrowser(); + 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. + if (browserOptions.persistent && !hasCustomArguments) + await browser._defaultContext!._loadDefaultContext(); return browser; } async launchServer(options: LaunchServerOptions = {}): Promise { + assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launchServer`. Use `browserType.launchPersistentContext` instead'); + const { port = 0 } = options; const logger = new RootLogger(options.logger); - return this._launchServer(options, 'server', logger, TimeoutSettings.computeDeadline(options.timeout, 30000)); + const { browserServer, transport } = await this._launchServer(options, false, logger, TimeoutSettings.computeDeadline(options.timeout, 30000)); + browserServer._webSocketWrapper = this._wrapTransportWithWebSocket(transport, logger, port); + return browserServer; } async connect(options: ConnectOptions): Promise { @@ -170,7 +172,12 @@ export abstract class BrowserTypeBase implements BrowserType { let transport: ConnectionTransport | undefined; try { transport = await WebSocketTransport.connect(options.wsEndpoint, logger, deadline); - const promise = this._innerConnectPromise(transport, options, logger); + const browserOptions: BrowserOptions = { + slowMo: options.slowMo, + logger, + }; + copyTestHooks(options, browserOptions); + const promise = this._innerCreateBrowser(transport, browserOptions, false); const browser = await helper.waitWithDeadline(promise, 'connect to browser', deadline, 'pw:browser*'); logger.stopLaunchRecording(); return browser; @@ -189,13 +196,7 @@ export abstract class BrowserTypeBase implements BrowserType { } } - async _innerConnectPromise(transport: ConnectionTransport, options: ConnectOptions, logger: RootLogger): Promise { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); - return this._connectToTransport(transport, { slowMo: options.slowMo, logger }); - } - - private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, logger: RootLogger, deadline: number, userDataDir?: string): Promise { + private async _launchServer(options: LaunchServerOptions, isPersistent: boolean, logger: RootLogger, deadline: number, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, transport: ConnectionTransport }> { const { ignoreDefaultArgs = false, args = [], @@ -204,21 +205,20 @@ export abstract class BrowserTypeBase implements BrowserType { handleSIGINT = true, handleSIGTERM = true, handleSIGHUP = true, - port = 0, } = options; - assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.'); - let temporaryUserDataDir: string | null = null; + const downloadsPath = await mkdtempAsync(DOWNLOADS_FOLDER); + const tempDirectories = [downloadsPath]; if (!userDataDir) { userDataDir = await mkdtempAsync(path.join(os.tmpdir(), `playwright_${this._name}dev_profile-`)); - temporaryUserDataDir = userDataDir; + tempDirectories.push(userDataDir); } const browserArguments = []; if (!ignoreDefaultArgs) - browserArguments.push(...this._defaultArgs(options, launchType, userDataDir)); + browserArguments.push(...this._defaultArgs(options, isPersistent, userDataDir)); else if (Array.isArray(ignoreDefaultArgs)) - browserArguments.push(...this._defaultArgs(options, launchType, userDataDir).filter(arg => ignoreDefaultArgs.indexOf(arg) === -1)); + browserArguments.push(...this._defaultArgs(options, isPersistent, userDataDir).filter(arg => ignoreDefaultArgs.indexOf(arg) === -1)); else browserArguments.push(...args); @@ -230,7 +230,7 @@ export abstract class BrowserTypeBase implements BrowserType { // "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({ + const { launchedProcess, gracefullyClose } = await launchProcess({ executablePath: executable, args: browserArguments, env: this._amendEnvironment(env, userDataDir, executable, browserArguments), @@ -239,7 +239,7 @@ export abstract class BrowserTypeBase implements BrowserType { handleSIGHUP, logger, pipe: !this._webSocketRegexNotPipe, - tempDir: temporaryUserDataDir || undefined, + tempDirectories, attemptToGracefullyClose: async () => { if ((options as any).__testHookGracefullyClose) await (options as any).__testHookGracefullyClose(); @@ -269,14 +269,20 @@ export abstract class BrowserTypeBase implements BrowserType { helper.killProcess(launchedProcess); throw e; } - browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, transport, downloadsPath, - launchType === 'server' ? this._wrapTransportWithWebSocket(transport, logger, port) : null); - return browserServer; + browserServer = new BrowserServer(launchedProcess, gracefullyClose); + return { browserServer, downloadsPath, transport }; } - abstract _defaultArgs(options: BrowserArgOptions, launchType: LaunchType, userDataDir: string): string[]; + abstract _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[]; abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise; abstract _wrapTransportWithWebSocket(transport: ConnectionTransport, logger: InnerLogger, port: number): WebSocketWrapper; abstract _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env; abstract _attemptToGracefullyCloseBrowser(transport: ConnectionTransport): void; } + +function copyTestHooks(from: object, to: object) { + for (const [key, value] of Object.entries(from)) { + if (key.startsWith('__testHook')) + (to as any)[key] = value; + } +} diff --git a/src/server/chromium.ts b/src/server/chromium.ts index 10a721fbab..7438bb8827 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -21,7 +21,7 @@ import { CRBrowser } from '../chromium/crBrowser'; import * as ws from 'ws'; import { Env } from './processLauncher'; import { kBrowserCloseMessageId } from '../chromium/crConnection'; -import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions, LaunchType } from './browserType'; +import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions } from './browserType'; import { WebSocketWrapper } from './browserServer'; import { ConnectionTransport, ProtocolRequest } from '../transport'; import { InnerLogger, logError } from '../logger'; @@ -66,7 +66,7 @@ export class Chromium extends BrowserTypeBase { return wrapTransportWithWebSocket(transport, logger, port); } - _defaultArgs(options: BrowserArgOptions, launchType: LaunchType, userDataDir: string): string[] { + _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[] { const { devtools, headless } = processBrowserArgOptions(options); const { args = [] } = options; const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir')); @@ -89,7 +89,7 @@ export class Chromium extends BrowserTypeBase { ); } chromeArguments.push(...args); - if (launchType === 'persistent') + if (isPersistent) chromeArguments.push('about:blank'); else chromeArguments.push('--no-startup-window'); diff --git a/src/server/electron.ts b/src/server/electron.ts index e3a3c9973d..9303b1371c 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -183,7 +183,7 @@ export class Electron { logger, pipe: true, cwd: options.cwd, - omitDownloads: true, + tempDirectories: [], attemptToGracefullyClose: () => app!.close(), onkill: (exitCode, signal) => { if (app) @@ -198,7 +198,7 @@ export class Electron { const chromeMatch = await waitForLine(launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/, helper.timeUntilDeadline(deadline), timeoutError); const chromeTransport = await WebSocketTransport.connect(chromeMatch[1], logger, deadline); - const browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, chromeTransport, undefined, null); + const browserServer = new BrowserServer(launchedProcess, gracefullyClose); const browser = await CRBrowser.connect(chromeTransport, { headful: true, logger, persistent: { viewport: null }, ownedServer: browserServer }); app = new ElectronApplication(logger, browser, nodeConnection); await app._init(); diff --git a/src/server/firefox.ts b/src/server/firefox.ts index 2157afaa50..ec0fae80be 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -22,7 +22,7 @@ import { FFBrowser } from '../firefox/ffBrowser'; import { kBrowserCloseMessageId } from '../firefox/ffConnection'; import { helper } from '../helper'; import { WebSocketWrapper } from './browserServer'; -import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions, LaunchType } from './browserType'; +import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions } from './browserType'; import { Env } from './processLauncher'; import { ConnectionTransport, SequenceNumberMixer } from '../transport'; import { InnerLogger, logError } from '../logger'; @@ -56,7 +56,7 @@ export class Firefox extends BrowserTypeBase { return wrapTransportWithWebSocket(transport, logger, port); } - _defaultArgs(options: BrowserArgOptions, launchType: LaunchType, userDataDir: string): string[] { + _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[] { const { devtools, headless } = processBrowserArgOptions(options); const { args = [] } = options; if (devtools) @@ -77,7 +77,7 @@ export class Firefox extends BrowserTypeBase { firefoxArguments.push(`-profile`, userDataDir); firefoxArguments.push('-juggler', '0'); firefoxArguments.push(...args); - if (launchType === 'persistent') + if (isPersistent) firefoxArguments.push('about:blank'); else firefoxArguments.push('-silent'); diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index 73c283ed98..b9ff33aaba 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -17,9 +17,6 @@ import * as childProcess from 'child_process'; import { Log, RootLogger } from '../logger'; -import * as fs from 'fs'; -import * as os from 'os'; -import * as path from 'path'; import * as readline from 'readline'; import * as removeFolder from 'rimraf'; import * as stream from 'stream'; @@ -28,8 +25,6 @@ import { TimeoutError } from '../errors'; import { helper } from '../helper'; const removeFolderAsync = util.promisify(removeFolder); -const mkdtempAsync = util.promisify(fs.mkdtemp); -const DOWNLOADS_FOLDER = path.join(os.tmpdir(), 'playwright_downloads-'); const browserLog: Log = { name: 'browser', @@ -55,10 +50,9 @@ export type LaunchProcessOptions = { handleSIGTERM?: boolean, handleSIGHUP?: boolean, pipe?: boolean, - tempDir?: string, + tempDirectories: string[], cwd?: string, - omitDownloads?: boolean, // Note: attemptToGracefullyClose should reject if it does not close the browser. attemptToGracefullyClose: () => Promise, @@ -69,10 +63,15 @@ export type LaunchProcessOptions = { type LaunchResult = { launchedProcess: childProcess.ChildProcess, gracefullyClose: () => Promise, - downloadsPath: string }; export async function launchProcess(options: LaunchProcessOptions): Promise { + const cleanup = async () => { + await Promise.all(options.tempDirectories.map(dir => { + return removeFolderAsync(dir).catch((err: Error) => console.error(err)); + })); + }; + const logger = options.logger; const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe']; logger._log(browserLog, ` ${options.executablePath} ${options.args.join(' ')}`); @@ -90,12 +89,12 @@ export async function launchProcess(options: LaunchProcessOptions): Promise void; - const result = new Promise((f, r) => reject = r); + let failed: (e: Error) => void; + const failedPromise = new Promise((f, r) => failed = f); spawnedProcess.once('error', error => { - reject(new Error('Failed to launch browser: ' + error)); + failed(new Error('Failed to launch browser: ' + error)); }); - return result; + return cleanup().then(() => failedPromise).then(e => Promise.reject(e)); } logger._log(browserLog, ` pid=${spawnedProcess.pid}`); @@ -109,8 +108,6 @@ export async function launchProcess(options: LaunchProcessOptions): Promise {}; const waitForClose = new Promise(f => fulfillClose = f); @@ -122,11 +119,8 @@ export async function launchProcess(options: LaunchProcessOptions): Promise console.error(err)).then(fulfillCleanup); + // Cleanup as process exits. + cleanup().then(fulfillCleanup); }); const listeners = [ helper.addEventListener(process, 'exit', killProcess) ]; @@ -174,16 +168,14 @@ export async function launchProcess(options: LaunchProcessOptions): Promise { diff --git a/src/server/webkit.ts b/src/server/webkit.ts index da093a8dfa..ddd45a121d 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -20,7 +20,7 @@ import { Env } from './processLauncher'; import * as path from 'path'; import { helper } from '../helper'; import { kBrowserCloseMessageId } from '../webkit/wkConnection'; -import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions, LaunchType } from './browserType'; +import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions } from './browserType'; import { ConnectionTransport, SequenceNumberMixer } from '../transport'; import * as ws from 'ws'; import { WebSocketWrapper } from './browserServer'; @@ -49,7 +49,7 @@ export class WebKit extends BrowserTypeBase { return wrapTransportWithWebSocket(transport, logger, port); } - _defaultArgs(options: BrowserArgOptions, launchType: LaunchType, userDataDir: string): string[] { + _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[] { const { devtools, headless } = processBrowserArgOptions(options); const { args = [] } = options; if (devtools) @@ -62,12 +62,12 @@ export class WebKit extends BrowserTypeBase { const webkitArguments = ['--inspector-pipe']; if (headless) webkitArguments.push('--headless'); - if (launchType === 'persistent') + if (isPersistent) webkitArguments.push(`--user-data-dir=${userDataDir}`); else webkitArguments.push(`--no-startup-window`); webkitArguments.push(...args); - if (launchType === 'persistent') + if (isPersistent) webkitArguments.push('about:blank'); return webkitArguments; }