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

chrome/browser/safe_browsing/database_manager.cc

Index: chrome/browser/safe_browsing/database_manager.cc
diff --git a/chrome/browser/safe_browsing/database_manager.cc b/chrome/browser/safe_browsing/database_manager.cc
index 712271ba657f0c31e19fc9bbd71aed6ef6fae19d..ba50c4716f95ce64bd29552b87785d973c516ced 100644
--- a/chrome/browser/safe_browsing/database_manager.cc
+++ b/chrome/browser/safe_browsing/database_manager.cc
@@ -112,6 +112,8 @@ SBThreatType GetThreatTypeFromListType(safe_browsing_util::ListType list_type) {
       return SB_THREAT_TYPE_URL_PHISHING;
     case safe_browsing_util::MALWARE:
       return SB_THREAT_TYPE_URL_MALWARE;
+    case safe_browsing_util::UNWANTEDURL:
+      return SB_THREAT_TYPE_URL_UNWANTED;
     case safe_browsing_util::BINURL:
       return SB_THREAT_TYPE_BINARY_MALWARE_URL;
     case safe_browsing_util::EXTENSIONBLACKLIST:
@@ -171,6 +173,7 @@ void SafeBrowsingDatabaseManager::Client::OnSafeBrowsingResult(
     switch (check.check_type) {
       case safe_browsing_util::MALWARE:
       case safe_browsing_util::PHISH:
+      case safe_browsing_util::UNWANTEDURL:
         DCHECK_EQ(1u, check.urls.size());
         OnCheckBrowseUrlResult(
             check.urls[0], check.url_results[0], check.url_metadata[0]);
@@ -217,6 +220,7 @@ SafeBrowsingDatabaseManager::SafeBrowsingDatabaseManager(
       enable_extension_blacklist_(false),
       enable_side_effect_free_whitelist_(false),
       enable_ip_blacklist_(false),
+      enable_unwanted_software_blacklist_(false),
       update_in_progress_(false),
       database_update_in_progress_(false),
       closing_database_(false),
@@ -253,6 +257,10 @@ SafeBrowsingDatabaseManager::SafeBrowsingDatabaseManager(
   // phishing protection for now.
   enable_ip_blacklist_ = enable_csd_whitelist_;
 
+  //FIXME: Figure out how to turn this on and off. It looks like the malware
+  //list is always on, should the uslist also be?
+  enable_unwanted_software_blacklist_ = true;
+
   enum SideEffectFreeWhitelistStatus {
     SIDE_EFFECT_FREE_WHITELIST_ENABLED,
     SIDE_EFFECT_FREE_WHITELIST_DISABLED,
@@ -409,6 +417,7 @@ bool SafeBrowsingDatabaseManager::CheckBrowseUrl(const GURL& url,
   std::vector<SBThreatType> expected_threats;
   expected_threats.push_back(SB_THREAT_TYPE_URL_MALWARE);
   expected_threats.push_back(SB_THREAT_TYPE_URL_PHISHING);
+  expected_threats.push_back(SB_THREAT_TYPE_URL_UNWANTED);
 
   const base::TimeTicks start = base::TimeTicks::Now();
   if (!MakeDatabaseAvailable()) {
@@ -421,32 +430,58 @@ bool SafeBrowsingDatabaseManager::CheckBrowseUrl(const GURL& url,
     return false;
   }
 
-  std::vector<SBPrefix> prefix_hits;
-  std::vector<SBFullHashResult> cache_hits;
+  std::vector<SBPrefix> browse_prefix_hits;
+  std::vector<SBFullHashResult> browse_cache_hits;
+  bool browse_prefix_match = database_->ContainsBrowseUrl(
+      url, &browse_prefix_hits, &browse_cache_hits);
 
-  bool prefix_match =
-      database_->ContainsBrowseUrl(url, &prefix_hits, &cache_hits);
+  std::vector<SBPrefix> unwanted_prefix_hits;
+  std::vector<SBFullHashResult> unwanted_cache_hits;
+  bool unwanted_prefix_match = database_->ContainsUnwantedSoftwareUrl(
+      url, &unwanted_prefix_hits, &unwanted_cache_hits);
 
   UMA_HISTOGRAM_TIMES("SB2.FilterCheck", base::TimeTicks::Now() - start);
 
-  if (!prefix_match)
+  if (!browse_prefix_match && !unwanted_prefix_match)
     return true;  // URL is okay.
 
-  // Needs to be asynchronous, since we could be in the constructor of a
-  // ResourceDispatcherHost event handler which can't pause there.
-  SafeBrowsingCheck* check = new SafeBrowsingCheck(std::vector<GURL>(1, url),
-                                                   std::vector<SBFullHash>(),
-                                                   client,
-                                                   safe_browsing_util::MALWARE,
-                                                   expected_threats);
-  check->need_get_hash = cache_hits.empty();
-  check->prefix_hits.swap(prefix_hits);
-  check->cache_hits.swap(cache_hits);
-  checks_.insert(check);
+  if (browse_prefix_match) {
+    // Needs to be asynchronous, since we could be in the constructor of a
+    // ResourceDispatcherHost event handler which can't pause there.
+    SafeBrowsingCheck* check =
+        new SafeBrowsingCheck(std::vector<GURL>(1, url),
+                              std::vector<SBFullHash>(),
+                              client,
+                              safe_browsing_util::MALWARE,
+                              expected_threats);
+    check->need_get_hash = browse_cache_hits.empty();
+    check->prefix_hits.swap(browse_prefix_hits);
+    check->cache_hits.swap(browse_cache_hits);
+    checks_.insert(check);
 
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
+  }
+
+  if (unwanted_prefix_match) {
+    // Needs to be asynchronous, since we could be in the constructor of a
+    // ResourceDispatcherHost event handler which can't pause there.
+    SafeBrowsingCheck* check =
+        new SafeBrowsingCheck(std::vector<GURL>(1, url),
+                              std::vector<SBFullHash>(),
+                              client,
+                              safe_browsing_util::UNWANTEDURL,
+                              expected_threats);
+    check->need_get_hash = unwanted_cache_hits.empty();
+    check->prefix_hits.swap(unwanted_prefix_hits);
+    check->cache_hits.swap(unwanted_cache_hits);
+    checks_.insert(check);

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

This is going to be a problem if both browse_prefix_match and
unwanted_prefix_match are true. The client isn't expecting to get two callbacks
for the same check.

[mattm, 2014/11/06 01:22:36]

Actually there should be no need for the separate checks, a single gethash check
always gets results for all lists, so you can have a single "if
(browse_prefix_match || unwanted_prefix_match)" since expected_threats contains
all three types.

[gab, 2014/11/06 17:47:46]

I caught on to do the same thing yesterday after sending this, I decided to
split it into two checks made by the client
SafeBrowsingDatabaseManager::CheckBrowseUrl and
SafeBrowsingDatabaseManager::CheckUnwantedSoftwareUrl.
Then the client (SafeBrowsingResourceThrottle) expects two callbacks and
forwards the most severe onwards (or the most severe to-date in the event of a
timeout).

Does that make sense?

I don't quite see how we could check two lists in a single SafeBrowsingCheck
here otherwise?

[gab, 2014/11/07 00:12:09]

Implementation in patch set 6.

[mattm, 2014/11/07 00:27:09]

SafeBrowsingCheck actually checks all lists, then filters the results based on
expected_threats. So you just combine the prefix_hits and cache_hits of the two
ContainsBrowseUrl and ContainsUWSUrl.

The one niggle is the "check_type" of SafeBrowsingCheck, but this is only used
to decide which callback to call, and as  the patch to
SafeBrowsingDatabaseManager::Client::OnSafeBrowsingResult above shows, you call
the same method in either case. Therefore using a single SafeBrowsingCheck with
check_type=MALWARE should be fine.

[gab, 2014/11/07 18:54:17]

I see, as discussed yesterday, makes sense. See latest update.

+
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
+  }
 
   return false;
 }
@@ -705,7 +740,8 @@ SafeBrowsingDatabase* SafeBrowsingDatabaseManager::GetDatabase() {
                                    enable_download_whitelist_,
                                    enable_extension_blacklist_,
                                    enable_side_effect_free_whitelist_,
-                                   enable_ip_blacklist_);
+                                   enable_ip_blacklist_,
+                                   enable_unwanted_software_blacklist_);
 
   database->Init(SafeBrowsingService::GetBaseFilename());
   {