[webkit-reviews] review granted: [Bug 85071] Fix repetitions & by animation support for path animations : [Attachment 139214] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 85071: Fix repetitions & by animation support for path animations
https://bugs.webkit.org/show_bug.cgi?id=85071

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=139214&action=review


r=me

> Source/WebCore/svg/SVGAnimatedPath.cpp:134
> +	   SVGPathParserFactory::self()->addToSVGPathByteStream(animatedPath,
toPath, repeatCount);

SVGPathParserFactory singleton doesn't add anything (except some overhead for
passing the this pointer), addToSVGPathByteStream and pals could be free
functions and the whole SVGPathParserFactory type removed.

> Source/WebCore/svg/SVGPathBlender.cpp:392
> -	   if (m_fromSource->hasMoreData() != m_toSource->hasMoreData())
> -	       return false;
> -	   if (!m_fromSource->hasMoreData() || !m_toSource->hasMoreData())
> +	   if (fromSourceHadData) {
> +	       if (m_fromSource->hasMoreData() != m_toSource->hasMoreData())
> +		   return false;
> +	       if (!m_fromSource->hasMoreData() || !m_toSource->hasMoreData())
> +		   break;
> +	   } else if (!m_toSource->hasMoreData())
>	       break;
>      }
>      return true;

You should replace all instances of 'break' in this function with 'return
true'. It will be shorter and read better.