[webkit-reviews] review granted: [Bug 10557] KCanvasPath should be replace by platform/Path : [Attachment 10231] General KCanvasPath removal

[bugzilla-request-daemon at opendarwin.org]

Eric Seidel <macdome at opendarwin.org> has granted Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 10557: KCanvasPath should be replace by platform/Path
http://bugzilla.opendarwin.org/show_bug.cgi?id=10557

Attachment 10231: General KCanvasPath removal
http://bugzilla.opendarwin.org/attachment.cgi?id=10231&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
If you're going to change to FloatRect, you might as well change to use
FloatSize as well here:

+Path KCanvasCreator::createRoundedRectangle(const FloatRect& box, float rx,
float ry) const

const FloatSize& roundingRadii

& could be moved here:
+const RenderPathList &KCanvasResource::clients() const

I don't think this needs to be a list here:
+typedef DeprecatedValueList<const RenderPath *> RenderPathList;

A vector would probably be better.

Can't this be an enum?
+    unsigned m_windRule : 1; // WindRule

Again, probably should be a vector:
+class KCClipDataList : public DeprecatedValueList<KCClipData>

I don't like these type of casts:
+		 m_clipper->addClipData(pathData, (WindRule)
pathStyle->svgStyle()->clipRule(), bbox);
Ideally they should just use the same enum.  No sense in having a separate one
for kcanvas anymore.

+	 DeprecatedString debugString() const;

should use use String.

So this patch looks great, but it could be better with a bit more tweaking.

There is really nothing in my review which would fully justify an r-.  It would
be really nice to get rid of the "List" classes and use Vectors instead before
landing.