RESOLVED FIXED 32758
SVGAngle / SVGPreserveAspectRatio should be POD types 
https://bugs.webkit.org/show_bug.cgi?id=32758
Summary SVGAngle / SVGPreserveAspectRatio should be POD types
Nikolas Zimmermann
Reported 2009-12-18 17:01:58 PST
The last two types which remain to be converted to POD types are SVGAngle/SVGPreserveAspectRatio. Just like it has been done for SVGLength before, change that, as there's no need to have these simple objects refcounted. Also this is a blocker for my upcoming patch: remove DOMObjectWithSVGContext, to basically rewrite SVG JS bindings to be more consistent with the HTML style of creating bindings (no special SVG context needed anymore soon...)
Attachments
Initial patch (37.60 KB, patch)
2009-12-18 18:29 PST, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
Updated patch (49.89 KB, patch)
2009-12-18 19:33 PST, Nikolas Zimmermann 		zimmermann: commit-queue-
DetailsFormatted DiffDiff
Updated patch v2 (50.86 KB, patch)
2009-12-18 20:20 PST, Nikolas Zimmermann 		eric: review+
zimmermann: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1 2009-12-18 18:29:32 PST
Created attachment 45217 [details] Initial patch Mechanical patch making SVGAngle / SVGPreserveAspectRatio non-refcounted types.
WebKit Review Bot
Comment 2 2009-12-18 18:32:10 PST
style-queue ran check-webkit-style on attachment 45217 [details] without any errors.
Nikolas Zimmermann
Comment 3 2009-12-18 19:33:48 PST
Created attachment 45220 [details] Updated patch Incorporated Eric's comments from IRC (remove SVGPreserveAspectRatio code duplication)
WebKit Review Bot
Comment 4 2009-12-18 19:38:35 PST
style-queue ran check-webkit-style on attachment 45220 [details] without any errors.
Nikolas Zimmermann
Comment 5 2009-12-18 20:20:17 PST
Created attachment 45222 [details] Updated patch v2 Yet another round, this time it should be ready for trunk.
WebKit Review Bot
Comment 6 2009-12-18 20:24:15 PST
style-queue ran check-webkit-style on attachment 45222 [details] without any errors.
Eric Seidel (no email)
Comment 7 2009-12-18 20:29:38 PST
Comment on attachment 45222 [details] Updated patch v2 Why remove: 184 if (orientAngle()) ? In general this looks great though. Does it break v8? The Chromium EWS should tell us.
Nikolas Zimmermann
Comment 8 2009-12-18 20:35:41 PST
(In reply to comment #7) > (From update of attachment 45222 [details]) > Why remove: > 184 if (orientAngle()) > ? > > In general this looks great though. Does it break v8? The Chromium EWS > should tell us. Landed in r52373. As the chromium ews bot is down, I'll just risk it, and watch the bots :-)
Adam Barth
Comment 9 2009-12-18 21:04:51 PST
> Landed in r52373. As the chromium ews bot is down, I'll just risk it, and watch > the bots :-) We need a better dashboard for seeing success. Turns out this patch passed: http://webkit-commit-queue.appspot.com/patch-status/chromium-ews/45222
Note You need to log in before you can comment on or make changes to this bug.