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..6d5e4903da9fdbe2b66b8dd7cbe0b908e62a7d64 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,19 @@
 #include <map>
 #include <memory>
 #include <string>
+#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 +52,45 @@ 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);
 };
 
+template <class T, size_t N>
+std::vector<T> AsVector(const T (&array)[N]) {
+  return std::vector<T>(std::begin(array), std::end(array));
+}
+
+void ExpectAndRetrieveExactlyOneEnabledConfig(Configuration* 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 +116,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 +155,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 +208,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 +270,17 @@ 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);
   }
 }
 
@@ -271,17 +308,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 +343,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 +378,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 +406,189 @@ 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, ConfigurationPriorities) {
+  const Configuration kConfigurationsByDecreasingPriority[] = {

[pkalinnikov, 2017/05/04 12:04:23]

nit: Can you make this const std::vector<Configuration> to get rid of AsVector?
And assign it directly to |expected_order_by_decreasing_priority|?
And rename it to |configurations_by_decreasing_priority|?

[engedy, 2017/05/05 12:25:42]

Done.

+      {ActivationLevel::ENABLED, ActivationScope::ACTIVATION_LIST,
+       ActivationList::SUBRESOURCE_FILTER},
+      {ActivationLevel::ENABLED, ActivationScope::ACTIVATION_LIST,
+       ActivationList::PHISHING_INTERSTITIAL},
+      {ActivationLevel::ENABLED, ActivationScope::ACTIVATION_LIST,
+       ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL},
+      {ActivationLevel::ENABLED, ActivationScope::ALL_SITES},
+      {ActivationLevel::ENABLED, ActivationScope::NO_SITES},
+      {ActivationLevel::DRYRUN, ActivationScope::ACTIVATION_LIST},
+      {ActivationLevel::DRYRUN, ActivationScope::ALL_SITES},
+      {ActivationLevel::DRYRUN, ActivationScope::NO_SITES},
+      {ActivationLevel::DISABLED, ActivationScope::ACTIVATION_LIST},
+      {ActivationLevel::DISABLED, ActivationScope::ALL_SITES},
+      {ActivationLevel::DISABLED, ActivationScope::NO_SITES},
+      Configuration() /* default constructor */

[pkalinnikov, 2017/05/04 12:04:23]

nit: comma?

[engedy, 2017/05/05 12:25:42]

I am not sure I understand. We don't have trailing commas anywhere in this test?

[pkalinnikov, 2017/05/05 13:29:58]

I suggested to put comma after /* default_constructor */, but I leave it up to
you.

[engedy, 2017/05/05 19:24:10]

OK, for consistency, let's not have a trailing comma.

+  };
+
+  const std::vector<Configuration> expected_order_by_decreasing_priority =
+      AsVector(kConfigurationsByDecreasingPriority);
+  std::vector<Configuration> reverse_order(
+      expected_order_by_decreasing_priority.rbegin(),
+      expected_order_by_decreasing_priority.rend());
+  subresource_filter::testing::ScopedSubresourceFilterConfigurator
+      scoped_configuration(std::move(reverse_order));

[pkalinnikov, 2017/05/04 12:04:23]

#include <utility> for std::move.

[engedy, 2017/05/05 12:25:42]

Done.

+  EXPECT_THAT(
+      GetEnabledConfigurations()->configs_by_decreasing_priority(),
+      ::testing::ElementsAreArray(expected_order_by_decreasing_priority));
+}
+
+TEST(SubresourceFilterFeaturesTest, MostSpecificRulesetFlavor) {
+  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"}},
+                    {"foo", {"", "a", "foo"}},
+                    {"foo", {"foo", "bar", "b"}}};
+
+  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()->GetMostSpecificRulesetFlavor());
   }
 }
+
+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->GetMostSpecificRulesetFlavor());
+}
+
+TEST(SubresourceFilterFeaturesTest,
+     EnabledConfigurations_FeatureEnabledWithNoParameters) {
+  ScopedExperimentalStateToggle scoped_experimental_state(
+      base::FeatureList::OVERRIDE_USE_DEFAULT, {});
+
+  const auto config_list = GetEnabledConfigurations();
+  EXPECT_THAT(config_list->configs_by_decreasing_priority(),
+              ::testing::ElementsAre(Configuration()));
+  EXPECT_EQ(std::string(), config_list->GetMostSpecificRulesetFlavor());
+}
+
+TEST(SubresourceFilterFeaturesTest, PresetForLiveRunOnPhishingSites) {
+  ExpectPresetCanBeEnabledByName(
+      Configuration::MakePresetForLiveRunOnPhishingSites(),
+      kPresetLiveRunOnPhishingSites);
+  ExpectPresetIsEquivalentToVariationParams(
+      Configuration::MakePresetForLiveRunOnPhishingSites(),
+      {{kActivationLevelParameterName, kActivationLevelEnabled},
+       {kActivationScopeParameterName, kActivationScopeActivationList},
+       {kActivationListsParameterName, kActivationListPhishingInterstitial}});
+}
+
+TEST(SubresourceFilterFeaturesTest,
+     PresetForPerformanceTestingDryRunOnAllSites) {
+  ExpectPresetCanBeEnabledByName(
+      Configuration::MakePresetForPerformanceTestingDryRunOnAllSites(),
+      kPresetPerformanceTestingDryRunOnAllSites);
+  ExpectPresetIsEquivalentToVariationParams(
+      Configuration::MakePresetForPerformanceTestingDryRunOnAllSites(),
+      {{kActivationLevelParameterName, kActivationLevelDryRun},
+       {kActivationScopeParameterName, kActivationScopeAllSites},
+       {kPerformanceMeasurementRateParameterName, "1.0"}});
+}
+
+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()));
+    EXPECT_EQ(std::string(), config_list->GetMostSpecificRulesetFlavor());
+  }
+}
+
+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},
+       {kRulesetFlavorParameterName, kTestRulesetFlavor}});
+
+  Configuration experimental_config(ActivationLevel::DRYRUN,
+                                    ActivationScope::ACTIVATION_LIST,
+                                    ActivationList::SUBRESOURCE_FILTER);
+  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->GetMostSpecificRulesetFlavor());
+}
+
 }  // namespace subresource_filter