Add the SocEng as a type for checking in CheckUrlForSubresourceFilter.

components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.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 <set>
 #include <string>
 
 #include "base/macros.h"
@@ skipping 33 matching lines @@
 const char kExampleUrlWithParams[] = "https://example.com/soceng?q=engsoc";
 const char kExampleUrl[] = "https://example.com";
 const char kExampleLoginUrl[] = "https://example.com/login";
 const char kUrlA[] = "https://example_a.com";
 const char kUrlB[] = "https://example_b.com";
 const char kUrlC[] = "https://example_c.com";
 const char kUrlD[] = "https://example_d.com";
 const char kSubframeName[] = "Child";
 const char kDisallowedUrl[] = "https://example.com/disallowed.html";
 
-const char kMatchesPatternHistogramName[] =
-    "SubresourceFilter.PageLoad.RedirectChainMatchPattern.";
+const char kMatchesHistogramName[] =
+    "SubresourecFilter.PageLoad.FinalURLMatch.";

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

nit: Sorry, I introduced a typo here: Subresource*

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

Done.

 const char kNavigationChainSize[] =
     "SubresourceFilter.PageLoad.RedirectChainLength.";
 
-// Human readable representation of expected redirect chain match patterns.
-// The explanations for the buckets given for the following redirect chain:
-// A->B->C->D, where A is initial URL and D is a final URL.
-enum RedirectChainMatchPattern {
-  EMPTY,             // No histograms were recorded.
-  F0M0L1,            // D is a Safe Browsing match.
-  F0M1L0,            // B or C, or both are Safe Browsing matches.
-  F0M1L1,            // B or C, or both and D are Safe Browsing matches.
-  F1M0L0,            // A is Safe Browsing match
-  F1M0L1,            // A and D are Safe Browsing matches.
-  F1M1L0,            // B and/or C and A are Safe Browsing matches.
-  F1M1L1,            // B and/or C and A and D are Safe Browsing matches.
-  NO_REDIRECTS_HIT,  // Redirect chain consists of single URL, aka no redirects
-                     // has happened, and this URL was a Safe Browsing hit.
-  NUM_HIT_PATTERNS,
+// Enum helps to setup test cases. In future, when checking first URL in the
+// redirect chain is implemented, the enum will be expanded.
+enum RecordedHistograms {

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

Could you also please remove RedirectChainMatchPattern in
subresource_filter_browsertest.cc?

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

Done.

+  EMPTY,

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

nit: Let's see if we can make these names more informative. How about:

NO_HISTOGRAMS = 0,
FINAL_URL_MATCHES = 1 << 0,
REDIRECT_CHAIN_LENGTH = 1 << 1,
BOTH_HISTOGRAMS = ...

Then we could make the checking starting at line 350 also easier to understand.

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

Done.

+  EMPTY_REDIRECTS,
+  MATCH_REDIRECTS,
 };
 
 std::string GetSuffixForList(const ActivationList& type) {
   switch (type) {
     case ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL:
       return "SocialEngineeringAdsInterstitial";
     case ActivationList::PHISHING_INTERSTITIAL:
       return "PhishingInterstital";
     case ActivationList::SUBRESOURCE_FILTER:
       return "SubresourceFilterOnly";
@@ skipping 63 matching lines @@
      safe_browsing::SB_THREAT_TYPE_URL_PHISHING,
      safe_browsing::ThreatPatternType::NONE},
     {ActivationDecision::ACTIVATED,
      subresource_filter::kActivationListSocialEngineeringAdsInterstitial,
      safe_browsing::SB_THREAT_TYPE_URL_PHISHING,
      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS},
     {ActivationDecision::ACTIVATED,
      subresource_filter::kActivationListPhishingInterstitial,
      safe_browsing::SB_THREAT_TYPE_URL_PHISHING,
      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS},
+    {ActivationDecision::ACTIVATED,
+     subresource_filter::kActivationListSubresourceFilter,
+     safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER,
+     safe_browsing::ThreatPatternType::NONE},
+    {ActivationDecision::ACTIVATION_LIST_NOT_MATCHED,
+     subresource_filter::kActivationListPhishingInterstitial,

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

I think this should be kActivationListSubresourceFilter.

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

Isn't it the test case above?

[engedy, 2017/05/05 08:19:33]

Nevermind me. :-)

+     safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER,
+     safe_browsing::ThreatPatternType::NONE},
+
 };
 
 struct ActivationScopeTestData {
   ActivationDecision expected_activation_decision;
   bool url_matches_activation_list;
   const char* const activation_scope;
 };
 
 const ActivationScopeTestData kActivationScopeTestData[] = {
     {ActivationDecision::ACTIVATED, false /* url_matches_activation_list */,
@@ skipping 27 matching lines @@
   ~MockSubresourceFilterClient() override = default;
 
   bool ShouldSuppressActivation(content::NavigationHandle* handle) override {
     return handle->IsInMainFrame() &&
            whitelisted_hosts_.find(handle->GetURL().host()) !=
                whitelisted_hosts_.end();
   }
 
   void WhitelistByContentSettings(const GURL& url) override {}
   void WhitelistInCurrentWebContents(const GURL& url) override {
     if (url.SchemeIsHTTPOrHTTPS())

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

nit: Can we ASSERT_TRUE(url.SchemeIsHTTPOrHTTPS()) here instead?

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

Done.

       whitelisted_hosts_.insert(url.host());
   }
 
   VerifiedRulesetDealer::Handle* GetRulesetDealer() override {
     return ruleset_dealer_;
   }
 
   MOCK_METHOD1(ToggleNotificationVisibility, void(bool));
 
  private:
   std::set<std::string> whitelisted_hosts_;
   // Owned by the test harness.

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

nit: Add blank line before comment.

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

Done.

   VerifiedRulesetDealer::Handle* ruleset_dealer_;
 
   DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterClient);
 };
 
+void ExpectSampleForSuffix(const std::string& suffix_to_check,
+                           const std::string& suffix,
+                           const base::HistogramTester& tester) {
+  tester.ExpectUniqueSample(kMatchesHistogramName + suffix_to_check,
+                            (suffix_to_check == suffix), 1);
+}
+
 }  // namespace
 
 class ContentSubresourceFilterDriverFactoryTest
     : public content::RenderViewHostTestHarness,
       public content::WebContentsObserver {
  public:
   ContentSubresourceFilterDriverFactoryTest() {}
   ~ContentSubresourceFilterDriverFactoryTest() override {}
 
   // content::RenderViewHostImplTestHarness:
@@ skipping 60 matching lines @@
     render_process_host->sink().ClearMessages();
   }
 
   void BlacklistURLWithRedirectsNavigateAndCommit(
       const std::vector<bool>& blacklisted_urls,
       const std::vector<GURL>& navigation_chain,
       safe_browsing::SBThreatType threat_type,
       safe_browsing::ThreatPatternType threat_type_metadata,
       const content::Referrer& referrer,
       ui::PageTransition transition,
-      RedirectChainMatchPattern expected_pattern,
+      RecordedHistograms expected_histograms,
       ActivationDecision expected_activation_decision) {
     const bool expected_activation =
         expected_activation_decision == ActivationDecision::ACTIVATED;
     base::HistogramTester tester;
     EXPECT_CALL(*client(), ToggleNotificationVisibility(false)).Times(1);
 
     std::unique_ptr<content::NavigationSimulator> navigation_simulator =
         content::NavigationSimulator::CreateRendererInitiated(
             navigation_chain.front(), main_rfh());
     navigation_simulator->SetReferrer(referrer);
     navigation_simulator->SetTransition(transition);
     navigation_simulator->Start();
 
     if (blacklisted_urls.front()) {
       factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
           navigation_chain.front(), navigation_chain, threat_type,
           threat_type_metadata);
     }
     ::testing::Mock::VerifyAndClearExpectations(client());
 
     for (size_t i = 1; i < navigation_chain.size(); ++i) {
       const GURL url = navigation_chain[i];
       if (i < blacklisted_urls.size() && blacklisted_urls[i]) {
         factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
             url, navigation_chain, threat_type, threat_type_metadata);
       }
       navigation_simulator->Redirect(url);
     }
+    std::string suffix;
+    ActivationList activation_list =
+        GetListForThreatTypeAndMetadata(threat_type, threat_type_metadata);
+    suffix = blacklisted_urls.back() ? GetSuffixForList(activation_list)
+                                     : std::string();
 
     navigation_simulator->Commit();
     ExpectActivationSignalForFrame(main_rfh(), expected_activation);
     EXPECT_EQ(expected_activation_decision,
               factory()->GetActivationDecisionForLastCommittedPageLoad());
 
     // Re-create a subframe now that the frame has navigated.
     content::RenderFrameHostTester* rfh_tester =
         content::RenderFrameHostTester::For(main_rfh());
     rfh_tester->AppendChild(kSubframeName);
-    ActivationList activation_list =
-        GetListForThreatTypeAndMetadata(threat_type, threat_type_metadata);
-
-    const std::string suffix(GetSuffixForList(activation_list));
     size_t all_pattern =

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

nit: s//all_matches/

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

Done.

-        tester.GetTotalCountsForPrefix(kMatchesPatternHistogramName).size();
+        tester.GetTotalCountsForPrefix(kMatchesHistogramName).size();
     size_t all_chain_size =
         tester.GetTotalCountsForPrefix(kNavigationChainSize).size();
-    if (expected_pattern != EMPTY) {
-      EXPECT_THAT(tester.GetAllSamples(kMatchesPatternHistogramName + suffix),
-                  ::testing::ElementsAre(base::Bucket(expected_pattern, 1)));
-      EXPECT_THAT(
-          tester.GetAllSamples(kNavigationChainSize + suffix),
-          ::testing::ElementsAre(base::Bucket(navigation_chain.size(), 1)));
-      // Check that we recorded only what is needed.
-      EXPECT_EQ(1u, all_pattern);
+    // Check that we recorded only what is needed.
+    if (expected_histograms == EMPTY) {
+      EXPECT_EQ(0u, all_pattern);
+      EXPECT_EQ(0U, all_chain_size);
+    } else {
+      EXPECT_EQ(3u, all_pattern);
+      ExpectSampleForSuffix("SocialEngineeringAdsInterstitial", suffix, tester);
+      ExpectSampleForSuffix("PhishingInterstital", suffix, tester);
+      ExpectSampleForSuffix("SubresourceFilterOnly", suffix, tester);
+    }
+
+    if (expected_histograms == MATCH_REDIRECTS) {
+      tester.ExpectUniqueSample(kNavigationChainSize + suffix,
+                                navigation_chain.size(), 1);
       EXPECT_EQ(1u, all_chain_size);
     } else {
-      EXPECT_EQ(0u, all_pattern);
-      EXPECT_EQ(0u, all_chain_size);
+      EXPECT_EQ(0U, all_chain_size);
     }
   }
 
   void NavigateSubframeAndExpectCheckResult(const GURL& url,
                                             bool expect_cancelled) {
     std::unique_ptr<content::NavigationSimulator> simulator =
         content::NavigationSimulator::CreateRendererInitiated(url,
                                                               GetSubframeRFH());
     simulator->Start();
     content::NavigationThrottle::ThrottleCheckResult result =
@@ skipping 15 matching lines @@
     ::testing::Mock::VerifyAndClearExpectations(client());
   }
 
   void NavigateAndExpectActivation(
       const std::vector<bool>& blacklisted_urls,
       const std::vector<GURL>& navigation_chain,
       safe_browsing::SBThreatType threat_type,
       safe_browsing::ThreatPatternType threat_type_metadata,
       const content::Referrer& referrer,
       ui::PageTransition transition,
-      RedirectChainMatchPattern expected_pattern,
+      RecordedHistograms expected_histograms,
       ActivationDecision expected_activation_decision) {
     const bool expected_activation =
         expected_activation_decision == ActivationDecision::ACTIVATED;
     BlacklistURLWithRedirectsNavigateAndCommit(
         blacklisted_urls, navigation_chain, threat_type, threat_type_metadata,
-        referrer, transition, expected_pattern, expected_activation_decision);
+        referrer, transition, expected_histograms,
+        expected_activation_decision);
 
     NavigateAndCommitSubframe(GURL(kExampleLoginUrl), expected_activation);
   }
 
   void NavigateAndExpectActivation(
       const std::vector<bool>& blacklisted_urls,
       const std::vector<GURL>& navigation_chain,
-      RedirectChainMatchPattern expected_pattern,
+      RecordedHistograms expected_histograms,
       ActivationDecision expected_activation_decision) {
     NavigateAndExpectActivation(
         blacklisted_urls, navigation_chain,
         safe_browsing::SB_THREAT_TYPE_URL_PHISHING,
         safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS,
-        content::Referrer(), ui::PAGE_TRANSITION_LINK, expected_pattern,
+        content::Referrer(), ui::PAGE_TRANSITION_LINK, expected_histograms,
         expected_activation_decision);
   }
 
   void EmulateFailedNavigationAndExpectNoActivation(const GURL& url) {
     EXPECT_CALL(*client(), ToggleNotificationVisibility(false)).Times(1);
 
     // With browser-side navigation enabled, ReadyToCommitNavigation is invoked
     // even for failed navigations. This is correctly simulated by
     // NavigationSimulator. Make sure no activation message is sent in this
     // case.
     content::NavigationSimulator::NavigateAndFailFromDocument(
         url, net::ERR_TIMED_OUT, main_rfh());
     ExpectActivationSignalForFrame(main_rfh(), false);
     ::testing::Mock::VerifyAndClearExpectations(client());
   }
 
   void EmulateInPageNavigation(
       const std::vector<bool>& blacklisted_urls,
-      RedirectChainMatchPattern expected_pattern,
+      RecordedHistograms expected_histograms,
       ActivationDecision expected_activation_decision) {
     // This test examines the navigation with the following sequence of events:
     //   DidStartProvisional(main, "example.com")
     //   ReadyToCommitNavigation(“example.com”)
     //   DidCommitProvisional(main, "example.com")
     //   DidStartProvisional(sub, "example.com/login")
     //   DidCommitProvisional(sub, "example.com/login")
     //   DidCommitProvisional(main, "example.com#ref")
 
     NavigateAndExpectActivation(blacklisted_urls, {GURL(kExampleUrl)},
-                                expected_pattern, expected_activation_decision);
+                                expected_histograms,
+                                expected_activation_decision);
     EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0);
     std::unique_ptr<content::NavigationSimulator> navigation_simulator =
         content::NavigationSimulator::CreateRendererInitiated(GURL(kExampleUrl),
                                                               main_rfh());
     navigation_simulator->CommitSameDocument();
     ExpectActivationSignalForFrame(main_rfh(), false);
     ::testing::Mock::VerifyAndClearExpectations(client());
   }
 
  protected:
@@ skipping 69 matching lines @@
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        ActivateForFrameHostDisabledFeature) {
   // Activation scope is set to NONE => no activation should happen even if URL
   // which is visited was a SB hit.
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_DISABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeAllSites,
       kActivationListSocialEngineeringAdsInterstitial);
   const GURL url(kExampleUrlWithParams);
-  NavigateAndExpectActivation({true}, {url}, NO_REDIRECTS_HIT,
+  NavigateAndExpectActivation({true}, {url}, MATCH_REDIRECTS,
                               ActivationDecision::ACTIVATION_DISABLED);
   factory()->client()->WhitelistInCurrentWebContents(url);
-  NavigateAndExpectActivation({true}, {url}, NO_REDIRECTS_HIT,
+  NavigateAndExpectActivation({true}, {url}, MATCH_REDIRECTS,
                               ActivationDecision::ACTIVATION_DISABLED);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest, NoActivationWhenNoMatch) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeActivationList,
       kActivationListSocialEngineeringAdsInterstitial);
-  NavigateAndExpectActivation({false}, {GURL(kExampleUrl)}, EMPTY,
+  NavigateAndExpectActivation({false}, {GURL(kExampleUrl)}, EMPTY_REDIRECTS,
                               ActivationDecision::ACTIVATION_LIST_NOT_MATCHED);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        SpecialCaseNavigationAllSitesEnabled) {
   // Check that when the experiment is enabled for all site, the activation
   // signal is always sent.
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeAllSites);
-  EmulateInPageNavigation({false}, EMPTY, ActivationDecision::ACTIVATED);
+  EmulateInPageNavigation({false}, EMPTY_REDIRECTS,
+                          ActivationDecision::ACTIVATED);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        SpecialCaseNavigationActivationListEnabled) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeActivationList,
       kActivationListSocialEngineeringAdsInterstitial);
-  EmulateInPageNavigation({true}, NO_REDIRECTS_HIT,
+  EmulateInPageNavigation({true}, MATCH_REDIRECTS,
                           ActivationDecision::ACTIVATED);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        SpecialCaseNavigationActivationListEnabledWithPerformanceMeasurement) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeActivationList,
       kActivationListSocialEngineeringAdsInterstitial,
       "1" /* performance_measurement_rate */);
-  EmulateInPageNavigation({true}, NO_REDIRECTS_HIT,
+  EmulateInPageNavigation({true}, MATCH_REDIRECTS,
                           ActivationDecision::ACTIVATED);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest, FailedNavigation) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeAllSites);
   const GURL url(kExampleUrl);
-  NavigateAndExpectActivation({false}, {url}, EMPTY,
+  NavigateAndExpectActivation({false}, {url}, EMPTY_REDIRECTS,
                               ActivationDecision::ACTIVATED);
   EmulateFailedNavigationAndExpectNoActivation(url);
 }
 
 // TODO(melandory): refactor the test so it no longer require the current
 // activation list to be matching.
 TEST_F(ContentSubresourceFilterDriverFactoryTest, RedirectPatternTest) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeActivationList,
       kActivationListSocialEngineeringAdsInterstitial);
   struct RedirectRedirectChainMatchPatternTestData {
     std::vector<bool> blacklisted_urls;
     std::vector<GURL> navigation_chain;
-    RedirectChainMatchPattern hit_expected_pattern;
+    RecordedHistograms hit_expected_histograms;
     ActivationDecision expected_activation_decision;
-  } kRedirectRedirectChainMatchPatternTestData[] = {
+  } kRedirectRecordedHistogramsTestData[] = {
       {{false},
        {GURL(kUrlA)},
-       EMPTY,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
-      {{true}, {GURL(kUrlA)}, NO_REDIRECTS_HIT, ActivationDecision::ACTIVATED},
+      {{true}, {GURL(kUrlA)}, MATCH_REDIRECTS, ActivationDecision::ACTIVATED},
       {{false, false},
        {GURL(kUrlA), GURL(kUrlB)},
-       EMPTY,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
       {{false, true},
        {GURL(kUrlA), GURL(kUrlB)},
-       F0M0L1,
+       MATCH_REDIRECTS,
        ActivationDecision::ACTIVATED},
       {{true, false},
        {GURL(kUrlA), GURL(kUrlB)},
-       F1M0L0,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
       {{true, true},
        {GURL(kUrlA), GURL(kUrlB)},
-       F1M0L1,
+       MATCH_REDIRECTS,
        ActivationDecision::ACTIVATED},
       {{false, false, false},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       EMPTY,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
       {{false, false, true},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F0M0L1,
+       MATCH_REDIRECTS,
        ActivationDecision::ACTIVATED},
       {{false, true, false},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F0M1L0,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
       {{false, true, true},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F0M1L1,
+       MATCH_REDIRECTS,
        ActivationDecision::ACTIVATED},
       {{true, false, false},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F1M0L0,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
       {{true, false, true},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F1M0L1,
+       MATCH_REDIRECTS,
        ActivationDecision::ACTIVATED},
       {{true, true, false},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F1M1L0,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
       {{true, true, true},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC)},
-       F1M1L1,
+       MATCH_REDIRECTS,
        ActivationDecision::ACTIVATED},
       {{false, true, false, false},
        {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC), GURL(kUrlD)},
-       F0M1L0,
+       EMPTY_REDIRECTS,
        ActivationDecision::ACTIVATION_LIST_NOT_MATCHED},
   };
 
-  for (size_t i = 0U; i < arraysize(kRedirectRedirectChainMatchPatternTestData);
-       ++i) {
-    auto test_data = kRedirectRedirectChainMatchPatternTestData[i];
+  for (const auto& test_data : kRedirectRecordedHistogramsTestData) {
     NavigateAndExpectActivation(
         test_data.blacklisted_urls, test_data.navigation_chain,
         safe_browsing::SB_THREAT_TYPE_URL_PHISHING,
         safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS,
         content::Referrer(), ui::PAGE_TRANSITION_LINK,
-        test_data.hit_expected_pattern, test_data.expected_activation_decision);
+        test_data.hit_expected_histograms,
+        test_data.expected_activation_decision);
     NavigateAndExpectActivation(
-        {false}, {GURL("https://dummy.com")}, EMPTY,
+        {false}, {GURL("https://dummy.com")}, EMPTY_REDIRECTS,
         ActivationDecision::ACTIVATION_LIST_NOT_MATCHED);
-#if defined(GOOGLE_CHROME_BUILD)
     NavigateAndExpectActivation(
         test_data.blacklisted_urls, test_data.navigation_chain,
         safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER,
         safe_browsing::ThreatPatternType::NONE, content::Referrer(),
-        ui::PAGE_TRANSITION_LINK, test_data.hit_expected_pattern,
+        ui::PAGE_TRANSITION_LINK, test_data.hit_expected_histograms,
         ActivationDecision::ACTIVATION_LIST_NOT_MATCHED);
-#endif
   }
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest, NotificationVisibility) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeAllSites);
-  NavigateAndExpectActivation({false}, {GURL(kExampleUrl)}, EMPTY,
+  NavigateAndExpectActivation({false}, {GURL(kExampleUrl)}, EMPTY_REDIRECTS,
                               ActivationDecision::ACTIVATED);
   EXPECT_CALL(*client(), ToggleNotificationVisibility(true)).Times(1);
   NavigateSubframeAndExpectCheckResult(GURL(kDisallowedUrl),
                                        true /* expect_cancelled */);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        SuppressNotificationVisibility) {
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeAllSites, "" /* activation_lists */,
       "" /* performance_measurement_rate */,
       "true" /* suppress_notifications */);
-  NavigateAndExpectActivation({false}, {GURL(kExampleUrl)}, EMPTY,
+  NavigateAndExpectActivation({false}, {GURL(kExampleUrl)}, EMPTY_REDIRECTS,
                               ActivationDecision::ACTIVATED);
   EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0);
   NavigateSubframeAndExpectCheckResult(GURL(kDisallowedUrl),
                                        true /* expect_cancelled */);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        InactiveMainFrame_SubframeNotFiltered) {
   GURL url(kExampleUrl);
-  NavigateAndExpectActivation({false}, {url}, EMPTY,
+  NavigateAndExpectActivation({false}, {url}, EMPTY_REDIRECTS,
                               ActivationDecision::ACTIVATION_DISABLED);
   NavigateSubframeAndExpectCheckResult(url, false /* expect_cancelled */);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest, WhitelistSiteOnReload) {
   const struct {
     content::Referrer referrer;
     ui::PageTransition transition;
     ActivationDecision expected_activation_decision;
   } kTestCases[] = {
@@ skipping 15 matching lines @@
     testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
         base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
         kActivationScopeAllSites, "" /* activation_lists */,
         "" /* performance_measurement_rate */, "" /* suppress_notifications */,
         "true" /* whitelist_site_on_reload */);
 
     NavigateAndExpectActivation(
         {false}, {GURL(kExampleUrl)},
         safe_browsing::SB_THREAT_TYPE_URL_PHISHING,
         safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS,
-        test_case.referrer, test_case.transition, EMPTY,
+        test_case.referrer, test_case.transition, EMPTY_REDIRECTS,
         test_case.expected_activation_decision);
     // Verify that if the first URL failed to activate, subsequent same-origin
     // navigations also fail to activate.
-    NavigateAndExpectActivation({false}, {GURL(kExampleUrlWithParams)}, EMPTY,
+    NavigateAndExpectActivation({false}, {GURL(kExampleUrlWithParams)},
+                                EMPTY_REDIRECTS,
                                 test_case.expected_activation_decision);
   }
 }
 
 TEST_P(ContentSubresourceFilterDriverFactoryActivationLevelTest,
        ActivateForFrameState) {
   const ActivationLevelTestData& test_data = GetParam();
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, test_data.activation_level,
       kActivationScopeActivationList,
       kActivationListSocialEngineeringAdsInterstitial);
 
   const GURL url(kExampleUrlWithParams);
-  NavigateAndExpectActivation({true}, {url}, NO_REDIRECTS_HIT,
+  NavigateAndExpectActivation({true}, {url}, MATCH_REDIRECTS,
                               test_data.expected_activation_decision);
   factory()->client()->WhitelistInCurrentWebContents(url);
   NavigateAndExpectActivation(
-      {true}, {GURL(kExampleUrlWithParams)}, NO_REDIRECTS_HIT,
+      {true}, {GURL(kExampleUrlWithParams)}, MATCH_REDIRECTS,
       GetActiveConfigurations()->the_one_and_only().activation_level ==
               ActivationLevel::DISABLED
           ? ActivationDecision::ACTIVATION_DISABLED
           : ActivationDecision::URL_WHITELISTED);
 }
 
 TEST_P(ContentSubresourceFilterDriverFactoryThreatTypeTest,
        ActivateForTheListType) {
   // Sets up the experiment in a way that the activation decision depends on the
   // list for which the Safe Browsing hit has happened.
   const ActivationListTestData& test_data = GetParam();
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       kActivationScopeActivationList, test_data.activation_list);
 
   const GURL test_url("https://example.com/nonsoceng?q=engsocnon");
   std::vector<GURL> navigation_chain;
 
   ActivationList effective_list = GetListForThreatTypeAndMetadata(
       test_data.threat_type, test_data.threat_type_metadata);
   NavigateAndExpectActivation(
       {false, false, false, true},
       {GURL(kUrlA), GURL(kUrlB), GURL(kUrlC), test_url}, test_data.threat_type,
       test_data.threat_type_metadata, content::Referrer(),
       ui::PAGE_TRANSITION_LINK,
-      effective_list != ActivationList::NONE ? F0M0L1 : EMPTY,
+      effective_list != ActivationList::NONE ? MATCH_REDIRECTS
+                                             : EMPTY_REDIRECTS,
       test_data.expected_activation_decision);
 };
 
 TEST_P(ContentSubresourceFilterDriverFactoryActivationScopeTest,
        ActivateForScopeType) {
   const ActivationScopeTestData& test_data = GetParam();
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       test_data.activation_scope,
       kActivationListSocialEngineeringAdsInterstitial);
 
   const GURL test_url(kExampleUrlWithParams);
 
-  RedirectChainMatchPattern expected_pattern =
-      test_data.url_matches_activation_list ? NO_REDIRECTS_HIT : EMPTY;
+  RecordedHistograms expected_histograms =
+      test_data.url_matches_activation_list ? MATCH_REDIRECTS : EMPTY_REDIRECTS;
   NavigateAndExpectActivation({test_data.url_matches_activation_list},
-                              {test_url}, expected_pattern,
+                              {test_url}, expected_histograms,
                               test_data.expected_activation_decision);
   if (test_data.url_matches_activation_list) {
     factory()->client()->WhitelistInCurrentWebContents(test_url);
     NavigateAndExpectActivation(
         {test_data.url_matches_activation_list}, {GURL(kExampleUrlWithParams)},
-        expected_pattern,
+        expected_histograms,
         GetActiveConfigurations()->the_one_and_only().activation_scope ==
                 ActivationScope::NO_SITES
             ? ActivationDecision::ACTIVATION_DISABLED
             : ActivationDecision::URL_WHITELISTED);
   }
 };
 
 // Only main frames with http/https schemes should activate, unless the
 // activation scope is for all sites.
 TEST_P(ContentSubresourceFilterDriverFactoryActivationScopeTest,
        ActivateForSupportedUrlScheme) {
   const ActivationScopeTestData& test_data = GetParam();
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
       test_data.activation_scope,
       kActivationListSocialEngineeringAdsInterstitial);
 
   // data URLs are also not supported, but not listed here, as it's not possible
   // for a page to redirect to them after https://crbug.com/594215 is fixed.
   const char* unsupported_urls[] = {"ftp://example.com/", "chrome://settings",
                                     "chrome-extension://some-extension",
                                     "file:///var/www/index.html"};
   const char* supported_urls[] = {"http://example.test",
                                   "https://example.test"};
   for (auto* url : unsupported_urls) {
     SCOPED_TRACE(url);
-    RedirectChainMatchPattern expected_pattern = EMPTY;
     NavigateAndExpectActivation(
-        {test_data.url_matches_activation_list}, {GURL(url)}, expected_pattern,
+        {test_data.url_matches_activation_list}, {GURL(url)}, EMPTY,
         GetActiveConfigurations()->the_one_and_only().activation_scope ==
                 ActivationScope::NO_SITES
             ? ActivationDecision::ACTIVATION_DISABLED
             : ActivationDecision::UNSUPPORTED_SCHEME);
   }
   for (auto* url : supported_urls) {
     SCOPED_TRACE(url);
-    RedirectChainMatchPattern expected_pattern =
-        test_data.url_matches_activation_list ? NO_REDIRECTS_HIT : EMPTY;
+    RecordedHistograms expected_histograms =
+        test_data.url_matches_activation_list ? MATCH_REDIRECTS
+                                              : EMPTY_REDIRECTS;
     NavigateAndExpectActivation({test_data.url_matches_activation_list},
-                                {GURL(url)}, expected_pattern,
+                                {GURL(url)}, expected_histograms,
                                 test_data.expected_activation_decision);
   }
 };
 
 INSTANTIATE_TEST_CASE_P(NoSocEngHit,
                         ContentSubresourceFilterDriverFactoryThreatTypeTest,
                         ::testing::ValuesIn(kActivationListTestData));
 
 INSTANTIATE_TEST_CASE_P(
     ActivationScopeTest,
     ContentSubresourceFilterDriverFactoryActivationScopeTest,
     ::testing::ValuesIn(kActivationScopeTestData));
 
 INSTANTIATE_TEST_CASE_P(
     ActivationLevelTest,
     ContentSubresourceFilterDriverFactoryActivationLevelTest,
     ::testing::ValuesIn(kActivationLevelTestData));
 
 }  // namespace subresource_filter