[webkit-reviews] review denied: [Bug 49437] SVGAnimations with IRI references via 'xlink:href' are slow : [Attachment 73731] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 49437: SVGAnimations with IRI references via 'xlink:href' are slow
https://bugs.webkit.org/show_bug.cgi?id=49437

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

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

Hm, I'm slightly worried that we have no new tests about this.
What happens when you change 'target' dynamically via JS, while the animation
is running? I guess m_targetElement won't be recached from reading the code...

I think you should try that and compare to Opera.

> WebCore/svg/SVGAnimateMotionElement.cpp:55
> -bool SVGAnimateMotionElement::hasValidTarget() const
> +bool SVGAnimateMotionElement::hasValidTarget()

Why is it a non-const version?

> WebCore/svg/SVGAnimateTransformElement.cpp:106
> +    SVGElement* targetElement = this->targetElement();

I'd rather use const_cast<SVGAnimateTransformElement*>(this)->targetElement()
here!