Throttle refresh drivers in hidden presshells (e.g. background tabs)

RESOLVED FIXED in mozilla2.0b5

Status

()

Core
Layout
P1
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

({dev-doc-complete})

Trunk
mozilla2.0b5
x86
Mac OS X
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

Attachments

(1 attachment)

How does a 1s timer sound?  200ms?
blocking2.0: --- → ?
Priority: -- → P1
Painting is suppressed after a 1s (or 200ms) timeout?
(1 second sounds fine IMHO.)
It doesn't need to be painted again until the presshell is shown, right?
I was thinking painting happens no more often than 1s or 200ms or something.  I would love to just suspend the refresh driver in a background tab altogether, but I seem to recall roc mentioning some sort of issues with plug-ins if this is done...
I don't remember saying anything about plugins. I do think that suspending completely could have unexpected side effects, but I think it's worth trying!
I thought that we currently didn't do any painting in background tabs (on Linux at least) because the widget was hidden.
I did wonder how removing the widget is going to affect that, though.
We don't do any painting of background tabs currently. But disabling the refresh driver does a lot more than that. It means we won't do any reflow, or style resolution, or even frame construction, unless script forces it by asking for layout information. It will also effectively stop animations --- CSS transitions, SMIL animation, and the new JS animation API.

Hmm, I suspect that stopping all animations completely is wrong. CSS and SMIL "animation end" events should fire at approximately the right time even in background tabs. We'll have to do something to ensure that, either using a very low frame rate or running the refresh driver every time an animation is supposed to end.
Created attachment 469364 [details] [diff] [review]
Proposed patch

This still runs the refresh driver (at 1Hz) even when throttled, per the comments in this bug and the comment I added in the patch.
Attachment #469364 - Flags: review?(roc)
That said, I could certainly make it so that the mozRequestAnimationFrame stuff doesn't keep a background tab refresh driver going.  There's no legacy or spec there to mess with us.  Do we want to do that?
Whiteboard: [need review]
Oh, for what it's worth I measured this in a debug build on the moving-green-square-and-recording-elapsed-times testcase from the mozRequestAnimationFrame bug.  I saw 40% CPU usage with tab in foreground, 26% with tab in background without this patch, <0.5% with tab in background with this patch.
(In reply to comment #7)
> That said, I could certainly make it so that the mozRequestAnimationFrame stuff
> doesn't keep a background tab refresh driver going.  There's no legacy or spec
> there to mess with us.  Do we want to do that?

I don't think we do. I think it's helpful for authors to have a guarantee that the event will fire, in any situation where they want to do something after the animation ends.
Attachment #469364 - Flags: review?(roc) → review+
Comment on attachment 469364 [details] [diff] [review]
Proposed patch

Requesting approval.  This should be reasonably safe, and improves our CPU usage and performance story when multiple tabs are open.
Attachment #469364 - Flags: approval2.0?
Whiteboard: [need review] → [need approval]
Attachment #469364 - Flags: approval2.0? → approval2.0+
Pushed http://hg.mozilla.org/mozilla-central/rev/0078400b865b
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [need approval]
Target Milestone: --- → mozilla2.0b5
blocking2.0: ? → beta6+
I can picture development scenarios in which knowing that your animation might be throttled would be good, so this should be documented.
Keywords: dev-doc-needed
This has been noted for the mozRequestAnimationFrame case:

https://developer.mozilla.org/en/DOM/window.mozRequestAnimationFrame

I think that's really the only place where this needs to be mentioned, so marking as doc complete.
Keywords: dev-doc-needed → dev-doc-complete
Depends on: 595752
You need to log in before you can comment on or make changes to this bug.