Last Comment Bug 586201 - Throttle refresh drivers in hidden presshells (e.g. background tabs)
: Throttle refresh drivers in hidden presshells (e.g. background tabs)
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Mac OS X
: P1 normal with 1 vote (vote)
: mozilla2.0b5
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on: 343515 595752
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-11 00:30 PDT by Boris Zbarsky [:bz]
Modified: 2010-10-25 23:45 PDT (History)
6 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta7+


Attachments
Proposed patch (6.64 KB, patch)
2010-08-25 23:59 PDT, Boris Zbarsky [:bz]
roc: review+
roc: approval2.0+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2010-08-11 00:30:36 PDT
How does a 1s timer sound?  200ms?
Comment 1 Karl Tomlinson (:karlt) 2010-08-11 01:08:02 PDT
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 [:bz] 2010-08-11 01:13:11 PDT
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) (Exited; email my personal email if necessary) 2010-08-11 01:50:34 PDT
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) 2010-08-11 02:29:13 PDT
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) (Exited; email my personal email if necessary) 2010-08-11 03:01:08 PDT
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 [:bz] 2010-08-25 23:59:05 PDT
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.
Comment 7 Boris Zbarsky [:bz] 2010-08-26 00:00:12 PDT
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 [:bz] 2010-08-26 00:01:18 PDT
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) (Exited; email my personal email if necessary) 2010-08-26 14:19:11 PDT
(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 [:bz] 2010-08-26 14:36:58 PDT
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 [:bz] 2010-08-26 23:28:03 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/0078400b865b
Comment 12 Eric Shepherd [:sheppy] 2010-09-11 10:11:29 PDT
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] 2010-09-12 22:58:46 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.