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 22 matching lines @@
 // Returns true with a probability given by |performance_measurement_rate| if
 // ThreadTicks is supported, otherwise returns false.
 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

[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
 
 // static
 void ContentSubresourceFilterDriverFactory::CreateForWebContents(
     content::WebContents* web_contents,
     std::unique_ptr<SubresourceFilterClient> client) {
   if (FromWebContents(web_contents))
     return;
@@ skipping 202 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(
-    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(
       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)
+  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:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace subresource_filter