Metrics for tracking Safe Browsing hit patterns for the Safe Browsing lists Subresource Filter.

components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc

Index: components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
diff --git a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
index 57bd1de7c13068b7955ce5f5b43c4f3dc3801fe5..73a4a4d167fa40e7dd090711dd42d214934cbd46 100644
--- a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
+++ b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
@@ -75,30 +75,28 @@ bool ContentSubresourceFilterDriverFactory::IsWhitelisted(
   return whitelisted_hosts_.find(url.host()) != whitelisted_hosts_.end();
 }
 
-bool ContentSubresourceFilterDriverFactory::IsHit(const GURL& url) const {
-  return safe_browsing_blacklisted_patterns_.find(url.host() + url.path()) !=
-         safe_browsing_blacklisted_patterns_.end();
-}
-
-
 void ContentSubresourceFilterDriverFactory::
     OnMainResourceMatchedSafeBrowsingBlacklist(
         const GURL& url,
         const std::vector<GURL>& redirect_urls,
         safe_browsing::SBThreatType threat_type,
         safe_browsing::ThreatPatternType threat_type_metadata) {
-  bool proceed = false;
-  if (GetCurrentActivationList() ==
-      ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL) {
-    proceed = (threat_type_metadata ==
-               safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);
-  } else if (GetCurrentActivationList() ==
-             ActivationList::PHISHING_INTERSTITIAL) {
-    proceed = (threat_type == safe_browsing::SB_THREAT_TYPE_URL_PHISHING);
+  bool is_phishing_interstitial =
+      (threat_type == safe_browsing::SB_THREAT_TYPE_URL_PHISHING);
+  bool is_soc_engineering_ads_interstitial =
+      threat_type_metadata ==
+      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS;
+  if (is_phishing_interstitial || is_soc_engineering_ads_interstitial)
+    AddToBlacklistedBySafebrowsing(url);
+
+  ActivationList activation_list_type = GetCurrentActivationList();
+
+  if ((activation_list_type == ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL &&
+       is_soc_engineering_ads_interstitial) ||
+      (activation_list_type == ActivationList::PHISHING_INTERSTITIAL &&
+       is_phishing_interstitial)) {
+    AddToActivationSet(url);
   }
-  if (!proceed)
-    return;
-  AddToActivationHitsSet(url);
 }
 
 void ContentSubresourceFilterDriverFactory::AddHostOfURLToWhitelistSet(
@@ -107,18 +105,12 @@ void ContentSubresourceFilterDriverFactory::AddHostOfURLToWhitelistSet(
     whitelisted_hosts_.insert(url.host());
 }
 
-void ContentSubresourceFilterDriverFactory::AddToActivationHitsSet(
-    const GURL& url) {
-  if (!url.host().empty() && url.SchemeIsHTTPOrHTTPS())
-    safe_browsing_blacklisted_patterns_.insert(url.host() + url.path());
-}
-
 bool ContentSubresourceFilterDriverFactory::ShouldActivateForMainFrameURL(
     const GURL& url) const {
   if (GetCurrentActivationScope() == ActivationScope::ALL_SITES)
     return !IsWhitelisted(url);
   else if (GetCurrentActivationScope() == ActivationScope::ACTIVATION_LIST)
-    return IsHit(url) && !IsWhitelisted(url);
+    return IsInActivationSet(url) && !IsWhitelisted(url);
   return false;
 }
 
@@ -153,6 +145,24 @@ ContentSubresourceFilterDriverFactory::DriverFromFrameHost(
   return iterator == frame_drivers_.end() ? nullptr : iterator->second.get();
 }
 
+void ContentSubresourceFilterDriverFactory::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (navigation_handle->IsInMainFrame()) {
+    safe_browsing_blacklisted_patterns_.clear();
+    activation_patterns_.clear();
+    navigation_chain_.clear();
+    navigation_chain_.push_back(navigation_handle->GetURL());
+
+    client_->ToggleNotificationVisibility(false);
+    activation_state_ = ActivationState::DISABLED;
+  }
+}
+
+void ContentSubresourceFilterDriverFactory::DidRedirectNavigation(
+    content::NavigationHandle* navigation_handle) {
+  navigation_chain_.push_back(navigation_handle->GetURL());
+}
+
 void ContentSubresourceFilterDriverFactory::RenderFrameCreated(
     content::RenderFrameHost* render_frame_host) {
   CreateDriverForFrameHostIfNeeded(render_frame_host);
@@ -163,15 +173,6 @@ void ContentSubresourceFilterDriverFactory::RenderFrameDeleted(
   frame_drivers_.erase(render_frame_host);
 }
 
-void ContentSubresourceFilterDriverFactory::DidStartNavigation(
-    content::NavigationHandle* navigation_handle) {
-  safe_browsing_blacklisted_patterns_.clear();
-  if (navigation_handle->IsInMainFrame()) {
-    client_->ToggleNotificationVisibility(false);
-    activation_state_ = ActivationState::DISABLED;
-  }
-}
-
 void ContentSubresourceFilterDriverFactory::ReadyToCommitNavigation(
     content::NavigationHandle* navigation_handle) {
   content::RenderFrameHost* render_frame_host =
@@ -196,6 +197,7 @@ void ContentSubresourceFilterDriverFactory::ReadyToCommitNavigationInternal(
     content::RenderFrameHost* render_frame_host,
     const GURL& url) {
   if (!render_frame_host->GetParent()) {
+    RecordRedirectChainMatchPattern();
     if (ShouldActivateForMainFrameURL(url)) {
       activation_state_ = GetMaximumActivationState();
       ActivateForFrameHostIfNeeded(render_frame_host, url);
@@ -207,4 +209,63 @@ void ContentSubresourceFilterDriverFactory::ReadyToCommitNavigationInternal(
   }
 }
 
+bool ContentSubresourceFilterDriverFactory::IsBlacklistedBySafeBrowsing(
+    const GURL& url) const {
+  return safe_browsing_blacklisted_patterns_set().find(url.host() +
+                                                       url.path()) !=
+         safe_browsing_blacklisted_patterns_set().end();
+}
+
+bool ContentSubresourceFilterDriverFactory::IsInActivationSet(
+    const GURL& url) const {
+  return activation_patterns_.find(url.host() + url.path()) !=
+         activation_patterns_.end();
+}
+
+void ContentSubresourceFilterDriverFactory::AddToBlacklistedBySafebrowsing(
+    const GURL& url) {
+  if (!url.host().empty() && url.SchemeIsHTTPOrHTTPS()) {
+    safe_browsing_blacklisted_patterns_.insert(url.host() + url.path());
+  }
+}
+
+void ContentSubresourceFilterDriverFactory::AddToActivationSet(
+    const GURL& url) {
+  if (!url.host().empty() && url.SchemeIsHTTPOrHTTPS()) {
+    activation_patterns_.insert(url.host() + url.path());
+  }
+}
+
+void ContentSubresourceFilterDriverFactory::RecordRedirectChainMatchPattern()
+    const {
+  int hits_hosts_size = safe_browsing_blacklisted_patterns_.size();
+  int hits_pattern = 0;
+  const int initialURLHitMask = 0x4;
+  const int redirectURLHitMask = 0x2;
+  const int finalURLHitMask = 0x1;
+  if (navigation_chain_.size() > 1) {
+    if (IsBlacklistedBySafeBrowsing(navigation_chain_.back())) {
+      hits_pattern |= finalURLHitMask;
+      hits_hosts_size--;
+    }
+    if (IsBlacklistedBySafeBrowsing(navigation_chain_.front())) {
+      hits_pattern |= initialURLHitMask;
+      hits_hosts_size--;
+    }
+    if (hits_hosts_size > 0) {
+      hits_pattern |= redirectURLHitMask;
+    }
+  } else {
+    if (hits_hosts_size)
+      hits_pattern = 0x8;  // One url hit.
+    else
+      return;
+  }
+  UMA_HISTOGRAM_ENUMERATION(
+      "SubresourceFilter.PageLoad.RedirectChainMatchPattern", hits_pattern,
+      0x10 /* max value */);
+  UMA_HISTOGRAM_COUNTS("SubresourceFilter.PageLoad.RedirectChainSize",

[rkaplow, 2016/10/27 16:29:41]

nit, we're suggesting using an explicit UMA_HISTOGRAM_COUNTSN (where the max is
explicit) instead of using the default.

[melandory, 2016/11/16 15:59:35]

Hm, haven't found the constant for the max allowed number of redirects in the
code base.

[engedy, 2016/11/18 14:12:03]

Hmm, I haven't found it either. Maybe 10 or 20?

+                       navigation_chain_.size());
+}
+
 }  // namespace subresource_filter