[webkit-reviews] review granted: [Bug 211668] Tighten up logic in DocumentTimelinesController::updateAnimationsAndSendEvents : [Attachment 398945] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun May 10 01:45:51 PDT 2020

Antoine Quint <graouts at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 211668: Tighten up logic in

Attachment 398945: Patch


--- Comment #4 from Antoine Quint <graouts at webkit.org> ---
Comment on attachment 398945
  --> https://bugs.webkit.org/attachment.cgi?id=398945

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

r=me, but there are a couple of things you should adjust before landing based
on feedback.

> Source/WebCore/animation/DocumentTimelinesController.cpp:136
> +	   // 2. Within events with equal scheduled event times, sort by their
composite order. FIXME: Need to do this.
> +	   return lhs->timelineTime() < rhs->timelineTime();

I'm not sure about that bit. How does this work if one of those two values is

> Source/WebCore/animation/DocumentTimelinesController.cpp:149
> +		   // FIXME: Should this use WebAnimation::remove instead of

Calling `WebAnimation::remove()` is not correct here. All we want to do is
remove the animation from the list of relevant animations for this timeline,
but we do not want to alter its animation-to-timeline or animation-to-effect
relationships, which is what `WebAnimation::remove()` would do. Indeed, there
may be JS references to that animation, and we cannot alter this object.

More information about the webkit-reviews mailing list