[webkit-reviews] review granted: [Bug 12936] Fix most remaining <use> bugs. : [Attachment 13437] Initial patch

[bugzilla-request-daemon at macosforge.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 12936: Fix most remaining <use> bugs.
http://bugs.webkit.org/show_bug.cgi?id=12936

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

------- Additional Comments from Darin Adler <darin at apple.com>
+    return svgElement;

Avoid reference count churn by saying return svgElement.release(), which allows
the RefPtr inside the function to give up the pointer without doing a ref
followed by a deref.

+    String widthString = String::number(width().value());
+    String heightString = String::number(height().value());

A shame to compute these both even when there's no width or height attribute. I
sugestmoving these expressions inside the if statements. You probably don't
need the local variables.

+		     String transformString = String::format("translate(%f,
%f)", use->x().value(), use->y().value());

I see this pattern repeated multiple times. I suggest some sort of helper
function to do this. Maybe a new function appendTransform(const FloatSize&) as
a member or just a helper function.

Do the two layout tests really cover all the problems?

r=me