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

chore(bidi): disable thottling of background tabs in Firefox #34381

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

whimboo
Copy link
Collaborator

@whimboo whimboo commented Jan 17, 2025

With this patch throttling of background tabs will no longer happen. This PR changes the following:

@yury-s at least one test is no longer timing out with Firefox. I'm not sure if others might benefit from that as well. Also depending on your usage of setTimeout in general you should see a big improvement.

Copy link
Contributor

Test results for "tests 1"

9 flaky ⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-clearcookies.spec.ts:92:3 › should remove cookies by domain @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/debug-controller.spec.ts:261:1 › should highlight inside iframe @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/proxy.spec.ts:93:11 › should proxy local network requests › by default › loopback address @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/proxy.spec.ts:93:11 › should proxy local network requests › by default › link-local @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/selector-generator.spec.ts:116:5 › selector generator › should escape text with slash @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/trace-viewer.spec.ts:169:1 › should show action context on locators and other common actions @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/trace-viewer.spec.ts:1719:1 › should toggle canvas rendering @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-set-input-files.spec.ts:205:3 › should upload multiple large files @webkit-ubuntu-22.04-node18

37588 passed, 648 skipped
✔️✔️✔️

Merge workflow run.

Copy link
Member

@yury-s yury-s left a comment

Choose a reason for hiding this comment

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

Nice! It looks like we don't use this preferences with juggler (see https://github.com/microsoft/playwright/tree/main/browser_patches/firefox/preferences), which means there is probably another preference that helps us avoid the throttling. I'll try to incorporate this settings to our other builds too and see if it makes a difference.

Update: I just noticed from your description that at lease 'layout.testing.top-level-always-active' was added a few days ago, no wonder we don't have it yet.

Copy link
Member

@yury-s yury-s left a comment

Choose a reason for hiding this comment

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

It's another reason to have a canonical set of preferences that should be used for testing in Firefox.

@yury-s yury-s merged commit d082805 into microsoft:main Jan 17, 2025
29 checks passed
@whimboo whimboo deleted the firefox-no-throttling branch January 20, 2025 11:24
@whimboo
Copy link
Collaborator Author

whimboo commented Jan 20, 2025

Nice! It looks like we don't use this preferences with juggler (see https://github.com/microsoft/playwright/tree/main/browser_patches/firefox/preferences), which means there is probably another preference that helps us avoid the throttling. I'll try to incorporate this settings to our other builds too and see if it makes a difference.

Yes, you probably want to add this preference there as well.

Update: I just noticed from your description that at lease 'layout.testing.top-level-always-active' was added a few days ago, no wonder we don't have it yet.

Correct. After investigation of the raf test case in your repository I noticed that this was a problem and requested a possibility to turn throttling of for rendering. Is there a way to see how this affects the non BiDi-tests for Firefox? Do you have some general performance numbers recorded to detect perf changes?

It's another reason to have a canonical set of preferences that should be used for testing in Firefox.

The preferences as used in this PR are not going to become default preferences when Marionette or WebDriver BiDi is active in Firefox. Our aim is to run as close as possible to the users settings, and running tests in parallel is usually not such a thing, but could be requested by the user. Maybe it could be some kind of configuration option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants