Bug 90300 – [chromium] Use CCThread::Task in compositor's RateLimiter instead of Timer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Use CCThread::Task in compositor's RateLimiter instead of Timer

[James Robinson]

Created attachment 150238 [details]
Patch

[James Robinson]

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.

[Adrienne Walker]

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.

[John Bauman]

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.

[James Robinson]

Do you think the Timer-based version does something different?

[James Robinson]

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

[James Robinson]

Committed r121601: <http://trac.webkit.org/changeset/121601>