WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
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 attachment
proposed patch, testcase, etc.
Aaron Colwell
Comment 1
2011-08-31 13:29:59 PDT
Created
attachment 105821
[details]
Patch
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
Created
attachment 109302
[details]
Patch
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
Created
attachment 109307
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug