[webkit-reviews] review granted: [Bug 19384] Implement path morphing for SVG animation : [Attachment 21488] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 19384: Implement path morphing for SVG animation
http://bugs.webkit.org/show_bug.cgi?id=19384

Attachment 21488: patch
http://bugs.webkit.org/attachment.cgi?id=21488&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
 161	     m_fromPath.clear();
 162	     m_toPath.clear();
 163	     m_fromPath = SVGPathSegList::create(SVGNames::dAttr);

Is the call to m_fromPath.clear() needed here?

 238		     SVGPathSeg* segment = m_animatedPath->getItem(n,
ec).get();

It seems unnecessarily dangerous to use a raw pointer here. If it was safe to
be a raw pointer, then I think getItem's return value should be a raw pointer.
I suggest using a RefPtr instead. There should be no additional ref count
churn, since you start with a PassRefPtr anyway.

Can getItem fail? if so, then don't we need to check the value of "ec"?

 188	     SVGPathSeg* from = fromList->getItem(n, ec).get();
 189	     SVGPathSeg* to = toList->getItem(n, ec).get();

Same two comments here.

 254	     result->appendItem(segment, ec);

Don't we need to check ec here? Or can this never fail?

 17 </svg>
018 \ No newline at end of file

Could you include a newline?

I'm going to say r=me, but please consider those RefPtr issues.