Add the SocEng as a type for checking in CheckUrlForSubresourceFilter.

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 7ba5c8223658a6982f4c36f09003b373a05efffa..0b63f083584d66f07708ca7fae7f76f32a744951 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
@@ -46,11 +46,10 @@ bool ShouldMeasurePerformanceForPageLoad(double performance_measurement_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)   \
+#define REPORT_REDIRECT_PATTERN_FOR_SUFFIX(suffix, match_pattern, chain_size)  \
   do {                                                                         \
-    UMA_HISTOGRAM_ENUMERATION(                                                 \
-        "SubresourceFilter.PageLoad.RedirectChainMatchPattern." suffix,        \
-        hits_pattern, 0x10);                                                   \
+    UMA_HISTOGRAM_COUNTS("SubresourceFilter.PageLoad.Match." suffix,           \

[engedy, 2017/04/20 11:16:11]

Have you considered making this UMA_HISTOGRAM_BOOLEAN, and record this histogram
all the time, so that we would have a baseline that we can compare the number of
matches to? 

[melandory, 2017/04/25 13:48:13]

Done.

+                         match_pattern);                                       \
     UMA_HISTOGRAM_COUNTS(                                                      \
         "SubresourceFilter.PageLoad.RedirectChainLength." suffix, chain_size); \
   } while (0)
@@ -347,33 +346,10 @@ void ContentSubresourceFilterDriverFactory::AddActivationListMatch(
     activation_list_matches_[DistillURLToHostAndPath(url)].insert(match_type);
 }
 
-int ContentSubresourceFilterDriverFactory::CalculateHitPatternForActivationList(
+bool ContentSubresourceFilterDriverFactory::CalculateMatchForActivationList(

[engedy, 2017/04/20 11:16:11]

nit: Let's just inline this method now that it became so trivial.

[melandory, 2017/04/25 13:48:13]

Done.

     ActivationList activation_list) const {
-  int hits_pattern = 0;
-  const int kInitialURLHitMask = 0x4;
-  const int kRedirectURLHitMask = 0x2;
-  const int kFinalURLHitMask = 0x1;
-
-  if (navigation_chain_.size() > 1) {
-    if (DidURLMatchActivationList(navigation_chain_.back(), activation_list))
-      hits_pattern |= kFinalURLHitMask;
-    if (DidURLMatchActivationList(navigation_chain_.front(), activation_list))
-      hits_pattern |= kInitialURLHitMask;
-
-    // Examine redirects.
-    for (size_t i = 1; i < navigation_chain_.size() - 1; ++i) {
-      if (DidURLMatchActivationList(navigation_chain_[i], activation_list)) {
-        hits_pattern |= kRedirectURLHitMask;
-        break;
-      }
-    }
-  } else {
-    if (navigation_chain_.size() &&
-        DidURLMatchActivationList(navigation_chain_.front(), activation_list)) {
-      hits_pattern = 0x8;  // One url hit.
-    }
-  }
-  return hits_pattern;
+  return navigation_chain_.size() &&

[engedy, 2017/04/20 11:16:11]

nit: Can |navigation_chain_| ever be empty here?

[melandory, 2017/04/25 13:48:13]

Done.

+         DidURLMatchActivationList(navigation_chain_.back(), activation_list);
 }
 
 void ContentSubresourceFilterDriverFactory::RecordRedirectChainMatchPattern()
@@ -387,21 +363,21 @@ void ContentSubresourceFilterDriverFactory::RecordRedirectChainMatchPattern()
 void ContentSubresourceFilterDriverFactory::
     RecordRedirectChainMatchPatternForList(
         ActivationList activation_list) const {
-  int hits_pattern = CalculateHitPatternForActivationList(activation_list);
-  if (!hits_pattern)
+  int match_pattern = CalculateMatchForActivationList(activation_list);
+  if (!match_pattern)
     return;
   size_t chain_size = navigation_chain_.size();
   switch (activation_list) {
     case ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL:
       REPORT_REDIRECT_PATTERN_FOR_SUFFIX("SocialEngineeringAdsInterstitial",
-                                         hits_pattern, chain_size);
+                                         match_pattern, chain_size);
       break;
     case ActivationList::PHISHING_INTERSTITIAL:
-      REPORT_REDIRECT_PATTERN_FOR_SUFFIX("PhishingInterstital", hits_pattern,
+      REPORT_REDIRECT_PATTERN_FOR_SUFFIX("PhishingInterstital", match_pattern,
                                          chain_size);
       break;
     case ActivationList::SUBRESOURCE_FILTER:
-      REPORT_REDIRECT_PATTERN_FOR_SUFFIX("SubresourceFilterOnly", hits_pattern,
+      REPORT_REDIRECT_PATTERN_FOR_SUFFIX("SubresourceFilterOnly", match_pattern,
                                          chain_size);
       break;
     default: