[webkit-reviews] review denied: [Bug 207713] Fix handling of animations which lose their effect : [Attachment 390675] Patch

[bugzilla-daemon at webkit.org]

Antoine Quint <graouts at webkit.org> has denied Sunny He <sunny_he at apple.com>'s
request for review:
Bug 207713: Fix handling of animations which lose their effect
https://bugs.webkit.org/show_bug.cgi?id=207713

Attachment 390675: Patch

https://bugs.webkit.org/attachment.cgi?id=390675&action=review




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

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

> Source/WebCore/ChangeLog:3
> +	   Fix handling of animations which lose their effect

I would suggest this bug is renamed "Ensure animations that lose their effect
don't schedule an animation update". It's more descriptive of the issue that is
fixed.

> Source/WebCore/ChangeLog:7
> +

You need a description for what your patch does. Something like "An active
animation for which the effect is removed may be considered for an upcoming
animation resolution. However, WebAnimation::timeToNextTick() expects a valid
effect to be available to be able to determine timing. We now check an
animation is relevant before calling timeToNextTick() and add an ASSERT() in
that function to catch cases where an animation effect might not be available."

> Source/WebCore/ChangeLog:8
> +	   Test: fast/animation/animation-null-effect.html

I would file this under LayoutTests/webanimations with other Web Animations
tests. The fast/animation directory only contains CSS-based animations tests.