[webkit-reviews] review granted: [Bug 43929] Use SVGPathByteStream to animate SVGPath : [Attachment 64250] 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 43929: Use SVGPathByteStream to animate SVGPath
https://bugs.webkit.org/show_bug.cgi?id=43929

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
WebCore/ChangeLog:9
 +	    the starting point to the end point according to the current
process.
s/process/progress value/

WebCore/ChangeLog:10
 +	    Cleanup SVGPathSegList and delete the now needless blending code.
Cleanup SVGPathSegList, by removing the unnecessary legacy blending code.

WebCore/ChangeLog:55452
 +  == Rolled over to ChangeLog-2010-05-24 ==	     No new tests because no
functional changes.
This looks wrong.

WebCore/svg/SVGAnimateElement.cpp:34
 +  #include <stdio.h>
Leftover.

WebCore/svg/SVGAnimateElement.cpp:136
 +		ASSERT(m_fromPath);
Maybe add an additional ASSERT(percentage >= 0) to assure percentage is always
positive.

WebCore/svg/SVGPathBlender.cpp:201
 +	    m_fromSource->parseSVGSegmentType(fromCommand);
You should check the return boolean, and abort if there's a problem.

WebCore/svg/SVGPathBlender.cpp:203
 +	    m_toSource->parseSVGSegmentType(toCommand);
Ditto.

WebCore/svg/SVGPathBlender.cpp:204
 +	    if (fromCommand != toCommand)
Because this line wouldn't catch it, if fromCommand and toCommand are both
Unknown.

WebCore/svg/SVGPathBlender.cpp:267
 +		return false;
I'm aware this would catch it, but I think the explicit check of the
parseSVGSegmentType return values is cleaner.

WebCore/svg/SVGPathBlender.cpp:270
 +		break;
Also check wheter m_toSource hasMoreData, also abort if not.

WebCore/svg/SVGPathBlender.cpp:32
 +	, m_consumer(0)
Please also use , m_progress(0) here, to avoid confusion.

r=me, if you fix all issues.