From 8eca6339c207c9f93d6775b8a6e7bc14b7eaa4f8 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 3 Aug 2022 17:32:29 -0700 Subject: [PATCH] =?UTF-8?q?feat(reuse):=20account=20for=20the=20browser=20?= =?UTF-8?q?launch=20args=20when=20reusing=20the=20bro=E2=80=A6=20(#16229)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit feat(reuse): account for the browser launch args when reusing the browsers --- .../src/grid/gridBrowserWorker.ts | 2 +- .../src/remote/playwrightConnection.ts | 39 ++++++++++++++++--- .../src/remote/playwrightServer.ts | 18 +++++++-- .../src/server/android/android.ts | 3 +- .../playwright-core/src/server/browser.ts | 1 + .../playwright-core/src/server/browserType.ts | 1 + .../src/server/chromium/chromium.ts | 1 + .../src/server/electron/electron.ts | 1 + packages/playwright-test/src/index.ts | 3 +- 9 files changed, 58 insertions(+), 11 deletions(-) diff --git a/packages/playwright-core/src/grid/gridBrowserWorker.ts b/packages/playwright-core/src/grid/gridBrowserWorker.ts index 7aca8dca2a..2638c516b0 100644 --- a/packages/playwright-core/src/grid/gridBrowserWorker.ts +++ b/packages/playwright-core/src/grid/gridBrowserWorker.ts @@ -23,7 +23,7 @@ function launchGridBrowserWorker(gridURL: string, agentId: string, workerId: str const log = debug(`pw:grid:worker:${workerId}`); log('created'); const ws = new WebSocket(gridURL.replace('http://', 'ws://') + `/registerWorker?agentId=${agentId}&workerId=${workerId}`); - new PlaywrightConnection('auto', ws, { enableSocksProxy: true, browserAlias, headless: true }, { playwright: null, browser: null }, log, async () => { + new PlaywrightConnection('auto', ws, { enableSocksProxy: true, browserAlias, launchOptions: {} }, { playwright: null, browser: null }, log, async () => { log('exiting process'); setTimeout(() => process.exit(0), 30000); // Meanwhile, try to gracefully close all browsers. diff --git a/packages/playwright-core/src/remote/playwrightConnection.ts b/packages/playwright-core/src/remote/playwrightConnection.ts index 3446fe19d3..812a9bf637 100644 --- a/packages/playwright-core/src/remote/playwrightConnection.ts +++ b/packages/playwright-core/src/remote/playwrightConnection.ts @@ -24,11 +24,12 @@ import { registry } from '../server'; import { SocksProxy } from '../common/socksProxy'; import type { Mode } from './playwrightServer'; import { assert } from '../utils'; +import type { LaunchOptions } from '../server/types'; type Options = { enableSocksProxy: boolean, browserAlias: string | null, - headless: boolean, + launchOptions: LaunchOptions, }; type PreLaunched = { @@ -99,7 +100,7 @@ export class PlaywrightConnection { const socksProxy = this._options.enableSocksProxy ? await this._enableSocksProxy(playwright) : undefined; const browser = await playwright[executable.browserName!].launch(serverSideCallMetadata(), { channel: executable.type === 'browser' ? undefined : executable.name, - headless: this._options.headless, + headless: this._options.launchOptions?.headless, }); // Close the browser on disconnect. @@ -132,15 +133,18 @@ export class PlaywrightConnection { this._debugLog(`engaged reuse browsers mode for ${this._options.browserAlias}`); const executable = this._executableForBrowerAlias(this._options.browserAlias!); const playwright = this._preLaunched.playwright!; - - let browser = playwright.allBrowsers().find(b => b.options.name === executable.browserName); + const requestedOptions = launchOptionsHash(this._options.launchOptions); + let browser = playwright.allBrowsers().find(b => { + const existingOptions = launchOptionsHash(b.options.originalLaunchOptions); + return existingOptions === requestedOptions; + }); const remaining = playwright.allBrowsers().filter(b => b !== browser); for (const r of remaining) await r.close(); if (!browser) { browser = await playwright[executable.browserName!].launch(serverSideCallMetadata(), { - channel: executable.type === 'browser' ? undefined : executable.name, + ...this._options.launchOptions, headless: false, }); browser.on(Browser.Events.Disconnected, () => { @@ -189,3 +193,28 @@ export class PlaywrightConnection { return executable; } } + +function launchOptionsHash(options: LaunchOptions) { + const copy = { ...options }; + for (const k of Object.keys(copy)) { + const key = k as keyof LaunchOptions; + if (copy[key] === defaultLaunchOptions[key]) + delete copy[key]; + } + for (const key of optionsThatAllowBrowserReuse) + delete copy[key]; + return JSON.stringify(copy); +} + +const defaultLaunchOptions: LaunchOptions = { + ignoreAllDefaultArgs: false, + handleSIGINT: false, + handleSIGTERM: false, + handleSIGHUP: false, + headless: true, + devtools: false, +}; + +const optionsThatAllowBrowserReuse: (keyof LaunchOptions)[] = [ + 'headless', +]; diff --git a/packages/playwright-core/src/remote/playwrightServer.ts b/packages/playwright-core/src/remote/playwrightServer.ts index cacf70dbf0..4357f48dd6 100644 --- a/packages/playwright-core/src/remote/playwrightServer.ts +++ b/packages/playwright-core/src/remote/playwrightServer.ts @@ -23,6 +23,7 @@ import { createPlaywright } from '../server/playwright'; import { PlaywrightConnection } from './playwrightConnection'; import { assert } from '../utils'; import { serverSideCallMetadata } from '../server/instrumentation'; +import type { LaunchOptions } from '../server/types'; const debugLog = debug('pw:server'); @@ -120,17 +121,28 @@ export class PlaywrightServer { const url = new URL('http://localhost' + (request.url || '')); const browserHeader = request.headers['x-playwright-browser']; const browserAlias = url.searchParams.get('browser') || (Array.isArray(browserHeader) ? browserHeader[0] : browserHeader) || null; - const headlessHeader = request.headers['x-playwright-headless']; - const headlessValue = url.searchParams.get('headless') || (Array.isArray(headlessHeader) ? headlessHeader[0] : headlessHeader); const proxyHeader = request.headers['x-playwright-proxy']; const proxyValue = url.searchParams.get('proxy') || (Array.isArray(proxyHeader) ? proxyHeader[0] : proxyHeader); const enableSocksProxy = this._options.enableSocksProxy && proxyValue === '*'; + + const launchOptionsHeader = request.headers['x-playwright-launch-options'] || ''; + let launchOptions: LaunchOptions = {}; + try { + launchOptions = JSON.parse(Array.isArray(launchOptionsHeader) ? launchOptionsHeader[0] : launchOptionsHeader); + } catch (e) { + } + + const headlessHeader = request.headers['x-playwright-headless']; + const headlessValue = url.searchParams.get('headless') || (Array.isArray(headlessHeader) ? headlessHeader[0] : headlessHeader); + if (headlessValue && headlessValue !== '0') + launchOptions.headless = true; + this._clientsCount++; const log = newLogger(); log(`serving connection: ${request.url}`); const connection = new PlaywrightConnection( this._mode, ws, - { enableSocksProxy, browserAlias, headless: headlessValue !== '0' }, + { enableSocksProxy, browserAlias, launchOptions }, { playwright: this._preLaunchedPlaywright, browser: this._options.preLaunchedBrowser || null }, log, () => this._clientsCount--); (ws as any)[kConnectionSymbol] = connection; diff --git a/packages/playwright-core/src/server/android/android.ts b/packages/playwright-core/src/server/android/android.ts index 996b007902..c0643cc06a 100644 --- a/packages/playwright-core/src/server/android/android.ts +++ b/packages/playwright-core/src/server/android/android.ts @@ -290,7 +290,8 @@ export class AndroidDevice extends SdkObject { browserProcess: new ClankBrowserProcess(androidBrowser), proxy: options.proxy, protocolLogger: helper.debugProtocolLogger(), - browserLogsCollector: new RecentLogsCollector() + browserLogsCollector: new RecentLogsCollector(), + originalLaunchOptions: {}, }; validateBrowserContextOptions(options, browserOptions); diff --git a/packages/playwright-core/src/server/browser.ts b/packages/playwright-core/src/server/browser.ts index 0c7a12d1f0..42197006f5 100644 --- a/packages/playwright-core/src/server/browser.ts +++ b/packages/playwright-core/src/server/browser.ts @@ -57,6 +57,7 @@ export type BrowserOptions = PlaywrightOptions & { browserLogsCollector: RecentLogsCollector, slowMo?: number; wsEndpoint?: string; // Only there when connected over web socket. + originalLaunchOptions: types.LaunchOptions; }; export abstract class Browser extends SdkObject { diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index 9ace20e4b0..d98655be24 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -123,6 +123,7 @@ export abstract class BrowserType extends SdkObject { protocolLogger, browserLogsCollector, wsEndpoint: options.useWebSocket ? (transport as WebSocketTransport).wsEndpoint : undefined, + originalLaunchOptions: options, }; if (persistent) validateBrowserContextOptions(persistent, browserOptions); diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index f28b314a9a..72805c5514 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -114,6 +114,7 @@ export class Chromium extends BrowserType { // users in normal (launch/launchServer) mode since otherwise connectOverCDP // does not work at all with proxies on Windows. proxy: { server: 'per-context' }, + originalLaunchOptions: {}, }; validateBrowserContextOptions(persistent, browserOptions); progress.throwIfAborted(); diff --git a/packages/playwright-core/src/server/electron/electron.ts b/packages/playwright-core/src/server/electron/electron.ts index 2541f6a186..d118e9a11c 100644 --- a/packages/playwright-core/src/server/electron/electron.ts +++ b/packages/playwright-core/src/server/electron/electron.ts @@ -226,6 +226,7 @@ export class Electron extends SdkObject { artifactsDir, downloadsPath: artifactsDir, tracesDir: artifactsDir, + originalLaunchOptions: {}, }; validateBrowserContextOptions(contextOptions, browserOptions); const browser = await CRBrowser.connect(chromeTransport, browserOptions); diff --git a/packages/playwright-test/src/index.ts b/packages/playwright-test/src/index.ts index d2ad26d4c4..c45bcf121b 100644 --- a/packages/playwright-test/src/index.ts +++ b/packages/playwright-test/src/index.ts @@ -106,7 +106,7 @@ export const test = _baseTest.extend({ (browserType as any)._defaultLaunchOptions = undefined; }, { scope: 'worker', auto: true }], - _connectedBrowser: [async ({ playwright, browserName, channel, headless, connectOptions }, use) => { + _connectedBrowser: [async ({ playwright, browserName, channel, headless, connectOptions, launchOptions }, use) => { if (!connectOptions) { await use(undefined); return; @@ -117,6 +117,7 @@ export const test = _baseTest.extend({ headers: { 'x-playwright-browser': channel || browserName, 'x-playwright-headless': headless ? '1' : '0', + 'x-playwright-launch-options': JSON.stringify(launchOptions), ...connectOptions.headers, }, timeout: connectOptions.timeout ?? 3 * 60 * 1000, // 3 minutes