Bug 67311 - Add EnabledAtRuntime support for constants.
Summary: Add EnabledAtRuntime support for constants.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-31 13:29 PDT by Aaron Colwell
Modified: 2011-09-30 11:19 PDT (History)
2 users (show)

See Also:

Attachments
Patch (3.18 KB, patch)
2011-08-31 13:29 PDT, Aaron Colwell 		no flags Details | Formatted Diff | Diff
Patch (3.19 KB, patch)
2011-09-30 09:51 PDT, Aaron Colwell 		no flags Details | Formatted Diff | Diff
Patch (3.46 KB, patch)
2011-09-30 10:20 PDT, Aaron Colwell 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Colwell 2011-08-31 13:29:26 PDT 
Add EnabledAtRuntime support for constants.
Comment 1 Aaron Colwell 2011-08-31 13:29:59 PDT 
Created attachment 105821 [details]
Patch
Comment 2 Aaron Colwell 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.
Comment 3 Aaron Colwell 2011-09-11 06:31:04 PDT 
ping
Comment 4 Aaron Colwell 2011-09-30 09:51:34 PDT 
Created attachment 109302 [details]
Patch
Comment 5 Aaron Colwell 2011-09-30 09:54:16 PDT 
Will you review this please.
Comment 6 Adam Barth 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.
Comment 7 Aaron Colwell 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.
Comment 8 Aaron Colwell 2011-09-30 10:20:50 PDT 
Created attachment 109307 [details]
Patch
Comment 9 WebKit Review Bot 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>
Comment 10 WebKit Review Bot 2011-09-30 11:19:33 PDT 
All reviewed patches have been landed.  Closing bug.