Expand FeatureList to support FieldTrial association.

base/feature_list.h

Index: base/feature_list.h
diff --git a/base/feature_list.h b/base/feature_list.h
index 6e4ad5859c4211bed7be3f2ef8ffa07943f01905..b5bee2e4cd91efeeb4d4cf7cb63202236c41fdc8 100644
--- a/base/feature_list.h
+++ b/base/feature_list.h
@@ -16,6 +16,8 @@
 
 namespace base {
 
+class FieldTrial;
+
 // Specifies whether a given feature is enabled or disabled by default.
 enum FeatureState {
   FEATURE_DISABLED_BY_DEFAULT,
@@ -83,6 +85,37 @@ class BASE_EXPORT FeatureList {
   void InitializeFromCommandLine(const std::string& enable_features,
                                  const std::string& disable_features);
 
+  // Specifies whether a feature override enables or disables the feature.
+  enum OverrideState {
+    OVERRIDE_DISABLE_FEATURE,
+    OVERRIDE_ENABLE_FEATURE,
+  };
+
+  // Registers a field trial to override the enabled state of the specified

[rkaplow, 2015/09/14 21:20:12]

shouldn't this just be override the state of the specificed feature... why do
you say 'enabled'?

[Alexei Svitkine (slow), 2015/09/14 22:03:43]

Hmm, I said "enabled state" instead of "state" to clarify what "state" refers
to. (Since perhaps a feature can have other states - e.g. one state is whether
it has an associated trial. So "enabled state" means whether the feature is
enabled or not.

But seems like this wording is confusing (at least, it was confusing to you).
Can you think of better wording?

[rkaplow, 2015/09/15 17:34:08]

Could have just been me on that read. Probably fine.

+  // feature to |override_state|. Command-line overrides still take precedence
+  // over field trials, so this will have no effect if the feature is being
+  // overridden from the command-line. The associated field trial will be
+  // activated when the feature state for this feature is queried. This should
+  // be called during registration, after InitializeFromCommandLine() has been
+  // called but before the instance is registered via SetInstance().
+  void RegisterFieldTrialOverride(const std::string& feature_name,
+                                  OverrideState override_state,
+                                  FieldTrial* field_trial);
+
+  // Creates and associates a field trial for reporting purposes corresponding
+  // to the command-line setting the feature state to |for_overridden_state|.
+  // This is a no-op and returns null if the specified feature is not being
+  // affected by a command-line override to the given state. Otherwise, it
+  // creates a field trial with name |field_trial_name| and a single group named
+  // |group_name| and returns it. The trial will be activated when the state of
+  // the feature is queried. This should be called during registration, after
+  // InitializeFromCommandLine() has been called but before the instance is
+  // registered via SetInstance().
+  FieldTrial* AssociateReportingFieldTrial(const std::string& feature_name,
+                                           OverrideState for_overridden_state,

[rkaplow, 2015/09/14 21:20:12]

seems a bit funny to call it for_overriden_state but not a big deal

[Alexei Svitkine (slow), 2015/09/14 22:03:43]

I guess I wanted to make it clear that the association is with a given
overridden state. I could just call it |overridden_state|, but seems it's not as
obvious then. Let me know what you think.

[rkaplow, 2015/09/15 17:34:08]

Hm, I'm still not mentally parsing how I'm supposed to be reading
'for_overriden_state' - still seems funny. Anyway not a big deal, up to you.

+                                           const std::string& field_trial_name,
+                                           const std::string& group_name);
+
   // Returns whether the given |feature| is enabled. Must only be called after
   // the singleton instance has been registered via SetInstance(). Additionally,
   // a feature with a given name must only have a single corresponding Feature
@@ -103,12 +136,6 @@ class BASE_EXPORT FeatureList {
  private:
   FRIEND_TEST_ALL_PREFIXES(FeatureListTest, CheckFeatureIdentity);
 
-  // Specifies whether a feature override enables or disables the feature.
-  enum OverrideState {
-    OVERRIDE_DISABLE_FEATURE,
-    OVERRIDE_ENABLE_FEATURE,
-  };
-
   // Finalizes the initialization state of the FeatureList, so that no further
   // overrides can be registered. This is called by SetInstance() on the
   // singleton feature list that is being registered.
@@ -121,9 +148,13 @@ class BASE_EXPORT FeatureList {
 
   // Registers an override for feature |feature_name|. The override specifies
   // whether the feature should be on or off (via |overridden_state|), which
-  // will take precedence over the feature's default state.
+  // will take precedence over the feature's default state. If |field_trial| is
+  // not null, registers the specified field trial object to be associated with
+  // the feature, which will activate the field trial when the feature state is
+  // queried.
   void RegisterOverride(const std::string& feature_name,
-                        OverrideState overridden_state);
+                        OverrideState overridden_state,
+                        FieldTrial* field_trial);
 
   // Verifies that there's only a single definition of a Feature struct for a
   // given feature name. Keeps track of the first seen Feature struct for each
@@ -136,9 +167,13 @@ class BASE_EXPORT FeatureList {
     // The overridden enable (on/off) state of the feature.
     const OverrideState overridden_state;
 
+    // An optional associated field trial, which will be activated when the
+    // state of the feature is queried for the first time.
+    base::FieldTrial* field_trial;
+
     // TODO(asvitkine): Expand this as more support is added.
 
-    explicit OverrideEntry(OverrideState overridden_state);
+    OverrideEntry(OverrideState overridden_state, FieldTrial* field_trial);
   };
   // Map from feature name to an OverrideEntry struct for the feature, if it
   // exists.