Skip to content

Commit

Permalink
fix: ensure that chromium based browsers do not send out a lot of fon…
Browse files Browse the repository at this point in the history
…t requests when global styles change (#28217)
  • Loading branch information
ryanthemanuel authored Nov 3, 2023
1 parent 8c4a106 commit 5dbebe6
Show file tree
Hide file tree
Showing 17 changed files with 286 additions and 119 deletions.
3 changes: 2 additions & 1 deletion cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ _Released 11/7/2023 (PENDING)_
**Bugfixes:**

- Fixed an issue determining visibility when an element is hidden by an ancestor with a shared edge. Fixes [#27514](https://github.com/cypress-io/cypress/issues/27514).
- Fixed an issue with 'other' targets (e.g. pdf documents embedded in an object tag) not fully loading. Fixes [#28228](https://github.com/cypress-io/cypress/issues/28228)
- Fixed an issue where in chromium based browsers, global style updates can trigger flooding of font face requests in DevTools and Test Replay. This can affect performance due to the flooding of messages in CDP. Fixes [#28150](https://github.com/cypress-io/cypress/issues/28150) and [#28215](https://github.com/cypress-io/cypress/issues/28215).
- Fixed an issue with 'other' targets (e.g. pdf documents embedded in an object tag) not fully loading. Fixes [#28228](https://github.com/cypress-io/cypress/issues/28228) and [#28162](https://github.com/cypress-io/cypress/issues/28162).
- Fixed an issue where network requests made from tabs/windows other than the main Cypress tab would be delayed. Fixes [#28113](https://github.com/cypress-io/cypress/issues/28113).
- Stopped processing CDP events at the end of a spec when Test Isolation is off and Test Replay is enabled. Addressed in [#28213](https://github.com/cypress-io/cypress/pull/28213).

Expand Down
4 changes: 0 additions & 4 deletions packages/driver/cypress/e2e/commands/actions/click.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -1678,16 +1678,12 @@ describe('src/cy/commands/actions/click', () => {
it('can scroll to and click elements in html with scroll-behavior: smooth', () => {
cy.get('html').invoke('css', 'scrollBehavior', 'smooth')
cy.get('#table tr:first').click()
// Validate that the scrollBehavior is still smooth even after the actionability fixes we do
cy.get('html').invoke('css', 'scrollBehavior').then((scrollBehavior) => expect(scrollBehavior).to.eq('smooth'))
})

// https://github.com/cypress-io/cypress/issues/3200
it('can scroll to and click elements in ancestor element with scroll-behavior: smooth', () => {
cy.get('#dom').invoke('css', 'scrollBehavior', 'smooth')
cy.get('#table tr:first').click()
// Validate that the scrollBehavior is still smooth even after the actionability fixes we do
cy.get('#dom').invoke('css', 'scrollBehavior').then((scrollBehavior) => expect(scrollBehavior).to.eq('smooth'))
})
})
})
Expand Down
25 changes: 1 addition & 24 deletions packages/driver/cypress/e2e/dom/visibility.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('src/cypress/dom/visibility', () => {
expect(fn()).to.be.true
})

it('returns false if window and body < window height', () => {
it('returns false window and body > window height', () => {
cy.$$('body').html('<div>foo</div>')

const win = cy.state('window')
Expand All @@ -65,29 +65,6 @@ describe('src/cypress/dom/visibility', () => {
expect(fn()).to.be.false
})

it('returns true if document element and body > window height', function () {
this.add('<div style="height: 1000px; width: 10px;" />')
const documentElement = Cypress.dom.wrap(cy.state('document').documentElement)

const fn = () => {
return dom.isScrollable(documentElement)
}

expect(fn()).to.be.true
})

it('returns false if document element and body < window height', () => {
cy.$$('body').html('<div>foo</div>')

const documentElement = Cypress.dom.wrap(cy.state('document').documentElement)

const fn = () => {
return dom.isScrollable(documentElement)
}

expect(fn()).to.be.false
})

it('returns false el is not scrollable', function () {
const noScroll = this.add(`\
<div style="height: 100px; overflow: auto;">
Expand Down
44 changes: 11 additions & 33 deletions packages/driver/src/cy/actionability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import $utils from './../cypress/utils'
import type { ElWindowPostion, ElViewportPostion, ElementPositioning } from '../dom/coordinates'
import $elements from '../dom/elements'
import $errUtils from '../cypress/error_utils'
import { callNativeMethod, getNativeProp } from '../dom/elements/nativeProps'
const debug = debugFn('cypress:driver:actionability')

const delay = 50
Expand Down Expand Up @@ -461,46 +460,24 @@ const verify = function (cy, $el, config, options, callbacks: VerifyCallbacks) {
// make scrolling occur instantly. we do this by adding a style tag
// and then removing it after we finish scrolling
// https://github.com/cypress-io/cypress/issues/3200
const addScrollBehaviorFix = (element: JQuery<HTMLElement>) => {
const affectedParents: Map<HTMLElement, string> = new Map()
const addScrollBehaviorFix = () => {
let style

try {
let parent: JQuery<HTMLElement> | null = element
const doc = $el.get(0).ownerDocument

do {
if ($dom.isScrollable(parent)) {
const parentElement = parent[0]
const style = getNativeProp(parentElement, 'style')
const styles = getComputedStyle(parentElement)

if (styles.scrollBehavior === 'smooth') {
affectedParents.set(parentElement, callNativeMethod(style, 'getStyleProperty', 'scroll-behavior'))
callNativeMethod(style, 'setStyleProperty', 'scroll-behavior', 'auto')
}
}

parent = $dom.getFirstScrollableParent(parent)
} while (parent)
style = doc.createElement('style')
style.innerHTML = '* { scroll-behavior: inherit !important; }'
// there's guaranteed to be a <script> tag, so that's the safest thing
// to query for and add the style tag after
doc.querySelector('script').after(style)
} catch (err) {
// the above shouldn't error, but out of an abundance of caution, we
// ignore any errors since this fix isn't worth failing the test over
}

return () => {
for (const [parent, value] of affectedParents) {
const style = getNativeProp(parent, 'style')

if (value === '') {
if (callNativeMethod(style, 'getStyleProperty', 'length') === 1) {
callNativeMethod(parent, 'removeAttribute', 'style')
} else {
callNativeMethod(style, 'removeProperty', 'scroll-behavior')
}
} else {
callNativeMethod(style, 'setStyleProperty', 'scroll-behavior', value)
}
}
affectedParents.clear()
if (style) style.remove()
}
}

Expand All @@ -523,7 +500,8 @@ const verify = function (cy, $el, config, options, callbacks: VerifyCallbacks) {
if (options.scrollBehavior !== false) {
// scroll the element into view
const scrollBehavior = scrollBehaviorOptionsMap[options.scrollBehavior]
const removeScrollBehaviorFix = addScrollBehaviorFix($el)

const removeScrollBehaviorFix = addScrollBehaviorFix()

debug('scrollIntoView:', $el[0])
$el.get(0).scrollIntoView({ block: scrollBehavior })
Expand Down
6 changes: 0 additions & 6 deletions packages/driver/src/dom/elements/complexElements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,6 @@ export const isScrollable = ($el) => {
return false
}

const documentElement = $document.getDocumentFromElement(el).documentElement

if (el === documentElement) {
return checkDocumentElement($window.getWindowByElement(el), el)
}

// if we're any other element, we do some css calculations
// to see that the overflow is correct and the scroll
// area is larger than the actual height or width
Expand Down
5 changes: 0 additions & 5 deletions packages/driver/src/dom/elements/nativeProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ const nativeGetters = {
body: descriptor('Document', 'body').get,
frameElement: Object.getOwnPropertyDescriptor(window, 'frameElement')!.get,
maxLength: _getMaxLength,
style: descriptor('HTMLElement', 'style').get,
}

const nativeSetters = {
Expand All @@ -225,16 +224,12 @@ const nativeMethods = {
execCommand: window.document.execCommand,
getAttribute: window.Element.prototype.getAttribute,
setAttribute: window.Element.prototype.setAttribute,
removeAttribute: window.Element.prototype.removeAttribute,
setSelectionRange: _nativeSetSelectionRange,
modify: window.Selection.prototype.modify,
focus: _nativeFocus,
hasFocus: window.document.hasFocus,
blur: _nativeBlur,
select: _nativeSelect,
getStyleProperty: window.CSSStyleDeclaration.prototype.getPropertyValue,
setStyleProperty: window.CSSStyleDeclaration.prototype.setProperty,
removeStyleProperty: window.CSSStyleDeclaration.prototype.removeProperty,
}

export const getNativeProp = function<T, K extends keyof T> (obj: T, prop: K): T[K] {
Expand Down
3 changes: 3 additions & 0 deletions packages/server/lib/browsers/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,9 @@ export = {
args.push(`--remote-debugging-port=${port}`)
args.push('--remote-debugging-address=127.0.0.1')

// control memory caching per execution context so that font flooding does not occur: https://github.com/cypress-io/cypress/issues/28215
args.push('--enable-features=ScopeMemoryCachePerContext')

return args
},

Expand Down
9 changes: 7 additions & 2 deletions packages/server/lib/cypress.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,19 +164,24 @@ module.exports = {
options.headed = false
}

const electronApp = require('./util/electron-app')

if (options.runProject && !options.headed) {
debug('scaling electron app in headless mode')
// scale the electron browser window
// to force retina screens to not
// upsample their images when offscreen
// rendering
require('./util/electron-app').scale()
electronApp.scale()
}

// control memory caching per execution context so that font flooding does not occur: https://github.com/cypress-io/cypress/issues/28215
electronApp.setScopeMemoryCachePerContext()

// make sure we have the appData folder
return Promise.all([
require('./util/app_data').ensure(),
require('./util/electron-app').setRemoteDebuggingPort(),
electronApp.setRemoteDebuggingPort(),
])
.then(() => {
// else determine the mode by
Expand Down
13 changes: 13 additions & 0 deletions packages/server/lib/util/electron-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ const setRemoteDebuggingPort = async () => {
}
}

const setScopeMemoryCachePerContext = () => {
try {
const { app } = require('electron')

app.commandLine.appendSwitch('enable-features', 'ScopeMemoryCachePerContext')
} catch (err) {
// Catch errors for when we're running outside of electron in development
return
}
}

const isRunning = () => {
// are we in the electron or the node process?
return Boolean(process.env.ELECTRON_RUN_AS_NODE || process.versions && process.versions.electron)
Expand All @@ -60,6 +71,8 @@ const isRunningAsElectronProcess = ({ debug } = {}) => {
module.exports = {
scale,

setScopeMemoryCachePerContext,

getRemoteDebuggingPort,

setRemoteDebuggingPort,
Expand Down
44 changes: 0 additions & 44 deletions system-tests/__snapshots__/protocol_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6678,50 +6678,6 @@ exports['component events - experimentalSingleTabRunMode: true'] = `
"pageLoading": [],
"resetTest": [],
"responseEndedWithEmptyBody": [
{
"requestId": "Any.Number",
"isCached": true,
"timings": {
"cdpRequestWillBeSentTimestamp": "Any.Number",
"cdpRequestWillBeSentReceivedTimestamp": "Any.Number",
"proxyRequestReceivedTimestamp": "Any.Number",
"cdpLagDuration": "Any.Number",
"proxyRequestCorrelationDuration": "Any.Number"
}
},
{
"requestId": "Any.Number",
"isCached": true,
"timings": {
"cdpRequestWillBeSentTimestamp": "Any.Number",
"cdpRequestWillBeSentReceivedTimestamp": "Any.Number",
"proxyRequestReceivedTimestamp": "Any.Number",
"cdpLagDuration": "Any.Number",
"proxyRequestCorrelationDuration": "Any.Number"
}
},
{
"requestId": "Any.Number",
"isCached": true,
"timings": {
"cdpRequestWillBeSentTimestamp": "Any.Number",
"cdpRequestWillBeSentReceivedTimestamp": "Any.Number",
"proxyRequestReceivedTimestamp": "Any.Number",
"cdpLagDuration": "Any.Number",
"proxyRequestCorrelationDuration": "Any.Number"
}
},
{
"requestId": "Any.Number",
"isCached": true,
"timings": {
"cdpRequestWillBeSentTimestamp": "Any.Number",
"cdpRequestWillBeSentReceivedTimestamp": "Any.Number",
"proxyRequestReceivedTimestamp": "Any.Number",
"cdpLagDuration": "Any.Number",
"proxyRequestCorrelationDuration": "Any.Number"
}
},
{
"requestId": "Any.Number",
"isCached": true,
Expand Down
103 changes: 103 additions & 0 deletions system-tests/lib/protocol-stubs/protocolStubFontFlooding.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import path from 'path'
import fs from 'fs-extra'
import type { AppCaptureProtocolInterface, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions, ResponseStreamTimedOutOptions } from '@packages/types'
import type { Readable } from 'stream'

const getFilePath = (filename) => {
return path.join(
path.resolve(__dirname),
'cypress',
'system-tests-protocol-dbs',
`${filename}.json`,
)
}

export class AppCaptureProtocol implements AppCaptureProtocolInterface {
private filename: string
private events = {
numberOfFontRequests: 0,
}
private cdpClient: any

getDbMetadata (): { offset: number, size: number } {
return {
offset: 0,
size: 0,
}
}

responseStreamReceived (options: ResponseStreamOptions): Readable {
return options.responseStream
}

connectToBrowser = async (cdpClient) => {
if (cdpClient) {
this.cdpClient = cdpClient
}

this.cdpClient.on('Network.requestWillBeSent', (params) => {
// For the font flooding test, we want to count the number of font requests.
// There should only be 2 requests. One for each test in the spec.
if (params.type === 'Font') {
this.events.numberOfFontRequests += 1
}
})
}

addRunnables = (runnables) => {
return Promise.resolve()
}

beforeSpec = ({ archivePath, db }) => {
this.filename = getFilePath(path.basename(db.name))

if (!fs.existsSync(archivePath)) {
// If a dummy file hasn't been created by the test, write a tar file so that it can be fake uploaded
fs.writeFileSync(archivePath, '')
}
}

async afterSpec (): Promise<void> {
try {
fs.outputFileSync(this.filename, JSON.stringify(this.events, null, 2))
} catch (e) {
console.log('error writing protocol events', e)
}
}

beforeTest = (test) => {
return Promise.resolve()
}

commandLogAdded = (log) => {
}

commandLogChanged = (log) => {
}

viewportChanged = (input) => {
}

urlChanged = (input) => {
}

pageLoading = (input) => {
}

preAfterTest = (test, options) => {
return Promise.resolve()
}

afterTest = (test) => {
return Promise.resolve()
}

responseEndedWithEmptyBody = (options: ResponseEndedWithEmptyBodyOptions) => {
}

responseStreamTimedOut (options: ResponseStreamTimedOutOptions): void {
}

resetTest (testId: string): void {
}
}
Loading

5 comments on commit 5dbebe6

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 5dbebe6 Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.4.1/linux-arm64/develop-5dbebe6e5e7fca3af9716bfa8b80830b87641cf4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 5dbebe6 Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.4.1/linux-x64/develop-5dbebe6e5e7fca3af9716bfa8b80830b87641cf4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 5dbebe6 Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.4.1/darwin-x64/develop-5dbebe6e5e7fca3af9716bfa8b80830b87641cf4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 5dbebe6 Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.4.1/darwin-arm64/develop-5dbebe6e5e7fca3af9716bfa8b80830b87641cf4/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 5dbebe6 Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.4.1/win32-x64/develop-5dbebe6e5e7fca3af9716bfa8b80830b87641cf4/cypress.tgz

Please sign in to comment.