Summary: | Convert SVGAnimatedBoolean to the new SVGAnimatedPropertyTearOff concept | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||||||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, eric, krit, mdelaney7, webkit-ews, webkit.review.bot, zimmermann | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 47905 | ||||||||||||
Attachments: |
|
Description
Nikolas Zimmermann
2010-10-27 14:35:59 PDT
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 |