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 df3f8c171f7ec4aa6ac1288197854833ce5457fc..2deb5cbe7cd7e0cf62da8f39f7cab44ee0cd513f 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
@@ -42,13 +42,15 @@ bool ShouldMeasurePerformanceForPageLoad(double performance_measurement_rate) {
 
 // Records histograms about the length of redirect chains, and about the pattern

[engedy, 2017/04/26 13:47:09]

nit: s/about the pattern of//

[melandory, 2017/04/26 15:02:20]

Done.

 // of whether each URL in the chain matched the activation list.

[engedy, 2017/04/26 13:47:09]

nit: s/each/the last/

[melandory, 2017/04/26 15:02:20]

Done.

-#define REPORT_REDIRECT_PATTERN_FOR_SUFFIX(suffix, hits_pattern, chain_size)   \
-  do {                                                                         \
-    UMA_HISTOGRAM_ENUMERATION(                                                 \
-        "SubresourceFilter.PageLoad.RedirectChainMatchPattern." suffix,        \
-        hits_pattern, 0x10);                                                   \
-    UMA_HISTOGRAM_COUNTS(                                                      \
-        "SubresourceFilter.PageLoad.RedirectChainLength." suffix, chain_size); \
+#define REPORT_REDIRECT_PATTERN_FOR_SUFFIX(suffix, match_pattern, chain_size) \

[engedy, 2017/04/26 13:47:09]

nit: s/match_pattern/did_match|is_match|matched/, and then use that name
consistently throughout this file.

[melandory, 2017/04/26 15:02:20]

Done.

+  do {                                                                        \
+    UMA_HISTOGRAM_BOOLEAN("SubresourecFilter.PageLoad.FinalURLMatch." suffix, \
+                          match_pattern);                                     \
+    if (match_pattern) {                                                      \
+      UMA_HISTOGRAM_COUNTS(                                                   \
+          "SubresourceFilter.PageLoad.RedirectChainLength." suffix,           \
+          chain_size);                                                        \
+    };                                                                        \
   } while (0)
 
 }  // namespace
@@ -271,35 +273,6 @@ void ContentSubresourceFilterDriverFactory::AddActivationListMatch(
     activation_list_matches_[DistillURLToHostAndPath(url)].insert(match_type);
 }
 
-int ContentSubresourceFilterDriverFactory::CalculateHitPatternForActivationList(
-    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;
-}
-
 void ContentSubresourceFilterDriverFactory::RecordRedirectChainMatchPattern()
     const {
   RecordRedirectChainMatchPatternForList(
@@ -311,21 +284,24 @@ void ContentSubresourceFilterDriverFactory::RecordRedirectChainMatchPattern()
 void ContentSubresourceFilterDriverFactory::
     RecordRedirectChainMatchPatternForList(
         ActivationList activation_list) const {
-  int hits_pattern = CalculateHitPatternForActivationList(activation_list);
-  if (!hits_pattern)
+  if (!navigation_chain_.back().has_host() ||

[engedy, 2017/04/26 13:47:09]

nit: Add DCHECK(!navigation_chain_.empty()); before.

[melandory, 2017/04/26 15:02:20]

Done.

+      !navigation_chain_.back().SchemeIsHTTPOrHTTPS()) {
     return;
+  }
+  bool match_pattern =
+      DidURLMatchActivationList(navigation_chain_.back(), activation_list);
   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: