Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unify cdp approach to fix devtools in electron #26573

Merged
merged 20 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .circleci/workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 >>
Expand All @@ -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 >>
Expand All @@ -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 >>
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
<!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
## 12.11.1

_Released 05/09/2023 (Pending)_
ryanthemanuel marked this conversation as resolved.
Show resolved Hide resolved

**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_
Expand Down
119 changes: 38 additions & 81 deletions packages/server/lib/browsers/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,6 +31,7 @@ const ELECTRON_DEBUG_EVENTS = [
]

let instance: BrowserInstance | null = null
let browserCriClient: BrowserCriClient | null = null

const tryToCall = function (win, method) {
try {
Expand All @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
})

Expand All @@ -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')
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion packages/server/lib/cypress.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions packages/server/lib/util/electron-app.js
Original file line number Diff line number Diff line change
@@ -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
}
}
Expand All @@ -26,6 +53,10 @@ const isRunningAsElectronProcess = ({ debug } = {}) => {
module.exports = {
scale,

getRemoteDebuggingPort,

setRemoteDebuggingPort,

isRunning,

isRunningAsElectronProcess,
Expand Down
Loading