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

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted 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 379246: Patch

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




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 379246
  --> https://bugs.webkit.org/attachment.cgi?id=379246
Patch

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

> Source/WebCore/svg/SVGAnimateElementBase.cpp:84
> +    if (auto* animator = this->animator())
> +	   return animator->isDiscrete();
> +
> +    return false;

Another way to write this is:

    auto* animator = this->animator();
    return animator && animator->isDiscrete();

Not sure which style I like better, but I have often chosen to do a null guard
with && rather than early exit.

> Source/WebCore/svg/SVGAnimateElementBase.h:48
> +    bool hasInvalidCSSAttributeType() const;

Doesn’t seem like we needed to move this line of code. Could have left it where
it was, after the overrides. Maybe was moved as part of intermediate steps?