Add the goog-unwanted-shavar list to a new SafeBrowsing PrefixSet.

chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc

Index: chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 3137d061d1ff7472410d114cffde4cb220fe34b5..27539a83fb3f547ba1dd9e7a8f123134c9263580 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -134,6 +134,16 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
                        std::vector<GURL>(1, url),
                        prefix_hits);
   }
+  bool ContainsUnwantedSoftwareUrl(
+      const GURL& url,
+      std::vector<SBPrefix>* prefix_hits,
+      std::vector<SBFullHashResult>* cache_hits) override {
+    cache_hits->clear();
+    return ContainsUrl(safe_browsing_util::UNWANTEDURL,
+                       safe_browsing_util::UNWANTEDURL,
+                       std::vector<GURL>(1, url),
+                       prefix_hits);
+  }
   bool ContainsDownloadUrl(const std::vector<GURL>& urls,
                            std::vector<SBPrefix>* prefix_hits) override {
     bool found = ContainsUrl(safe_browsing_util::BINURL,
@@ -241,7 +251,8 @@ class TestSafeBrowsingDatabaseFactory : public SafeBrowsingDatabaseFactory {
       bool enable_download_whitelist,
       bool enable_extension_blacklist,
       bool enable_side_effect_free_whitelist,
-      bool enable_ip_blacklist) override {
+      bool enable_ip_blacklist,
+      bool enabled_unwanted_software_list) override {
     db_ = new TestSafeBrowsingDatabase();
     return db_;
   }
@@ -724,24 +735,58 @@ class TestSBClient
     content::RunMessageLoop();  // Will stop in OnCheckDownloadUrlResult.
   }
 
+  void CheckBrowseUrl(const GURL& url) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&TestSBClient::CheckBrowseUrlOnIOThread,
+                   this, url));
+    content::RunMessageLoop();  // Will stop in OnCheckBrowseUrlResult.
+  }
+
  private:
   friend class base::RefCountedThreadSafe<TestSBClient>;
   ~TestSBClient() override {}
 
   void CheckDownloadUrlOnIOThread(const std::vector<GURL>& url_chain) {
-    safe_browsing_service_->database_manager()->
+    bool synchronous_safe_signal = safe_browsing_service_->database_manager()->
         CheckDownloadUrl(url_chain, this);
+    if (synchronous_safe_signal) {
+      threat_type_ = SB_THREAT_TYPE_SAFE;
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                              base::Bind(&TestSBClient::CheckDone, this));
+    }
+  }
+
+  void CheckBrowseUrlOnIOThread(const GURL& url) {
+    // The async CheckDone() hook will not be called when we have a synchronous
+    // safe signal, handle it right away.
+    bool synchronous_safe_signal = safe_browsing_service_->database_manager()->
+        CheckBrowseUrl(url, this);
+    if (synchronous_safe_signal) {
+      threat_type_ = SB_THREAT_TYPE_SAFE;
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                              base::Bind(&TestSBClient::CheckDone, this));
+    }
   }
 
   // Called when the result of checking a download URL is known.
-  void OnCheckDownloadUrlResult(const std::vector<GURL>& url_chain,
+  void OnCheckDownloadUrlResult(const std::vector<GURL>& /* url_chain */,
                                 SBThreatType threat_type) override {
     threat_type_ = threat_type;
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(&TestSBClient::DownloadCheckDone, this));
+                            base::Bind(&TestSBClient::CheckDone, this));
+  }
+
+  // Called when the result of checking a browse URL is known.
+  void OnCheckBrowseUrlResult(const GURL& /* url */,
+                              SBThreatType threat_type,
+                              const std::string& /* metadata */ ) override {
+    threat_type_ = threat_type;
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(&TestSBClient::CheckDone, this));
   }
 
-  void DownloadCheckDone() {
+  void CheckDone() {
     base::MessageLoopForUI::current()->Quit();
   }
 
@@ -776,6 +821,91 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrl) {
   EXPECT_EQ(SB_THREAT_TYPE_BINARY_MALWARE_URL, client->GetThreatType());
 }
 
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckUnwantedSoftwareUrl) {
+  const GURL bad_url = test_server()->GetURL(kMalwareFile);
+  {
+    scoped_refptr<TestSBClient> client(new TestSBClient);
+
+    // Since bad_url is not in database, it is considered to be
+    // safe.
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_SAFE, client->GetThreatType());
+
+    SBFullHashResult full_hash_result;
+    GenUrlFullhashResult(bad_url, safe_browsing_util::UNWANTEDURL,
+                         &full_hash_result);
+    SetupResponseForUrl(bad_url, full_hash_result);
+
+    // Now, the bad_url is not safe since it is added to download
+    // database.
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_URL_UNWANTED, client->GetThreatType());
+  }
+
+  // The unwantedness should survive across multiple clients.
+  {
+    scoped_refptr<TestSBClient> client(new TestSBClient);
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_URL_UNWANTED, client->GetThreatType());
+  }
+
+  // An unwanted URL also marked as malware should be flagged as malware.
+  {
+    scoped_refptr<TestSBClient> client(new TestSBClient);
+
+    SBFullHashResult full_hash_result;
+    GenUrlFullhashResult(bad_url, safe_browsing_util::MALWARE,
+                         &full_hash_result);
+    SetupResponseForUrl(bad_url, full_hash_result);
+
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, client->GetThreatType());
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckBrowseUrl) {
+  const GURL bad_url = test_server()->GetURL(kMalwareFile);
+  {
+    scoped_refptr<TestSBClient> client(new TestSBClient);
+
+    // Since bad_url is not in database, it is considered to be
+    // safe.
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_SAFE, client->GetThreatType());
+
+    SBFullHashResult full_hash_result;
+    GenUrlFullhashResult(bad_url, safe_browsing_util::MALWARE,
+                         &full_hash_result);
+    SetupResponseForUrl(bad_url, full_hash_result);
+
+    // Now, the bad_url is not safe since it is added to download
+    // database.
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, client->GetThreatType());
+  }
+
+  // The unwantedness should survive across multiple clients.
+  {
+    scoped_refptr<TestSBClient> client(new TestSBClient);
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, client->GetThreatType());
+  }
+
+  // Adding the unwanted state to an existing malware URL should have no impact
+  // (i.e. a malware hit should still prevail).
+  {
+    scoped_refptr<TestSBClient> client(new TestSBClient);
+
+    SBFullHashResult full_hash_result;
+    GenUrlFullhashResult(bad_url, safe_browsing_util::UNWANTEDURL,
+                         &full_hash_result);
+    SetupResponseForUrl(bad_url, full_hash_result);

[mattm, 2014/11/11 01:29:11]

Looks like SetupResponseForUrl just overwrites the previous response, not adds
to it.

[gab, 2014/11/11 23:39:12]

Right, I caught on to that as well while doing further testing (when I
originally wrote CheckUnwantedUrl I thought the last bit would fail, but it
surprisingly didn't... which is why I wrote the CheckBrowseUrl test to confirm I
wasn't crazy but that one failed... which is when I realized the test was hiding
the real behavior with this. After fixing the test: CheckUnwantedUrl failed as
originally expected and then I added product logic to extract the severest
threat as desired).

+
+    client->CheckBrowseUrl(bad_url);
+    EXPECT_EQ(SB_THREAT_TYPE_URL_MALWARE, client->GetThreatType());
+  }
+}
+
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrlRedirects) {
   GURL original_url = test_server()->GetURL(kEmptyPage);
   GURL badbin_url = test_server()->GetURL(kMalwareFile);