From d5cf73405c4ffb1055e8cbcd12a0974c46b3ba55 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Fri, 21 Apr 2023 15:33:46 -0500 Subject: [PATCH 01/16] feat: unify cdp approach in electron --- packages/server/lib/browsers/electron.ts | 102 ++++++----------------- packages/server/lib/environment.js | 3 + 2 files changed, 27 insertions(+), 78 deletions(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 4a639209acef..7562744cffeb 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -4,21 +4,21 @@ import path from 'path' import Debug from 'debug' import menu from '../gui/menu' import * as Windows from '../gui/windows' -import { CdpAutomation, screencastOpts, CdpCommand, CdpEvent } from './cdp_automation' +import { CdpAutomation, screencastOpts } from './cdp_automation' import * as savedState from '../saved_state' import utils from './utils' import * as errors from '../errors' import type { Browser, BrowserInstance } from './types' -import type { BrowserWindow, WebContents } from 'electron' +import type { BrowserWindow } from 'electron' import type { Automation } from '../automation' import type { BrowserLaunchOpts, Preferences, RunModeVideoApi } from '@packages/types' import memory from './memory' +import { BrowserCriClient } from './browser-cri-client' // TODO: unmix these two types type ElectronOpts = Windows.WindowOptions & BrowserLaunchOpts const debug = Debug('cypress:server:browsers:electron') -const debugVerbose = Debug('cypress-verbose:server:browsers:electron') // additional events that are nice to know about to be logged // https://electronjs.org/docs/api/browser-window#instance-events @@ -30,6 +30,7 @@ const ELECTRON_DEBUG_EVENTS = [ ] let instance: BrowserInstance | null = null +let browserCriClient: BrowserCriClient | null = null const tryToCall = function (win, method) { try { @@ -46,38 +47,31 @@ const tryToCall = function (win, method) { } const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) { - async function sendCommand (method: CdpCommand, data?: object) { - return tryToCall(win, () => { - return win.webContents.debugger.sendCommand - .call(win.webContents.debugger, method, data) - }) - } + if (!options.onError) throw new Error('Missing onError in electron#_launch') - const on = (eventName: CdpEvent, cb) => { - win.webContents.debugger.on('message', (event, method, params) => { - if (method === eventName) { - cb(params) - } - }) - } + browserCriClient = await BrowserCriClient.create(['127.0.0.1'], 9999, 'electron', options.onError, () => {}) + + const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') const sendClose = () => { win.destroy() } - const automation = await CdpAutomation.create(sendCommand, on, sendClose, parent) + const automation = await CdpAutomation.create(pageCriClient.send, pageCriClient.on, sendClose, parent) automation.onRequest = _.wrap(automation.onRequest, async (fn, message, data) => { switch (message) { case 'take:screenshot': { + // TODO: can we get rid of this now? + // after upgrading to Electron 8, CDP screenshots can hang if a screencast is not also running // workaround: start and stop screencasts between screenshots // @see https://github.com/cypress-io/cypress/pull/6555#issuecomment-596747134 if (!options.videoApi) { - await sendCommand('Page.startScreencast', screencastOpts()) + await pageCriClient.send('Page.startScreencast', screencastOpts()) const ret = await fn(message, data) - await sendCommand('Page.stopScreencast') + await pageCriClient.send('Page.stopScreencast') return ret } @@ -202,11 +196,7 @@ export = { win.maximize() } - const launched = await this._launch(win, url, automation, preferences, options.videoApi) - - automation.use(await _getAutomation(win, preferences, automation)) - - return launched + return await this._launch(win, url, automation, preferences, options.videoApi) }, _launchChild (e, url, parent, projectRoot, state, options, automation) { @@ -247,8 +237,6 @@ export = { }) }) - this._attachDebugger(win.webContents) - const ua = options.userAgent if (ua) { @@ -288,7 +276,7 @@ export = { ]) // enabling can only happen once the window has loaded - await this._enableDebugger(win.webContents) + await this._enableDebugger() await win.loadURL(url) this._listenToOnBeforeHeaders(win) @@ -296,56 +284,10 @@ export = { return win }, - _attachDebugger (webContents) { - try { - webContents.debugger.attach('1.3') - debug('debugger attached') - } catch (err) { - debug('debugger attached failed %o', { err }) - throw err - } - - const originalSendCommand = webContents.debugger.sendCommand - - webContents.debugger.sendCommand = async function (message, data) { - debugVerbose('debugger: sending %s with params %o', message, data) - - try { - const res = await originalSendCommand.call(webContents.debugger, message, data) - - let debugRes = res - - if (debug.enabled && (_.get(debugRes, 'data.length') > 100)) { - debugRes = _.clone(debugRes) - debugRes.data = `${debugRes.data.slice(0, 100)} [truncated]` - } - - debugVerbose('debugger: received response to %s: %o', message, debugRes) - - return res - } catch (err) { - debug('debugger: received error on %s: %o', message, err) - throw err - } - } - - webContents.debugger.sendCommand('Browser.getVersion') - - webContents.debugger.on('detach', (event, reason) => { - debug('debugger detached due to %o', { reason }) - }) - - webContents.debugger.on('message', (event, method, params) => { - if (method === 'Console.messageAdded') { - debug('console message: %o', params.message) - } - }) - }, - - _enableDebugger (webContents: WebContents) { + _enableDebugger () { debug('debugger: enable Console and Network') - return webContents.debugger.sendCommand('Console.enable') + return browserCriClient?.currentlyAttachedTarget?.send('Console.enable') }, _handleDownloads (win, dir, automation) { @@ -373,7 +315,7 @@ export = { // avoid adding redundant `will-download` handlers if session is reused for next spec win.on('closed', () => session.removeListener('will-download', onWillDownload)) - return win.webContents.debugger.sendCommand('Page.setDownloadBehavior', { + return browserCriClient?.currentlyAttachedTarget?.send('Page.setDownloadBehavior', { behavior: 'allow', downloadPath: dir, }) @@ -456,7 +398,7 @@ export = { webContents.userAgent = userAgent // In addition to the session, also set the user-agent optimistically through CDP. @see https://github.com/cypress-io/cypress/issues/23597 - webContents.debugger.sendCommand('Network.setUserAgentOverride', { + browserCriClient?.currentlyAttachedTarget?.send('Network.setUserAgentOverride', { userAgent, }) @@ -476,7 +418,11 @@ export = { /** * Clear instance state for the electron instance, this is normally called on kill or on exit, for electron there isn't any state to clear. */ - clearInstanceState () {}, + clearInstanceState () { + // Do nothing on failure here since we're shutting down anyway + browserCriClient?.close().catch() + browserCriClient = null + }, async connectToNewSpec (browser: Browser, options: ElectronOpts, automation: Automation) { if (!options.url) throw new Error('Missing url in connectToNewSpec') diff --git a/packages/server/lib/environment.js b/packages/server/lib/environment.js index 4e26ec204a6a..eaf2f176a9a7 100644 --- a/packages/server/lib/environment.js +++ b/packages/server/lib/environment.js @@ -69,6 +69,9 @@ try { // ensure we get the most accurate memory usage app.commandLine.appendSwitch('enable-precise-memory-info') + // set up remote debugging port + app.commandLine.appendSwitch('remote-debugging-port', '9999') + if (os.platform() === 'linux') { app.disableHardwareAcceleration() } From 030c2378352dd7fe032993ec5a4e232c6d659f62 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Fri, 21 Apr 2023 16:34:58 -0500 Subject: [PATCH 02/16] fix issue with video --- packages/server/lib/browsers/electron.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 7562744cffeb..56800315b566 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -49,7 +49,9 @@ const tryToCall = function (win, method) { const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) { if (!options.onError) throw new Error('Missing onError in electron#_launch') - browserCriClient = await BrowserCriClient.create(['127.0.0.1'], 9999, 'electron', options.onError, () => {}) + if (!browserCriClient) { + browserCriClient = await BrowserCriClient.create(['127.0.0.1'], 9999, 'electron', options.onError, () => {}) + } const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') @@ -237,6 +239,11 @@ export = { }) }) + await win.loadURL('about:blank') + const cdpAutomation = await this._getAutomation(win, options, automation) + + automation.use(cdpAutomation) + const ua = options.userAgent if (ua) { @@ -265,11 +272,6 @@ export = { this._clearCache(win.webContents), ]) - await win.loadURL('about:blank') - const cdpAutomation = await this._getAutomation(win, options, automation) - - automation.use(cdpAutomation) - await Promise.all([ videoApi && recordVideo(cdpAutomation, videoApi), this._handleDownloads(win, options.downloadsFolder, automation), From 451d2f5d19a0f726f887a4ce4d218dd5126f5207 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Fri, 21 Apr 2023 21:16:46 -0500 Subject: [PATCH 03/16] fix issue with tests --- packages/server/lib/environment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/lib/environment.js b/packages/server/lib/environment.js index eaf2f176a9a7..d4d30fd1d505 100644 --- a/packages/server/lib/environment.js +++ b/packages/server/lib/environment.js @@ -70,7 +70,7 @@ try { app.commandLine.appendSwitch('enable-precise-memory-info') // set up remote debugging port - app.commandLine.appendSwitch('remote-debugging-port', '9999') + app.commandLine.appendSwitch('remote-debugging-port', '8315') if (os.platform() === 'linux') { app.disableHardwareAcceleration() From 8ea0797e0a7654a527574538d0516633a3a512fc Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Sat, 22 Apr 2023 07:38:38 -0500 Subject: [PATCH 04/16] fix port - run ci --- packages/server/lib/browsers/electron.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 56800315b566..a9d318bcb496 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -50,7 +50,7 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) if (!options.onError) throw new Error('Missing onError in electron#_launch') if (!browserCriClient) { - browserCriClient = await BrowserCriClient.create(['127.0.0.1'], 9999, 'electron', options.onError, () => {}) + browserCriClient = await BrowserCriClient.create(['127.0.0.1'], 8315, 'electron', options.onError, () => {}) } const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') From 2abde3284d50698cd3bc6b4731dda50cdce7efe8 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Sun, 23 Apr 2023 19:31:46 -0500 Subject: [PATCH 05/16] fix tests --- packages/server/lib/browsers/electron.ts | 7 ++- packages/server/lib/cypress.js | 5 +- packages/server/lib/environment.js | 3 - packages/server/lib/util/electron-app.js | 31 ++++++++++ .../test/unit/browsers/electron_spec.js | 60 ++++++++++++------- 5 files changed, 76 insertions(+), 30 deletions(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index a9d318bcb496..2ea0335e67c1 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -14,6 +14,7 @@ import type { Automation } from '../automation' import type { BrowserLaunchOpts, Preferences, RunModeVideoApi } from '@packages/types' import memory from './memory' import { BrowserCriClient } from './browser-cri-client' +import { getRemoteDebuggingPort } from '../util/electron-app' // TODO: unmix these two types type ElectronOpts = Windows.WindowOptions & BrowserLaunchOpts @@ -49,9 +50,9 @@ const tryToCall = function (win, method) { const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) { if (!options.onError) throw new Error('Missing onError in electron#_launch') - if (!browserCriClient) { - browserCriClient = await BrowserCriClient.create(['127.0.0.1'], 8315, 'electron', options.onError, () => {}) - } + const port = getRemoteDebuggingPort() + + browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, 'electron', options.onError, () => {}) const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') diff --git a/packages/server/lib/cypress.js b/packages/server/lib/cypress.js index 45c72b4335ad..03d3c9f73dcc 100644 --- a/packages/server/lib/cypress.js +++ b/packages/server/lib/cypress.js @@ -176,7 +176,10 @@ module.exports = { } // make sure we have the appData folder - return require('./util/app_data').ensure() + return Promise.all([ + require('./util/app_data').ensure(), + require('./util/electron-app').setRemoteDebuggingPort(), + ]) .then(() => { // else determine the mode by // the passed in arguments / options diff --git a/packages/server/lib/environment.js b/packages/server/lib/environment.js index d4d30fd1d505..4e26ec204a6a 100644 --- a/packages/server/lib/environment.js +++ b/packages/server/lib/environment.js @@ -69,9 +69,6 @@ try { // ensure we get the most accurate memory usage app.commandLine.appendSwitch('enable-precise-memory-info') - // set up remote debugging port - app.commandLine.appendSwitch('remote-debugging-port', '8315') - if (os.platform() === 'linux') { app.disableHardwareAcceleration() } diff --git a/packages/server/lib/util/electron-app.js b/packages/server/lib/util/electron-app.js index 1877ad70f0e9..2f9cbaa5c2a8 100644 --- a/packages/server/lib/util/electron-app.js +++ b/packages/server/lib/util/electron-app.js @@ -1,9 +1,36 @@ +const getPort = require('get-port') + const scale = () => { try { const { app } = require('electron') return app.commandLine.appendSwitch('force-device-scale-factor', '1') } catch (err) { + // Catch errors for when we're running outside of electron in development + return + } +} + +const getRemoteDebuggingPort = () => { + try { + const { app } = require('electron') + + return app.commandLine.getSwitchValue('remote-debugging-port') + } catch (err) { + // Catch errors for when we're running outside of electron in development + return + } +} + +const setRemoteDebuggingPort = async () => { + try { + const port = await getPort() + const { app } = require('electron') + + // set up remote debugging port + app.commandLine.appendSwitch('remote-debugging-port', String(port)) + } catch (err) { + // Catch errors for when we're running outside of electron in development return } } @@ -26,6 +53,10 @@ const isRunningAsElectronProcess = ({ debug } = {}) => { module.exports = { scale, + getRemoteDebuggingPort, + + setRemoteDebuggingPort, + isRunning, isRunningAsElectronProcess, diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 27d1c545b8a9..8b6c0f66cf3a 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -11,6 +11,8 @@ const Windows = require(`../../../lib/gui/windows`) const electron = require(`../../../lib/browsers/electron`) const savedState = require(`../../../lib/saved_state`) const { Automation } = require(`../../../lib/automation`) +const { BrowserCriClient } = require('../../../lib/browsers/browser-cri-client') +const electronApp = require('../../../lib/util/electron-app') const ELECTRON_PID = 10001 @@ -26,6 +28,7 @@ describe('lib/browsers/electron', () => { browser: { isHeadless: false, }, + onError: () => {}, } this.automation = new Automation('foo', 'bar', 'baz') @@ -54,16 +57,31 @@ describe('lib/browsers/electron', () => { clearCache: sinon.stub(), }, getOSProcessId: sinon.stub().returns(ELECTRON_PID), - 'debugger': { - attach: sinon.stub().returns(), - sendCommand: sinon.stub().resolves(), - on: sinon.stub().returns(), - }, }, }) sinon.stub(Windows, 'installExtension').returns() sinon.stub(Windows, 'removeAllExtensions').returns() + sinon.stub(electronApp, 'getRemoteDebuggingPort').resolves(1234) + + // mock CRI client during testing + this.pageCriClient = { + send: sinon.stub().resolves(), + // Page: { + // screencastFrame: sinon.stub().returns(), + // }, + // close: sinon.stub().resolves(), + on: sinon.stub(), + } + + this.browserCriClient = { + attachToTargetUrl: sinon.stub().resolves(this.pageCriClient), + currentlyAttachedTarget: this.pageCriClient, + // close: sinon.stub().resolves(), + // ensureMinimumProtocolVersion: sinon.stub().withArgs('1.3').resolves(), + } + + sinon.stub(BrowserCriClient, 'create').resolves(this.browserCriClient) this.stubForOpen = function () { sinon.stub(electron, '_render').resolves(this.win) @@ -189,7 +207,6 @@ describe('lib/browsers/electron', () => { context('._launch', () => { beforeEach(() => { sinon.stub(menu, 'set') - sinon.stub(electron, '_attachDebugger').resolves() sinon.stub(electron, '_clearCache').resolves() sinon.stub(electron, '_setProxy').resolves() sinon.stub(electron, '_setUserAgent') @@ -197,13 +214,13 @@ describe('lib/browsers/electron', () => { }) it('sets menu.set whether or not its in headless mode', function () { - return electron._launch(this.win, this.url, this.automation, { show: true }) + return electron._launch(this.win, this.url, this.automation, { show: true, onError: () => {} }) .then(() => { expect(menu.set).to.be.calledWith({ withInternalDevTools: true }) }).then(() => { menu.set.reset() - return electron._launch(this.win, this.url, this.automation, { show: false }) + return electron._launch(this.win, this.url, this.automation, { show: false, onError: () => {} }) }).then(() => { expect(menu.set).not.to.be.called }) @@ -214,7 +231,7 @@ describe('lib/browsers/electron', () => { .then(() => { expect(electron._setUserAgent).not.to.be.called }).then(() => { - return electron._launch(this.win, this.url, this.automation, { userAgent: 'foo' }) + return electron._launch(this.win, this.url, this.automation, { userAgent: 'foo', onError: () => {} }) }).then(() => { expect(electron._setUserAgent).to.be.calledWith(this.win.webContents, 'foo') }) @@ -225,7 +242,7 @@ describe('lib/browsers/electron', () => { .then(() => { expect(electron._setProxy).not.to.be.called }).then(() => { - return electron._launch(this.win, this.url, this.automation, { proxyServer: 'foo' }) + return electron._launch(this.win, this.url, this.automation, { proxyServer: 'foo', onError: () => {} }) }).then(() => { expect(electron._setProxy).to.be.calledWith(this.win.webContents, 'foo') }) @@ -295,7 +312,7 @@ describe('lib/browsers/electron', () => { return electron._launch(this.win, this.url, this.automation, this.options) .then(() => { - expect(this.win.webContents.debugger.sendCommand).to.be.calledWith('Page.setDownloadBehavior', { + expect(this.pageCriClient.send).to.be.calledWith('Page.setDownloadBehavior', { behavior: 'allow', downloadPath: 'downloads', }) @@ -507,13 +524,10 @@ describe('lib/browsers/electron', () => { describe('setUserAgent with experimentalModifyObstructiveThirdPartyCode', () => { let userAgent - let originalSendCommandSpy beforeEach(function () { userAgent = '' this.win.webContents.session.getUserAgent.callsFake(() => userAgent) - // set a reference to the original sendCommand as it is decorated in electron.ts. This way, we can assert on the spy - originalSendCommandSpy = this.win.webContents.debugger.sendCommand }) describe('disabled', function () { @@ -523,7 +537,7 @@ describe('lib/browsers/electron', () => { return electron._launch(this.win, this.url, this.automation, this.options) .then(() => { expect(this.win.webContents.session.setUserAgent).not.to.be.called - expect(originalSendCommandSpy).not.to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).not.to.be.calledWith('Network.setUserAgentOverride', { userAgent, }) }) @@ -544,7 +558,7 @@ describe('lib/browsers/electron', () => { .then(() => { expect(this.win.webContents.session.setUserAgent).to.be.calledWith('foobar') expect(this.win.webContents.session.setUserAgent).not.to.be.calledWith('barbaz') - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: 'foobar', }) }) @@ -558,7 +572,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) @@ -572,7 +586,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) @@ -586,7 +600,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.113 Safari/537.36' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) @@ -600,7 +614,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Teams/1.5.00.4689 Chrome/85.0.4183.121 Safari/537.36' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) @@ -614,7 +628,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Typora/0.9.93 Chrome/83.0.4103.119 Safari/E7FBAF' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) @@ -629,7 +643,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) @@ -643,7 +657,7 @@ describe('lib/browsers/electron', () => { const expectedUA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36' expect(this.win.webContents.session.setUserAgent).to.have.been.calledWith(expectedUA) - expect(originalSendCommandSpy).to.be.calledWith('Network.setUserAgentOverride', { + expect(this.pageCriClient.send).to.be.calledWith('Network.setUserAgentOverride', { userAgent: expectedUA, }) }) From 26c49bfd2d8832f346abe9d5970ec71a5aea07a5 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Sun, 23 Apr 2023 19:46:39 -0500 Subject: [PATCH 06/16] fix tests --- .../server/test/integration/cypress_spec.js | 19 ++++++++++++++----- .../test/unit/browsers/electron_spec.js | 6 ------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/server/test/integration/cypress_spec.js b/packages/server/test/integration/cypress_spec.js index aa33184b1bbf..b815f520f806 100644 --- a/packages/server/test/integration/cypress_spec.js +++ b/packages/server/test/integration/cypress_spec.js @@ -125,11 +125,6 @@ function mockEE () { ee.loadURL = () => {} ee.focusOnWebView = () => {} ee.webContents = { - debugger: { - on: sinon.stub(), - attach: sinon.stub(), - sendCommand: sinon.stub().resolves(), - }, getOSProcessId: sinon.stub(), setUserAgent: sinon.stub(), session: { @@ -1055,6 +1050,20 @@ describe('lib/cypress', () => { }) it('electron', function () { + // mock CRI client during testing + this.pageCriClient = { + send: sinon.stub().resolves(), + on: sinon.stub(), + } + + this.browserCriClient = { + attachToTargetUrl: sinon.stub().resolves(this.pageCriClient), + currentlyAttachedTarget: this.pageCriClient, + close: sinon.stub().resolves(), + } + + sinon.stub(BrowserCriClient, 'create').resolves(this.browserCriClient) + videoCapture.start.returns() return cypress.start([ diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 8b6c0f66cf3a..09c7d5ff5f21 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -67,18 +67,12 @@ describe('lib/browsers/electron', () => { // mock CRI client during testing this.pageCriClient = { send: sinon.stub().resolves(), - // Page: { - // screencastFrame: sinon.stub().returns(), - // }, - // close: sinon.stub().resolves(), on: sinon.stub(), } this.browserCriClient = { attachToTargetUrl: sinon.stub().resolves(this.pageCriClient), currentlyAttachedTarget: this.pageCriClient, - // close: sinon.stub().resolves(), - // ensureMinimumProtocolVersion: sinon.stub().withArgs('1.3').resolves(), } sinon.stub(BrowserCriClient, 'create').resolves(this.browserCriClient) From 198b47b33cf8c01fb8cdf5168e362c69ba03e8ab Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Sun, 23 Apr 2023 20:19:57 -0500 Subject: [PATCH 07/16] improve cleanup --- packages/server/lib/browsers/electron.ts | 8 +++++++- .../server/test/integration/cypress_spec.js | 19 +++++++++++-------- .../test/unit/browsers/electron_spec.js | 4 ++++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 2ea0335e67c1..d7334ef45264 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -52,7 +52,9 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) const port = getRemoteDebuggingPort() - browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, 'electron', options.onError, () => {}) + if (!browserCriClient) { + browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, 'electron', options.onError, () => {}) + } const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') @@ -114,6 +116,10 @@ async function recordVideo (cdpAutomation: CdpAutomation, videoApi: RunModeVideo } export = { + // For testing + _clearBrowserCriClient () { + browserCriClient = null + }, _defaultOptions (projectRoot: string | undefined, state: Preferences, options: BrowserLaunchOpts, automation: Automation): ElectronOpts { const _this = this diff --git a/packages/server/test/integration/cypress_spec.js b/packages/server/test/integration/cypress_spec.js index b815f520f806..effe98ea43fa 100644 --- a/packages/server/test/integration/cypress_spec.js +++ b/packages/server/test/integration/cypress_spec.js @@ -1050,19 +1050,19 @@ describe('lib/cypress', () => { }) it('electron', function () { - // mock CRI client during testing - this.pageCriClient = { - send: sinon.stub().resolves(), + // during testing, do not try to connect to the remote interface or + // use the Chrome remote interface client + const criClient = { on: sinon.stub(), + send: sinon.stub(), } - - this.browserCriClient = { - attachToTargetUrl: sinon.stub().resolves(this.pageCriClient), - currentlyAttachedTarget: this.pageCriClient, + const browserCriClient = { + ensureMinimumProtocolVersion: sinon.stub().resolves(), + attachToTargetUrl: sinon.stub().resolves(criClient), close: sinon.stub().resolves(), } - sinon.stub(BrowserCriClient, 'create').resolves(this.browserCriClient) + sinon.stub(BrowserCriClient, 'create').resolves(browserCriClient) videoCapture.start.returns() @@ -1077,6 +1077,9 @@ describe('lib/cypress', () => { onNewWindow: sinon.match.func, }) + expect(BrowserCriClient.create).to.have.been.calledOnce + expect(browserCriClient.attachToTargetUrl).to.have.been.calledOnce + this.expectExitWith(0) }) }) diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 09c7d5ff5f21..a1839b34ca93 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -91,6 +91,10 @@ describe('lib/browsers/electron', () => { } }) + afterEach(function () { + electron._clearBrowserCriClient() + }) + context('.connectToNewSpec', () => { it('calls open with the browser, url, options, and automation', async function () { sinon.stub(electron, 'open').withArgs({ isHeaded: true }, 'http://www.example.com', { url: 'http://www.example.com' }, this.automation) From 75bb681592efc5d52e30b5a6f3c96e8204ff6c50 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 24 Apr 2023 08:56:09 -0500 Subject: [PATCH 08/16] remove code that is no longer needed --- packages/server/lib/browsers/electron.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index d7334ef45264..0f0ed5f19b15 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -66,23 +66,6 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) automation.onRequest = _.wrap(automation.onRequest, async (fn, message, data) => { switch (message) { - case 'take:screenshot': { - // TODO: can we get rid of this now? - - // after upgrading to Electron 8, CDP screenshots can hang if a screencast is not also running - // workaround: start and stop screencasts between screenshots - // @see https://github.com/cypress-io/cypress/pull/6555#issuecomment-596747134 - if (!options.videoApi) { - await pageCriClient.send('Page.startScreencast', screencastOpts()) - const ret = await fn(message, data) - - await pageCriClient.send('Page.stopScreencast') - - return ret - } - - return fn(message, data) - } case 'focus:browser:window': { win.show() From 05f21023e52a582983f3681c5f7704d031c5bd69 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 24 Apr 2023 09:55:52 -0500 Subject: [PATCH 09/16] Revert "remove code that is no longer needed" This reverts commit 75bb681592efc5d52e30b5a6f3c96e8204ff6c50. --- packages/server/lib/browsers/electron.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 0f0ed5f19b15..d7334ef45264 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -66,6 +66,23 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) automation.onRequest = _.wrap(automation.onRequest, async (fn, message, data) => { switch (message) { + case 'take:screenshot': { + // TODO: can we get rid of this now? + + // after upgrading to Electron 8, CDP screenshots can hang if a screencast is not also running + // workaround: start and stop screencasts between screenshots + // @see https://github.com/cypress-io/cypress/pull/6555#issuecomment-596747134 + if (!options.videoApi) { + await pageCriClient.send('Page.startScreencast', screencastOpts()) + const ret = await fn(message, data) + + await pageCriClient.send('Page.stopScreencast') + + return ret + } + + return fn(message, data) + } case 'focus:browser:window': { win.show() From b936a61d78034a2312c0f3d7f2eb249acc8ec78e Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 24 Apr 2023 09:56:28 -0500 Subject: [PATCH 10/16] minor rework --- packages/server/lib/browsers/electron.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index d7334ef45264..944399fd45dc 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -67,8 +67,6 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) automation.onRequest = _.wrap(automation.onRequest, async (fn, message, data) => { switch (message) { case 'take:screenshot': { - // TODO: can we get rid of this now? - // after upgrading to Electron 8, CDP screenshots can hang if a screencast is not also running // workaround: start and stop screencasts between screenshots // @see https://github.com/cypress-io/cypress/pull/6555#issuecomment-596747134 From b8927b5cfc4e6e79e2cfcedc3e4185bf92382b93 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 24 Apr 2023 12:14:10 -0500 Subject: [PATCH 11/16] add changelog --- cli/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index a58671cb1ec9..de8bd9fe7d1d 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -11,6 +11,7 @@ _Released 04/25/2023 (PENDING)_ - Fixed an issue where setting `videoCompression` to `0` would cause the video output to be broken. `0` is now treated as false. Addresses [#5191](https://github.com/cypress-io/cypress/issues/5191) and [#24595](https://github.com/cypress-io/cypress/issues/24595). - Fixed an issue on the [Debug page](https://on.cypress.io/debug-page) where the passing run status would appear even if the Cypress Cloud organization was over its monthly test result limit. Addresses [#26528](https://github.com/cypress-io/cypress/issues/26528). +- Fixed an issue in Electron where devtools gets out of sync with the DOM occasionally. Addresses [#15932](https://github.com/cypress-io/cypress/issues/15932). ## 12.10.0 From b3c8e1e13e35016b028b46cb4b6bde5606782679 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 24 Apr 2023 13:25:33 -0500 Subject: [PATCH 12/16] improve cleanup --- packages/server/lib/browsers/electron.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 944399fd45dc..d331ebfe6787 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -503,11 +503,15 @@ export = { return win.webContents.getOSProcessId() }) + const clearInstanceState = this.clearInstanceState + instance = _.extend(events, { pid: mainPid, allPids: [mainPid], browserWindow: win, kill (this: BrowserInstance) { + clearInstanceState() + if (this.isProcessExit) { // if the process is exiting, all BrowserWindows will be destroyed anyways return From 116deb80724079243a67ffe28f994a513bdb262b Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 24 Apr 2023 16:52:55 -0500 Subject: [PATCH 13/16] build binary --- .circleci/workflows.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.circleci/workflows.yml b/.circleci/workflows.yml index c6147aad94d2..983d3dfe2a1c 100644 --- a/.circleci/workflows.yml +++ b/.circleci/workflows.yml @@ -30,7 +30,7 @@ mainBuildFilters: &mainBuildFilters - /^release\/\d+\.\d+\.\d+$/ # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - 'update-v8-snapshot-cache-on-develop' - - 'matth/misc/telemetry' + - 'ryanm/feat/unify-cdp-approach-in-electron' # usually we don't build Mac app - it takes a long time # but sometimes we want to really confirm we are doing the right thing @@ -41,6 +41,7 @@ macWorkflowFilters: &darwin-workflow-filters - equal: [ develop, << pipeline.git.branch >> ] # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ] + - equal: [ 'ryanm/feat/unify-cdp-approach-in-electron', << pipeline.git.branch >> ] - matches: pattern: /^release\/\d+\.\d+\.\d+$/ value: << pipeline.git.branch >> @@ -51,6 +52,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters - equal: [ develop, << pipeline.git.branch >> ] # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ] + - equal: [ 'ryanm/feat/unify-cdp-approach-in-electron', << pipeline.git.branch >> ] - matches: pattern: /^release\/\d+\.\d+\.\d+$/ value: << pipeline.git.branch >> @@ -71,6 +73,7 @@ windowsWorkflowFilters: &windows-workflow-filters # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - equal: [ 'windows-flake', << pipeline.git.branch >> ] - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ] + - equal: [ 'ryanm/feat/unify-cdp-approach-in-electron', << pipeline.git.branch >> ] - matches: pattern: /^release\/\d+\.\d+\.\d+$/ value: << pipeline.git.branch >> @@ -136,7 +139,7 @@ commands: - run: name: Check current branch to persist artifacts command: | - if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "update-v8-snapshot-cache-on-develop" && "$CIRCLE_BRANCH" != "matth/chore/add-circle-ci-detector" ]]; then + if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "update-v8-snapshot-cache-on-develop" && "$CIRCLE_BRANCH" != "ryanm/feat/unify-cdp-approach-in-electron" ]]; then echo "Not uploading artifacts or posting install comment for this branch." circleci-agent step halt fi From 848df947b518615d0ea3a6e17413dd2f44f9971e Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Thu, 27 Apr 2023 12:48:36 -0500 Subject: [PATCH 14/16] Update CHANGELOG.md --- cli/CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 2ea32480f083..b9d128b176be 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,4 +1,13 @@ + +## 12.11.1 + +_Released 05/09/2023 (Pending)_ + +**Bugfixes:** + +- Fixed an issue in Electron where devtools gets out of sync with the DOM occasionally. Addresses [#15932](https://github.com/cypress-io/cypress/issues/15932). + ## 12.11.0 _Released 04/26/2023_ @@ -12,7 +21,6 @@ _Released 04/26/2023_ - Fixed an issue where setting `videoCompression` to `0` would cause the video output to be broken. `0` is now treated as false. Addresses [#5191](https://github.com/cypress-io/cypress/issues/5191) and [#24595](https://github.com/cypress-io/cypress/issues/24595). - Fixed an issue on the [Debug page](https://on.cypress.io/debug-page) where the passing run status would appear even if the Cypress Cloud organization was over its monthly test result limit. Addresses [#26528](https://github.com/cypress-io/cypress/issues/26528). -- Fixed an issue in Electron where devtools gets out of sync with the DOM occasionally. Addresses [#15932](https://github.com/cypress-io/cypress/issues/15932). **Misc:** From fdaaddd3a7a811988211678ec70d99bb300b12d2 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Thu, 27 Apr 2023 12:50:05 -0500 Subject: [PATCH 15/16] Update CHANGELOG.md --- cli/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index b9d128b176be..76ea9498bd9f 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,5 +1,4 @@ - ## 12.11.1 _Released 05/09/2023 (Pending)_ From 3fbbc9daf0113fea97711352a1d4d22c2c9ee3c3 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Thu, 27 Apr 2023 12:51:39 -0500 Subject: [PATCH 16/16] Update cli/CHANGELOG.md --- cli/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 76ea9498bd9f..e558e39b6320 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,7 +1,7 @@ ## 12.11.1 -_Released 05/09/2023 (Pending)_ +_Released 05/09/2023 (PENDING)_ **Bugfixes:**