[Origin Trials] Clarify the distinction between trials and features

[Origin Trials] Clarify the distinction between trials and features

There is some confusion in the origin trial method names and comments
between the name of a feature, which is declared in
RuntimeEnabledFeatures, and a trial, which can enable multiple features.

This CL attempts to make a distinction between the two, most visibly by
renaming OriginTrialContext::isFeatureEnabled to
OriginTrialContext::isTrialEnabled.

R=chasej@chromium.org

Committed: https://crrev.com/a420fe39c98570d80347675134d3738af20c63b7
Cr-Commit-Position: refs/heads/master@{#424033}

[iclelland, 2016-10-06 13:34:53]

The CQ bit was checked by iclelland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 13:35:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[iclelland, 2016-10-06 13:35:41]

+r chasej, PTAL and see if you think this will help with some of the confusion
around the use of these methods

Thanks!

[chasej, 2016-10-06 14:57:39]

Thanks for the cleanups here.

For consistency, we should probably change OriginTrialPolicy and the component
installer. This CL addresses the most confusing parts, so I think it's fine to
defer the other renames. I'll make a note to address those classes when
implementing the token revocation.

https://codereview.chromium.org/2398793003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/origin_trials/OriginTrialContext.h (right):

https://codereview.chromium.org/2398793003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/origin_trials/OriginTrialContext.h:81: bool
featureBindingsInstalled(const String& featureName);
What about these methods? It doesn't look they are used anymore? Seems like
either these should be removed, or renamed to use "trial" instead of "feature".

https://codereview.chromium.org/2398793003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/origin_trials/OriginTrialContext.h:95:
HashSet<String> m_bindingsInstalled;
Ditto for my comment above about remove or rename.

[iclelland, 2016-10-06 14:59:04]

https://codereview.chromium.org/2398793003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/origin_trials/OriginTrialContext.h (right):

https://codereview.chromium.org/2398793003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/origin_trials/OriginTrialContext.h:81: bool
featureBindingsInstalled(const String& featureName);
On 2016/10/06 14:57:39, chasej wrote:
> What about these methods? It doesn't look they are used anymore? Seems like
> either these should be removed, or renamed to use "trial" instead of
"feature".

I was preparing another CL to remove them entirely; I don't care which one lands
first, really. I just avoided renaming them here since they're going away.

[commit-bot: I haz the power, 2016-10-06 15:11:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-06 15:11:39]

Dry run: This issue passed the CQ dry run.

[chasej, 2016-10-06 15:18:51]

lgtm

[haraken, 2016-10-06 16:01:19]

LGTM

[iclelland, 2016-10-06 17:31:24]

On 2016/10/06 16:01:19, haraken wrote:
> LGTM

Thanks!

[iclelland, 2016-10-06 17:31:30]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2016-10-06 17:32:02]

The CQ bit was unchecked by iclelland@chromium.org

[iclelland, 2016-10-07 20:30:43]

The CQ bit was checked by iclelland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 20:31:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 21:52:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 21:52:42]

Dry run: This issue passed the CQ dry run.

[iclelland, 2016-10-08 01:02:38]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2016-10-08 01:02:39]

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/2398793003/#ps20001
(title: "Rebase; -DurableStorage, +WebVR")

[commit-bot: I haz the power, 2016-10-08 01:03:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-08 01:10:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-08 01:12:39]

Description was changed from

==========
[Origin Trials] Clarify the distinction between trials and features

There is some confusion in the origin trial method names and comments
between the name of a feature, which is declared in
RuntimeEnabledFeatures, and a trial, which can enable multiple features.

This CL attempts to make a distinction between the two, most visibly by
renaming OriginTrialContext::isFeatureEnabled to
OriginTrialContext::isTrialEnabled.

R=chasej@chromium.org
==========

to

==========
[Origin Trials] Clarify the distinction between trials and features

There is some confusion in the origin trial method names and comments
between the name of a feature, which is declared in
RuntimeEnabledFeatures, and a trial, which can enable multiple features.

This CL attempts to make a distinction between the two, most visibly by
renaming OriginTrialContext::isFeatureEnabled to
OriginTrialContext::isTrialEnabled.

R=chasej@chromium.org

Committed: https://crrev.com/a420fe39c98570d80347675134d3738af20c63b7
Cr-Commit-Position: refs/heads/master@{#424033}
==========

[commit-bot: I haz the power, 2016-10-08 01:12:40]

Patchset 2 (id:??) landed as
https://crrev.com/a420fe39c98570d80347675134d3738af20c63b7
Cr-Commit-Position: refs/heads/master@{#424033}