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

chrome/browser/safe_browsing/safe_browsing_database.cc

 // Copyright (c) 2012 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 "chrome/browser/safe_browsing/safe_browsing_database.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/bind.h"
@@ skipping 39 matching lines @@
     FILE_PATH_LITERAL(" Download Whitelist");
 // Filename suffix for the extension blacklist store.
 const base::FilePath::CharType kExtensionBlacklistDBFile[] =
     FILE_PATH_LITERAL(" Extension Blacklist");
 // Filename suffix for the side-effect free whitelist store.
 const base::FilePath::CharType kSideEffectFreeWhitelistDBFile[] =
     FILE_PATH_LITERAL(" Side-Effect Free Whitelist");
 // Filename suffix for the csd malware IP blacklist store.
 const base::FilePath::CharType kIPBlacklistDBFile[] =
     FILE_PATH_LITERAL(" IP Blacklist");
+// Filename suffix for the unwanted software blacklist store.
+const base::FilePath::CharType kUnwantedSoftwareDBFile[] =
+    FILE_PATH_LITERAL(" UwS List");
 
 // Filename suffix for browse store.
 // TODO(shess): "Safe Browsing Bloom Prefix Set" is full of win.
 // Unfortunately, to change the name implies lots of transition code
 // for little benefit.  If/when file formats change (say to put all
 // the data in one file), that would be a convenient point to rectify
 // this.
 // TODO(shess): This shouldn't be OS-driven <http://crbug.com/394379>
 #if defined(OS_ANDROID)
 // NOTE(shess): This difference is also reflected in the list name in
@@ skipping 45 matching lines @@
 // Generate the set of full hashes to check for |url|.  If
 // |include_whitelist_hashes| is true we will generate additional path-prefixes
 // to match against the csd whitelist.  E.g., if the path-prefix /foo is on the
 // whitelist it should also match /foo/bar which is not the case for all the
 // other lists.  We'll also always add a pattern for the empty path.
 // TODO(shess): This function is almost the same as
 // |CompareFullHashes()| in safe_browsing_util.cc, except that code
 // does an early exit on match.  Since match should be the infrequent
 // case (phishing or malware found), consider combining this function
 // with that one.
-void BrowseFullHashesToCheck(const GURL& url,
-                             bool include_whitelist_hashes,
-                             std::vector<SBFullHash>* full_hashes) {
+void UrlToFullHashes(const GURL& url,
+                     bool include_whitelist_hashes,
+                     std::vector<SBFullHash>* full_hashes) {
   std::vector<std::string> hosts;
   if (url.HostIsIPAddress()) {
     hosts.push_back(url.host());
   } else {
     safe_browsing_util::GenerateHostsToCheck(url, &hosts);
   }
 
   std::vector<std::string> paths;
   safe_browsing_util::GeneratePathsToCheck(url, &paths);
 
@@ skipping 13 matching lines @@
       }
     }
   }
 }
 
 // Get the prefixes matching the download |urls|.
 void GetDownloadUrlPrefixes(const std::vector<GURL>& urls,
                             std::vector<SBPrefix>* prefixes) {
   std::vector<SBFullHash> full_hashes;
   for (size_t i = 0; i < urls.size(); ++i)
-    BrowseFullHashesToCheck(urls[i], false, &full_hashes);
+    UrlToFullHashes(urls[i], false, &full_hashes);
 
   for (size_t i = 0; i < full_hashes.size(); ++i)
     prefixes->push_back(full_hashes[i].prefix);
 }
 
 // Helper function to compare addprefixes in |store| with |prefixes|.
 // The |list_bit| indicates which list (url or hash) to compare.
 //
 // Returns true if there is a match, |*prefix_hits| (if non-NULL) will contain
 // the actual matching prefixes.
@@ skipping 73 matching lines @@
     DCHECK_EQ(safe_browsing_util::GetListId(listname) % 2,
               static_cast<int>(i % 2));
     DCHECK_NE(safe_browsing_util::GetListId(listname),
               safe_browsing_util::INVALID);
     lists->push_back(SBListChunkRanges(listname));
     lists->back().adds.swap(adds[i]);
     lists->back().subs.swap(subs[i]);
   }
 }
 
-void UpdateChunkRangesForLists(SafeBrowsingStore* store,
-                               const std::string& listname0,
-                               const std::string& listname1,
-                               std::vector<SBListChunkRanges>* lists) {
+void UpdateChunkRangesForBrowseLists(SafeBrowsingStore* store,
+                                     const std::string& listname0,
+                                     const std::string& listname1,
+                                     std::vector<SBListChunkRanges>* lists) {
   std::vector<std::string> listnames;
   listnames.push_back(listname0);
   listnames.push_back(listname1);
   UpdateChunkRanges(store, listnames, lists);
 }
 
 void UpdateChunkRangesForList(SafeBrowsingStore* store,
                               const std::string& listname,
                               std::vector<SBListChunkRanges>* lists) {
   UpdateChunkRanges(store, std::vector<std::string>(1, listname), lists);
 }
 
 // This code always checks for non-zero file size.  This helper makes
 // that less verbose.
 int64 GetFileSizeOrZero(const base::FilePath& file_path) {
   int64 size_64;
   if (!base::GetFileSize(file_path, &size_64))
     return 0;
   return size_64;
 }
 
-// Helper for ContainsBrowseUrlHashes().  Returns true if an un-expired match
+// Helper for PrefixSetContainsUrlHashes().  Returns true if an un-expired match
 // for |full_hash| is found in |cache|, with any matches appended to |results|
 // (true can be returned with zero matches).  |expire_base| is used to check the
 // cache lifetime of matches, expired matches will be discarded from |cache|.
 bool GetCachedFullHash(std::map<SBPrefix, SBCachedFullHashResult>* cache,
                        const SBFullHash& full_hash,
                        const base::Time& expire_base,
                        std::vector<SBFullHashResult>* results) {
   // First check if there is a valid cached result for this prefix.
   std::map<SBPrefix, SBCachedFullHashResult>::iterator
       citer = cache->find(full_hash.prefix);
@@ skipping 21 matching lines @@
 
 // The default SafeBrowsingDatabaseFactory.
 class SafeBrowsingDatabaseFactoryImpl : public SafeBrowsingDatabaseFactory {
  public:
   SafeBrowsingDatabase* CreateSafeBrowsingDatabase(
       bool enable_download_protection,
       bool enable_client_side_whitelist,
       bool enable_download_whitelist,
       bool enable_extension_blacklist,
       bool enable_side_effect_free_whitelist,
-      bool enable_ip_blacklist) override {
+      bool enable_ip_blacklist,
+      bool enable_unwanted_software_list) override {
     return new SafeBrowsingDatabaseNew(
         new SafeBrowsingStoreFile,
         enable_download_protection ? new SafeBrowsingStoreFile : NULL,
         enable_client_side_whitelist ? new SafeBrowsingStoreFile : NULL,
         enable_download_whitelist ? new SafeBrowsingStoreFile : NULL,
         enable_extension_blacklist ? new SafeBrowsingStoreFile : NULL,
         enable_side_effect_free_whitelist ? new SafeBrowsingStoreFile : NULL,
-        enable_ip_blacklist ? new SafeBrowsingStoreFile : NULL);
+        enable_ip_blacklist ? new SafeBrowsingStoreFile : NULL,
+        enable_unwanted_software_list ? new SafeBrowsingStoreFile : NULL);
   }
 
   SafeBrowsingDatabaseFactoryImpl() { }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingDatabaseFactoryImpl);
 };
 
 // static
 SafeBrowsingDatabaseFactory* SafeBrowsingDatabase::factory_ = NULL;
 
 // Factory method, non-thread safe. Caller has to make sure this s called
 // on SafeBrowsing Thread.
 // TODO(shess): There's no need for a factory any longer.  Convert
 // SafeBrowsingDatabaseNew to SafeBrowsingDatabase, and have Create()
 // callers just construct things directly.
 SafeBrowsingDatabase* SafeBrowsingDatabase::Create(
     bool enable_download_protection,
     bool enable_client_side_whitelist,
     bool enable_download_whitelist,
     bool enable_extension_blacklist,
     bool enable_side_effect_free_whitelist,
-    bool enable_ip_blacklist) {
+    bool enable_ip_blacklist,
+    bool enable_unwanted_software_list) {
   if (!factory_)
     factory_ = new SafeBrowsingDatabaseFactoryImpl();
   return factory_->CreateSafeBrowsingDatabase(
       enable_download_protection,
       enable_client_side_whitelist,
       enable_download_whitelist,
       enable_extension_blacklist,
       enable_side_effect_free_whitelist,
-      enable_ip_blacklist);
+      enable_ip_blacklist,
+      enable_unwanted_software_list);
 }
 
 SafeBrowsingDatabase::~SafeBrowsingDatabase() {
 }
 
 // static
 base::FilePath SafeBrowsingDatabase::BrowseDBFilename(
     const base::FilePath& db_base_filename) {
   return base::FilePath(db_base_filename.value() + kBrowseDBFile);
 }
@@ skipping 39 matching lines @@
     const base::FilePath& db_filename) {
   return base::FilePath(db_filename.value() + kSideEffectFreeWhitelistDBFile);
 }
 
 // static
 base::FilePath SafeBrowsingDatabase::IpBlacklistDBFilename(
     const base::FilePath& db_filename) {
   return base::FilePath(db_filename.value() + kIPBlacklistDBFile);
 }
 
+// static
+base::FilePath SafeBrowsingDatabase::UnwantedSoftwareDBFilename(
+    const base::FilePath& db_filename) {
+  return base::FilePath(db_filename.value() + kUnwantedSoftwareDBFile);
+}
+
 SafeBrowsingStore* SafeBrowsingDatabaseNew::GetStore(const int list_id) {
   if (list_id == safe_browsing_util::PHISH ||
       list_id == safe_browsing_util::MALWARE) {
     return browse_store_.get();
   } else if (list_id == safe_browsing_util::BINURL) {
     return download_store_.get();
   } else if (list_id == safe_browsing_util::CSDWHITELIST) {
     return csd_whitelist_store_.get();
   } else if (list_id == safe_browsing_util::DOWNLOADWHITELIST) {
     return download_whitelist_store_.get();
   } else if (list_id == safe_browsing_util::EXTENSIONBLACKLIST) {
     return extension_blacklist_store_.get();
   } else if (list_id == safe_browsing_util::SIDEEFFECTFREEWHITELIST) {
     return side_effect_free_whitelist_store_.get();
   } else if (list_id == safe_browsing_util::IPBLACKLIST) {
     return ip_blacklist_store_.get();
+  } else if (list_id == safe_browsing_util::UNWANTEDURL) {
+    return unwanted_software_store_.get();
   }
   return NULL;
 }
 
 // static
 void SafeBrowsingDatabase::RecordFailure(FailureType failure_type) {
   UMA_HISTOGRAM_ENUMERATION("SB2.DatabaseFailure", failure_type,
                             FAILURE_DATABASE_MAX);
 }
 
 SafeBrowsingDatabaseNew::SafeBrowsingDatabaseNew()
     : creation_loop_(base::MessageLoop::current()),
       browse_store_(new SafeBrowsingStoreFile),
       corruption_detected_(false),
       change_detected_(false),
       reset_factory_(this) {
   DCHECK(browse_store_.get());
   DCHECK(!download_store_.get());
   DCHECK(!csd_whitelist_store_.get());
   DCHECK(!download_whitelist_store_.get());
   DCHECK(!extension_blacklist_store_.get());
   DCHECK(!side_effect_free_whitelist_store_.get());
   DCHECK(!ip_blacklist_store_.get());
+  DCHECK(!unwanted_software_store_.get());
 }
 
 SafeBrowsingDatabaseNew::SafeBrowsingDatabaseNew(
     SafeBrowsingStore* browse_store,
     SafeBrowsingStore* download_store,
     SafeBrowsingStore* csd_whitelist_store,
     SafeBrowsingStore* download_whitelist_store,
     SafeBrowsingStore* extension_blacklist_store,
     SafeBrowsingStore* side_effect_free_whitelist_store,
-    SafeBrowsingStore* ip_blacklist_store)
+    SafeBrowsingStore* ip_blacklist_store,
+    SafeBrowsingStore* unwanted_software_store)
     : creation_loop_(base::MessageLoop::current()),
       browse_store_(browse_store),
       download_store_(download_store),
       csd_whitelist_store_(csd_whitelist_store),
       download_whitelist_store_(download_whitelist_store),
       extension_blacklist_store_(extension_blacklist_store),
       side_effect_free_whitelist_store_(side_effect_free_whitelist_store),
       ip_blacklist_store_(ip_blacklist_store),
+      unwanted_software_store_(unwanted_software_store),
       corruption_detected_(false),
       reset_factory_(this) {
   DCHECK(browse_store_.get());
 }
 
 SafeBrowsingDatabaseNew::~SafeBrowsingDatabaseNew() {
   // The DCHECK is disabled due to crbug.com/338486 .
   // DCHECK_EQ(creation_loop_, base::MessageLoop::current());
 }
 
 void SafeBrowsingDatabaseNew::Init(const base::FilePath& filename_base) {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
 
   // This should not be run multiple times.
   DCHECK(filename_base_.empty());
 
   filename_base_ = filename_base;
 
   // TODO(shess): The various stores are really only necessary while doing
-  // updates, or when querying a store directly (see |ContainsDownloadUrl()|).
+  // updates (see |UpdateFinished()|) or when querying a store directly (see
+  // |ContainsDownloadUrl()|).
   // The store variables are also tested to see if a list is enabled.  Perhaps
   // the stores could be refactored into an update object so that they are only
   // live in memory while being actively used.  The sense of enabled probably
   // belongs in protocol_manager or database_manager.
 
   browse_store_->Init(
       BrowseDBFilename(filename_base_),
       base::Bind(&SafeBrowsingDatabaseNew::HandleCorruptDatabase,
                  base::Unretained(this)));
 
   {
     // NOTE: There is no need to grab the lock in this function, since
     // until it returns, there are no pointers to this class on other
     // threads.  Then again, that means there is no possibility of
     // contention on the lock...
     base::AutoLock locked(lookup_lock_);
-    browse_gethash_cache_.clear();
-    LoadPrefixSet();
+    prefix_gethash_cache_.clear();
+    LoadPrefixSet(BrowseDBFilename(filename_base_), &browse_prefix_set_,
+                  FAILURE_BROWSE_PREFIX_SET_READ);
   }
 
   if (download_store_.get()) {
     download_store_->Init(
         DownloadDBFilename(filename_base_),
         base::Bind(&SafeBrowsingDatabaseNew::HandleCorruptDatabase,
                    base::Unretained(this)));
   }
 
   if (csd_whitelist_store_.get()) {
@@ skipping 72 matching lines @@
         base::Bind(&SafeBrowsingDatabaseNew::HandleCorruptDatabase,
                    base::Unretained(this)));
 
     std::vector<SBAddFullHash> full_hashes;
     if (ip_blacklist_store_->GetAddFullHashes(&full_hashes)) {
       LoadIpBlacklist(full_hashes);
     } else {
       LoadIpBlacklist(std::vector<SBAddFullHash>());  // Clear the list.
     }
   }
+
+  if (unwanted_software_store_.get()) {
+    unwanted_software_store_->Init(
+        UnwantedSoftwareDBFilename(filename_base_),
+        base::Bind(&SafeBrowsingDatabaseNew::HandleCorruptDatabase,
+                   base::Unretained(this)));
+    LoadPrefixSet(UnwantedSoftwareDBFilename(filename_base_),
+                  &unwanted_software_prefix_set_,
+                  FAILURE_UNWANTED_SOFTWARE_PREFIX_SET_READ);

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

I guess this should be protected by lookup_lock too (probably not needed, as the
comment above says, but should be consistent).

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

Done (and moved it up to have it contextually close to similar code).

+  }
 }
 
 bool SafeBrowsingDatabaseNew::ResetDatabase() {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
 
   // Delete files on disk.
   // TODO(shess): Hard to see where one might want to delete without a
   // reset.  Perhaps inline |Delete()|?
   if (!Delete())
     return false;
 
   // Reset objects in memory.
   {
     base::AutoLock locked(lookup_lock_);
-    browse_gethash_cache_.clear();
+    prefix_gethash_cache_.clear();
     browse_prefix_set_.reset();
     side_effect_free_whitelist_prefix_set_.reset();
     ip_blacklist_.clear();
+    unwanted_software_prefix_set_.reset();
   }
   // Wants to acquire the lock itself.
   WhitelistEverything(&csd_whitelist_);
   WhitelistEverything(&download_whitelist_);
   return true;
 }
 
 bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
     const GURL& url,
     std::vector<SBPrefix>* prefix_hits,
     std::vector<SBFullHashResult>* cache_hits) {
+  return PrefixSetContainsUrl(url, browse_prefix_set_.get(), prefix_hits,

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

the browse_prefix_set_.get() and unwanted_software_prefix_set_.get() needs to be
done inside (the same) lookup_lock_ that protects the access, which is the
reason for the odd ordering of the code in the old version.

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

Ah I see makes sense now :-)! Tweaked to make it so.

+                              cache_hits);
+}
+
+bool SafeBrowsingDatabaseNew::ContainsUnwantedSoftwareUrl(
+    const GURL& url,
+    std::vector<SBPrefix>* prefix_hits,
+    std::vector<SBFullHashResult>* cache_hits) {
+  return PrefixSetContainsUrl(url, unwanted_software_prefix_set_.get(),
+                              prefix_hits, cache_hits);
+}
+
+bool SafeBrowsingDatabaseNew::PrefixSetContainsUrl(
+    const GURL& url,
+    safe_browsing::PrefixSet* prefix_set,
+    std::vector<SBPrefix>* prefix_hits,
+    std::vector<SBFullHashResult>* cache_hits) {
   // Clear the results first.
   prefix_hits->clear();
   cache_hits->clear();
 
+  // |prefix_set| is empty until it is either read from disk, or the first
+  // update populates it.  Bail out without a hit if not yet available.
+  if (!prefix_set)
+    return false;
+
   std::vector<SBFullHash> full_hashes;
-  BrowseFullHashesToCheck(url, false, &full_hashes);
+  UrlToFullHashes(url, false, &full_hashes);
   if (full_hashes.empty())
     return false;
 
-  return ContainsBrowseUrlHashes(full_hashes, prefix_hits, cache_hits);
+  return PrefixSetContainsUrlHashes(full_hashes, prefix_set, prefix_hits,
+                                    cache_hits);
 }
 
-bool SafeBrowsingDatabaseNew::ContainsBrowseUrlHashes(
+bool SafeBrowsingDatabaseNew::ContainsBrowseUrlHashesForTesting(
     const std::vector<SBFullHash>& full_hashes,
     std::vector<SBPrefix>* prefix_hits,
     std::vector<SBFullHashResult>* cache_hits) {
+  return PrefixSetContainsUrlHashes(full_hashes, browse_prefix_set_.get(),
+                                    prefix_hits, cache_hits);
+}
+
+bool SafeBrowsingDatabaseNew::PrefixSetContainsUrlHashes(
+    const std::vector<SBFullHash>& full_hashes,
+    safe_browsing::PrefixSet* prefix_set,
+    std::vector<SBPrefix>* prefix_hits,
+    std::vector<SBFullHashResult>* cache_hits) {
   // Used to determine cache expiration.
   const base::Time now = base::Time::Now();
 
   // This function is called on the I/O thread, prevent changes to
   // filter and caches.
   base::AutoLock locked(lookup_lock_);
 
-  // |browse_prefix_set_| is empty until it is either read from disk, or the
-  // first update populates it.  Bail out without a hit if not yet
-  // available.
-  if (!browse_prefix_set_.get())
-    return false;
-
   for (size_t i = 0; i < full_hashes.size(); ++i) {
-    if (!GetCachedFullHash(&browse_gethash_cache_,
+    if (!GetCachedFullHash(&prefix_gethash_cache_,
                            full_hashes[i],
                            now,
                            cache_hits)) {
       // No valid cached result, check the database.
-      if (browse_prefix_set_->Exists(full_hashes[i]))
+      if (prefix_set->Exists(full_hashes[i]))
         prefix_hits->push_back(full_hashes[i].prefix);
     }
   }
 
   // Multiple full hashes could share prefix, remove duplicates.
   std::sort(prefix_hits->begin(), prefix_hits->end());
   prefix_hits->erase(std::unique(prefix_hits->begin(), prefix_hits->end()),
                      prefix_hits->end());
 
   return !prefix_hits->empty() || !cache_hits->empty();
@@ skipping 14 matching lines @@
                           safe_browsing_util::BINURL % 2,
                           prefixes,
                           prefix_hits);
 }
 
 bool SafeBrowsingDatabaseNew::ContainsCsdWhitelistedUrl(const GURL& url) {
   // This method is theoretically thread-safe but we expect all calls to
   // originate from the IO thread.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   std::vector<SBFullHash> full_hashes;
-  BrowseFullHashesToCheck(url, true, &full_hashes);
+  UrlToFullHashes(url, true, &full_hashes);
   return ContainsWhitelistedHashes(csd_whitelist_, full_hashes);
 }
 
 bool SafeBrowsingDatabaseNew::ContainsDownloadWhitelistedUrl(const GURL& url) {
   std::vector<SBFullHash> full_hashes;
-  BrowseFullHashesToCheck(url, true, &full_hashes);
+  UrlToFullHashes(url, true, &full_hashes);
   return ContainsWhitelistedHashes(download_whitelist_, full_hashes);
 }
 
 bool SafeBrowsingDatabaseNew::ContainsExtensionPrefixes(
     const std::vector<SBPrefix>& prefixes,
     std::vector<SBPrefix>* prefix_hits) {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
   if (!extension_blacklist_store_)
     return false;
 
@@ skipping 219 matching lines @@
     const std::vector<SBPrefix>& prefixes,
     const std::vector<SBFullHashResult>& full_hits,
     const base::TimeDelta& cache_lifetime) {
   const base::Time expire_after = base::Time::Now() + cache_lifetime;
 
   // This is called on the I/O thread, lock against updates.
   base::AutoLock locked(lookup_lock_);
 
   // Create or reset all cached results for these prefixes.
   for (size_t i = 0; i < prefixes.size(); ++i) {
-    browse_gethash_cache_[prefixes[i]] = SBCachedFullHashResult(expire_after);
+    prefix_gethash_cache_[prefixes[i]] = SBCachedFullHashResult(expire_after);
   }
 
   // Insert any fullhash hits. Note that there may be one, multiple, or no
   // fullhashes for any given entry in |prefixes|.
   for (size_t i = 0; i < full_hits.size(); ++i) {
     const SBPrefix prefix = full_hits[i].hash.prefix;
-    browse_gethash_cache_[prefix].full_hashes.push_back(full_hits[i]);
+    prefix_gethash_cache_[prefix].full_hashes.push_back(full_hits[i]);
   }
 }
 
 bool SafeBrowsingDatabaseNew::UpdateStarted(
     std::vector<SBListChunkRanges>* lists) {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
   DCHECK(lists);
 
   // If |BeginUpdate()| fails, reset the database.
   if (!browse_store_->BeginUpdate()) {
@@ skipping 34 matching lines @@
     HandleCorruptDatabase();
     return false;
   }
 
   if (ip_blacklist_store_ && !ip_blacklist_store_->BeginUpdate()) {
     RecordFailure(FAILURE_IP_BLACKLIST_UPDATE_BEGIN);
     HandleCorruptDatabase();
     return false;
   }
 
+  if (unwanted_software_store_ && !unwanted_software_store_->BeginUpdate()) {
+    RecordFailure(FAILURE_UNWANTED_SOFTWARE_DATABASE_UPDATE_BEGIN);
+    HandleCorruptDatabase();
+    return false;
+  }
+
   {
     base::AutoLock locked(lookup_lock_);
     // Cached fullhash results must be cleared on every database update (whether
     // successful or not.)
-    browse_gethash_cache_.clear();
+    prefix_gethash_cache_.clear();
   }
 
-  UpdateChunkRangesForLists(browse_store_.get(),
-                            safe_browsing_util::kMalwareList,
-                            safe_browsing_util::kPhishingList,
-                            lists);
+  UpdateChunkRangesForBrowseLists(browse_store_.get(),
+                                  safe_browsing_util::kMalwareList,
+                                  safe_browsing_util::kPhishingList,
+                                  lists);
 
   // NOTE(shess): |download_store_| used to contain kBinHashList, which has been
   // deprecated.  Code to delete the list from the store shows ~15k hits/day as
   // of Feb 2014, so it has been removed.  Everything _should_ be resilient to
   // extra data of that sort.
   UpdateChunkRangesForList(download_store_.get(),
                            safe_browsing_util::kBinUrlList, lists);
 
   UpdateChunkRangesForList(csd_whitelist_store_.get(),
                            safe_browsing_util::kCsdWhiteList, lists);
 
   UpdateChunkRangesForList(download_whitelist_store_.get(),
                            safe_browsing_util::kDownloadWhiteList, lists);
 
   UpdateChunkRangesForList(extension_blacklist_store_.get(),
                            safe_browsing_util::kExtensionBlacklist, lists);
 
   UpdateChunkRangesForList(side_effect_free_whitelist_store_.get(),
                            safe_browsing_util::kSideEffectFreeWhitelist, lists);
 
   UpdateChunkRangesForList(ip_blacklist_store_.get(),
                            safe_browsing_util::kIPBlacklist, lists);
 
+  UpdateChunkRangesForList(unwanted_software_store_.get(),
+                           safe_browsing_util::kUnwantedUrlList, lists);
+
   corruption_detected_ = false;
   change_detected_ = false;
   return true;
 }
 
 void SafeBrowsingDatabaseNew::UpdateFinished(bool update_succeeded) {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
 
   // The update may have failed due to corrupt storage (for instance,
   // an excessive number of invalid add_chunks and sub_chunks).
@@ skipping 23 matching lines @@
 
     if (side_effect_free_whitelist_store_ &&
         !side_effect_free_whitelist_store_->CheckValidity()) {
       DLOG(ERROR) << "Safe-browsing side-effect free whitelist database "
                   << "corrupt.";
     }
 
     if (ip_blacklist_store_ && !ip_blacklist_store_->CheckValidity()) {
       DLOG(ERROR) << "Safe-browsing IP blacklist database corrupt.";
     }
+
+    if (unwanted_software_store_ &&
+        !unwanted_software_store_->CheckValidity()) {
+      DLOG(ERROR) << "Unwanted software url list database corrupt.";
+    }
   }
 
   if (corruption_detected_)
     return;
 
   // Unroll the transaction if there was a protocol error or if the
   // transaction was empty.  This will leave the prefix set, the
   // pending hashes, and the prefix miss cache in place.
   if (!update_succeeded || !change_detected_) {
     // Track empty updates to answer questions at http://crbug.com/72216 .
     if (update_succeeded && !change_detected_)
       UMA_HISTOGRAM_COUNTS("SB2.DatabaseUpdateKilobytes", 0);
     browse_store_->CancelUpdate();
     if (download_store_.get())
       download_store_->CancelUpdate();
     if (csd_whitelist_store_.get())
       csd_whitelist_store_->CancelUpdate();
     if (download_whitelist_store_.get())
       download_whitelist_store_->CancelUpdate();
     if (extension_blacklist_store_)
       extension_blacklist_store_->CancelUpdate();
     if (side_effect_free_whitelist_store_)
       side_effect_free_whitelist_store_->CancelUpdate();
     if (ip_blacklist_store_)
       ip_blacklist_store_->CancelUpdate();
+    if (unwanted_software_store_)
+      unwanted_software_store_->CancelUpdate();
     return;
   }
 
   if (download_store_) {
     int64 size_bytes = UpdateHashPrefixStore(
         DownloadDBFilename(filename_base_),
         download_store_.get(),
         FAILURE_DOWNLOAD_DATABASE_UPDATE_FINISH);
     UMA_HISTOGRAM_COUNTS("SB2.DownloadDatabaseKilobytes",
                          static_cast<int>(size_bytes / 1024));
   }
 
-  UpdateBrowseStore();
+  UpdatePrefixSetUrlStore(BrowseDBFilename(filename_base_),
+                          browse_store_.get(),
+                          &browse_prefix_set_,
+                          FAILURE_BROWSE_DATABASE_UPDATE_FINISH,
+                          FAILURE_BROWSE_PREFIX_SET_WRITE);
+
   UpdateWhitelistStore(CsdWhitelistDBFilename(filename_base_),
                        csd_whitelist_store_.get(),
                        &csd_whitelist_);
   UpdateWhitelistStore(DownloadWhitelistDBFilename(filename_base_),
                        download_whitelist_store_.get(),
                        &download_whitelist_);
 
   if (extension_blacklist_store_) {
     int64 size_bytes = UpdateHashPrefixStore(
         ExtensionBlacklistDBFilename(filename_base_),
         extension_blacklist_store_.get(),
         FAILURE_EXTENSION_BLACKLIST_UPDATE_FINISH);
     UMA_HISTOGRAM_COUNTS("SB2.ExtensionBlacklistKilobytes",
                          static_cast<int>(size_bytes / 1024));
   }
 
   if (side_effect_free_whitelist_store_)
     UpdateSideEffectFreeWhitelistStore();
 
   if (ip_blacklist_store_)
     UpdateIpBlacklistStore();
+
+  if (unwanted_software_store_) {
+    UpdatePrefixSetUrlStore(UnwantedSoftwareDBFilename(filename_base_),
+                            unwanted_software_store_.get(),
+                            &unwanted_software_prefix_set_,
+                            FAILURE_UNWANTED_SOFTWARE_DATABASE_UPDATE_FINISH,
+                            FAILURE_UNWANTED_SOFTWARE_PREFIX_SET_WRITE);
+  }
 }
 
 void SafeBrowsingDatabaseNew::UpdateWhitelistStore(
     const base::FilePath& store_filename,
     SafeBrowsingStore* store,
     SBWhitelist* whitelist) {
   if (!store)
     return;
 
   // Note: |builder| will not be empty.  The current data store implementation
@@ skipping 25 matching lines @@
   if (!store->FinishUpdate(&builder, &add_full_hashes_result))
     RecordFailure(failure_type);
 
 #if defined(OS_MACOSX)
   base::mac::SetFileBackupExclusion(store_filename);
 #endif
 
   return GetFileSizeOrZero(store_filename);
 }
 
-void SafeBrowsingDatabaseNew::UpdateBrowseStore() {
+void SafeBrowsingDatabaseNew::UpdatePrefixSetUrlStore(
+    const base::FilePath& db_filename,
+    SafeBrowsingStore* url_store,
+    scoped_ptr<safe_browsing::PrefixSet>* prefix_set,
+    FailureType finish_failure_type,
+    FailureType write_failure_type) {
   // Measure the amount of IO during the filter build.
   base::IoCounters io_before, io_after;
   base::ProcessHandle handle = base::GetCurrentProcessHandle();
   scoped_ptr<base::ProcessMetrics> metric(
 #if !defined(OS_MACOSX)
       base::ProcessMetrics::CreateProcessMetrics(handle)
 #else
       // Getting stats only for the current process is enough, so NULL is fine.
       base::ProcessMetrics::CreateProcessMetrics(handle, NULL)
 #endif
   );
 
   // IoCounters are currently not supported on Mac, and may not be
   // available for Linux, so we check the result and only show IO
   // stats if they are available.
   const bool got_counters = metric->GetIOCounters(&io_before);
 
   const base::TimeTicks before = base::TimeTicks::Now();
 
   // TODO(shess): Perhaps refactor to let builder accumulate full hashes on the
   // fly?  Other clients use the SBAddFullHash vector, but AFAICT they only use
   // the SBFullHash portion.  It would need an accessor on PrefixSet.
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes;
-  if (!browse_store_->FinishUpdate(&builder, &add_full_hashes)) {
-    RecordFailure(FAILURE_BROWSE_DATABASE_UPDATE_FINISH);
+  if (!url_store->FinishUpdate(&builder, &add_full_hashes)) {
+    RecordFailure(finish_failure_type);
     return;
   }
 
   std::vector<SBFullHash> full_hash_results;
   for (size_t i = 0; i < add_full_hashes.size(); ++i) {
     full_hash_results.push_back(add_full_hashes[i].full_hash);
   }
 
-  scoped_ptr<safe_browsing::PrefixSet>
-      prefix_set(builder.GetPrefixSet(full_hash_results));
+  scoped_ptr<safe_browsing::PrefixSet> new_prefix_set(
+      builder.GetPrefixSet(full_hash_results));
 
   // Swap in the newly built filter.
   {
     base::AutoLock locked(lookup_lock_);
-    browse_prefix_set_.swap(prefix_set);
+    prefix_set->swap(new_prefix_set);
   }
 
   UMA_HISTOGRAM_LONG_TIMES("SB2.BuildFilter", base::TimeTicks::Now() - before);
 
   // Persist the prefix set to disk.  Since only this thread changes
-  // |browse_prefix_set_|, there is no need to lock.
-  WritePrefixSet();
+  // |prefix_set|, there is no need to lock.

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

Comment is (even more) confusing now that prefix_set is a local variable.
I guess it should at least say |*prefix_set|.

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

Agreed, re-worded comment.

I also think we can make this code better by using scoped_ptr<const PrefixSet>
instead of scoped_ptr<PrefixSet> to store PrefixSets to have stronger guarantees
that nobody else is touching them, rather than assuming everybody we hand them
to plays by the rules...

I'll follow up with a CL for this.

[mattm, 2014/11/12 03:24:30]

Sounds reasonable, though I don't think it completely guarantees it (would still
depend on no other thread changing the scoped_ptr itself).

+  WritePrefixSet(db_filename, prefix_set->get(), write_failure_type);
 
   // Gather statistics.
   if (got_counters && metric->GetIOCounters(&io_after)) {
     UMA_HISTOGRAM_COUNTS("SB2.BuildReadKilobytes",
                          static_cast<int>(io_after.ReadTransferCount -
                                           io_before.ReadTransferCount) / 1024);
     UMA_HISTOGRAM_COUNTS("SB2.BuildWriteKilobytes",
                          static_cast<int>(io_after.WriteTransferCount -
                                           io_before.WriteTransferCount) / 1024);
     UMA_HISTOGRAM_COUNTS("SB2.BuildReadOperations",
                          static_cast<int>(io_after.ReadOperationCount -
                                           io_before.ReadOperationCount));
     UMA_HISTOGRAM_COUNTS("SB2.BuildWriteOperations",
                          static_cast<int>(io_after.WriteOperationCount -
                                           io_before.WriteOperationCount));
   }
 
-  const base::FilePath browse_filename = BrowseDBFilename(filename_base_);
-  const int64 file_size = GetFileSizeOrZero(browse_filename);
+  const int64 file_size = GetFileSizeOrZero(db_filename);
   UMA_HISTOGRAM_COUNTS("SB2.BrowseDatabaseKilobytes",
                        static_cast<int>(file_size / 1024));
 
 #if defined(OS_MACOSX)
-  base::mac::SetFileBackupExclusion(browse_filename);
+  base::mac::SetFileBackupExclusion(db_filename);
 #endif
 }
 
 void SafeBrowsingDatabaseNew::UpdateSideEffectFreeWhitelistStore() {
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes_result;
 
   if (!side_effect_free_whitelist_store_->FinishUpdate(
           &builder, &add_full_hashes_result)) {
     RecordFailure(FAILURE_SIDE_EFFECT_FREE_WHITELIST_UPDATE_FINISH);
@@ skipping 73 matching lines @@
 
   // NOTE(shess): ResetDatabase() should remove the corruption, so this should
   // only happen once.  If you are here because you are hitting this after a
   // restart, then I would be very interested in working with you to figure out
   // what is happening, since it may affect real users.
   DLOG(FATAL) << "SafeBrowsing database was corrupt and reset";
 }
 
 // TODO(shess): I'm not clear why this code doesn't have any
 // real error-handling.
-void SafeBrowsingDatabaseNew::LoadPrefixSet() {
+void SafeBrowsingDatabaseNew::LoadPrefixSet(
+    const base::FilePath& db_filename,
+    scoped_ptr<safe_browsing::PrefixSet>* prefix_set,
+    FailureType read_failure_type) {
+  if (!prefix_set)
+    return;
+
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
   DCHECK(!filename_base_.empty());
 
-  const base::FilePath browse_filename = BrowseDBFilename(filename_base_);
-  const base::FilePath browse_prefix_set_filename =
-      PrefixSetForFilename(browse_filename);
+  const base::FilePath prefix_set_filename = PrefixSetForFilename(db_filename);
 
   // Only use the prefix set if database is present and non-empty.
-  if (!GetFileSizeOrZero(browse_filename))
+  if (!GetFileSizeOrZero(db_filename))
     return;
 
   // Cleanup any stale bloom filter (no longer used).
   // TODO(shess): Track existence to drive removal of this code?
   const base::FilePath bloom_filter_filename =
-      BloomFilterForFilename(browse_filename);
+      BloomFilterForFilename(db_filename);
   base::DeleteFile(bloom_filter_filename, false);
 
   const base::TimeTicks before = base::TimeTicks::Now();
-  browse_prefix_set_ = safe_browsing::PrefixSet::LoadFile(
-      browse_prefix_set_filename);
+  *prefix_set = safe_browsing::PrefixSet::LoadFile(prefix_set_filename);
   UMA_HISTOGRAM_TIMES("SB2.PrefixSetLoad", base::TimeTicks::Now() - before);
 
-  if (!browse_prefix_set_.get())
-    RecordFailure(FAILURE_BROWSE_PREFIX_SET_READ);
+  if (!prefix_set->get())
+    RecordFailure(read_failure_type);
 }
 
 bool SafeBrowsingDatabaseNew::Delete() {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
   DCHECK(!filename_base_.empty());
 
   // TODO(shess): This is a mess.  SafeBrowsingFileStore::Delete() closes the
   // store before calling DeleteStore().  DeleteStore() deletes transient files
   // in addition to the main file.  Probably all of these should be converted to
   // a helper which calls Delete() if the store exists, else DeleteStore() on
@@ skipping 54 matching lines @@
       side_effect_free_whitelist_prefix_set_filename,
       false);
   if (!r9)
     RecordFailure(FAILURE_SIDE_EFFECT_FREE_WHITELIST_PREFIX_SET_DELETE);
 
   const bool r10 = base::DeleteFile(IpBlacklistDBFilename(filename_base_),
                                     false);
   if (!r10)
     RecordFailure(FAILURE_IP_BLACKLIST_DELETE);
 
-  return r1 && r2 && r3 && r4 && r5 && r6 && r7 && r8 && r9 && r10;
+  const bool r11 =
+      base::DeleteFile(UnwantedSoftwareDBFilename(filename_base_), false);
+  if (!r11)
+    RecordFailure(FAILURE_UNWANTED_SOFTWARE_PREFIX_SET_DELETE);
+
+  return r1 && r2 && r3 && r4 && r5 && r6 && r7 && r8 && r9 && r10 && r11;
 }
 
-void SafeBrowsingDatabaseNew::WritePrefixSet() {
+void SafeBrowsingDatabaseNew::WritePrefixSet(
+    const base::FilePath& db_filename,
+    safe_browsing::PrefixSet* prefix_set,
+    FailureType write_failure_type) {
   DCHECK_EQ(creation_loop_, base::MessageLoop::current());
 
-  if (!browse_prefix_set_.get())
+  if (!prefix_set)
     return;
 
-  const base::FilePath browse_filename = BrowseDBFilename(filename_base_);
-  const base::FilePath browse_prefix_set_filename =
-      PrefixSetForFilename(browse_filename);
+  const base::FilePath prefix_set_filename = PrefixSetForFilename(db_filename);
 
   const base::TimeTicks before = base::TimeTicks::Now();
-  const bool write_ok = browse_prefix_set_->WriteFile(
-      browse_prefix_set_filename);
+  const bool write_ok = prefix_set->WriteFile(prefix_set_filename);
   UMA_HISTOGRAM_TIMES("SB2.PrefixSetWrite", base::TimeTicks::Now() - before);
 
-  const int64 file_size = GetFileSizeOrZero(browse_prefix_set_filename);
+  const int64 file_size = GetFileSizeOrZero(prefix_set_filename);
   UMA_HISTOGRAM_COUNTS("SB2.PrefixSetKilobytes",
                        static_cast<int>(file_size / 1024));
 
   if (!write_ok)
-    RecordFailure(FAILURE_BROWSE_PREFIX_SET_WRITE);
+    RecordFailure(write_failure_type);
 
 #if defined(OS_MACOSX)
-  base::mac::SetFileBackupExclusion(browse_prefix_set_filename);
+  base::mac::SetFileBackupExclusion(prefix_set_filename);
 #endif
 }
 
 void SafeBrowsingDatabaseNew::WhitelistEverything(SBWhitelist* whitelist) {
   base::AutoLock locked(lookup_lock_);
   whitelist->second = true;
   whitelist->first.clear();
 }
 
 void SafeBrowsingDatabaseNew::LoadWhitelist(
@@ skipping 68 matching lines @@
 bool SafeBrowsingDatabaseNew::IsMalwareIPMatchKillSwitchOn() {
   SBFullHash malware_kill_switch = SBFullHashForString(kMalwareIPKillSwitchUrl);
   std::vector<SBFullHash> full_hashes;
   full_hashes.push_back(malware_kill_switch);
   return ContainsWhitelistedHashes(csd_whitelist_, full_hashes);
 }
 
 bool SafeBrowsingDatabaseNew::IsCsdWhitelistKillSwitchOn() {
   return csd_whitelist_.second;
 }