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..d5f45d10c0c702a5656a70928395282cf6f2a24c 100644 |
--- a/base/test/scoped_feature_list.cc |
+++ b/base/test/scoped_feature_list.cc |
@@ -4,24 +4,65 @@ |
#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 { |
namespace { |
-static std::string GetFeatureString( |
+static std::vector<std::string> GetFeatureString( |
Ilya Sherman
2017/04/21 22:43:33
nit: s/String/Vector
Ilya Sherman
2017/04/21 22:43:34
nit: No need for "static" within an anonymous name
|
const std::initializer_list<base::Feature>& features) { |
- std::string output; |
- for (const base::Feature& feature : features) { |
- if (!output.empty()) |
- output += ","; |
- output += feature.name; |
- } |
+ std::vector<std::string> output; |
+ for (const base::Feature& feature : features) |
Ilya Sherman
2017/04/21 22:43:33
nit: Please add curly braces. I'm not sure whethe
|
+ output.push_back(feature.name); |
+ |
return output; |
} |
+static StringPiece GetFeatureName(const StringPiece& feature) { |
Ilya Sherman
2017/04/21 22:43:34
Please add documentation for this function.
Ilya Sherman
2017/04/21 22:43:34
Per the StringPiece documentation, it's preferable
|
+ StringPiece feature_name = feature; |
+ |
+ // Remove started *. |
Ilya Sherman
2017/04/21 22:43:33
This just repeats what the code does. Please inst
|
+ if (feature_name.starts_with("*")) |
+ feature_name.substr(1); |
Ilya Sherman
2017/04/21 22:43:34
Did you mean to assign this back to feature_name?
|
+ |
+ // Remove field_trial info. |
+ std::size_t index = feature_name.find("<"); |
+ if (index != std::string::npos) |
+ feature_name = feature_name.substr(0, index); |
+ |
+ return feature_name; |
+} |
+ |
+// Given features from FeatureList::GetFeatureOverrides, add them to overrides |
+// if it is not existing in already_overridden_feature_names or overrides. |
Ilya Sherman
2017/04/21 22:43:33
If you take my advice about passing a unioned set
|
+static void OverrideFeatures( |
+ const std::string& features, |
+ std::vector<std::string>* already_overridden_feature_names, |
+ std::vector<std::string>* overrides) { |
+ std::vector<StringPiece> features_list = |
+ SplitStringPiece(features, ",", TRIM_WHITESPACE, SPLIT_WANT_NONEMPTY); |
+ |
+ for (StringPiece& feature : features_list) { |
Ilya Sherman
2017/04/21 22:43:34
nit: Chromium strongly frowns upon non-const refer
|
+ StringPiece feature_name = GetFeatureName(feature); |
+ |
+ if (std::count(already_overridden_feature_names->begin(), |
+ already_overridden_feature_names->end(), feature_name) > 0) |
+ continue; |
+ |
+ if (std::count(overrides->begin(), overrides->end(), feature_name) > 0) |
+ continue; |
+ |
+ overrides->push_back(feature.as_string()); |
+ } |
+} |
+ |
} // namespace |
ScopedFeatureList::ScopedFeatureList() {} |
@@ -40,13 +81,6 @@ void ScopedFeatureList::Init() { |
InitWithFeatureList(std::move(feature_list)); |
} |
-void ScopedFeatureList::InitWithFeatures( |
- const std::initializer_list<base::Feature>& enabled_features, |
- const std::initializer_list<base::Feature>& disabled_features) { |
- InitFromCommandLine(GetFeatureString(enabled_features), |
- GetFeatureString(disabled_features)); |
-} |
- |
void ScopedFeatureList::InitWithFeatureList( |
std::unique_ptr<FeatureList> feature_list) { |
DCHECK(!original_feature_list_); |
@@ -63,11 +97,36 @@ void ScopedFeatureList::InitFromCommandLine( |
} |
void ScopedFeatureList::InitAndEnableFeature(const base::Feature& feature) { |
- InitFromCommandLine(feature.name, std::string()); |
+ InitWithFeatures({feature}, {}); |
} |
void ScopedFeatureList::InitAndDisableFeature(const base::Feature& feature) { |
- InitFromCommandLine(std::string(), feature.name); |
+ InitWithFeatures({}, {feature}); |
+} |
+ |
+void ScopedFeatureList::InitWithFeatures( |
Ilya Sherman
2017/04/21 22:43:34
Please position this within the .cc file in the sa
|
+ const std::initializer_list<base::Feature>& enabled_features, |
+ const std::initializer_list<base::Feature>& disabled_features) { |
+ std::vector<std::string> merged_enabled_features = |
+ GetFeatureString(enabled_features); |
+ std::vector<std::string> merged_disabled_features = |
+ GetFeatureString(disabled_features); |
Ilya Sherman
2017/04/21 22:43:34
Please construct a set containing the union of the
|
+ |
+ base::FeatureList* feature_list = base::FeatureList::GetInstance(); |
+ if (feature_list) { |
+ std::string enabled_features; |
+ std::string disabled_features; |
+ base::FeatureList::GetInstance()->GetFeatureOverrides(&enabled_features, |
+ &disabled_features); |
+ OverrideFeatures(enabled_features, &merged_disabled_features, |
Ilya Sherman
2017/04/21 22:43:34
The second param is not an outparam, so please pas
|
+ &merged_enabled_features); |
+ OverrideFeatures(disabled_features, &merged_enabled_features, |
+ &merged_disabled_features); |
+ } |
+ |
+ std::string enabled = JoinString(merged_enabled_features, ","); |
+ std::string disabled = JoinString(merged_disabled_features, ","); |
+ InitFromCommandLine(enabled, disabled); |
} |
} // namespace test |