RESOLVED FIXED 67311
Add EnabledAtRuntime support for constants.
https://bugs.webkit.org/show_bug.cgi?id=67311
Summary Add EnabledAtRuntime support for constants.
Aaron Colwell
Reported 2011-08-31 13:29:26 PDT
Add EnabledAtRuntime support for constants.
Attachments
Patch (3.18 KB, patch)
2011-08-31 13:29 PDT, Aaron Colwell
no flags
Patch (3.19 KB, patch)
2011-09-30 09:51 PDT, Aaron Colwell
no flags
Patch (3.46 KB, patch)
2011-09-30 10:20 PDT, Aaron Colwell
no flags
Aaron Colwell
Comment 1 2011-08-31 13:29:59 PDT
Aaron Colwell
Comment 2 2011-08-31 13:33:54 PDT
Currently V8 code generation allows EnabledAtRuntime on methods and attributes, but not constants. This patch allows constants to be runtime enabled so experimental features that have constants, like MediaSource (https://bugs.webkit.org/show_bug.cgi?id=67306), can completely hide themselves in the DOM. Without this patch the attributes and methods would be hidden, but the constants would be visible.
Aaron Colwell
Comment 3 2011-09-11 06:31:04 PDT
ping
Aaron Colwell
Comment 4 2011-09-30 09:51:34 PDT
Aaron Colwell
Comment 5 2011-09-30 09:54:16 PDT
Will you review this please.
Adam Barth
Comment 6 2011-09-30 10:01:03 PDT
Comment on attachment 109302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109302&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2113 > # FIXME: we need the static_cast here only because of one constant, NodeFilter.idl > # defines "const unsigned long SHOW_ALL = 0xFFFFFFFF". It would be better if we > # handled this here, and converted it to a -1 constant in the c++ output. Can you move this comment inside the else branch? That's really where it belongs. > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2253 > + static const BatchedConstant constData = {"${name}", static_cast<signed int>(${value})}; > + batchConfigureConstants(desc, proto, &constData, 1); Its too bad we don't get much of a batching savings here, but I don't see how to avoid that. Maybe if we grouped by condition? That doesn't seem worthwhile.
Aaron Colwell
Comment 7 2011-09-30 10:20:24 PDT
Comment on attachment 109302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109302&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2113 >> # handled this here, and converted it to a -1 constant in the c++ output. > > Can you move this comment inside the else branch? That's really where it belongs. Done. >> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2253 >> + batchConfigureConstants(desc, proto, &constData, 1); > > Its too bad we don't get much of a batching savings here, but I don't see how to avoid that. Maybe if we grouped by condition? That doesn't seem worthwhile. Yeah I didn't like that either, but I figured grouping by condition was going to make this very difficult to read for not a whole lot of gain.
Aaron Colwell
Comment 8 2011-09-30 10:20:50 PDT
WebKit Review Bot
Comment 9 2011-09-30 11:19:28 PDT
Comment on attachment 109307 [details] Patch Clearing flags on attachment: 109307 Committed r96409: <http://trac.webkit.org/changeset/96409>
WebKit Review Bot
Comment 10 2011-09-30 11:19:33 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.