From 0e3dfd1169ec841352cd6fde330a8415f6034396 Mon Sep 17 00:00:00 2001 From: Jennifer Shehane Date: Thu, 2 May 2024 11:38:44 -0400 Subject: [PATCH] fix: pass all chromium flags through to Electron (#29443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: pass all chromium flags through to Electron * Add changelog entry * exclude webkit from test for navigator.webdriver being true * add debug log for switches in electron * Fix the unit test so it passes now * Update packages/server/lib/environment.js Co-authored-by: Matt Schile * Update packages/server/lib/environment.js Yes 😝 Probably why a better test would be good around this. Co-authored-by: Matt Schile * Update packages/server/test/unit/util/chromium_flags_spec.js Co-authored-by: Bill Glesias * lint fix --------- Co-authored-by: Matt Schile Co-authored-by: Bill Glesias --- cli/CHANGELOG.md | 4 + .../driver/cypress/e2e/issues/27939.cy.js | 8 ++ .../driver/cypress/fixtures/issue-27939.html | 10 ++ packages/server/lib/browsers/chrome.ts | 95 +-------------- packages/server/lib/environment.js | 30 +---- packages/server/lib/util/chromium_flags.ts | 111 ++++++++++++++++++ .../test/unit/util/chromium_flags_spec.js | 30 +++++ 7 files changed, 170 insertions(+), 118 deletions(-) create mode 100644 packages/driver/cypress/e2e/issues/27939.cy.js create mode 100644 packages/driver/cypress/fixtures/issue-27939.html create mode 100644 packages/server/lib/util/chromium_flags.ts create mode 100644 packages/server/test/unit/util/chromium_flags_spec.js diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index ce948985b5..ad46fff7af 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -7,6 +7,10 @@ _Released 5/7/2024 (PENDING)_ - Added more descriptive error messages when Test Replay fails to record or upload. Addresses [#29022](https://github.com/cypress-io/cypress/issues/29022). +**Bugfixes:** + +- We now pass the same default Chromium flags to Electron as we do to Chrome. As a result of this change, the application under test's `navigator.webdriver` property will now correctly be `true` when testing in Electron. Fixes [#27939](https://github.com/cypress-io/cypress/issues/27939). + **Dependency Updates:** - Updated electron from `27.1.3` to `27.3.10` to address [CVE-2024-3156](https://nvd.nist.gov/vuln/detail/CVE-2024-3156). Addressed in [#29431](https://github.com/cypress-io/cypress/pull/29431). diff --git a/packages/driver/cypress/e2e/issues/27939.cy.js b/packages/driver/cypress/e2e/issues/27939.cy.js new file mode 100644 index 0000000000..b361a5d96a --- /dev/null +++ b/packages/driver/cypress/e2e/issues/27939.cy.js @@ -0,0 +1,8 @@ +// Chrome, Firefox, and Electron all have navigator.webdriver set to true in Cypress tests. +// TODO: Webkit should have this set, this needs to be fixed https://github.com/cypress-io/cypress/issues/29446 +// https://w3c.github.io/webdriver/#interface +// https://github.com/cypress-io/cypress/issues/27939 +it('visit', { browser: '!webkit' }, () => { + cy.visit('/fixtures/issue-27939.html') + cy.get('#content').should('have.text', 'navigator.webdriver is true') +}) diff --git a/packages/driver/cypress/fixtures/issue-27939.html b/packages/driver/cypress/fixtures/issue-27939.html new file mode 100644 index 0000000000..dc4675e833 --- /dev/null +++ b/packages/driver/cypress/fixtures/issue-27939.html @@ -0,0 +1,10 @@ + + + +

+ + + \ No newline at end of file diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index 7b72bf29d0..1c4c034436 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -23,6 +23,7 @@ import memory from './memory' import type { BrowserLaunchOpts, BrowserNewTabOpts, ProtocolManagerShape, RunModeVideoApi } from '@packages/types' import type { CDPSocketServer } from '@packages/socket/lib/cdp-socket' +import { DEFAULT_CHROME_FLAGS } from '../util/chromium_flags' const debug = debugModule('cypress:server:browsers:chrome') @@ -47,98 +48,6 @@ type ChromePreferences = { const pathToExtension = extension.getPathToV3Extension() const pathToTheme = extension.getPathToTheme() -const disabledFeatures = [ - // Disable manual option and popup prompt of Chrome translation - // https://github.com/cypress-io/cypress/issues/28225 - 'Translate', - // Disables "Enhanced ad privacy in Chrome" dialog - // https://github.com/cypress-io/cypress/issues/29199 - 'PrivacySandboxSettings4', -] - -// Common Chrome Flags for Automation -// https://github.com/GoogleChrome/chrome-launcher/blob/master/docs/chrome-flags-for-tools.md -const DEFAULT_ARGS = [ - '--test-type', - '--ignore-certificate-errors', - '--start-maximized', - '--silent-debugger-extension-api', - '--no-default-browser-check', - '--no-first-run', - '--noerrdialogs', - '--enable-fixed-layout', - '--disable-popup-blocking', - '--disable-password-generation', - '--disable-single-click-autofill', - '--disable-prompt-on-repos', - '--disable-background-timer-throttling', - '--disable-renderer-backgrounding', - '--disable-renderer-throttling', - '--disable-backgrounding-occluded-windows', - '--disable-restore-session-state', - '--disable-new-profile-management', - '--disable-new-avatar-menu', - '--allow-insecure-localhost', - '--reduce-security-for-testing', - '--enable-automation', - '--disable-print-preview', - '--disable-component-extensions-with-background-pages', - - '--disable-device-discovery-notifications', - - // https://github.com/cypress-io/cypress/issues/2376 - '--autoplay-policy=no-user-gesture-required', - - // http://www.chromium.org/Home/chromium-security/site-isolation - // https://github.com/cypress-io/cypress/issues/1951 - '--disable-site-isolation-trials', - - // the following come from chromedriver - // https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/chromedriver/chrome_launcher.cc&sq=package:chromium&l=70 - '--metrics-recording-only', - '--disable-prompt-on-repost', - '--disable-hang-monitor', - '--disable-sync', - // this flag is causing throttling of XHR callbacks for - // as much as 30 seconds. If you VNC in and open dev tools or - // click on a button, it'll "instantly" work. with this - // option enabled, it will time out some of our tests in circle - // "--disable-background-networking" - '--disable-web-resources', - '--safebrowsing-disable-download-protection', - '--disable-client-side-phishing-detection', - '--disable-component-update', - // Simulate when chrome needs an update. - // This prevents an 'update' from displaying til the given date - `--simulate-outdated-no-au='Tue, 31 Dec 2099 23:59:59 GMT'`, - '--disable-default-apps', - - `--disable-features=${disabledFeatures.join(',')}`, - - // These flags are for webcam/WebRTC testing - // https://github.com/cypress-io/cypress/issues/2704 - '--use-fake-ui-for-media-stream', - '--use-fake-device-for-media-stream', - - // so Cypress commands don't get throttled - // https://github.com/cypress-io/cypress/issues/5132 - '--disable-ipc-flooding-protection', - - // misc. options puppeteer passes - // https://github.com/cypress-io/cypress/issues/3633 - '--disable-backgrounding-occluded-window', - '--disable-breakpad', - '--password-store=basic', - '--use-mock-keychain', - - // write shared memory files into '/tmp' instead of '/dev/shm' - // https://github.com/cypress-io/cypress/issues/5336 - '--disable-dev-shm-usage', - - // enable precise memory info so performance.memory returns more accurate values - '--enable-precise-memory-info', -] - let browserCriClient: BrowserCriClient | undefined /** @@ -384,7 +293,7 @@ export = { }, _getArgs (browser: Browser, options: BrowserLaunchOpts, port: string) { - const args = ([] as string[]).concat(DEFAULT_ARGS) + const args = ([] as string[]).concat(DEFAULT_CHROME_FLAGS) if (os.platform() === 'linux') { args.push('--disable-gpu') diff --git a/packages/server/lib/environment.js b/packages/server/lib/environment.js index c6976bf7d1..91bbee6342 100644 --- a/packages/server/lib/environment.js +++ b/packages/server/lib/environment.js @@ -1,4 +1,5 @@ require('./util/fs') +const DEFAULT_ELECTRON_FLAGS = require('./util/chromium_flags').DEFAULT_ELECTRON_FLAGS const os = require('os') @@ -43,31 +44,10 @@ try { app, } = require('electron') - app.commandLine.appendSwitch('disable-renderer-backgrounding', true) - app.commandLine.appendSwitch('ignore-certificate-errors', true) - - // These flags are for webcam/WebRTC testing - // https://github.com/cypress-io/cypress/issues/2704 - app.commandLine.appendSwitch('use-fake-ui-for-media-stream') - app.commandLine.appendSwitch('use-fake-device-for-media-stream') - - // https://github.com/cypress-io/cypress/issues/2376 - app.commandLine.appendSwitch('autoplay-policy', 'no-user-gesture-required') - - // allows webSecurity: false to work as expected in webPreferences - // https://github.com/electron/electron/issues/18214 - app.commandLine.appendSwitch('disable-site-isolation-trials') - - // prevent electron from using /dev/shm, which can cause crashes in Docker - // https://github.com/cypress-io/cypress/issues/15814 - app.commandLine.appendSwitch('disable-dev-shm-usage') - - // prevent navigation throttling when navigating in the browser rapid fire - // https://github.com/cypress-io/cypress/pull/20271 - app.commandLine.appendSwitch('disable-ipc-flooding-protection') - - // ensure we get the most accurate memory usage - app.commandLine.appendSwitch('enable-precise-memory-info') + debug('appending default switches for electron: %O', DEFAULT_ELECTRON_FLAGS) + DEFAULT_ELECTRON_FLAGS.forEach(({ name, value }) => { + value ? app.commandLine.appendSwitch(name, value) : app.commandLine.appendSwitch(name) + }) if (os.platform() === 'linux') { app.disableHardwareAcceleration() diff --git a/packages/server/lib/util/chromium_flags.ts b/packages/server/lib/util/chromium_flags.ts new file mode 100644 index 0000000000..b4c2996deb --- /dev/null +++ b/packages/server/lib/util/chromium_flags.ts @@ -0,0 +1,111 @@ +const disabledFeatures = [ + // Disable manual option and popup prompt of Chrome translation + // https://github.com/cypress-io/cypress/issues/28225 + 'Translate', + // Disables "Enhanced ad privacy in Chrome" dialog + // https://github.com/cypress-io/cypress/issues/29199 + 'PrivacySandboxSettings4', +] + +// Common Chrome Flags for Automation +// https://github.com/GoogleChrome/chrome-launcher/blob/master/docs/chrome-flags-for-tools.md +const DEFAULT_FLAGS = [ + 'test-type', + 'ignore-certificate-errors', + 'start-maximized', + 'silent-debugger-extension-api', + 'no-default-browser-check', + 'no-first-run', + 'noerrdialogs', + 'enable-fixed-layout', + 'disable-popup-blocking', + 'disable-password-generation', + 'disable-single-click-autofill', + 'disable-prompt-on-repos', + 'disable-background-timer-throttling', + 'disable-renderer-backgrounding', + 'disable-renderer-throttling', + 'disable-backgrounding-occluded-windows', + 'disable-restore-session-state', + 'disable-new-profile-management', + 'disable-new-avatar-menu', + 'allow-insecure-localhost', + 'reduce-security-for-testing', + 'enable-automation', + 'disable-print-preview', + 'disable-component-extensions-with-background-pages', + + 'disable-device-discovery-notifications', + + // https://github.com/cypress-io/cypress/issues/2376 + 'autoplay-policy=no-user-gesture-required', + + // http://www.chromium.org/Home/chromium-security/site-isolation + // https://github.com/electron/electron/issues/18214 + // https://github.com/cypress-io/cypress/issues/1951 + 'disable-site-isolation-trials', + + // the following come from chromedriver + // https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/chromedriver/chrome_launcher.cc&sq=package:chromium&l=70 + 'metrics-recording-only', + 'disable-prompt-on-repost', + 'disable-hang-monitor', + 'disable-sync', + // this flag is causing throttling of XHR callbacks for + // as much as 30 seconds. If you VNC in and open dev tools or + // click on a button, it'll "instantly" work. with this + // option enabled, it will time out some of our tests in circle + // "disable-background-networking" + 'disable-web-resources', + 'safebrowsing-disable-download-protection', + 'disable-client-side-phishing-detection', + 'disable-component-update', + // Simulate when chrome needs an update. + // This prevents an 'update' from displaying til the given date + `simulate-outdated-no-au='Tue, 31 Dec 2099 23:59:59 GMT'`, + 'disable-default-apps', + + `disable-features=${disabledFeatures.join(',')}`, + + // These flags are for webcam/WebRTC testing + // https://github.com/cypress-io/cypress/issues/2704 + 'use-fake-ui-for-media-stream', + 'use-fake-device-for-media-stream', + + // prevent navigation throttling when navigating in the browser rapid fire + // https://github.com/cypress-io/cypress/issues/5132 + // https://github.com/cypress-io/cypress/pull/20271 + 'disable-ipc-flooding-protection', + + // misc. options puppeteer passes + // https://github.com/cypress-io/cypress/issues/3633 + 'disable-backgrounding-occluded-window', + 'disable-breakpad', + 'password-store=basic', + 'use-mock-keychain', + + // write shared memory files into '/tmp' instead of '/dev/shm' + // https://github.com/cypress-io/cypress/issues/5336 + // https://github.com/cypress-io/cypress/issues/15814 + 'disable-dev-shm-usage', + + // enable precise memory info so performance.memory returns more accurate values + 'enable-precise-memory-info', +] + +// prepend -- to each flag and concatenate them together +export const formatChromeFlags = (flags) => flags.map((flag) => `--${flag}`) + +// create an array of objects with name and value properties +// for each flag, splitting the flag on the first = character +export const formatElectronFlags = (flags) => { + return flags.map((flag) => { + const [name, value] = flag.split('=') + + return value ? { name, value } : { name } + }) +} + +export const DEFAULT_CHROME_FLAGS = formatChromeFlags(DEFAULT_FLAGS) + +export const DEFAULT_ELECTRON_FLAGS = formatElectronFlags(DEFAULT_CHROME_FLAGS) diff --git a/packages/server/test/unit/util/chromium_flags_spec.js b/packages/server/test/unit/util/chromium_flags_spec.js new file mode 100644 index 0000000000..4e71ad291a --- /dev/null +++ b/packages/server/test/unit/util/chromium_flags_spec.js @@ -0,0 +1,30 @@ +require('../../spec_helper') + +const { formatChromeFlags, formatElectronFlags } = require(`../../../lib/util/chromium_flags`) + +describe('lib/util/chromium_flags', () => { + context('#formatChromeFlags', () => { + it('formats flags with --', () => { + const flags = ['one', 'two', 'three'] + const chromeFlags = formatChromeFlags(flags) + + expect(chromeFlags).to.deep.eq(['--one', '--two', '--three']) + }) + }) + + context('#formatElectronFlags', () => { + it('formats flags as objects with name', () => { + const flags = ['one', 'two', 'three'] + const electronFlags = formatElectronFlags(flags) + + expect(electronFlags).to.deep.eq([{ name: 'one' }, { name: 'two' }, { name: 'three' }]) + }) + + it('formats flags as objects with name/value pairs', () => { + const flags = ['one=1', 'two=2', 'three'] + const electronFlags = formatElectronFlags(flags) + + expect(electronFlags).to.deep.eq([{ name: 'one', value: '1' }, { name: 'two', value: '2' }, { name: 'three' }]) + }) + }) +})