Add the client for accessing Subresource Filter only list.

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

Index: chrome/browser/subresource_filter/subresource_filter_browsertest.cc
diff --git a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
index a31489eb968af42b49d733345c5e23f8c424b24d..553fdce94527e278f5dbbbf2c6c01b7a4f2c2978 100644
--- a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
+++ b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/v4_test_utils.h"
 #include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -30,6 +31,7 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/safe_browsing_db/test_database_manager.h"
 #include "components/safe_browsing_db/util.h"
+#include "components/safe_browsing_db/v4_database.h"
 #include "components/security_interstitials/content/unsafe_resource.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features.h"
@@ -57,7 +59,7 @@ namespace {
 
 // The path to a multi-frame document used for tests.
 static constexpr const char kTestFrameSetPath[] =
-    "subresource_filter/frame_set.html";
+    "/subresource_filter/frame_set.html";
 
 // Names of DocumentLoad histograms.
 constexpr const char kDocumentLoadActivationLevel[] =
@@ -98,61 +100,34 @@ constexpr const char kEvaluationWallDuration[] =
 constexpr const char kEvaluationCPUDuration[] =
     "SubresourceFilter.SubresourceLoad.Evaluation.CPUDuration";
 
+#if defined(GOOGLE_CHROME_BUILD)
+// Names of navigation chain patterns diagram.

[engedy, 2017/03/10 14:39:32]

nit: ... histograms.

[melandory, 2017/03/15 13:41:35]

Done.

+const char kMatchesPatternHistogramName[] =
+    "SubresourceFilter.PageLoad.RedirectChainMatchPattern";
+const char kNavigationChainSize[] =
+    "SubresourceFilter.PageLoad.RedirectChainLength";
+const char kSubresourceFilterOnlySuffix[] = ".SubresourceFilterOnly";
+#endif
+
 // Other histograms.
 const char kSubresourceFilterPromptHistogram[] =
     "SubresourceFilter.Prompt.NumVisibility";
 
-// Database manager that allows any URL to be configured as blacklisted for
-// testing.
-class FakeSafeBrowsingDatabaseManager
-    : public safe_browsing::TestSafeBrowsingDatabaseManager {
- public:
-  FakeSafeBrowsingDatabaseManager() {}
-
-  void AddBlacklistedURL(const GURL& url,
-                         safe_browsing::SBThreatType threat_type) {
-    url_to_threat_type_[url] = threat_type;
-  }
-
- protected:
-  ~FakeSafeBrowsingDatabaseManager() override {}
-
-  bool CheckBrowseUrl(const GURL& url, Client* client) override {
-    if (!url_to_threat_type_.count(url))
-      return true;
-
-    content::BrowserThread::PostTask(
-        content::BrowserThread::IO, FROM_HERE,
-        base::Bind(&Client::OnCheckBrowseUrlResult, base::Unretained(client),
-                   url, url_to_threat_type_[url],
-                   safe_browsing::ThreatMetadata()));
-    return false;
-  }
-
-  bool CheckResourceUrl(const GURL& url, Client* client) override {
-    return true;
-  }
-
-  bool IsSupported() const override { return true; }
-  bool ChecksAreAlwaysAsync() const override { return false; }
-  bool CanCheckResourceType(
-      content::ResourceType /* resource_type */) const override {
-    return true;
-  }
-
-  safe_browsing::ThreatSource GetThreatSource() const override {
-    return safe_browsing::ThreatSource::LOCAL_PVER4;
-  }
-
-  bool CheckExtensionIDs(const std::set<std::string>& extension_ids,
-                         Client* client) override {
-    return true;
-  }
-
- private:
-  std::map<GURL, safe_browsing::SBThreatType> url_to_threat_type_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeSafeBrowsingDatabaseManager);
+// 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,
 };
 
 // UI manager that never actually shows any interstitials, but emulates as if
@@ -180,12 +155,6 @@ GURL GetURLWithFragment(const GURL& url, base::StringPiece fragment) {
   return url.ReplaceComponents(replacements);
 }
 
-GURL GetURLWithQuery(const GURL& url, base::StringPiece query) {
-  GURL::Replacements replacements;
-  replacements.SetQueryStr(query);
-  return url.ReplaceComponents(replacements);
-}
-
 }  // namespace
 
 namespace subresource_filter {
@@ -229,17 +198,42 @@ class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest {
   // As a workaround, enable the feature here, then enable the feature once
   // again + set up the field trials later.
   void SetUpCommandLine(base::CommandLine* command_line) override {
-    command_line->AppendSwitchASCII(switches::kEnableFeatures,
-                                    kSafeBrowsingSubresourceFilter.name);
+    command_line->AppendSwitchASCII(
+        switches::kEnableFeatures,
+        std::string(kSafeBrowsingSubresourceFilter.name) +

[engedy, 2017/03/10 14:39:32]

nit: Let's make this nicer by either appending two separate switches (if
possible), or using base::JoinString({..}, ',');

[melandory, 2017/03/15 13:41:36]

Done.

+            ", SafeBrowsingV4OnlyEnabled");
   }
 
-  void SetUpInProcessBrowserTestFixture() override {
-    fake_safe_browsing_database_ = new FakeSafeBrowsingDatabaseManager();
-    test_safe_browsing_factory_.SetTestDatabaseManager(
-        fake_safe_browsing_database_.get());
-    test_safe_browsing_factory_.SetTestUIManager(new FakeSafeBrowsingUIManager);
-    safe_browsing::SafeBrowsingService::RegisterFactory(
-        &test_safe_browsing_factory_);
+  void SetUp() override {
+    sb_factory_ =
+        base::MakeUnique<safe_browsing::TestSafeBrowsingServiceFactory>(
+            safe_browsing::V4FeatureList::V4UsageStatus::V4_ONLY);
+    sb_factory_->SetTestUIManager(new FakeSafeBrowsingUIManager());
+    safe_browsing::SafeBrowsingService::RegisterFactory(sb_factory_.get());
+
+    store_factory_ = new safe_browsing::TestV4StoreFactory();

[engedy, 2017/03/10 14:39:32]

nit: Not used outside of this function.

[melandory, 2017/03/15 13:41:35]

Done.

+    safe_browsing::V4Database::RegisterStoreFactoryForTest(
+        base::WrapUnique(store_factory_));
+
+    v4_db_factory_ = new safe_browsing::TestV4DatabaseFactory();
+    safe_browsing::V4Database::RegisterDatabaseFactoryForTest(
+        base::WrapUnique(v4_db_factory_));
+
+    v4_get_hash_factory_ =
+        new safe_browsing::TestV4GetHashProtocolManagerFactory();
+    safe_browsing::V4GetHashProtocolManager::RegisterFactory(
+        base::WrapUnique(v4_get_hash_factory_));
+    InProcessBrowserTest::SetUp();
+  }
+
+  void TearDown() override {
+    InProcessBrowserTest::TearDown();
+    // Unregister test factories after InProcessBrowserTest::TearDown
+    // (which destructs SafeBrowsingService).
+    safe_browsing::V4GetHashProtocolManager::RegisterFactory(nullptr);
+    safe_browsing::V4Database::RegisterDatabaseFactoryForTest(nullptr);
+    safe_browsing::V4Database::RegisterStoreFactoryForTest(nullptr);
+    safe_browsing::SafeBrowsingService::RegisterFactory(nullptr);
   }
 
   void SetUpOnMainThread() override {
@@ -262,9 +256,24 @@ class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest {
     return embedded_test_server()->base_url().Resolve(relative_url);
   }
 
+  void MarkUrlForListIdUnexpired(

[engedy, 2017/03/10 14:39:32]

nit: Add comment for what `unexpired` means, or name this something like
MarkUrlAsMatchingListWithId.

[melandory, 2017/03/15 13:41:36]

Done.

+      const GURL& bad_url,
+      const safe_browsing::ListIdentifier& list_id,
+      safe_browsing::ThreatPatternType threat_pattern_type) {
+    safe_browsing::FullHashInfo full_hash_info =
+        GetFullHashInfoWithMetadata(bad_url, list_id, threat_pattern_type);
+    v4_db_factory_->MarkPrefixAsBad(list_id, full_hash_info.full_hash);
+    v4_get_hash_factory_->AddToFullHashCache(full_hash_info);
+  }
+
   void ConfigureAsPhishingURL(const GURL& url) {
-    fake_safe_browsing_database_->AddBlacklistedURL(
-        url, safe_browsing::SB_THREAT_TYPE_URL_PHISHING);
+    MarkUrlForListIdUnexpired(url, safe_browsing::GetUrlSocEngId(),
+                              safe_browsing::ThreatPatternType::NONE);
+  }
+
+  void ConfigureAsSubresourceFilterOnlyURL(const GURL& url) {
+    MarkUrlForListIdUnexpired(url, safe_browsing::GetUrlSubresourceFilterId(),
+                              safe_browsing::ThreatPatternType::NONE);
   }
 
   content::WebContents* web_contents() {
@@ -340,14 +349,20 @@ class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest {
 
  private:
   TestRulesetCreator ruleset_creator_;
-  safe_browsing::TestSafeBrowsingServiceFactory test_safe_browsing_factory_;
-  scoped_refptr<FakeSafeBrowsingDatabaseManager> fake_safe_browsing_database_;
 
   std::unique_ptr<ScopedSubresourceFilterFeatureToggle> scoped_feature_toggle_;
   TestRulesetPublisher test_ruleset_publisher_;
   const bool measure_performance_;
   const bool whitelist_site_on_reload_;
 
+  std::unique_ptr<safe_browsing::TestSafeBrowsingServiceFactory> sb_factory_;
+  // Owned by the V4Database.
+  safe_browsing::TestV4DatabaseFactory* v4_db_factory_;
+  // Owned by the V4GetHashProtocolManager.
+  safe_browsing::TestV4GetHashProtocolManagerFactory* v4_get_hash_factory_;
+  // Owned by the V4Database.
+  safe_browsing::TestV4StoreFactory* store_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterBrowserTestImpl);
 };
 
@@ -479,9 +494,9 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, SubFrameActivation) {
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
                        HistoryNavigationActivation) {
-  GURL url_without_activation(GetTestUrl(kTestFrameSetPath));
-  GURL url_with_activation(
-      GetURLWithQuery(url_without_activation, "activation"));
+  GURL url_with_activation(GetTestUrl(kTestFrameSetPath));
+  GURL url_without_activation(
+      embedded_test_server()->GetURL("a.com", kTestFrameSetPath));
   ConfigureAsPhishingURL(url_with_activation);
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
@@ -996,4 +1011,53 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterWhitelistSiteOnReloadBrowserTest,
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 }
 
+#if defined(GOOGLE_CHROME_BUILD)
+// This test is only enabled when GOOGLE_CHROME_BUILD is true because the store
+// that this test uses is only populated on GOOGLE_CHROME_BUILD builds.
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    ExpectRedirectPatternHistogramsAreRecordedForSubresourceFilterOnlyMatch) {
+  ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
+      "suffix-that-does-not-match-anything"));
+
+  GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
+  ConfigureAsSubresourceFilterOnlyURL(url);
+
+  base::HistogramTester tester;
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  EXPECT_THAT(tester.GetAllSamples(std::string(kMatchesPatternHistogramName) +
+                                   std::string(kSubresourceFilterOnlySuffix)),
+              ::testing::ElementsAre(base::Bucket(NO_REDIRECTS_HIT, 1)));
+  EXPECT_THAT(tester.GetAllSamples(std::string(kNavigationChainSize) +
+                                   std::string(kSubresourceFilterOnlySuffix)),
+              ::testing::ElementsAre(base::Bucket(1, 1)));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    ExpectRedirectPatternHistogramsAreRecordedForSubresourceFilterOnlyRedirectMatch) {
+  ASSERT_NO_FATAL_FAILURE(
+      SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
+  const std::string initial_host("a.com");
+  const std::string redirected_host("b.com");
+
+  GURL redirect_url(embedded_test_server()->GetURL(
+      redirected_host, "/subresource_filter/frame_with_included_script.html"));
+  GURL url(embedded_test_server()->GetURL(
+      initial_host, "/server-redirect?" + redirect_url.spec()));
+
+  ConfigureAsSubresourceFilterOnlyURL(url.GetOrigin());
+  base::HistogramTester tester;
+  ui_test_utils::NavigateToURL(browser(), url);
+  EXPECT_THAT(tester.GetAllSamples(std::string(kMatchesPatternHistogramName) +
+                                   std::string(kSubresourceFilterOnlySuffix)),
+              ::testing::IsEmpty());
+
+  EXPECT_THAT(tester.GetAllSamples(std::string(kNavigationChainSize) +
+                                   std::string(kSubresourceFilterOnlySuffix)),
+              ::testing::IsEmpty());
+}
+#endif
+
 }  // namespace subresource_filter