Skip to content

Commit

Permalink
fix: performance regression due to enabling ScopeMemoryCachePerContext (
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanthemanuel authored Nov 14, 2023
1 parent e308603 commit 16d6430
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 36 deletions.
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ _Released 11/21/2023 (PENDING)_

**Bugfixes:**

- Fixed a regression in [`13.5.0`](https://docs.cypress.io/guides/references/changelog/13.5.0) where requests cached within a given spec may take longer to load than they did previously. Addresses [#28295](https://github.com/cypress-io/cypress/issues/28295).
- Fixed an issue where pages opened in a new tab were missing response headers, causing them not to load properly. Fixes [#28293](https://github.com/cypress-io/cypress/issues/28293) and [#28303](https://github.com/cypress-io/cypress/issues/28303).
- We now pass a flag to Chromium browsers to disable default component extensions. This is a common flag passed during browser automation. Fixed in [#28294](https://github.com/cypress-io/cypress/pull/28294).

Expand Down
13 changes: 13 additions & 0 deletions packages/driver/cypress/e2e/commands/actions/click.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -1678,12 +1678,25 @@ 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/28150
it('can scroll to and click elements in html with scroll-behavior: smooth and overflow-y: auto', () => {
cy.get('html').invoke('css', 'scrollBehavior', 'smooth')
cy.get('body').invoke('css', 'overflow-y', 'auto')
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: 24 additions & 1 deletion 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 window and body > window height', () => {
it('returns false if window and body < window height', () => {
cy.$$('body').html('<div>foo</div>')

const win = cy.state('window')
Expand All @@ -65,6 +65,29 @@ 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: 33 additions & 11 deletions packages/driver/src/cy/actionability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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 @@ -460,24 +461,46 @@ 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 = () => {
let style
const addScrollBehaviorFix = (element: JQuery<HTMLElement>) => {
const affectedParents: Map<HTMLElement, string> = new Map()

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

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)
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.getParent(parent)
} while (parent.length)
} 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 () => {
if (style) style.remove()
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()
}
}

Expand All @@ -500,8 +523,7 @@ 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()
const removeScrollBehaviorFix = addScrollBehaviorFix($el)

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

// If we're at the documentElement, we check its size against the window
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: 5 additions & 0 deletions packages/driver/src/dom/elements/nativeProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ const nativeGetters = {
body: descriptor('Document', 'body').get,
frameElement: Object.getOwnPropertyDescriptor(window, 'frameElement')!.get,
maxLength: _getMaxLength,
style: descriptor('HTMLElement', 'style').get,
}

const nativeSetters = {
Expand All @@ -224,12 +225,16 @@ 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: 0 additions & 3 deletions packages/server/lib/browsers/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,6 @@ 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: 2 additions & 7 deletions packages/server/lib/cypress.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,24 +164,19 @@ 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
electronApp.scale()
require('./util/electron-app').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(),
electronApp.setRemoteDebuggingPort(),
require('./util/electron-app').setRemoteDebuggingPort(),
])
.then(() => {
// else determine the mode by
Expand Down
13 changes: 0 additions & 13 deletions packages/server/lib/util/electron-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,6 @@ 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 @@ -71,8 +60,6 @@ const isRunningAsElectronProcess = ({ debug } = {}) => {
module.exports = {
scale,

setScopeMemoryCachePerContext,

getRemoteDebuggingPort,

setRemoteDebuggingPort,
Expand Down
44 changes: 44 additions & 0 deletions system-tests/__snapshots__/protocol_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6678,6 +6678,50 @@ 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
2 changes: 1 addition & 1 deletion system-tests/test/protocol_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ describe('capture-protocol', () => {
return systemTests.exec(this, {
key: 'f858a2bc-b469-4e48-be67-0876339ee7e1',
project: 'protocol',
spec: 'protocol.cy.js,test-isolation.cy.js',
record: true,
expectedExitCode: 0,
port: 2121,
spec: 'protocol.cy.js,test-isolation.cy.js',
config: {
hosts: {
'*foobar.com': '127.0.0.1',
Expand Down

5 comments on commit 16d6430

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 16d6430 Nov 14, 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.5.1/linux-arm64/develop-16d643082718d2e64a4ac9b056d1e7b4a10adf76/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 16d6430 Nov 14, 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.5.1/linux-x64/develop-16d643082718d2e64a4ac9b056d1e7b4a10adf76/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 16d6430 Nov 14, 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.5.1/darwin-x64/develop-16d643082718d2e64a4ac9b056d1e7b4a10adf76/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 16d6430 Nov 14, 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.5.1/win32-x64/develop-16d643082718d2e64a4ac9b056d1e7b4a10adf76/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 16d6430 Nov 14, 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.5.1/darwin-arm64/develop-16d643082718d2e64a4ac9b056d1e7b4a10adf76/cypress.tgz

Please sign in to comment.