[webkit-reviews] review granted: [Bug 63296] Convert AnimatedString to SVGAnimatorFactory concept : [Attachment 98465] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 63296: Convert AnimatedString to SVGAnimatorFactory concept
https://bugs.webkit.org/show_bug.cgi?id=63296

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98465&action=review

Looks good to me, I've discussed on IRC with Dirk, he actually already
explained both my and Robs question in an earlier comment. r=me.

> Source/WebCore/svg/SVGAnimateElement.cpp:352
> -	       valueToApply = m_animatedString;
> +	       valueToApply = String();

Dirk explained this in an earlier comment, it's safe and can be used to avoid
m_animatedString completely.

> Source/WebCore/svg/SVGAnimatorFactory.h:61
> -	       return adoptPtr(new SVGAnimatedLengthAnimator(animationElement,
contextElement));
> +	       return adoptPtr(new SVGAnimatedStringAnimator(animationElement,
contextElement));

To avoid the confusion use "return nullptr" here. It doesn't matter what's
returned here, the code is never reached. But it's certainly better to return 0
than some other value.