Bug 48179 - Convert SVGAngle to the new SVGPropertyTearOff concept, reimplement it properly
Summary: Convert SVGAngle to the new SVGPropertyTearOff concept, reimplement it properly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 47905
  Show dependency treegraph
 
Reported: 2010-10-23 03:45 PDT by Nikolas Zimmermann
Modified: 2010-10-24 09:13 PDT (History)
7 users (show)

See Also:


Attachments
Patch (82.55 KB, patch)
2010-10-23 16:18 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (82.55 KB, patch)
2010-10-23 16:26 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v3 (82.63 KB, patch)
2010-10-24 02:33 PDT, Nikolas Zimmermann
krit: review+
Details | Formatted Diff | Diff
Fix WebGL follow-up patch (9.55 KB, patch)
2010-10-24 04:54 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Fix WebGL follow-up patch - v2 (7.14 KB, patch)
2010-10-24 08:55 PDT, Nikolas Zimmermann
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2010-10-23 03:45:54 PDT
Convert SVGAngle to the new SVGPropertyTearOff concept.
Comment 1 Nikolas Zimmermann 2010-10-23 16:18:33 PDT
Created attachment 71658 [details]
Patch

Tested local mac debug build on leopard and chromium build on snow leopard.
SVG pixel tests pass w/o regressions. Still have to do a full tests run, going to report soon.
The new SVGAngle tests which requires ScriptTypeSetting for attribute setters works perfectly in V8 & JSC, verified that twice :-)
Comment 2 WebKit Review Bot 2010-10-23 16:21:45 PDT
Attachment 71658 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/svg/SVGAnimatedAngle.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Nikolas Zimmermann 2010-10-23 16:26:07 PDT
Created attachment 71659 [details]
Patch v2

Fixed style issue.
Comment 4 Nikolas Zimmermann 2010-10-23 16:54:04 PDT
No layout test regression, did a full run-webkit-tests run.
Comment 5 Early Warning System Bot 2010-10-23 17:10:27 PDT
Attachment 71659 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4607060
Comment 6 Eric Seidel (no email) 2010-10-23 22:05:16 PDT
Attachment 71659 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4670061
Comment 7 Nikolas Zimmermann 2010-10-24 02:25:09 PDT
Hm, I just tried a clean build on leopard, worked fine. The chrome build, from scratch, also worked fine. I guess I have to try another snow leopard (non-chrome) build.
Comment 8 Nikolas Zimmermann 2010-10-24 02:33:40 PDT
Created attachment 71677 [details]
Patch v3

*grml* Diffed my working tree against the "Patch v2", turns out I forgot to include a CodeGenerator.pm change. With that one it builds fine...
Comment 9 Dirk Schulze 2010-10-24 03:32:08 PDT
Comment on attachment 71677 [details]
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=71677&action=review

> WebCore/bindings/scripts/CodeGeneratorJS.pm:1848
> +                                           push(@implContent, "    if (!ec)\n"); 
> +                                        push(@implContent, "        imp->commitChange();\n");

Indention looks wrong

Looks great, please take a look at the bots, since EWS will take some while.

r=me
Comment 10 Nikolas Zimmermann 2010-10-24 03:39:50 PDT
Committed r70410: <http://trac.webkit.org/changeset/70410>
Comment 11 Nikolas Zimmermann 2010-10-24 04:47:38 PDT
Ouch, I was not aware the webgl tests don't run on Leopard. I broke them, investigating.
Comment 12 Nikolas Zimmermann 2010-10-24 04:54:24 PDT
Created attachment 71678 [details]
Fix WebGL follow-up patch
Comment 13 Dirk Schulze 2010-10-24 04:56:58 PDT
Comment on attachment 71678 [details]
Fix WebGL follow-up patch

r=me
Comment 14 Nikolas Zimmermann 2010-10-24 04:59:18 PDT
Comment on attachment 71678 [details]
Fix WebGL follow-up patch

Landed speculative WebGL fix in r70411.
Hope these tests work again, otherwhise I have to roll it out.
Comment 15 Nikolas Zimmermann 2010-10-24 06:13:59 PDT
(In reply to comment #14)
> (From update of attachment 71678 [details])
> Landed speculative WebGL fix in r70411.
> Hope these tests work again, otherwhise I have to roll it out.

That did not fix it, running a local snow leopard build now, to figure out whats happening.
Will take some time...
Comment 16 Nikolas Zimmermann 2010-10-24 08:55:00 PDT
Created attachment 71688 [details]
Fix WebGL follow-up patch - v2
Comment 17 Dimitri Glazkov (Google) 2010-10-24 09:06:42 PDT
Comment on attachment 71688 [details]
Fix WebGL follow-up patch - v2

ok
Comment 18 Nikolas Zimmermann 2010-10-24 09:13:34 PDT
Committed r70414: <http://trac.webkit.org/changeset/70414>