[webkit-reviews] review denied: [Bug 201926] Assertion fires when animating a discrete property with values range and multiple animators : [Attachment 379060] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 201926: Assertion fires when animating a discrete property with values
range and multiple animators
https://bugs.webkit.org/show_bug.cgi?id=201926

Attachment 379060: Patch

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




--- Comment #5 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 379060
  --> https://bugs.webkit.org/attachment.cgi?id=379060
Patch

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

> Source/WebCore/ChangeLog:8
> +	   The first animator of a property is considered the result element.
The

What is "the result element"?

>> Source/WebCore/ChangeLog:22
>> +	    isDiscreteAnimator(). This should also be done in
calculateFromAndByValues().
> 
> Conventionally, the fix for something like this would be to have
isDiscreteAnimator() call animator() so it is created rather than relying on a
side effect of requiring the caller to call animator() first. Maybe there’s a
good reason not to do that, but it seems dangerous to rely on side effects and
the other of calling functions.

Maybe isDiscreteAnimator() should assert that animator() is not null, or the
isDiscreteAnimator() should be on animator itself.