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
+  // 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,
+                                           const std::string& field_trial_name,
+                                           const std::string& group_name);

[Ilya Sherman, 2015/09/16 00:50:43]

This API doesn't make it especially obvious that AssociateReportingFieldTrial()
should always precede calls to RegisterFieldTrialOverride(), and that the latter
should only be called if AssociateReportingFieldTrial() doesn't associate.  It
would be nice to document this, and possibly add test coverage for the behavior
if both are called.

+
   // 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.

[Ilya Sherman, 2015/09/16 00:50:43]

Please document ownership and lifetime expectations.

[Alexei Svitkine (slow), 2015/09/22 21:19:59]

Done.

+    base::FieldTrial* field_trial;
+
     // TODO(asvitkine): Expand this as more support is added.

[Ilya Sherman, 2015/09/16 00:50:43]

nit: Ok to remove this?  You removed a similar comment in the .cc file.

[Alexei Svitkine (slow), 2015/09/22 21:19:59]

There's still the kill-switch semantics that are planned to be added. I've
restored the comment in the .cc file.

 
-    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.