Bug 32758 - SVGAngle / SVGPreserveAspectRatio should be POD types
: SVGAngle / SVGPreserveAspectRatio should be POD types
Status: RESOLVED FIXED
: WebKit
SVG
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-12-18 17:01 PST by
Modified: 2009-12-18 21:04 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 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 From 2009-12-18 18:29:32 PST -------
Created an attachment (id=45217) [details]
Initial patch

Mechanical patch making SVGAngle / SVGPreserveAspectRatio non-refcounted types.
------- Comment #2 From 2009-12-18 18:32:10 PST -------
style-queue ran check-webkit-style on attachment 45217 [details] without any errors.
------- Comment #3 From 2009-12-18 19:33:48 PST -------
Created an attachment (id=45220) [details]
Updated patch

Incorporated Eric's comments from IRC (remove SVGPreserveAspectRatio code duplication)
------- Comment #4 From 2009-12-18 19:38:35 PST -------
style-queue ran check-webkit-style on attachment 45220 [details] without any errors.
------- Comment #5 From 2009-12-18 20:20:17 PST -------
Created an attachment (id=45222) [details]
Updated patch v2

Yet another round, this time it should be ready for trunk.
------- Comment #6 From 2009-12-18 20:24:15 PST -------
style-queue ran check-webkit-style on attachment 45222 [details] without any errors.
------- Comment #7 From 2009-12-18 20:29:38 PST -------
(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.
------- Comment #8 From 2009-12-18 20:35:41 PST -------
(In reply to comment #7)
> (From update of attachment 45222 [details] [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 From 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