Converting SVGAnimatedBoolean is trivial, but it needs a new type: SVGAnimatedStaticPropertyTearOff, which is used for properties that don't create tear offs when accessing baseVal/animVal. For example with SVGLength you can do: var foo = rect.x.baseVal; // SVGRectElement -> SVGAnimatedLength -> SVGLength foo.value = 50; In this case a SVGAnimatedPropertyTearOff is created, and a SVGPropertyTearOff to represent the SVGLength. For SVGExternalResourcesRequired (one of the two SVGAnimatedBoolean types), this is not possible, as baseVal returns a POD type (boolean). var blub = rect.externalResourcesRequired.baseVal; // blub is a boolean blub = false; // does NOT effect the rect.externalResourcesRequired object The old concept used JSSVGStaticPODTypeWrapper for these types, we need something similar in the new concept, called SVGAnimatedStaticPropertyTearOff.
Created attachment 72096 [details] Patch Did not test on a local chrome build, let's wait for EWS to see whether it builds. But the CodeGenerator changes are trivial this time, so the likelihood is large, that it may just work.
Attachment 72096 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/svg/SVGScriptElement.h:26: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 52 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 72097 [details] Patch v2 Fix style issue.
Attachment 72097 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4850045
Attachment 72097 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4789056
Created attachment 72167 [details] Patch v3 Need to apply the same fix as ObjC received to V8, to avoid including "V8boolean.h".
Attachment 72167 [details] did not build on qt: Build output: http://queues.webkit.org/results/4837054
Created attachment 72174 [details] Patch v4 Oops, fix some last minute changes that broke the build. Reattaching a working version, tested a local mac leopard debug build, zero regressions.
Attachment 72167 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4846067
Attachment 72167 [details] did not build on mac: Build output: http://queues.webkit.org/results/4800064
(In reply to comment #10) > Attachment 72167 [details] did not build on mac: > Build output: http://queues.webkit.org/results/4800064 But you build it without regressions on Mac ;-)
Comment on attachment 72174 [details] Patch v4 View in context: https://bugs.webkit.org/attachment.cgi?id=72174&action=review Looks great, waiting for mac and chromium bots before I review it. (If no one else beats me) > WebCore/svg/properties/SVGAnimatedProperty.h:-88 > - RefPtr<SVGProperty> m_baseVal; > - RefPtr<SVGProperty> m_animVal; I guess you moved this out, because animatedStaticPro doesn't need it.
(In reply to comment #12) > (From update of attachment 72174 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=72174&action=review > > Looks great, waiting for mac and chromium bots before I review it. (If no one else beats me) > > > WebCore/svg/properties/SVGAnimatedProperty.h:-88 > > - RefPtr<SVGProperty> m_baseVal; > > - RefPtr<SVGProperty> m_animVal; > > I guess you moved this out, because animatedStaticPro doesn't need it. Yes, exactly - mac bot worked fine, and cr-mac as well.
(In reply to comment #11) > (In reply to comment #10) > > Attachment 72167 [details] [details] did not build on mac: > > Build output: http://queues.webkit.org/results/4800064 > > But you build it without regressions on Mac ;-) Yes, if you look closer, that's the result from the last patch v3, not patch v4.
Comment on attachment 72174 [details] Patch v4 r=me
Landed in r70857. Waiting for bots before closing bug.
(In reply to comment #16) > Landed in r70857. Waiting for bots before closing bug. Forgot to remove an alert() from LayoutTests/svg/dom/SVGScriptElement/resources/script-load4.js before landing, fixed in r70860.
http://trac.webkit.org/changeset/70857 might have broken Qt Linux Release