Add support for multiple simultaneous subresource_filter::Configurations.

components/subresource_filter/core/browser/subresource_filter_features.cc

Index: components/subresource_filter/core/browser/subresource_filter_features.cc
diff --git a/components/subresource_filter/core/browser/subresource_filter_features.cc b/components/subresource_filter/core/browser/subresource_filter_features.cc
index 06b06f1b3a6f2be15c0e3ddf5aa1321fe1380e7e..1795ef08f3274530b8ea32c99e90ca713700bb90 100644
--- a/components/subresource_filter/core/browser/subresource_filter_features.cc
+++ b/components/subresource_filter/core/browser/subresource_filter_features.cc
@@ -4,7 +4,9 @@
 
 #include "components/subresource_filter/core/browser/subresource_filter_features.h"
 
+#include <algorithm>
 #include <string>
+#include <tuple>
 #include <utility>
 
 #include "base/lazy_instance.h"
@@ -112,6 +114,58 @@ Configuration ParseFieldTrialConfiguration() {
   return configuration;
 }
 
+std::vector<Configuration> MakeConfigVector(Configuration config) {
+  std::vector<Configuration> configs;
+  configs.push_back(std::move(config));
+  return configs;
+}
+
+size_t GetActivationLevelPriority(ActivationLevel activation_level) {
+  constexpr ActivationLevel kValuesInIncreasingOrderOfPriority[] = {

[Charlie Harrison, 2017/04/26 21:28:11]

I see the appeal here but maybe it would just be easier to implement these as
switch/case statements?

You get the compile time checking for free without static asserts, you don't
need to use std::find, and I don't think updating them will be too big a hassle.

[engedy, 2017/05/03 12:04:45]

Done.

+      ActivationLevel::DISABLED, ActivationLevel::DRYRUN,
+      ActivationLevel::ENABLED};
+  static_assert(ActivationLevel::LAST == ActivationLevel::ENABLED,
+                "Priority not defined for ActivationLevel.");
+  const auto* const it =
+      std::find(std::begin(kValuesInIncreasingOrderOfPriority),
+                std::end(kValuesInIncreasingOrderOfPriority), activation_level);
+  DCHECK(it < std::end(kValuesInIncreasingOrderOfPriority));
+  return it - std::begin(kValuesInIncreasingOrderOfPriority);
+}
+
+size_t GetActivationScopePriority(ActivationScope activation_scope) {
+  // The more specific ACTIVATION_LIST trumps the blanket scopes.
+  constexpr ActivationScope kValuesInIncreasingOrderOfPriority[] = {
+      ActivationScope::NO_SITES, ActivationScope::ALL_SITES,
+      ActivationScope::ACTIVATION_LIST};
+  static_assert(ActivationScope::LAST == ActivationScope::ALL_SITES,
+                "Priority not defined for ActivationScope.");
+  const auto* const it =
+      std::find(std::begin(kValuesInIncreasingOrderOfPriority),
+                std::end(kValuesInIncreasingOrderOfPriority), activation_scope);
+  DCHECK(it < std::end(kValuesInIncreasingOrderOfPriority));
+  return it - std::begin(kValuesInIncreasingOrderOfPriority);
+}
+
+// Returns the priority of the |configuration|, where a greater value indicates
+// higher priority.
+std::tuple<size_t, size_t, size_t> GetConfigurationPriority(
+    const Configuration& configuration) {
+  return std::make_tuple(
+      GetActivationLevelPriority(configuration.activation_level),
+      GetActivationScopePriority(configuration.activation_scope),
+      static_cast<size_t>(configuration.activation_list));
+}
+
+std::vector<Configuration> SortConfigsByDecreasingPriority(
+    std::vector<Configuration> configs) {
+  std::sort(configs.begin(), configs.end(),
+            [](const Configuration& a, const Configuration& b) {
+              return GetConfigurationPriority(a) > GetConfigurationPriority(b);

[Charlie Harrison, 2017/04/26 21:28:11]

How does this operator work (can you include a comment)? I assume it compares by
the first elt and goes on to the next elt in case of a tie?

[engedy, 2017/05/03 12:04:45]

Yep, added comment.

+            });
+  return configs;
+}
+
 base::LazyInstance<base::Lock>::Leaky g_active_configurations_lock =
     LAZY_INSTANCE_INITIALIZER;
 
@@ -163,10 +217,26 @@ Configuration::Configuration(Configuration&&) = default;
 Configuration::~Configuration() = default;
 Configuration& Configuration::operator=(Configuration&&) = default;
 
+ConfigurationList::ConfigurationList(std::vector<Configuration> configs)
+    : ordered_configs_(SortConfigsByDecreasingPriority(std::move(configs))) {}
 ConfigurationList::ConfigurationList(Configuration config)
-    : config_(std::move(config)) {}
+    : ConfigurationList(MakeConfigVector(std::move(config))) {}
 ConfigurationList::~ConfigurationList() = default;
 
+std::string ConfigurationList::GetMostSpecificRulesetFlavor() const {

[Charlie Harrison, 2017/04/26 21:28:11]

Please comment what "most specific" means and how you've implemented it. I don't
readily see how the comparator yields the most specific.

[engedy, 2017/05/03 12:04:45]

Documented in the header, and cleaned up the implementation.

+  if (ordered_configs_.empty())
+    return std::string();
+  const auto it_max = std::max_element(
+      ordered_configs_.begin(), ordered_configs_.end(),
+      [](const Configuration& a, const Configuration& b) {
+        if (a.ruleset_flavor.size() == b.ruleset_flavor.size())
+          return a.ruleset_flavor < b.ruleset_flavor;
+        return a.ruleset_flavor.size() < b.ruleset_flavor.size();
+      });
+  DCHECK(it_max != ordered_configs_.end());
+  return it_max->ruleset_flavor;
+}
+
 scoped_refptr<ConfigurationList> GetActiveConfigurations() {
   base::AutoLock lock(g_active_configurations_lock.Get());
   if (!g_active_configurations.Get()) {