From 16d643082718d2e64a4ac9b056d1e7b4a10adf76 Mon Sep 17 00:00:00 2001
From: Ryan Manuel <ryanm@cypress.io>
Date: Tue, 14 Nov 2023 11:10:34 -0600
Subject: [PATCH] fix: performance regression due to enabling
 ScopeMemoryCachePerContext (#28327)

---
 cli/CHANGELOG.md                              |  1 +
 .../cypress/e2e/commands/actions/click.cy.js  | 13 ++++++
 .../driver/cypress/e2e/dom/visibility.cy.ts   | 25 ++++++++++-
 packages/driver/src/cy/actionability.ts       | 44 ++++++++++++++-----
 .../src/dom/elements/complexElements.ts       |  7 +++
 .../driver/src/dom/elements/nativeProps.ts    |  5 +++
 packages/server/lib/browsers/chrome.ts        |  3 --
 packages/server/lib/cypress.js                |  9 +---
 packages/server/lib/util/electron-app.js      | 13 ------
 system-tests/__snapshots__/protocol_spec.js   | 44 +++++++++++++++++++
 system-tests/test/protocol_spec.js            |  2 +-
 11 files changed, 130 insertions(+), 36 deletions(-)

diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md
index 672a2ab47fbe..6268ac5f0c3b 100644
--- a/cli/CHANGELOG.md
+++ b/cli/CHANGELOG.md
@@ -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).
 
diff --git a/packages/driver/cypress/e2e/commands/actions/click.cy.js b/packages/driver/cypress/e2e/commands/actions/click.cy.js
index 97fe26a7b6a0..fe7ff9d81ffd 100644
--- a/packages/driver/cypress/e2e/commands/actions/click.cy.js
+++ b/packages/driver/cypress/e2e/commands/actions/click.cy.js
@@ -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'))
         })
       })
     })
diff --git a/packages/driver/cypress/e2e/dom/visibility.cy.ts b/packages/driver/cypress/e2e/dom/visibility.cy.ts
index eed827b883b1..5dc7495afc30 100644
--- a/packages/driver/cypress/e2e/dom/visibility.cy.ts
+++ b/packages/driver/cypress/e2e/dom/visibility.cy.ts
@@ -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')
@@ -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;">
diff --git a/packages/driver/src/cy/actionability.ts b/packages/driver/src/cy/actionability.ts
index 8cdea1e51cbe..6e485c405622 100644
--- a/packages/driver/src/cy/actionability.ts
+++ b/packages/driver/src/cy/actionability.ts
@@ -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
@@ -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()
     }
   }
 
@@ -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 })
diff --git a/packages/driver/src/dom/elements/complexElements.ts b/packages/driver/src/dom/elements/complexElements.ts
index 10cfa37d4461..711ea8f32ba4 100644
--- a/packages/driver/src/dom/elements/complexElements.ts
+++ b/packages/driver/src/dom/elements/complexElements.ts
@@ -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
diff --git a/packages/driver/src/dom/elements/nativeProps.ts b/packages/driver/src/dom/elements/nativeProps.ts
index 2e6407edc3b8..615aa3489f14 100644
--- a/packages/driver/src/dom/elements/nativeProps.ts
+++ b/packages/driver/src/dom/elements/nativeProps.ts
@@ -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 = {
@@ -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] {
diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts
index 883ec13c3c1a..b2aeb8abe6de 100644
--- a/packages/server/lib/browsers/chrome.ts
+++ b/packages/server/lib/browsers/chrome.ts
@@ -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
   },
 
diff --git a/packages/server/lib/cypress.js b/packages/server/lib/cypress.js
index f4b65ed21939..3d029d570edb 100644
--- a/packages/server/lib/cypress.js
+++ b/packages/server/lib/cypress.js
@@ -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
diff --git a/packages/server/lib/util/electron-app.js b/packages/server/lib/util/electron-app.js
index 78c6135c7013..1d9c2a6e3acc 100644
--- a/packages/server/lib/util/electron-app.js
+++ b/packages/server/lib/util/electron-app.js
@@ -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)
@@ -71,8 +60,6 @@ const isRunningAsElectronProcess = ({ debug } = {}) => {
 module.exports = {
   scale,
 
-  setScopeMemoryCachePerContext,
-
   getRemoteDebuggingPort,
 
   setRemoteDebuggingPort,
diff --git a/system-tests/__snapshots__/protocol_spec.js b/system-tests/__snapshots__/protocol_spec.js
index 33670e438897..2e9ad628f700 100644
--- a/system-tests/__snapshots__/protocol_spec.js
+++ b/system-tests/__snapshots__/protocol_spec.js
@@ -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,
diff --git a/system-tests/test/protocol_spec.js b/system-tests/test/protocol_spec.js
index 7f868174cdd6..ff1ed2bb739a 100644
--- a/system-tests/test/protocol_spec.js
+++ b/system-tests/test/protocol_spec.js
@@ -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',