Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, similar to the approach
for the [MeasureAs] attribute.

BUG=586594
Committed: https://crrev.com/0c7efcf9ce5d2223c6aba69b94be77da64adea2b
Cr-Commit-Position: refs/heads/master@{#386080}

[chasej, 2016-04-04 21:04:39]

Description was changed from

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. In addition, an explicit check will cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

BUG=586594
==========

to

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, and should be easier
to change for all the types of IDL members in the future.

BUG=586594
==========

[chasej, 2016-04-04 21:07:58]

chasej@chromium.org changed reviewers:
+ haraken@chromium.org, iclelland@chromium.org

[chasej, 2016-04-04 21:07:59]

haraken, iclelland, please take a look.

I'd suggest this be reviewed after the cleanup CLs (which start with
https://codereview.chromium.org/1828973003/).

[chasej, 2016-04-04 21:11:47]

Description was changed from

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, and should be easier
to change for all the types of IDL members in the future.

BUG=586594
==========

to

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, similar to the approach
for the [MeasureAs] attribute.

BUG=586594
==========

[haraken, 2016-04-05 01:23:18]

LGTM, thanks for splitting the CLs! It was easy to review :)

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_utilities.py (right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:412: if not
feature_name or trial_enabled:

Do we still need this check?

[iclelland, 2016-04-05 13:38:59]

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_utilities.py (right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:370: def
origin_trial_enabled(definition_or_member, interface):
This function's name suggests that it is a boolean value; it actually returns a
method to call, or None if origin trials are not enabled for a particular
feature. (And, by side effect, adds the appropriate #includes). Is there a
better name for this?

(I like the function, it simplifies a lot of other code, but the final result is
that I had to go looking here to see what it actually *did*)

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:374: if
(is_origin_trial_enabled and 'RuntimeEnabled' in extended_attributes):
Is there a compelling reason for this check to be here (as opposed to in
runtime_enabled, or both) Or could it be checked at a higher level?

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:375: raise
Exception('[OriginTrialEnabled] and [RuntimeEnabled] must '
I'd expect a more specific exception type to be raised, but that might be a
consequence of where this code is right now.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:398: """Returns the
name of the RuntimeEnabledFeatures function.
Document the new side effect

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp
(right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp:20:
#include "platform/RuntimeEnabledFeatures.h"
Curious -- do you know why this wasn't included before?

[chasej, 2016-04-05 19:50:22]

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_utilities.py (right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:370: def
origin_trial_enabled(definition_or_member, interface):
On 2016/04/05 13:38:58, iclelland wrote:
> This function's name suggests that it is a boolean value; it actually returns
a
> method to call, or None if origin trials are not enabled for a particular
> feature. (And, by side effect, adds the appropriate #includes). Is there a
> better name for this?
> 
> (I like the function, it simplifies a lot of other code, but the final result
is
> that I had to go looking here to see what it actually *did*)

The function is implemented similar to the deprecate_as() and measure_as()
functions. Those functions seem to be named after the extended attribute
(without any prefix/suffix), so I used similar naming for this function.
Admittedly, the fact that the attribute name includes "enabled" confuses things.
 
Looking at all the functions in this file, there doesn't seem to be a majority
convention for extended attributes. The runtime_enabled_function_name() is an
obvious counter-example to the approach I took.

Other than adding the results of these functions to the bindings context, the
usage seems rather separated (i.e. you have to look in the *.cpp template
files). Given that, I'm not sure what naming approach is best. Suggestions for a
better name are welcome.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:374: if
(is_origin_trial_enabled and 'RuntimeEnabled' in extended_attributes):
On 2016/04/05 13:38:58, iclelland wrote:
> Is there a compelling reason for this check to be here (as opposed to in
> runtime_enabled, or both) Or could it be checked at a higher level?

I had the check here because it really belongs to [OriginTrialEnabled], not to
[RuntimeEnabled]. This could be checked at a higher level, but then it's not
encapsulated as nicely. Every caller that uses origin_trial_enabled() would have
to know/remember to add the check.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:375: raise
Exception('[OriginTrialEnabled] and [RuntimeEnabled] must '
On 2016/04/05 13:38:58, iclelland wrote:
> I'd expect a more specific exception type to be raised, but that might be a
> consequence of where this code is right now.

Other than a few specific scenarios, most of the bindings script seem to raise
either Exception or ValueError. I didn't notice a specific pattern to either.
I'm open to suggestions here.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:398: """Returns the
name of the RuntimeEnabledFeatures function.
On 2016/04/05 13:38:58, iclelland wrote:
> Document the new side effect

Done.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:412: if not
feature_name or trial_enabled:
On 2016/04/05 01:23:18, haraken wrote:
> 
> Do we still need this check?

You're right, we probably don't need this check anymore, given the
check/exception in the origin_trial_enabled() function.

Removed.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp
(right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp:20:
#include "platform/RuntimeEnabledFeatures.h"
On 2016/04/05 13:38:59, iclelland wrote:
> Curious -- do you know why this wasn't included before?

I'm pretty sure this was a bug. See line 615 in v8_interface.py, the
constant_context() function (in the original version). There was logic to add
the includes for OriginTrials, but not for RuntimeEnabled. Compare to line 162
in the same file, which has a check to add the include. TestInterface2.idl only
has [RuntimeEnabled] applied to a constant, which would have been affected by
the changes in this CL. Specifically, I moved the logic to add the
RuntimeEnabled include into the runtime_enabled_function_name().

[iclelland, 2016-04-07 14:00:32]

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_utilities.py (right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:370: def
origin_trial_enabled(definition_or_member, interface):
On 2016/04/05 19:50:22, chasej wrote:
> On 2016/04/05 13:38:58, iclelland wrote:
> > This function's name suggests that it is a boolean value; it actually
returns
> a
> > method to call, or None if origin trials are not enabled for a particular
> > feature. (And, by side effect, adds the appropriate #includes). Is there a
> > better name for this?
> > 
> > (I like the function, it simplifies a lot of other code, but the final
result
> is
> > that I had to go looking here to see what it actually *did*)
> 
> The function is implemented similar to the deprecate_as() and measure_as()
> functions. Those functions seem to be named after the extended attribute
> (without any prefix/suffix), so I used similar naming for this function.
> Admittedly, the fact that the attribute name includes "enabled" confuses
things.
>  
> Looking at all the functions in this file, there doesn't seem to be a majority
> convention for extended attributes. The runtime_enabled_function_name() is an
> obvious counter-example to the approach I took.
> 
> Other than adding the results of these functions to the bindings context, the
> usage seems rather separated (i.e. you have to look in the *.cpp template
> files). Given that, I'm not sure what naming approach is best. Suggestions for
a
> better name are welcome.
> 

With the obvious parallels to RuntimeEnabled code, I think I'd be happy with
(the slightly longer) origin_trial_enabled_function_name

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:374: if
(is_origin_trial_enabled and 'RuntimeEnabled' in extended_attributes):
On 2016/04/05 19:50:22, chasej wrote:
> On 2016/04/05 13:38:58, iclelland wrote:
> > Is there a compelling reason for this check to be here (as opposed to in
> > runtime_enabled, or both) Or could it be checked at a higher level?
> 
> I had the check here because it really belongs to [OriginTrialEnabled], not to
> [RuntimeEnabled]. This could be checked at a higher level, but then it's not
> encapsulated as nicely. Every caller that uses origin_trial_enabled() would
have
> to know/remember to add the check.

It just seems like an obvious symmetry, that it could be checked in either
place, and the only difference is that this method is *newer*.

If it doesn't makes sense to put this check in v8_attributes.py, where the
includes.add lines were removed, then I'm okay with it being here. (And see the
comments in V8TestInterface2.cpp as well)

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_utilities.py:375: raise
Exception('[OriginTrialEnabled] and [RuntimeEnabled] must '
On 2016/04/05 19:50:22, chasej wrote:
> On 2016/04/05 13:38:58, iclelland wrote:
> > I'd expect a more specific exception type to be raised, but that might be a
> > consequence of where this code is right now.
> 
> Other than a few specific scenarios, most of the bindings script seem to raise
> either Exception or ValueError. I didn't notice a specific pattern to either.
> I'm open to suggestions here.

Acknowledged. All I had seen (without looking *too* hard) was ValueError, which
doesn't seem to fit here. If there's nothing else in common use here, and no
user-defined exceptions, then adding them here seems out-of-scope.

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp
(right):

https://codereview.chromium.org/1861433002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp:20:
#include "platform/RuntimeEnabledFeatures.h"
On 2016/04/05 19:50:22, chasej wrote:
> On 2016/04/05 13:38:59, iclelland wrote:
> > Curious -- do you know why this wasn't included before?
> 
> I'm pretty sure this was a bug. See line 615 in v8_interface.py, the
> constant_context() function (in the original version). There was logic to add
> the includes for OriginTrials, but not for RuntimeEnabled. Compare to line 162
> in the same file, which has a check to add the include. TestInterface2.idl
only
> has [RuntimeEnabled] applied to a constant, which would have been affected by
> the changes in this CL. Specifically, I moved the logic to add the
> RuntimeEnabled include into the runtime_enabled_function_name().

This sort of bug being present and unnoticed makes me more comfortable with the
way you're adding new includes now. The fact that the old method missed cases
like this suggests that the changes in this CL are definite improvements.

[chasej, 2016-04-07 21:17:41]

chasej@chromium.org changed reviewers:
+ jyasskin@chromium.org

[chasej, 2016-04-07 21:17:42]

iclelland, please take another look.

jyasskin, please take a look at bluetooth/.

[Jeffrey Yasskin, 2016-04-08 00:07:49]

LGTM assuming our flag will still work.

https://codereview.chromium.org/1861433002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/IDLExtendedAttributes.md (right):

https://codereview.chromium.org/1861433002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:1032: Summary: Like
`[RuntimeEnabled]`, it controls at runtime whether bindings are exposed, but
uses a different mechanism for enabling experimental features.
Can you clarify that OriginTrialEnabled=WebBluetooth still results in a 
RuntimeEnabledFeatures::setWebBluetoothEnabled() function we can call in
response to the about:flags flag being enabled?

[ortuno, 2016-04-08 14:49:58]

ortuno@chromium.org changed reviewers:
+ ortuno@chromium.org

[ortuno, 2016-04-08 14:49:59]

I just tried this patch with the tests from http://crrev.com/1858293002 and I
can confirm our flag works as expected.

[iclelland, 2016-04-08 14:51:32]

LGTM (one nit, maybe not in scope for this CL)

However, maybe we want to wait for the WebUSB cl to land, and then take care of
both of the updates in this one?

https://codereview.chromium.org/1861433002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/1861433002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:49: from v8_utilities
import (cpp_name_or_partial, capitalize, cpp_name, gc_type,
nit: Why the inconsistency here? I see that it could be used with the module
name prefixed (because of the import on the previous line), but it seems like
either all of these functions should be fully scoped, or none of them.

https://codereview.chromium.org/1861433002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/IDLExtendedAttributes.md (right):

https://codereview.chromium.org/1861433002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:1032: Summary: Like
`[RuntimeEnabled]`, it controls at runtime whether bindings are exposed, but
uses a different mechanism for enabling experimental features.
On 2016/04/08 00:07:49, Jeffrey Yasskin wrote:
> Can you clarify that OriginTrialEnabled=WebBluetooth still results in a 
> RuntimeEnabledFeatures::setWebBluetoothEnabled() function we can call in
> response to the about:flags flag being enabled?

Yes -- that is triggered by the presence of "WebBluetooth" in
RuntimeEnabledFeatures.in, though, and doesn't require any support from bindings
generation.

[chasej, 2016-04-08 15:11:14]

https://codereview.chromium.org/1861433002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/1861433002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:49: from v8_utilities
import (cpp_name_or_partial, capitalize, cpp_name, gc_type,
On 2016/04/08 14:51:31, iclelland wrote:
> nit: Why the inconsistency here? I see that it could be used with the module
> name prefixed (because of the import on the previous line), but it seems like
> either all of these functions should be fully scoped, or none of them.

This file was inconsistent to begin with. The import of
"origin_trial_enabled_function" wasn't actually used. The references in code
were all prefixed. Other files (e.g. v8_attributes.py, v8_methods.py) did not
import the specific function. I removed the import to make it more consistent
with usage in this file, and across the files.

I believe this inconsistency was inherited from the import of
runtime_enabled_function_name, in the initial implementation of
[APIExperimentEnabled]. You'll see runtime_enabled_function_name is used both
with and without prefix in this file. I'd guess that wasn't intentional, but I
think that should be addressed separately, if necessary.

[chasej, 2016-04-08 15:13:46]

On 2016/04/08 14:51:32, iclelland wrote:
> LGTM (one nit, maybe not in scope for this CL)
> 
> However, maybe we want to wait for the WebUSB cl to land, and then take care
of
> both of the updates in this one?

Either way, one CL has to land first, and the other has to rebase/change. I'd
prefer to land this one first, and keep all the WebUSB changes in the other CL.
Assuming this lands first, I'll help with the WebUSB CL.

[chasej, 2016-04-08 15:15:11]

The CQ bit was checked by chasej@chromium.org

[chasej, 2016-04-08 15:15:11]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1861433002/#ps100001
(title: "Correct IDL for Web Bluetooth")

[commit-bot: I haz the power, 2016-04-08 15:15:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1861433002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1861433002/100001

[commit-bot: I haz the power, 2016-04-08 15:21:44]

Description was changed from

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, similar to the approach
for the [MeasureAs] attribute.

BUG=586594
==========

to

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, similar to the approach
for the [MeasureAs] attribute.

BUG=586594
==========

[commit-bot: I haz the power, 2016-04-08 15:21:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-04-08 15:22:51]

Description was changed from

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, similar to the approach
for the [MeasureAs] attribute.

BUG=586594
==========

to

==========
Make [OriginTrialEnabled] and [RuntimeEnabled] mutually exclusive

The original intent of the [OriginTrialEnabled] extended attribute was
that it would be used as a drop-in replacement for [RuntimeEnabled]. The
implementation for enabling via origin trials includes a check for
runtime feature flags, so there is no need to use both attributes. When
implemented, a bug actually made both attributes required, in order to
generate the desired bindings code.

This CL fixes the bug so that the [RuntimeEnabled] attribute is not
required. An explicit check was added to cause an error if both
attributes are used on the same IDL member. The goal of the check is
avoid confusion - the intended usage of experimental features should be
clearer by marking with [RuntimeEnabled] vs [OriginTrialEnabled].

The implementation was also refactored, as the bindings logic was
mostly copy/paste reuse across attributes, constants, interfaces
and methods. The logic is more centralized now, similar to the approach
for the [MeasureAs] attribute.

BUG=586594

Committed: https://crrev.com/0c7efcf9ce5d2223c6aba69b94be77da64adea2b
Cr-Commit-Position: refs/heads/master@{#386080}
==========

[commit-bot: I haz the power, 2016-04-08 15:22:52]

Patchset 6 (id:??) landed as
https://crrev.com/0c7efcf9ce5d2223c6aba69b94be77da64adea2b
Cr-Commit-Position: refs/heads/master@{#386080}