[webkit-reviews] review requested: [Bug 81484] [chromium] Animation events should only be used for synchronizing animation start times : [Attachment 132950] Patch

[bugzilla-daemon at webkit.org]

vollick at chromium.org has asked	for review:
Bug 81484: [chromium] Animation events should only be used for synchronizing
animation start times
https://bugs.webkit.org/show_bug.cgi?id=81484

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

------- Additional Comments from vollick at chromium.org
(In reply to comment #3)
CCAnimationEvents.cpp is toast.

(In reply to comment #4)
> (From update of attachment 132827 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=132827&action=review
>
> Drive by language question: is monotonic time a synonym for wall clock time
here?
>
I've been using monotonicTime to refer to times obtained via
monotonicallyIncreasingTime() and wallClockTime for times obtained using
currentTime()

> >
Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp:391
> > +	     if (m_activeAnimations[i]->runState() ==
CCActiveAnimation::Running &&
!m_activeAnimations[i]->needsSynchronizedStartTime()) {
> >		 double trimmed =
m_activeAnimations[i]->trimTimeToCurrentIteration(monotonicTime);
> >
> > +		 // Animation assumes its initial value until it gets the
synchronized start time
> > +		 // from the impl thread and can start ticking.
> > +		 if (m_activeAnimations[i]->needsSynchronizedStartTime())
> > +		     trimmed = 0;
>
> I don't understand this second conditional here.  It looks like it will never
be true, based on the first conditional.
Good catch. The first conditional was meant to be deleted. Fixed.