Add the SocEng as a type for checking in CheckUrlForSubresourceFilter.

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 28 matching lines @@
 bool ShouldMeasurePerformanceForPageLoad(double performance_measurement_rate) {
   if (!base::ThreadTicks::IsSupported())
     return false;
   return performance_measurement_rate == 1 ||
          (performance_measurement_rate > 0 &&
           base::RandDouble() < 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)
 
 }  // namespace
 
 // static
 void ContentSubresourceFilterDriverFactory::CreateForWebContents(
     content::WebContents* web_contents,
     std::unique_ptr<SubresourceFilterClient> client) {
@@ skipping 276 matching lines @@
 
 void ContentSubresourceFilterDriverFactory::AddActivationListMatch(
     const GURL& url,
     ActivationList match_type) {
   if (match_type == ActivationList::NONE)
     return;
   if (url.has_host() && url.SchemeIsHTTPOrHTTPS())
     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()
     const {
   RecordRedirectChainMatchPatternForList(
       ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL);
   RecordRedirectChainMatchPatternForList(ActivationList::PHISHING_INTERSTITIAL);
   RecordRedirectChainMatchPatternForList(ActivationList::SUBRESOURCE_FILTER);
 }
 
 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:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace subresource_filter