[Experimental Framework] Rename IDL attribute to OriginTrialEnabled

[Experimental Framework] Rename IDL attribute to OriginTrialEnabled

R=haraken@chromium.org
TBR=chasej@chromium.org
BUG=575808
Committed: https://crrev.com/48848bca460b8ce12e89ca6314ef121063465fa2
Cr-Commit-Position: refs/heads/master@{#375123}

[iclelland, 2016-02-11 04:13:13]

haraken, PTAL (and CQ if OK)

Thanks!

[haraken, 2016-02-11 05:46:56]

LGTM

https://codereview.chromium.org/1685393002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right):

https://codereview.chromium.org/1685393002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/v8_attributes.py:106:
'origin_trial_enabled': v8_utilities.origin_trial_enabled_function(attribute), 
# [OriginTrialEnabled]

Alphabetical order please. (The same comment for other parts in this CL.)

[iclelland, 2016-02-11 15:21:57]

https://codereview.chromium.org/1685393002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right):

https://codereview.chromium.org/1685393002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/v8_attributes.py:106:
'origin_trial_enabled': v8_utilities.origin_trial_enabled_function(attribute), 
# [OriginTrialEnabled]
On 2016/02/11 05:46:56, haraken wrote:
> 
> Alphabetical order please. (The same comment for other parts in this CL.)

Thanks - I missed that. I have moved these ones into order, but have not
corrected any existing out-of-order context dict entries.

[chasej, 2016-02-11 20:01:50]

I saw the latest patch addressed some of my offline comments - thanks for that.

There's a few other things remaining.

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

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:611: # [DeprecateAs],
[Reflect], [RuntimeEnabled]
I think this comment should be updated. As is, it could be confusing to mention
[RuntimeEnabled], but then the function has no reference to that attribute. 
Probably some explanation that [OriginTrialEnabled] includes the check for
[RuntimeEnabled] which is why there's no code here for that attribute.

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/utilities.cpp (right):

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/utilities.cpp:58: {% macro
check_origin_trial_internal(errorName, experiment_name) %}
Should rename experiment_name, as in earlier comments.

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl
(right):

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl:41:
RuntimeEnabled=FeatureName,
I would like to see ExperimentName renamed, to get rid of any usages of
"experiment".  That would wrap up the renaming in this CL.

However, based on offline discussion, both [OriginTrialEnabled] and
[RuntimeEnabled] should be using the same value, namely the runtime flag name
(as would be defined in RuntimeEnabledFeatures.in).  Further, there will be
larger changes to how the [OriginTrialEnabled] attribute works.

I'd suggest to rename/fix here, if it doesn't have a cascading effect on the
tests/results files.  If it require updates to many other files, I'm fine to
leave it for subsequent CLs.

[iclelland, 2016-02-11 21:37:37]

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

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:611: # [DeprecateAs],
[Reflect], [RuntimeEnabled]
On 2016/02/11 20:01:50, chasej wrote:
> I think this comment should be updated. As is, it could be confusing to
mention
> [RuntimeEnabled], but then the function has no reference to that attribute. 
> Probably some explanation that [OriginTrialEnabled] includes the check for
> [RuntimeEnabled] which is why there's no code here for that attribute.

[RuntimeEnabled] is referenced on line 631 here, which computes
'runtime_enabled_function'. It's just not called out in comments, which I can
remedy.

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/utilities.cpp (right):

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/utilities.cpp:58: {% macro
check_origin_trial_internal(errorName, experiment_name) %}
On 2016/02/11 20:01:50, chasej wrote:
> Should rename experiment_name, as in earlier comments.

Done.

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl
(right):

https://codereview.chromium.org/1685393002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl:41:
RuntimeEnabled=FeatureName,
On 2016/02/11 20:01:50, chasej wrote:
> I would like to see ExperimentName renamed, to get rid of any usages of
> "experiment".  That would wrap up the renaming in this CL.
> 
> However, based on offline discussion, both [OriginTrialEnabled] and
> [RuntimeEnabled] should be using the same value, namely the runtime flag name
> (as would be defined in RuntimeEnabledFeatures.in).  Further, there will be
> larger changes to how the [OriginTrialEnabled] attribute works.
> 
> I'd suggest to rename/fix here, if it doesn't have a cascading effect on the
> tests/results files.  If it require updates to many other files, I'm fine to
> leave it for subsequent CLs.

Done.

[chasej, 2016-02-11 23:45:45]

lgtm

[iclelland, 2016-02-12 04:02:07]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2016-02-12 04:02:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, chasej@chromium.org
Link to the patchset: https://codereview.chromium.org/1685393002/#ps80001
(title: "Change existing usage of APIExperimentEnabled")

[commit-bot: I haz the power, 2016-02-12 04:03:54]

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

[commit-bot: I haz the power, 2016-02-12 05:10:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-16 22:41:41]

Description was changed from

==========
[Experimental Framework] Rename IDL attribute to OriginTrialEnabled

R=haraken@chromium.org
TBR=chasej@chromium.org
BUG=575808
==========

to

==========
[Experimental Framework] Rename IDL attribute to OriginTrialEnabled

R=haraken@chromium.org
TBR=chasej@chromium.org
BUG=575808

Committed: https://crrev.com/48848bca460b8ce12e89ca6314ef121063465fa2
Cr-Commit-Position: refs/heads/master@{#375123}
==========

[commit-bot: I haz the power, 2016-02-16 22:41:41]

Patchset 5 (id:??) landed as
https://crrev.com/48848bca460b8ce12e89ca6314ef121063465fa2
Cr-Commit-Position: refs/heads/master@{#375123}