Bug 106940 - Suspend CSS animations for background tabs
Summary: Suspend CSS animations for background tabs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-01-15 13:19 PST by Beth Dakin
Modified: 2013-01-15 15:45 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.34 KB, patch)
2013-01-15 13:20 PST, Beth Dakin
dino: review+
Details | Formatted Diff | Diff
Patch in Page (1.40 KB, patch)
2013-01-15 15:25 PST, Beth Dakin
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2013-01-15 13:19:14 PST
We should suspend CSS animations when a page is in the background.

<rdar://problem/12961725>
Comment 1 Beth Dakin 2013-01-15 13:20:57 PST
Created attachment 182838 [details]
Patch
Comment 2 Dean Jackson 2013-01-15 13:46:53 PST
Comment on attachment 182838 [details]
Patch

I'm surprised that this is all it takes. I wonder if it should be a setting so ports could toggle it?
Comment 3 Simon Fraser (smfr) 2013-01-15 13:53:18 PST
Comment on attachment 182838 [details]
Patch

IIRC these suspend/resume calls don't work very well; if JS runs and starts an animation while animations are "suspended" it will actually run.
Comment 4 Dean Jackson 2013-01-15 13:54:22 PST
Also, we have some bugs with fill mode iirc. But, we'll just have to fix all this :(
Comment 5 Simon Fraser (smfr) 2013-01-15 13:55:18 PST
We also have Frame::suspendActiveDOMObjectsAndAnimations (added in r109548). Should we be using that instead? FocusController doesn't seem like the right place to do this.
Comment 6 Beth Dakin 2013-01-15 15:23:10 PST
Actually, I think I found a place that I prefer for this code to live. I think it should be grouped with Kiran's DOM timer throttling (see http://trac.webkit.org/changeset/130720 ) which is in Page::setVisibilityState(). As far as I can tell, Page::setVisibilityState() and FocusController::setContainingWindowIsVisible() are pretty much equivalent, but it would be nice to have all of this throttling/suspending in the same place. I am going to post a new patch that moves this code to Page::setVisibilityState().
Comment 7 Beth Dakin 2013-01-15 15:25:16 PST
Created attachment 182855 [details]
Patch in Page

We might want to re-name HIDDEN_PAGE_DOM_TIMER_THROTTLING to HIDDEN_PAGE_THROTTLING if we go with this.
Comment 8 Simon Fraser (smfr) 2013-01-15 15:29:44 PST
Comment on attachment 182855 [details]
Patch in Page

I like this better but I don't think it should be inside #if ENABLE(HIDDEN_PAGE_DOM_TIMER_THROTTLING).
Comment 9 Beth Dakin 2013-01-15 15:45:43 PST
http://trac.webkit.org/changeset/139800