Plumb base::Feature field trial association to renderers.

base/feature_list.cc

Index: base/feature_list.cc
diff --git a/base/feature_list.cc b/base/feature_list.cc
index c06ba8b3bfdf6be87e55321f08b52a8c71d2bac9..b4005a15fe4c77b930e0dfcf05df27ef5e98c6ce 100644
--- a/base/feature_list.cc
+++ b/base/feature_list.cc
@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 
 namespace base {
 
@@ -20,6 +21,16 @@ namespace {
 // have more control over initialization timing. Leaky.
 FeatureList* g_instance = nullptr;
 
+// Some characters are not allowed to appear in feature names or the associated
+// field trial names, as they are used as special characters for command-line
+// serialization. This function checks that the strings are ASCII (since they
+// are used in command-line API functions that require ASCII) and if there are

[Ilya Sherman, 2015/11/30 18:54:40]

Optional nit: s/if/whether helps clarity IMO

[Alexei Svitkine (slow), 2015/11/30 19:44:35]

Done.

+// any reserved characters present, returning true if the string is valid. Only
+// called in DCHECKs.
+bool ValidateFeatureOrFieldTrialName(const std::string& name) {

[Ilya Sherman, 2015/11/30 18:54:40]

nit: Perhaps "IsValidFeatureOrFieldTrialName"?

[Alexei Svitkine (slow), 2015/11/30 19:44:35]

Done.

+  return IsStringASCII(name) && name.find_first_of(",<") == std::string::npos;
+}
+
 }  // namespace
 
 FeatureList::FeatureList() : initialized_(false) {}
@@ -33,12 +44,8 @@ void FeatureList::InitializeFromCommandLine(
 
   // Process disabled features first, so that disabled ones take precedence over
   // enabled ones (since RegisterOverride() uses insert()).
-  for (const auto& feature_name : SplitFeatureListString(disable_features)) {
-    RegisterOverride(feature_name, OVERRIDE_DISABLE_FEATURE, nullptr);
-  }
-  for (const auto& feature_name : SplitFeatureListString(enable_features)) {
-    RegisterOverride(feature_name, OVERRIDE_ENABLE_FEATURE, nullptr);
-  }
+  RegisterOverridesFromCommandLine(disable_features, OVERRIDE_DISABLE_FEATURE);
+  RegisterOverridesFromCommandLine(enable_features, OVERRIDE_ENABLE_FEATURE);
 }
 
 bool FeatureList::IsFeatureOverriddenFromCommandLine(
@@ -91,18 +98,23 @@ void FeatureList::GetFeatureOverrides(std::string* enable_overrides,
   disable_overrides->clear();
 
   for (const auto& entry : overrides_) {
+    std::string* target_list = nullptr;
     switch (entry.second.overridden_state) {
       case OVERRIDE_ENABLE_FEATURE:
-        if (!enable_overrides->empty())
-          enable_overrides->push_back(',');
-        enable_overrides->append(entry.first);
+        target_list = enable_overrides;
         break;
       case OVERRIDE_DISABLE_FEATURE:
-        if (!disable_overrides->empty())
-          disable_overrides->push_back(',');
-        disable_overrides->append(entry.first);
+        target_list = disable_overrides;
         break;
     }
+
+    if (!target_list->empty())
+      target_list->push_back(',');
+    target_list->append(entry.first);
+    if (entry.second.field_trial) {
+      target_list->push_back('<');
+      target_list->append(entry.second.field_trial->trial_name());
+    }
   }
 }
 
@@ -151,6 +163,7 @@ void FeatureList::FinalizeInitialization() {
 
 bool FeatureList::IsFeatureEnabled(const Feature& feature) {
   DCHECK(initialized_);
+  DCHECK(ValidateFeatureOrFieldTrialName(feature.name)) << feature.name;
   DCHECK(CheckFeatureIdentity(feature)) << feature.name;
 
   auto it = overrides_.find(feature.name);
@@ -168,15 +181,39 @@ bool FeatureList::IsFeatureEnabled(const Feature& feature) {
   return feature.default_state == FEATURE_ENABLED_BY_DEFAULT;
 }
 
-void FeatureList::RegisterOverride(const std::string& feature_name,
+void FeatureList::RegisterOverridesFromCommandLine(
+    const std::string& feature_list,
+    OverrideState overridden_state) {
+  for (const auto& value : SplitFeatureListString(feature_list)) {
+    StringPiece feature_name(value);
+    base::FieldTrial* trial = nullptr;
+
+    // The entry may be of the form FeatureName<FieldTrialName - in which case,
+    // this splits off the field trial name and associates it with the override.
+    std::string::size_type pos = feature_name.find('<');
+    if (pos != std::string::npos) {
+      feature_name.set(value.data(), pos);
+      trial = base::FieldTrialList::Find(value.substr(pos + 1));
+    }
+
+    RegisterOverride(feature_name, overridden_state, trial);
+  }
+}
+
+void FeatureList::RegisterOverride(StringPiece feature_name,
                                    OverrideState overridden_state,
                                    FieldTrial* field_trial) {
   DCHECK(!initialized_);
+  if (field_trial) {
+    DCHECK(ValidateFeatureOrFieldTrialName(field_trial->trial_name()))
+        << field_trial->trial_name();
+  }
+
   // Note: The semantics of insert() is that it does not overwrite the entry if
   // one already exists for the key. Thus, only the first override for a given
   // feature name takes effect.
   overrides_.insert(std::make_pair(
-      feature_name, OverrideEntry(overridden_state, field_trial)));
+      feature_name.as_string(), OverrideEntry(overridden_state, field_trial)));
 }
 
 bool FeatureList::CheckFeatureIdentity(const Feature& feature) {