fix: remove offscreen mode from headless Electron windows

[lukeapage]

• Closes Tests slow down since upgrading cypress from 4.4.1 #7930

User facing changelog

Switch off offscreen rendering which can cause a 8x performance slowdown for headless electron runs on slow computers. Problem last introduced in cypress 4.5.0.

Additional details

Offscreen rendering:
https://www.electronjs.org/docs/tutorial/offscreen-rendering

passes every frame to a 'paint' event, which cypress does not listen too.

In electron 8.2.1 it was fixed and switched back on, so when cypress upgraded from 8.2.0 to 8.2.3, it caused a performance regression.

The performance regression on slow computers is about 8x slower. On fast computers you don't see a difference - I expect the multi-process nature of electron and cypress means that more cores mitigate the extra cpu needed.

The option was introduced here, 3 years ago:
5bde862

My guess is it was a mistake. @brian-mann do you remember?

[cypress-bot]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

[CLAassistant]

All committers have signed the CLA.

[jennifer-shehane]

Would be nice to have a test for this. Performance tests are really challenging though. 🤔

[lukeapage]

I hoped I could see a marked difference in your test runs, but perhaps down to whatever hardware used, there’s no big time difference there. Given the difficulty of performance tests and that this is removing bad code from 3 years ago I’d ask that be done as a separate feature.

[bahmutov]

interesting, I wonder how we can qualify and test this to prevent regressions from this happening

[bahmutov]

Question: the test run videos are still captured though?

[lukeapage]

how do I test videos? I can verify windows but it would be good to double check Mac and Linux too - this Pr has no link to a dashboard like other prs I see.

[lukeapage]

I can confirm that videos still work on windows.

[jennifer-shehane]

Video recording seems fine on Mac

[jennifer-shehane]

I got some notes from @brian-mann on why this option was originally added:

• hiding would leak elements like select drop downs to the screen
• we used to only be able to record and control the FPS this way
• we used to control the screen size and DPI consistently at 1x DPI
• performance
• did not use the GPU to render which prevented display issues

Hopefully he can comment with some more detailed thoughts, but we may want to look into removing this more closely. I remember the dropdown issue anyway and could look at that.

[brian-mann]

Adding to my notes that @jennifer-shehane listed...

With offscreen we get the monitor DPI controlled for and normalized everywhere. If we take off offscreen we have to use the RDP protocol to normalize it for all users. In our e2e tests we manually do that, but that will not be available to other users, so we'll have to do it for them.

[flotwig]

In addition to the reasons that @jennifer-shehane mentioned, Cypress /was/ using the paint event to capture video of the running tests, so for a time, there was a good reason to use offscreen rendering. We switched to using Chrome DevTools Protocol for recording in 4.2.0, so that is why you are able to disable offscreen rendering and still have video recording work.

The issue where dropdowns and other elements would appear on screen during --headless on macOS, without offscreen: true, was like, 8 major versions of Electron ago (1.x? 2.0.18?), so it's possible that it's been fixed now that we're on Electron 9. Someone with a Mac can confirm.

As long as we can ensure that issue is fixed, and that 1x DPI is still enforced somehow with offscreen: false, LGTM.

---

Here is the code @brian-mann is referring to that forces 1x DPI in our e2e tests:

cypress/packages/server/test/support/fixtures/projects/e2e/cypress/support/index.js

Lines 4 to 25 in ee60b01

	const before = _.once(function () {
	if (Cypress.isBrowser([{ name: '!electron', family: 'chromium' }])) {
	return Cypress.automation('remote:debugger:protocol', {
	command: 'Emulation.setDeviceMetricsOverride',
	params: {
	width: 1280,
	height: 720,
	deviceScaleFactor: 1,
	mobile: false,
	screenWidth: 1280,
	screenHeight: 720,
	},
	})
	.then(() => {
	// can't tell expect() not to log, so manually throwing here
	if (window.devicePixelRatio !== 1) {
	throw new Error('Setting devicePixelRatio to 1 failed')
	}
	})
	}
	})

We don't force the 1x override in Electron or Firefox though, so I'm not sure what that says for how important this is...

[lukeapage]

Remember also that electron 8 - 8.2.0 had offscreen disabled so the option did nothing:
https://www.electronjs.org/blog/electron-8-0
I believe that cypress used this range in 4.3.0-4.4.1
So I’d expect bugs reported for these cypress versions that were resolved with 4.5.0

[lukeapage]

What do you want to do? Make it an option or spend more time testing it?

[brian-mann]

@jennifer-shehane and I paired earlier on this and she's testing out the various scenarios. Once we can confirm everything behaves the same and there are no gaps we can merge this in.

[jennifer-shehane]

I don't see the weird <select> rendering issue that we used to when running Electron.

I'm also not seeing the behavior of the video change based on the DPI of my screen.

dismising