Bug 32758

Summary:

SVGAngle / SVGPreserveAspectRatio should be POD types

Product:

WebKit

Reporter:

Nikolas Zimmermann <zimmermann>

Component:

SVG

Assignee:

Nikolas Zimmermann <zimmermann>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, eric, krit, staikos, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Initial patch	none
Updated patch	zimmermann: commit-queue-
Updated patch v2	eric: review+, zimmermann: commit-queue-

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	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
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.