Bug 67621 - [chromium] REGRESSION(94353): requestAnimationFrame not throttled in compositing path
Summary: [chromium] REGRESSION(94353): requestAnimationFrame not throttled in composit...
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: Nat Duca
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-05 18:00 PDT by James Robinson
Modified: 2011-09-06 12:02 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.05 KB, patch)
2011-09-06 11:34 PDT, Nat Duca
no flags Details | Formatted Diff | Diff
Okay (1.92 KB, patch)
2011-09-06 11:57 PDT, Nat Duca
jamesr: review+
jamesr: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2011-09-05 18:00:33 PDT
See http://webstuff.nfshost.com/anim-timing/raftime.html.  When compositing is enabled, requestAnimationFrame is uncapped.

Regression range is 94351 - 94379.  Will bisect.
Comment 1 James Robinson 2011-09-05 20:34:16 PDT
Locally confirmed that it's http://trac.webkit.org/changeset/94353. I'm not really sure how...
Comment 2 Nat Duca 2011-09-05 22:18:51 PDT
Oy! Well I guess we know what I'm doing on Tuesday then. :)
Comment 3 Nat Duca 2011-09-06 11:34:24 PDT
Created attachment 106452 [details]
Patch
Comment 4 James Robinson 2011-09-06 11:42:48 PDT
Comment on attachment 106452 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:91
> +    commitIfNeeded(false);

actually we don't want to do layout here either. it's the responsibility of the caller to WebWidget::paint() to call animateAndLayout() before calling this function

> Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:237
> +    if (!alreadyCalledLayout)
> +        m_layerTreeHost->animateAndLayout(frameBeginTime);

i think you can just delete this completely
Comment 5 Nat Duca 2011-09-06 11:55:08 PDT
> i think you can just delete this completely
I did this because we're going to have to eventually invert that. But, I'll do as you wish.
Comment 6 James Robinson 2011-09-06 11:56:54 PDT
Eventually we'll need it, but not for compositeAndReadback().  That path will not.
Comment 7 Nat Duca 2011-09-06 11:57:05 PDT
Created attachment 106456 [details]
Okay
Comment 8 Nat Duca 2011-09-06 12:02:41 PDT
Committed r94587: <http://trac.webkit.org/changeset/94587>