Add support for multiple simultaneous subresource_filter::Configurations.

components/subresource_filter/core/browser/subresource_filter_features_unittest.cc

Index: components/subresource_filter/core/browser/subresource_filter_features_unittest.cc
diff --git a/components/subresource_filter/core/browser/subresource_filter_features_unittest.cc b/components/subresource_filter/core/browser/subresource_filter_features_unittest.cc
index 28b8f5108fc4637e128663879796331a3f62516f..b713fb5e61e0297a1d75bde0a2c36071220e3d0f 100644
--- a/components/subresource_filter/core/browser/subresource_filter_features_unittest.cc
+++ b/components/subresource_filter/core/browser/subresource_filter_features_unittest.cc
@@ -7,17 +7,20 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <utility>
+#include <vector>
 
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/field_trial_params.h"
+#include "base/strings/string_util.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"
 #include "components/variations/variations_associated_data.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace subresource_filter {
-namespace testing {
 
 namespace {
 
@@ -50,14 +53,41 @@ class ScopedExperimentalStateToggle {
  private:
   base::FieldTrialList field_trial_list_;
 
-  ScopedSubresourceFilterConfigurator scoped_configurator_;
+  testing::ScopedSubresourceFilterConfigurator scoped_configurator_;
   base::test::ScopedFeatureList scoped_feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedExperimentalStateToggle);
 };
 
+void ExpectAndRetrieveExactlyOneEnabledConfig(Configuration* actual_config) {
+  DCHECK(actual_config);
+  const auto config_list = GetEnabledConfigurations();
+  ASSERT_EQ(1u, config_list->configs_by_decreasing_priority().size());
+  *actual_config = config_list->configs_by_decreasing_priority().front();
+}
+
+void ExpectPresetCanBeEnabledByName(Configuration preset, const char* name) {
+  ScopedExperimentalStateToggle scoped_experimental_state(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE,
+      {{kEnablePresetsParameterName, name}});
+
+  const auto config_list = GetEnabledConfigurations();
+  EXPECT_THAT(config_list->configs_by_decreasing_priority(),
+              ::testing::ElementsAre(preset, Configuration()));
+}
+
+void ExpectPresetIsEquivalentToVariationParams(
+    Configuration preset,
+    std::map<std::string, std::string> variation_params) {
+  ScopedExperimentalStateToggle scoped_experimental_state(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE, variation_params);
+
+  Configuration experimental_configuration;
+  ExpectAndRetrieveExactlyOneEnabledConfig(&experimental_configuration);
+  EXPECT_EQ(preset, experimental_configuration);
+}
+
 }  // namespace
-}  // namespace testing
 
 TEST(SubresourceFilterFeaturesTest, ActivationLevel) {
   const struct {
@@ -83,18 +113,18 @@ TEST(SubresourceFilterFeaturesTest, ActivationLevel) {
     SCOPED_TRACE(::testing::Message("ActivationLevelParam = \"")
                  << test_case.activation_level_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kActivationLevelParameterName, test_case.activation_level_param},
          {kActivationScopeParameterName, kActivationScopeNoSites}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
     EXPECT_EQ(test_case.expected_activation_level,
-              actual_configuration.activation_level);
-    EXPECT_EQ(ActivationScope::NO_SITES, actual_configuration.activation_scope);
+              actual_configuration.activation_options.activation_level);
+    EXPECT_EQ(ActivationScope::NO_SITES,
+              actual_configuration.activation_conditions.activation_scope);
   }
 }
 
@@ -122,18 +152,18 @@ TEST(SubresourceFilterFeaturesTest, ActivationScope) {
     SCOPED_TRACE(::testing::Message("ActivationScopeParam = \"")
                  << test_case.activation_scope_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kActivationLevelParameterName, kActivationLevelDisabled},
          {kActivationScopeParameterName, test_case.activation_scope_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
-    EXPECT_EQ(ActivationLevel::DISABLED, actual_configuration.activation_level);
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
+    EXPECT_EQ(ActivationLevel::DISABLED,
+              actual_configuration.activation_options.activation_level);
     EXPECT_EQ(test_case.expected_activation_scope,
-              actual_configuration.activation_scope);
+              actual_configuration.activation_conditions.activation_scope);
   }
 }
 
@@ -175,19 +205,24 @@ TEST(SubresourceFilterFeaturesTest, ActivationLevelAndScope) {
        kActivationScopeAllSites, ActivationScope::NO_SITES}};
 
   for (const auto& test_case : kTestCases) {
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    SCOPED_TRACE(::testing::Message("Enabled = ") << test_case.feature_enabled);
+    SCOPED_TRACE(::testing::Message("ActivationLevelParam = \"")
+                 << test_case.activation_level_param << "\"");
+    SCOPED_TRACE(::testing::Message("ActivationScopeParam = \"")
+                 << test_case.activation_scope_param << "\"");
+
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kActivationLevelParameterName, test_case.activation_level_param},
          {kActivationScopeParameterName, test_case.activation_scope_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
     EXPECT_EQ(test_case.expected_activation_level,
-              actual_configuration.activation_level);
+              actual_configuration.activation_options.activation_level);
     EXPECT_EQ(test_case.expected_activation_scope,
-              actual_configuration.activation_scope);
+              actual_configuration.activation_conditions.activation_scope);
   }
 }
 
@@ -232,18 +267,56 @@ TEST(SubresourceFilterFeaturesTest, ActivationList) {
     SCOPED_TRACE(::testing::Message("ActivationListParam = \"")
                  << test_case.activation_list_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kActivationLevelParameterName, kActivationLevelDisabled},
          {kActivationScopeParameterName, kActivationScopeNoSites},
          {kActivationListsParameterName, test_case.activation_list_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
     EXPECT_EQ(test_case.expected_activation_list,
-              actual_configuration.activation_list);
+              actual_configuration.activation_conditions.activation_list);
+  }
+}
+
+TEST(SubresourceFilterFeaturesTest, ActivationPriority) {
+  const struct {
+    bool feature_enabled;
+    const char* activation_priority_param;
+    int expected_priority;
+  } kTestCases[] = {{false, "", 0},
+                    {false, "not_an_integer", 0},
+                    {false, "100", 0},
+                    {true, "", 0},
+                    {true, "not_an_integer", 0},
+                    {true, "0.5not_an_integer", 0},
+                    {true, "garbage42", 0},
+                    {true, "42garbage", 42},
+                    {true, "0", 0},
+                    {true, "1", 1},
+                    {true, "-1", -1},
+                    {true, "2.9", 2},
+                    {true, "-2.9", -2},
+                    {true, "2e0", 2},
+                    {true, "100", 100}};
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(::testing::Message("Enabled = ") << test_case.feature_enabled);
+    SCOPED_TRACE(::testing::Message("Priority = \"")
+                 << test_case.activation_priority_param << "\"");
+
+    ScopedExperimentalStateToggle scoped_experimental_state(
+        test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
+                                  : base::FeatureList::OVERRIDE_USE_DEFAULT,
+        {{kActivationPriorityParameterName,
+          test_case.activation_priority_param}});
+
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
+    EXPECT_EQ(test_case.expected_priority,
+              actual_configuration.activation_conditions.priority);
   }
 }
 
@@ -271,17 +344,17 @@ TEST(SubresourceFilterFeaturesTest, PerfMeasurementRate) {
     SCOPED_TRACE(::testing::Message("PerfMeasurementParam = \"")
                  << test_case.perf_measurement_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kPerformanceMeasurementRateParameterName,
           test_case.perf_measurement_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
-    EXPECT_EQ(test_case.expected_perf_measurement_rate,
-              actual_configuration.performance_measurement_rate);
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
+    EXPECT_EQ(
+        test_case.expected_perf_measurement_rate,
+        actual_configuration.activation_options.performance_measurement_rate);
   }
 }
 
@@ -306,17 +379,17 @@ TEST(SubresourceFilterFeaturesTest, SuppressNotifications) {
     SCOPED_TRACE(::testing::Message("SuppressNotificationsParam = \"")
                  << test_case.suppress_notifications_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kSuppressNotificationsParameterName,
           test_case.suppress_notifications_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
-    EXPECT_EQ(test_case.expected_suppress_notifications_value,
-              actual_configuration.should_suppress_notifications);
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
+    EXPECT_EQ(
+        test_case.expected_suppress_notifications_value,
+        actual_configuration.activation_options.should_suppress_notifications);
   }
 }
 
@@ -341,17 +414,17 @@ TEST(SubresourceFilterFeaturesTest, WhitelistSiteOnReload) {
     SCOPED_TRACE(::testing::Message("WhitelistSiteOnReloadParam = \"")
                  << test_case.whitelist_site_on_reload_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kWhitelistSiteOnReloadParameterName,
           test_case.whitelist_site_on_reload_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
     EXPECT_EQ(test_case.expected_whitelist_site_on_reload_value,
-              actual_configuration.should_whitelist_site_on_reload);
+              actual_configuration.activation_options
+                  .should_whitelist_site_on_reload);
   }
 }
 
@@ -369,16 +442,188 @@ TEST(SubresourceFilterFeaturesTest, RulesetFlavor) {
     SCOPED_TRACE(::testing::Message("Flavor = \"")
                  << test_case.ruleset_flavor_param << "\"");
 
-    testing::ScopedExperimentalStateToggle scoped_experimental_state(
+    ScopedExperimentalStateToggle scoped_experimental_state(
         test_case.feature_enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
                                   : base::FeatureList::OVERRIDE_USE_DEFAULT,
         {{kRulesetFlavorParameterName, test_case.ruleset_flavor_param}});
 
-    const auto active_configurations = GetActiveConfigurations();
-    const Configuration& actual_configuration =
-        active_configurations->the_one_and_only();
+    Configuration actual_configuration;
+    ExpectAndRetrieveExactlyOneEnabledConfig(&actual_configuration);
     EXPECT_EQ(std::string(test_case.expected_ruleset_flavor_value),
-              actual_configuration.ruleset_flavor);
+              actual_configuration.general_settings.ruleset_flavor);
   }
 }
+
+TEST(SubresourceFilterFeaturesTest, LexicographicallyGreatestRulesetFlavor) {
+  const struct {
+    const char* expected_ruleset_flavor_selected;
+    std::vector<std::string> ruleset_flavors;
+  } kTestCases[] = {{"", std::vector<std::string>()},
+                    {"", {""}},
+                    {"a", {"a"}},
+                    {"e", {"e"}},
+                    {"foo", {"foo"}},
+                    {"", {"", ""}},
+                    {"a", {"a", ""}},
+                    {"a", {"", "a"}},
+                    {"a", {"a", "a"}},
+                    {"c", {"b", "", "c"}},
+                    {"b", {"", "b", "a"}},
+                    {"aa", {"", "a", "aa"}},
+                    {"b", {"", "a", "aa", "b"}},
+                    {"foo", {"foo", "bar", "b", ""}},
+                    {"2.1", {"2", "2.1", "1.3", ""}},
+                    {"3", {"2", "2.1", "1.3", "3"}}};
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(::testing::Message()
+                 << "ruleset_flavors: "
+                 << ::testing::PrintToString(test_case.ruleset_flavors));
+
+    std::vector<Configuration> configs;
+    for (const auto& ruleset_flavor : test_case.ruleset_flavors) {
+      Configuration config;
+      config.general_settings.ruleset_flavor = ruleset_flavor;
+      configs.push_back(std::move(config));
+    }
+
+    subresource_filter::testing::ScopedSubresourceFilterConfigurator
+        scoped_configuration(std::move(configs));
+    EXPECT_EQ(test_case.expected_ruleset_flavor_selected,
+              GetEnabledConfigurations()
+                  ->lexicographically_greatest_ruleset_flavor());
+  }
+}
+
+TEST(SubresourceFilterFeaturesTest, EnabledConfigurations_FeatureDisabled) {
+  ScopedExperimentalStateToggle scoped_experimental_state(
+      base::FeatureList::OVERRIDE_USE_DEFAULT,
+      std::map<std::string, std::string>());
+
+  const auto config_list = GetEnabledConfigurations();
+  EXPECT_THAT(config_list->configs_by_decreasing_priority(),
+              ::testing::ElementsAre(Configuration()));
+  EXPECT_EQ(std::string(),
+            config_list->lexicographically_greatest_ruleset_flavor());
+}
+
+TEST(SubresourceFilterFeaturesTest,
+     EnabledConfigurations_FeatureEnabledWithNoParameters) {

[Charlie Harrison, 2017/05/05 21:16:37]

Same as the previous test? Or maybe my eyes are just glazing over?

[engedy, 2017/05/05 21:31:53]

No, it's my eyes. Fixed. :)

+  ScopedExperimentalStateToggle scoped_experimental_state(
+      base::FeatureList::OVERRIDE_USE_DEFAULT,
+      std::map<std::string, std::string>());
+
+  const auto config_list = GetEnabledConfigurations();
+  EXPECT_THAT(config_list->configs_by_decreasing_priority(),
+              ::testing::ElementsAre(Configuration()));
+  EXPECT_EQ(std::string(),
+            config_list->lexicographically_greatest_ruleset_flavor());
+}
+
+TEST(SubresourceFilterFeaturesTest, PresetForLiveRunOnPhishingSites) {
+  ExpectPresetCanBeEnabledByName(
+      Configuration::MakePresetForLiveRunOnPhishingSites(),
+      kPresetLiveRunOnPhishingSites);
+  ExpectPresetIsEquivalentToVariationParams(
+      Configuration::MakePresetForLiveRunOnPhishingSites(),
+      {{kActivationLevelParameterName, kActivationLevelEnabled},
+       {kActivationScopeParameterName, kActivationScopeActivationList},
+       {kActivationListsParameterName, kActivationListPhishingInterstitial},
+       {kActivationPriorityParameterName, "100"}});
+}
+
+TEST(SubresourceFilterFeaturesTest,
+     PresetForPerformanceTestingDryRunOnAllSites) {
+  ExpectPresetCanBeEnabledByName(
+      Configuration::MakePresetForPerformanceTestingDryRunOnAllSites(),
+      kPresetPerformanceTestingDryRunOnAllSites);
+  ExpectPresetIsEquivalentToVariationParams(
+      Configuration::MakePresetForPerformanceTestingDryRunOnAllSites(),
+      {{kActivationLevelParameterName, kActivationLevelDryRun},
+       {kActivationScopeParameterName, kActivationScopeAllSites},
+       {kActivationPriorityParameterName, "50"},
+       {kPerformanceMeasurementRateParameterName, "1.0"}});
+}
+
+TEST(SubresourceFilterFeaturesTest, ConfigurationPriorities) {
+  const std::vector<Configuration> expected_order_by_decreasing_priority = {
+      Configuration::MakePresetForLiveRunOnPhishingSites(),
+      Configuration::MakePresetForPerformanceTestingDryRunOnAllSites(),
+      Configuration() /* default constructor */
+  };
+
+  std::vector<Configuration> reverse_order(

[Charlie Harrison, 2017/05/05 21:16:37]

Optional: I think a test which does 2 1 3 or 3 1 2 is more general than just
reversing the priorities.

[engedy, 2017/05/05 21:31:53]

Agreed. Done.

+      expected_order_by_decreasing_priority.rbegin(),
+      expected_order_by_decreasing_priority.rend());
+  subresource_filter::testing::ScopedSubresourceFilterConfigurator
+      scoped_configuration(std::move(reverse_order));
+  EXPECT_THAT(
+      GetEnabledConfigurations()->configs_by_decreasing_priority(),
+      ::testing::ElementsAreArray(expected_order_by_decreasing_priority));
+}
+
+TEST(SubresourceFilterFeaturesTest, EnabledConfigurations_MultiplePresets) {
+  const std::string kPhishing(kPresetLiveRunOnPhishingSites);
+  const std::string kPerfTest(kPresetPerformanceTestingDryRunOnAllSites);
+  const struct {
+    std::string preset_name_list;
+  } kTestCases[] = {
+      {kPhishing + "," + kPerfTest},
+      {kPerfTest + "," + kPhishing},
+      {base::ToUpperASCII(kPhishing) + "," + base::ToUpperASCII(kPerfTest)},
+      {",, ," + kPerfTest + ",," + kPhishing},
+      {"garbage,garbage2," + kPerfTest + "," + kPhishing}};
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(::testing::Message()
+                 << "preset_name_list: " << test_case.preset_name_list);
+
+    ScopedExperimentalStateToggle scoped_experimental_state(
+        base::FeatureList::OVERRIDE_ENABLE_FEATURE,
+        {{kEnablePresetsParameterName, test_case.preset_name_list}});
+
+    const auto config_list = GetEnabledConfigurations();
+    EXPECT_THAT(
+        config_list->configs_by_decreasing_priority(),
+        ::testing::ElementsAre(
+            Configuration::MakePresetForLiveRunOnPhishingSites(),
+            Configuration::MakePresetForPerformanceTestingDryRunOnAllSites(),
+            Configuration()));

[Charlie Harrison, 2017/05/05 21:16:38]

Where does this one come from?

[engedy, 2017/05/05 21:31:53]

This is the experimental configuration. Added comment. Could be convinced to
remove this, it just makes for really ugly special casing in tests.

+    EXPECT_EQ(std::string(),
+              config_list->lexicographically_greatest_ruleset_flavor());
+  }
+}
+
+TEST(SubresourceFilterFeaturesTest,
+     EnabledConfigurations_MultiplePresetsAndExperimentalConfig) {
+  const std::string kPhishing(kPresetLiveRunOnPhishingSites);
+  const std::string kPerfTest(kPresetPerformanceTestingDryRunOnAllSites);
+  const std::string kTestRulesetFlavor("foobar");
+
+  ScopedExperimentalStateToggle scoped_experimental_state(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE,
+      {{kEnablePresetsParameterName, kPhishing + "," + kPerfTest},
+       {kActivationLevelParameterName, kActivationLevelDryRun},
+       {kActivationScopeParameterName, kActivationScopeActivationList},
+       {kActivationListsParameterName, kActivationListSubresourceFilter},
+       {kActivationPriorityParameterName, "75"},
+       {kRulesetFlavorParameterName, kTestRulesetFlavor}});
+
+  Configuration experimental_config(ActivationLevel::DRYRUN,
+                                    ActivationScope::ACTIVATION_LIST,
+                                    ActivationList::SUBRESOURCE_FILTER);
+  experimental_config.activation_conditions.priority = 75;
+  experimental_config.general_settings.ruleset_flavor = kTestRulesetFlavor;
+
+  const auto config_list = GetEnabledConfigurations();
+  EXPECT_THAT(
+      config_list->configs_by_decreasing_priority(),
+      ::testing::ElementsAre(
+          Configuration::MakePresetForLiveRunOnPhishingSites(),
+          experimental_config,
+          Configuration::MakePresetForPerformanceTestingDryRunOnAllSites()));
+  EXPECT_EQ(kTestRulesetFlavor,
+            config_list->lexicographically_greatest_ruleset_flavor());
+}
+
 }  // namespace subresource_filter