Introduce subresource_filter::Configuration.

components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
 
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "base/time/time.h"
@@ skipping 16 matching lines @@
 
 namespace {
 
 const char kWebContentsUserDataKey[] =
     "web_contents_subresource_filter_driver_factory";
 
 std::string DistillURLToHostAndPath(const GURL& url) {
   return url.host() + url.path();
 }
 
-// Returns true with a probability of GetPerformanceMeasurementRate() if
-// ThreadTicks is supported, otherwise returns false.
-bool ShouldMeasurePerformanceForPageLoad() {
+// Returns true with a probability given by |performance_measurement_rate| in
+// |configuration| if ThreadTicks is supported, otherwise returns false.
+bool ShouldMeasurePerformanceForPageLoad(const Configuration& configuration) {
   if (!base::ThreadTicks::IsSupported())
     return false;
   // TODO(pkalinnikov): Cache |rate| and other variation params in
   // ContentSubresourceFilterDriverFactory.
-  const double rate = GetPerformanceMeasurementRate();
+  const double rate = configuration.performance_measurement_rate;
   return rate == 1 || (rate > 0 && base::RandDouble() < rate);
 }
 
 // Records histograms about the length of redirect chains, and about the pattern
 // of whether each URL in the chain matched the activation list.
 #define REPORT_REDIRECT_PATTERN_FOR_SUFFIX(suffix, hits_pattern, chain_size)   \
   do {                                                                         \
     UMA_HISTOGRAM_ENUMERATION(                                                 \
         "SubresourceFilter.PageLoad.RedirectChainMatchPattern." suffix,        \
         hits_pattern, 0x10);                                                   \
@@ skipping 84 matching lines @@
 
 void ContentSubresourceFilterDriverFactory::AddHostOfURLToWhitelistSet(
     const GURL& url) {
   if (url.has_host() && url.SchemeIsHTTPOrHTTPS())
     whitelisted_hosts_.insert(url.host());
 }
 
 ContentSubresourceFilterDriverFactory::ActivationDecision
 ContentSubresourceFilterDriverFactory::ComputeActivationDecisionForMainFrameURL(
     const GURL& url) const {
-  if (GetMaximumActivationLevel() == ActivationLevel::DISABLED)
+  if (configuration_.activation_level == ActivationLevel::DISABLED)
     return ActivationDecision::ACTIVATION_DISABLED;
 
-  ActivationScope scope = GetCurrentActivationScope();
-  if (scope == ActivationScope::NO_SITES)
+  if (configuration_.activation_scope == ActivationScope::NO_SITES)
     return ActivationDecision::ACTIVATION_DISABLED;
 
   if (!url.SchemeIsHTTPOrHTTPS())
     return ActivationDecision::UNSUPPORTED_SCHEME;
   if (IsWhitelisted(url))
     return ActivationDecision::URL_WHITELISTED;
 
-  switch (scope) {
+  switch (configuration_.activation_scope) {
     case ActivationScope::ALL_SITES:
       return ActivationDecision::ACTIVATED;
     case ActivationScope::ACTIVATION_LIST: {
       // The logic to ensure only http/https URLs are activated lives in
       // AddActivationListMatch to ensure the activation list only has relevant
       // entries.
       DCHECK(url.SchemeIsHTTPOrHTTPS() ||
-             !DidURLMatchActivationList(url, GetCurrentActivationList()));
+             !DidURLMatchActivationList(url, configuration_.activation_list));
       bool should_activate =
-          DidURLMatchActivationList(url, GetCurrentActivationList());
-      if (GetCurrentActivationList() == ActivationList::PHISHING_INTERSTITIAL) {
+          DidURLMatchActivationList(url, configuration_.activation_list);
+      if (configuration_.activation_list ==
+          ActivationList::PHISHING_INTERSTITIAL) {
         // Handling special case, where activation on the phishing sites also
         // mean the activation on the sites with social engineering metadata.
         should_activate |= DidURLMatchActivationList(
             url, ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL);
       }
       return should_activate ? ActivationDecision::ACTIVATED
                              : ActivationDecision::ACTIVATION_LIST_NOT_MATCHED;
     }
     default:
       return ActivationDecision::ACTIVATION_DISABLED;
@@ skipping 23 matching lines @@
       navigation_handle->GetNetErrorCode() != net::OK) {
     return;
   }
 
   const GURL& url = navigation_handle->GetURL();
   const content::Referrer& referrer = navigation_handle->GetReferrer();
   ui::PageTransition transition = navigation_handle->GetPageTransition();
 
   RecordRedirectChainMatchPattern();
 
-  if (ShouldWhitelistSiteOnReload() &&
+  if (configuration_.should_whitelist_site_on_reload &&
       NavigationIsPageReload(url, referrer, transition)) {
     // Whitelist this host for the current as well as subsequent navigations.
     AddHostOfURLToWhitelistSet(url);
   }
 
   activation_decision_ = ComputeActivationDecisionForMainFrameURL(url);
   DCHECK(activation_decision_ != ActivationDecision::UNKNOWN);
   if (activation_decision_ != ActivationDecision::ACTIVATED) {
     ResetActivationState();
     return;
   }
 
-  activation_level_ = GetMaximumActivationLevel();
+  activation_level_ = configuration_.activation_level;
   measure_performance_ = activation_level_ != ActivationLevel::DISABLED &&
-                         ShouldMeasurePerformanceForPageLoad();
+                         ShouldMeasurePerformanceForPageLoad(configuration_);
   ActivationState state = ActivationState(activation_level_);
   state.measure_performance = measure_performance_;
   throttle_manager_->NotifyPageActivationComputed(navigation_handle, state);
 }
 
 void ContentSubresourceFilterDriverFactory::OnFirstSubresourceLoadDisallowed() {
-  if (ShouldSuppressNotifications())
+  if (configuration_.should_suppress_notifications)
     return;
 
   client_->ToggleNotificationVisibility(activation_level_ ==
                                         ActivationLevel::ENABLED);
 }
 
 bool ContentSubresourceFilterDriverFactory::ShouldSuppressActivation(
     content::NavigationHandle* navigation_handle) {
   // Never suppress subframe navigations.
   return navigation_handle->IsInMainFrame() &&
          IsWhitelisted(navigation_handle->GetURL());
 }
 
 void ContentSubresourceFilterDriverFactory::ResetActivationState() {
   navigation_chain_.clear();
   activation_list_matches_.clear();
   activation_level_ = ActivationLevel::DISABLED;
   measure_performance_ = false;
   aggregated_document_statistics_ = DocumentLoadStatistics();
 }
 
 void ContentSubresourceFilterDriverFactory::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame() &&
       !navigation_handle->IsSameDocument()) {
+    // TODO(https://crbug.com/708181): Somewhat hacky workaround to allow tests
+    // to change the configuration after construction. Can be removed once the
+    // Safe Browsing navigation throttle handles all activation decisions.
+    configuration_ = GetActiveConfiguration();

[Charlie Harrison, 2017/04/05 20:43:04]

Eek this scares me, especially if code accidentally gets added in the meantime
that uses configuration_ before DidStartNavigation.

What about keeping this as a unique_ptr and exposing a
set_configuration_for_testing()?  It means more code to cleanup, unfortunately.

I would also be okay with using base::Optional, so that we get DCHECks around
picking configuration_.value().

[engedy, 2017/04/07 08:40:58]

Much relieved. I was worried that it might not scare you and if I pick one of
these more verbose alternatives, I might get accused of over-engineering things.
:-)

How about this?

     activation_decision_ = ActivationDecision::UNKNOWN;
     ResetActivationState();
     navigation_chain_.push_back(navigation_handle->GetURL());
     client_->ToggleNotificationVisibility(false);
   }
 }
 
 void ContentSubresourceFilterDriverFactory::DidRedirectNavigation(
     content::NavigationHandle* navigation_handle) {
   DCHECK(!navigation_handle->IsSameDocument());
@@ skipping 129 matching lines @@
       REPORT_REDIRECT_PATTERN_FOR_SUFFIX("SubresourceFilterOnly", hits_pattern,
                                          chain_size);
       break;
     default:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace subresource_filter