[webkit-reviews] review granted: [Bug 81106] [chromium] Infrastructure to allow animating layers to be only partially updated : [Attachment 132579] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted vollick at chromium.org's request
for review:
Bug 81106: [chromium] Infrastructure to allow animating layers to be only
partially updated
https://bugs.webkit.org/show_bug.cgi?id=81106

Attachment 132579: Patch
https://bugs.webkit.org/attachment.cgi?id=132579&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=132579&action=review


Looking good!  A few comments inline.

> Source/WebCore/platform/graphics/chromium/LayerChromium.cpp:547
> +

nit: extra newline

>
Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp:30
> +#include "TransformOperations.h"

are we using this #include anywhere?

> Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.h:43
> +class TransformOperations;

I don't see this forward declaration being used anywhere - can we remove it?  I
think it's nice if this class doesn't have to depend on TransformOperations

> Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp:396
> +    m_pendingBeginFrameRequest = adoptPtr(new
BeginFrameAndCommitState(monotonicallyIncreasingTime()));

I think I made this change in a different way in another patch.

Unfortunately on the main thread we need to use a currentTime() based clock -
stuff like requestAnimationFrame needs it.  We plan to change that eventually,
but for now you either have to use that clock or add some renormalizing logic
before you pass the time into the CCLayerTreeHostClient.

> Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h:95
> +	   BeginFrameAndCommitState(double frameBeginTime) :
frameBeginTime(frameBeginTime) { }

explicit, prz