Skip to content

Commit

Permalink
fix(electron): stall node while browser is starting
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Nov 15, 2022
1 parent 6258ed5 commit 1177680
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 52 deletions.
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/android/android.ts
Expand Up @@ -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-');
Expand Down Expand Up @@ -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`);
Expand Down
39 changes: 2 additions & 37 deletions packages/playwright-core/src/server/chromium/chromium.ts
Expand Up @@ -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-');

Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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;
Expand Down
54 changes: 54 additions & 0 deletions 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'
];
4 changes: 3 additions & 1 deletion packages/playwright-core/src/server/electron/electron.ts
Expand Up @@ -80,6 +80,8 @@ export class ElectronApplication extends SdkObject {
await electronHandle.evaluate(({ app }) => app.quit());
});
this._nodeSession.send('Runtime.enable', {}).catch(e => {});
// Delay loading the app until browser is started and the browser targets are configured to auto-attach.
this._nodeSession.send('Runtime.evaluate', { expression: '__playwright_run()' });
}

process(): childProcess.ChildProcess {
Expand Down Expand Up @@ -125,7 +127,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;
Expand Down
31 changes: 31 additions & 0 deletions 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);
};
3 changes: 0 additions & 3 deletions 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(() => {
Expand Down
10 changes: 3 additions & 7 deletions tests/electron/electron-app.spec.ts
Expand Up @@ -131,32 +131,29 @@ 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();
window.addBrowserView(view);
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();
});

test('should return same browser window for browser view pages', 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();
window.addBrowserView(view);
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);
Expand All @@ -181,9 +178,8 @@ test('should record video', async ({ playwright }, testInfo) => {
expect(fs.statSync(videoPath).size).toBeGreaterThan(0);
});

test('should be able to get the first window when with a delayed navigation', async ({ playwright }) => {
test.only('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')],
Expand Down
4 changes: 2 additions & 2 deletions tests/webview2/webView2Test.ts
Expand Up @@ -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>(traceViewerFixtures).extend<PageTestFixtures, PageWorkerFixtures>({
browserVersion: [process.env.PWTEST_WEBVIEW2_CHROMIUM_VERSION, { scope: 'worker' }],
Expand All @@ -39,7 +39,7 @@ export const webView2Test = baseTest.extend<TraceViewerFixtures>(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}`),
}
});
Expand Down

0 comments on commit 1177680

Please sign in to comment.