[webkit-reviews] review granted: [Bug 87873] [chromium] Single thread proxy should not tick animations unless the layer renderer has been initialized : [Attachment 145124] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted vollick at chromium.org's request
for review:
Bug 87873: [chromium] Single thread proxy should not tick animations unless the
layer renderer has been initialized
https://bugs.webkit.org/show_bug.cgi?id=87873

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

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


Looks good

> Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp:1474
> +	   m_layerTreeHost->finishAllRendering();

We don't need this call any more, do we?

> Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp:1475
> +	   endTestAfterDelay(CCSingleThreadProxy::animationTimerDelay() *
1000);

I think it's worth having a comment here explaining why this value is chosen
(since we're setting the timer against the animation timer).  This will round
down since it's truncating from a double to millis - might be safer to
explicitly ceil() here so if the timer implementation does something else we
can be sure we don't run before it.