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

[bugzilla-daemon at webkit.org]

vollick at chromium.org has asked	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 132816: Patch
https://bugs.webkit.org/attachment.cgi?id=132816&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #17)
> (From update of attachment 132579 [details])
> 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
>
Fixed.
> >
Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp:30
> > +#include "TransformOperations.h"
>
> are we using this #include anywhere?
>
Nope. Removed.
> >
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
>
Removed.
> > 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.
>
It also turned out that we get to
CCLayerTreeHost::updateAnimations(frameBeginTime) along another code path that
passes the currentTime() rather than the monotonicallyIncreasingTime().

I've reverted these changes and I grab the monotonicallyIncreasingTime() in
updateAnimations instead.
> > Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h:95
> > +	     BeginFrameAndCommitState(double frameBeginTime) :
frameBeginTime(frameBeginTime) { }
>
> explicit, prz

Reverted.