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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: handle timer throttling in DevTools to avoid timeouts #11987

Merged
merged 2 commits into from Jan 21, 2021

Conversation

patrickhulce
Copy link
Collaborator

@patrickhulce patrickhulce commented Jan 21, 2021

Summary
tl;dr the newly enabled timer throttling messes with our CPU idle check

This should address our timeout issue in DevTools. I'm not sure how to test for this specific issue since I'm sure the reason it wasn't caught to begin with is that asserting certain tests don't timeout is a flaky chromium web test we wouldn't be able to land anyway 馃槵

Related Issues/PRs
fixes #11983

@patrickhulce patrickhulce requested a review from a team as a code owner January 21, 2021 18:21
@patrickhulce patrickhulce requested review from connorjclark and removed request for a team January 21, 2021 18:21
@google-cla google-cla bot added the cla: yes label Jan 21, 2021

// Chrome 88 introduced heavy throttling of timers which means our `setTimeout` will be executed
// at some point farish (several hundred ms) into the future and the time at which it executes isn't
// a reliable indicator of long task existence, instead we check if any information has changed.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to be fair to chrome, we were relying on something that was never promised, just because you ask for something to be done in 50ms doesn't mean it will happen within 100ms even if everything else is idle, it just happened to be true for the past two decades or so :)

Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

LGTM

light suggestion to remove (wait-for) from the commit title.

@patrickhulce patrickhulce changed the title core(wait-for): handle timer throttling in DevTools to avoid timeouts core: handle timer throttling in DevTools to avoid timeouts Jan 21, 2021
@patrickhulce patrickhulce merged commit 73af3c8 into master Jan 21, 2021
@patrickhulce patrickhulce deleted the timer_throttle_fix branch January 21, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chrome throttling of background timers means every page times out in DevTools
3 participants