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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 3 19:06:04 PDT 2008

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

Attachment 21488: patch

------- 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,

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.

More information about the webkit-reviews mailing list