From 61dc43610b397d6a654ac810de886e1df19a81ef Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 14 Nov 2022 19:05:25 -0800 Subject: [PATCH] fix(electron): stall node while browser is starting --- .../src/server/android/android.ts | 4 +- .../src/server/chromium/chromium.ts | 39 +------------- .../src/server/chromium/chromiumSwitches.ts | 54 +++++++++++++++++++ .../src/server/electron/electron.ts | 10 +++- .../src/server/electron/loader.ts | 31 +++++++++++ tests/electron/electron-app.js | 3 -- tests/electron/electron-app.spec.ts | 8 +-- tests/webview2/webView2Test.ts | 4 +- 8 files changed, 101 insertions(+), 52 deletions(-) create mode 100644 packages/playwright-core/src/server/chromium/chromiumSwitches.ts create mode 100644 packages/playwright-core/src/server/electron/loader.ts diff --git a/packages/playwright-core/src/server/android/android.ts b/packages/playwright-core/src/server/android/android.ts index ae0aa9644ca27..3b5182ae12048 100644 --- a/packages/playwright-core/src/server/android/android.ts +++ b/packages/playwright-core/src/server/android/android.ts @@ -35,7 +35,7 @@ import { gracefullyCloseSet } from '../../utils/processLauncher'; import { TimeoutSettings } from '../../common/timeoutSettings'; import type * as channels from '@protocol/channels'; import { SdkObject, serverSideCallMetadata } from '../instrumentation'; -import { DEFAULT_ARGS } from '../chromium/chromium'; +import { chromiumSwitches } from '../chromium/chromiumSwitches'; import { registry } from '../registry'; const ARTIFACTS_FOLDER = path.join(os.tmpdir(), 'playwright-artifacts-'); @@ -269,7 +269,7 @@ export class AndroidDevice extends SdkObject { '--disable-fre', '--no-default-browser-check', `--remote-debugging-socket-name=${socketName}`, - ...DEFAULT_ARGS, + ...chromiumSwitches, ].join(' '); debug('pw:android')('Starting', pkg, commandLine); await this._backend.runCommand(`shell:echo "${commandLine}" > /data/local/tmp/chrome-command-line`); diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index 2619d17c977f3..7e2b24458cba2 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -48,6 +48,7 @@ import https from 'https'; import { registry } from '../registry'; import { ManualPromise } from '../../utils/manualPromise'; import { validateBrowserContextOptions } from '../browserContext'; +import { chromiumSwitches } from './chromiumSwitches'; const ARTIFACTS_FOLDER = path.join(os.tmpdir(), 'playwright-artifacts-'); @@ -282,7 +283,7 @@ export class Chromium extends BrowserType { throw new Error('Playwright manages remote debugging connection itself.'); if (args.find(arg => !arg.startsWith('-'))) throw new Error('Arguments can not specify page to be opened'); - const chromeArguments = [...DEFAULT_ARGS]; + const chromeArguments = [...chromiumSwitches]; // See https://github.com/microsoft/playwright/issues/7362 if (os.platform() === 'darwin') @@ -325,42 +326,6 @@ export class Chromium extends BrowserType { } } -export const DEFAULT_ARGS = [ - '--disable-field-trial-config', // https://source.chromium.org/chromium/chromium/src/+/main:testing/variations/README.md - '--disable-background-networking', - '--enable-features=NetworkService,NetworkServiceInProcess', - '--disable-background-timer-throttling', - '--disable-backgrounding-occluded-windows', - '--disable-back-forward-cache', // Avoids surprises like main request not being intercepted during page.goBack(). - '--disable-breakpad', - '--disable-client-side-phishing-detection', - '--disable-component-extensions-with-background-pages', - '--disable-component-update', // Avoids unneeded network activity after startup. - '--no-default-browser-check', - '--disable-default-apps', - '--disable-dev-shm-usage', - '--disable-extensions', - // AvoidUnnecessaryBeforeUnloadCheckSync - https://github.com/microsoft/playwright/issues/14047 - // Translate - https://github.com/microsoft/playwright/issues/16126 - '--disable-features=ImprovedCookieControls,LazyFrameLoading,GlobalMediaControls,DestroyProfileOnBrowserClose,MediaRouter,DialMediaRouteProvider,AcceptCHFrame,AutoExpandDetailsElement,CertificateTransparencyComponentUpdater,AvoidUnnecessaryBeforeUnloadCheckSync,Translate', - '--allow-pre-commit-input', - '--disable-hang-monitor', - '--disable-ipc-flooding-protection', - '--disable-popup-blocking', - '--disable-prompt-on-repost', - '--disable-renderer-backgrounding', - '--disable-sync', - '--force-color-profile=srgb', - '--metrics-recording-only', - '--no-first-run', - '--enable-automation', - '--password-store=basic', - '--use-mock-keychain', - // See https://chromium-review.googlesource.com/c/chromium/src/+/2436773 - '--no-service-autorun', - '--export-tagged-pdf' -]; - async function urlToWSEndpoint(progress: Progress, endpointURL: string) { if (endpointURL.startsWith('ws')) return endpointURL; diff --git a/packages/playwright-core/src/server/chromium/chromiumSwitches.ts b/packages/playwright-core/src/server/chromium/chromiumSwitches.ts new file mode 100644 index 0000000000000..9fbf9b1423149 --- /dev/null +++ b/packages/playwright-core/src/server/chromium/chromiumSwitches.ts @@ -0,0 +1,54 @@ +/** + * Copyright 2017 Google Inc. All rights reserved. + * Modifications copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// No dependencies as it is used from the Electron loader. + +export const chromiumSwitches = [ + '--disable-field-trial-config', // https://source.chromium.org/chromium/chromium/src/+/main:testing/variations/README.md + '--disable-background-networking', + '--enable-features=NetworkService,NetworkServiceInProcess', + '--disable-background-timer-throttling', + '--disable-backgrounding-occluded-windows', + '--disable-back-forward-cache', // Avoids surprises like main request not being intercepted during page.goBack(). + '--disable-breakpad', + '--disable-client-side-phishing-detection', + '--disable-component-extensions-with-background-pages', + '--disable-component-update', // Avoids unneeded network activity after startup. + '--no-default-browser-check', + '--disable-default-apps', + '--disable-dev-shm-usage', + '--disable-extensions', + // AvoidUnnecessaryBeforeUnloadCheckSync - https://github.com/microsoft/playwright/issues/14047 + // Translate - https://github.com/microsoft/playwright/issues/16126 + '--disable-features=ImprovedCookieControls,LazyFrameLoading,GlobalMediaControls,DestroyProfileOnBrowserClose,MediaRouter,DialMediaRouteProvider,AcceptCHFrame,AutoExpandDetailsElement,CertificateTransparencyComponentUpdater,AvoidUnnecessaryBeforeUnloadCheckSync,Translate', + '--allow-pre-commit-input', + '--disable-hang-monitor', + '--disable-ipc-flooding-protection', + '--disable-popup-blocking', + '--disable-prompt-on-repost', + '--disable-renderer-backgrounding', + '--disable-sync', + '--force-color-profile=srgb', + '--metrics-recording-only', + '--no-first-run', + '--enable-automation', + '--password-store=basic', + '--use-mock-keychain', + // See https://chromium-review.googlesource.com/c/chromium/src/+/2436773 + '--no-service-autorun', + '--export-tagged-pdf' +]; diff --git a/packages/playwright-core/src/server/electron/electron.ts b/packages/playwright-core/src/server/electron/electron.ts index d1251e017b2eb..618a0229629a1 100644 --- a/packages/playwright-core/src/server/electron/electron.ts +++ b/packages/playwright-core/src/server/electron/electron.ts @@ -79,7 +79,12 @@ export class ElectronApplication extends SdkObject { const electronHandle = await this._nodeElectronHandlePromise; await electronHandle.evaluate(({ app }) => app.quit()); }); - this._nodeSession.send('Runtime.enable', {}).catch(e => {}); + } + + async initialize() { + await this._nodeSession.send('Runtime.enable', {}); + // Delay loading the app until browser is started and the browser targets are configured to auto-attach. + await this._nodeSession.send('Runtime.evaluate', { expression: '__playwright_run()' }); } process(): childProcess.ChildProcess { @@ -125,7 +130,7 @@ export class Electron extends SdkObject { controller.setLogName('browser'); return controller.run(async progress => { let app: ElectronApplication | undefined = undefined; - const electronArguments = [...args, '--inspect=0', '--remote-debugging-port=0']; + const electronArguments = [require.resolve('./loader'), options.cwd || process.cwd(), ...args, '--inspect=0', '--remote-debugging-port=0']; if (os.platform() === 'linux') { const runningAsRoot = process.geteuid && process.geteuid() === 0; @@ -231,6 +236,7 @@ export class Electron extends SdkObject { validateBrowserContextOptions(contextOptions, browserOptions); const browser = await CRBrowser.connect(chromeTransport, browserOptions); app = new ElectronApplication(this, browser, nodeConnection, launchedProcess); + await app.initialize(); return app; }, TimeoutSettings.timeout(options)); } diff --git a/packages/playwright-core/src/server/electron/loader.ts b/packages/playwright-core/src/server/electron/loader.ts new file mode 100644 index 0000000000000..b5124e61b72fe --- /dev/null +++ b/packages/playwright-core/src/server/electron/loader.ts @@ -0,0 +1,31 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const { app } = require('electron'); +const path = require('path'); +const { chromiumSwitches } = require('../chromium/chromiumSwitches'); + +const appPath = path.resolve(process.argv[2], process.argv[3]); + +for (const arg of chromiumSwitches) { + const match = arg.match(/--([^=]*)=?(.*)/)!; + app.commandLine.appendSwitch(match[1], match[2]); + app.getAppPath = () => path.dirname(appPath); +} + +(globalThis as any).__playwright_run = () => { + require(appPath); +}; diff --git a/tests/electron/electron-app.js b/tests/electron/electron-app.js index c2faa9644d73f..0e688711994a6 100644 --- a/tests/electron/electron-app.js +++ b/tests/electron/electron-app.js @@ -1,9 +1,6 @@ const { app, protocol } = require('electron'); const path = require('path'); -app.commandLine.appendSwitch('disable-features', 'AutoExpandDetailsElement'); -app.commandLine.appendSwitch('allow-pre-commit-input') - app.on('window-all-closed', e => e.preventDefault()); app.whenReady().then(() => { diff --git a/tests/electron/electron-app.spec.ts b/tests/electron/electron-app.spec.ts index 27c8e78cf95e0..e5f3c7c68e90b 100644 --- a/tests/electron/electron-app.spec.ts +++ b/tests/electron/electron-app.spec.ts @@ -131,7 +131,6 @@ test('should create page for browser view', async ({ playwright }) => { const app = await playwright._electron.launch({ args: [path.join(__dirname, 'electron-window-app.js')], }); - const browserViewPagePromise = app.waitForEvent('window'); await app.evaluate(async electron => { const window = electron.BrowserWindow.getAllWindows()[0]; const view = new electron.BrowserView(); @@ -139,8 +138,7 @@ test('should create page for browser view', async ({ playwright }) => { await view.webContents.loadURL('about:blank'); view.setBounds({ x: 0, y: 0, width: 256, height: 256 }); }); - await browserViewPagePromise; - expect(app.windows()).toHaveLength(2); + await expect.poll(() => app.windows().length).toBe(2); await app.close(); }); @@ -148,7 +146,6 @@ test('should return same browser window for browser view pages', async ({ playwr const app = await playwright._electron.launch({ args: [path.join(__dirname, 'electron-window-app.js')], }); - const browserViewPagePromise = app.waitForEvent('window'); await app.evaluate(async electron => { const window = electron.BrowserWindow.getAllWindows()[0]; const view = new electron.BrowserView(); @@ -156,7 +153,7 @@ test('should return same browser window for browser view pages', async ({ playwr await view.webContents.loadURL('about:blank'); view.setBounds({ x: 0, y: 0, width: 256, height: 256 }); }); - await browserViewPagePromise; + await expect.poll(() => app.windows().length).toBe(2); const [firstWindowId, secondWindowId] = await Promise.all( app.windows().map(async page => { const bwHandle = await app.browserWindow(page); @@ -183,7 +180,6 @@ test('should record video', async ({ playwright }, testInfo) => { test('should be able to get the first window when with a delayed navigation', async ({ playwright }) => { test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/17765' }); - test.fixme(); const app = await playwright._electron.launch({ args: [path.join(__dirname, 'electron-window-app-delayed-loadURL.js')], diff --git a/tests/webview2/webView2Test.ts b/tests/webview2/webView2Test.ts index 0e8a0c08e080e..fb4685cef4ef3 100644 --- a/tests/webview2/webView2Test.ts +++ b/tests/webview2/webView2Test.ts @@ -23,7 +23,7 @@ import type { TraceViewerFixtures } from '../config/traceViewerFixtures'; import { traceViewerFixtures } from '../config/traceViewerFixtures'; export { expect } from '@playwright/test'; import { TestChildProcess } from '../config/commonFixtures'; -import { DEFAULT_ARGS } from '../../packages/playwright-core/lib/server/chromium/chromium'; +import { chromiumSwitches } from '../../packages/playwright-core/lib/server/chromium/chromiumSwitches'; export const webView2Test = baseTest.extend(traceViewerFixtures).extend({ browserVersion: [process.env.PWTEST_WEBVIEW2_CHROMIUM_VERSION, { scope: 'worker' }], @@ -39,7 +39,7 @@ export const webView2Test = baseTest.extend(traceViewerFixt shell: true, env: { ...process.env, - WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS: `--remote-debugging-port=${cdpPort} ${DEFAULT_ARGS.join(' ')}`, + WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS: `--remote-debugging-port=${cdpPort} ${chromiumSwitches.join(' ')}`, WEBVIEW2_USER_DATA_FOLDER: path.join(fs.realpathSync.native(os.tmpdir()), `playwright-webview2-tests/user-data-dir-${testInfo.workerIndex}`), } });