fix: unify cdp approach to fix devtools in electron

.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

cli/CHANGELOG.md

@@ -12,6 +12,7 @@ _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:**
 

packages/server/lib/browsers/electron.ts

@@ -4,21 +4,22 @@ 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'
+import { getRemoteDebuggingPort } from '../util/electron-app'
 
 // 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 +31,7 @@ const ELECTRON_DEBUG_EVENTS = [
 ]
 
 let instance: BrowserInstance | null = null
+let browserCriClient: BrowserCriClient | null = null
 
 const tryToCall = function (win, method) {
   try {
@@ -46,26 +48,21 @@ 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)
-      }
-    })
+  const port = getRemoteDebuggingPort()
+
+  if (!browserCriClient) {
+    browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, '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) {
@@ -74,10 +71,10 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent)
         // 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
         }
@@ -117,6 +114,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
 
@@ -202,11 +203,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,7 +244,10 @@ export = {
       })
     })
 
-    this._attachDebugger(win.webContents)
+    await win.loadURL('about:blank')
+    const cdpAutomation = await this._getAutomation(win, options, automation)
+
+    automation.use(cdpAutomation)
 
     const ua = options.userAgent
 
@@ -277,75 +277,24 @@ 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),
     ])
 
     // enabling can only happen once the window has loaded
-    await this._enableDebugger(win.webContents)
+    await this._enableDebugger()
 
     await win.loadURL(url)
     this._listenToOnBeforeHeaders(win)
 
     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 +322,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 +405,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 +425,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')
@@ -550,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

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

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,

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 () {
+          // 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(),
+          }
+          const browserCriClient = {
+            ensureMinimumProtocolVersion: sinon.stub().resolves(),
+            attachToTargetUrl: sinon.stub().resolves(criClient),
+            close: sinon.stub().resolves(),
+          }
+
+          sinon.stub(BrowserCriClient, 'create').resolves(browserCriClient)
+
           videoCapture.start.returns()
 
           return cypress.start([
@@ -1068,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)
           })
         })