Bug 90300 - [chromium] Use CCThread::Task in compositor's RateLimiter instead of Timer
Summary: [chromium] Use CCThread::Task in compositor's RateLimiter instead of Timer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-29 12:50 PDT by James Robinson
Modified: 2012-06-29 16:45 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.78 KB, patch)
2012-06-29 12:51 PDT, James Robinson
enne: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-06-29 12:50:53 PDT
[chromium] Use CCThread::Task in compositor's RateLimiter instead of Timer
Comment 1 James Robinson 2012-06-29 12:51:22 PDT
Created attachment 150238 [details]
Patch
Comment 2 James Robinson 2012-06-29 12:52:19 PDT
Motivation is to cut dependency on WebCore::Timer.

Tested manually on FishIE, seems to work fine.  Unit test coverage would be nice, but that's not a new issue.
Comment 3 Adrienne Walker 2012-06-29 14:43:29 PDT
Comment on attachment 150238 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150238&action=review

R=me.  Looks reasonable to me.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:132
> +    RateLimiterMap::iterator it = m_rateLimiters.begin();
> +    if (it != m_rateLimiters.end())
> +        it->second->stop();

It's kind of dodgy that we didn't have this before, since something that outlived the host could have taken a ref on the RateLimiter.
Comment 4 John Bauman 2012-06-29 15:06:19 PDT
I tested with something like this before, and noticed in traces that this would sometimes cause the rate limit task to be pushed into the next frame (after the composite), which could cause the frame time to be more erratic.
Comment 5 James Robinson 2012-06-29 15:25:10 PDT
Do you think the Timer-based version does something different?
Comment 6 James Robinson 2012-06-29 16:45:17 PDT
(In reply to comment #4)
> I tested with something like this before, and noticed in traces that this would sometimes cause the rate limit task to be pushed into the next frame (after the composite), which could cause the frame time to be more erratic.

I'm going to go ahead since the dependency is a big pain and I don't think this will change the timing much, but please file a bug if you see any bad behaviors.  We also have canvas deferral on by default, which is another way to try to tackle some of these issues.
Comment 7 James Robinson 2012-06-29 16:45:47 PDT
Committed r121601: <http://trac.webkit.org/changeset/121601>