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

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

How does a 1s timer sound?  200ms?

Comment 1

[Karl Tomlinson (:karlt)]

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?

Comment 2

[Boris Zbarsky [:bzbarsky]]

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...

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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!

Comment 4

[Karl Tomlinson (:karlt)]

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.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: 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.

Comment 7

[Boris Zbarsky [:bzbarsky]]

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?

Comment 8

[Boris Zbarsky [:bzbarsky]]

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.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(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.

Comment 10

[Boris Zbarsky [:bzbarsky]]

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.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/0078400b865b

Comment 12

[Eric Shepherd [:sheppy]]

I can picture development scenarios in which knowing that your animation might be throttled would be good, so this should be documented.

Comment 13

[Eric Shepherd [:sheppy]]

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.