Convert the overlay scrollbar flag to use FeatureList

base/test/scoped_feature_list.cc

Index: base/test/scoped_feature_list.cc
diff --git a/base/test/scoped_feature_list.cc b/base/test/scoped_feature_list.cc
index f0f3f4edfba1f1d333ef3188836cf14273795739..8f8535898bbfeac9b99ac9e103d62aa76542c1a7 100644
--- a/base/test/scoped_feature_list.cc
+++ b/base/test/scoped_feature_list.cc
@@ -4,7 +4,12 @@
 
 #include "base/test/scoped_feature_list.h"
 
+#include <algorithm>
 #include <string>
+#include <vector>
+
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 
 namespace base {
 namespace test {
@@ -70,5 +75,66 @@ void ScopedFeatureList::InitAndDisableFeature(const base::Feature& feature) {
   InitFromCommandLine(std::string(), feature.name);
 }
 
+void OverrideFeature(const StringPiece& feature,
+                     std::vector<std::string>* check_list,
+                     std::vector<std::string>* add_to_list) {
+  // Start with * means use default.
+  if (feature.starts_with("*"))
+    return;
+
+  // Remove field_trial info.
+  StringPiece feature_name = feature;
+  std::size_t index = feature.find("<");
+  if (index != std::string::npos)
+    feature_name = feature.substr(0, index);
+
+  if (std::any_of(
+          check_list->begin(), check_list->end(),
+          [&feature_name](std::string& s) { return feature_name == s; }))
+    return;
+
+  if (std::any_of(
+          add_to_list->begin(), add_to_list->end(),
+          [&feature_name](std::string& s) { return feature_name == s; }))
+    return;
+
+  add_to_list->push_back(feature_name.as_string());
+}
+
+void ScopedFeatureList::InitWithFeatureListAndOverrides(
+    base::FeatureList* feature_list,
+    const std::vector<std::string>& override_enabled_features,
+    const std::vector<std::string>& override_disabled_features) {
+  std::vector<std::string> merged_enabled_features(override_enabled_features);
+  std::vector<std::string> merged_disabled_features(override_disabled_features);
+
+  if (feature_list) {

[dcheng, 2017/04/18 00:18:05]

I still don't understand why we pass in feature_list, it's completely unused
other than this null check.

+    std::string enabled_features;
+    std::string disabled_features;
+    base::FeatureList::GetInstance()->GetFeatureOverrides(&enabled_features,
+                                                          &disabled_features);
+
+    std::vector<StringPiece> enabled_features_list = SplitStringPiece(
+        enabled_features, ",", TRIM_WHITESPACE, SPLIT_WANT_NONEMPTY);
+
+    std::vector<StringPiece> disabled_features_list = SplitStringPiece(
+        disabled_features, ",", TRIM_WHITESPACE, SPLIT_WANT_NONEMPTY);
+
+    for (StringPiece& feature : enabled_features_list) {
+      OverrideFeature(feature, &merged_disabled_features,
+                      &merged_enabled_features);
+    }
+
+    for (StringPiece& feature : disabled_features_list) {
+      OverrideFeature(feature, &merged_enabled_features,
+                      &merged_disabled_features);
+    }

[dcheng, 2017/04/18 00:18:05]

I think it just feels weird that:
1. GetFeatureOverrides() turns its internal override map into command-line
strings.
2. which we need to crack and parse here.
3. which we then feed back to FeatureList
4. which has to parse the input and turn it back into a list of overrides

I would suggest skipping steps 2 and 3 and just concatenating any input features
with the command-line that already exists. WDYT?

[dcheng, 2017/04/18 15:54:57]

I don't think this comment has been addressed.

Basically, I'm proposing that the input is just additional_disabled_features and
additional_enabled_features, and we just smash the the current disabled +
additional disabled together with a , to separate them; same for the current
enabled + additional enabled.

+  }
+
+  std::string enabled = JoinString(merged_enabled_features, ",");
+  std::string disabled = JoinString(merged_disabled_features, ",");
+  InitFromCommandLine(enabled, disabled);
+}
+
 }  // namespace test
 }  // namespace base