Bug 32758 - SVGAngle / SVGPreserveAspectRatio should be POD types
: SVGAngle / SVGPreserveAspectRatio should be POD types
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: SVG
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Nikolas Zimmermann
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-18 17:01 PST by Nikolas Zimmermann
Modified: 2009-12-18 21:04 PST (History)
5 users (show)

See Also:


Attachments
Initial patch (37.60 KB, patch)
2009-12-18 18:29 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Updated patch (49.89 KB, patch)
2009-12-18 19:33 PST, Nikolas Zimmermann
zimmermann: commit‑queue-
Details | Formatted Diff | Diff
Updated patch v2 (50.86 KB, patch)
2009-12-18 20:20 PST, Nikolas Zimmermann
eric: review+
zimmermann: commit‑queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 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...)
Comment 1 Nikolas Zimmermann 2009-12-18 18:29:32 PST
Created attachment 45217 [details]
Initial patch

Mechanical patch making SVGAngle / SVGPreserveAspectRatio non-refcounted types.
Comment 2 WebKit Review Bot 2009-12-18 18:32:10 PST
style-queue ran check-webkit-style on attachment 45217 [details] without any errors.
Comment 3 Nikolas Zimmermann 2009-12-18 19:33:48 PST
Created attachment 45220 [details]
Updated patch

Incorporated Eric's comments from IRC (remove SVGPreserveAspectRatio code duplication)
Comment 4 WebKit Review Bot 2009-12-18 19:38:35 PST
style-queue ran check-webkit-style on attachment 45220 [details] without any errors.
Comment 5 Nikolas Zimmermann 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.
Comment 6 WebKit Review Bot 2009-12-18 20:24:15 PST
style-queue ran check-webkit-style on attachment 45222 [details] without any errors.
Comment 7 Eric Seidel 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.
Comment 8 Nikolas Zimmermann 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 :-)
Comment 9 Adam Barth 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