Transition safe browsing from bloom filter to prefix set.

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 13 matching lines @@
 #if defined(OS_MACOSX)
 #include "base/mac/mac_util.h"
 #endif
 
 using content::BrowserThread;
 
 namespace {
 
 // Filename suffix for the bloom filter.
 const FilePath::CharType kBloomFilterFile[] = FILE_PATH_LITERAL(" Filter 2");
+// Filename suffix for the prefix set.
+const FilePath::CharType kPrefixSetFile[] = FILE_PATH_LITERAL(" Prefix Set");
 // Filename suffix for download store.
 const FilePath::CharType kDownloadDBFile[] = FILE_PATH_LITERAL(" Download");
 // Filename suffix for client-side phishing detection whitelist store.
 const FilePath::CharType kCsdWhitelistDBFile[] =
     FILE_PATH_LITERAL(" Csd Whitelist");
 // Filename suffix for the download whitelist store.
 const FilePath::CharType kDownloadWhitelistDBFile[] =
     FILE_PATH_LITERAL(" Download Whitelist");
 // Filename suffix for browse store.
-// TODO(lzheng): change to a better name when we change the file format.
+// 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.
 const FilePath::CharType kBrowseDBFile[] = FILE_PATH_LITERAL(" Bloom");
 
 // The maximum staleness for a cached entry.
 const int kMaxStalenessMinutes = 45;
 
 // Maximum number of entries we allow in any of the whitelists.
 // If a whitelist on disk contains more entries then all lookups to
 // the whitelist will be considered a match.
 const size_t kMaxWhitelistSize = 5000;
 
@@ skipping 197 matching lines @@
   if (subs_deleted > 0)
     UMA_HISTOGRAM_COUNTS("SB2.DownloadBinhashSubsDeleted", subs_deleted);
 }
 
 // Order |SBAddFullHash| on the prefix part.  |SBAddPrefixLess()| from
 // safe_browsing_store.h orders on both chunk-id and prefix.
 bool SBAddFullHashPrefixLess(const SBAddFullHash& a, const SBAddFullHash& b) {
   return a.full_hash.prefix < b.full_hash.prefix;
 }
 
-// Helper to reduce code duplication.
-safe_browsing::PrefixSet* CreateEmptyPrefixSet() {
-  return new safe_browsing::PrefixSet(std::vector<SBPrefix>());
+// Track what LoadBloomFilterOrPrefixSet() loaded.
+enum FilterLoad {
+  FILTER_LOAD,                 // All calls.
+  FILTER_LOADED_PREFIX_SET,    // Cases loaded from prefix set.
+  FILTER_LOADED_BLOOM_FILTER,  // Cases loaded from bloom filter.
+
+  // Memory space for histograms is determined by the max.  ALWAYS ADD
+  // NEW VALUES BEFORE THIS ONE.
+  FILTER_LOAD_MAX
+};
+
+void RecordFilterLoad(FilterLoad event_type) {
+  UMA_HISTOGRAM_ENUMERATION("SB2.FilterLoad", event_type,
+                            FILTER_LOAD_MAX);
 }
 
-// Generate |PrefixSet| and |BloomFilter| instances from the contents
-// of |add_prefixes|.
-void FiltersFromAddPrefixes(
-    const SBAddPrefixes& add_prefixes,
-    scoped_refptr<BloomFilter>* bloom_filter,
-    scoped_ptr<safe_browsing::PrefixSet>* prefix_set) {
-  const int filter_size =
-      BloomFilter::FilterSizeForKeyCount(add_prefixes.size());
-  *bloom_filter = new BloomFilter(filter_size);
-  if (add_prefixes.empty()) {
-    prefix_set->reset(CreateEmptyPrefixSet());
-    return;
-  }
-
-  // TODO(shess): If |add_prefixes| were sorted by the prefix, it
-  // could be passed directly to |PrefixSet()|, removing the need for
-  // |prefixes|.
-  std::vector<SBPrefix> prefixes;
-  prefixes.reserve(add_prefixes.size());
-  for (SBAddPrefixes::const_iterator iter = add_prefixes.begin();
-       iter != add_prefixes.end(); ++iter) {
-    prefixes.push_back(iter->prefix);
-  }
-  std::sort(prefixes.begin(), prefixes.end());
-
-  for (std::vector<SBPrefix>::const_iterator iter = prefixes.begin();
-       iter != prefixes.end(); ++iter) {
-    bloom_filter->get()->Insert(*iter);
-  }
-
-  prefix_set->reset(new safe_browsing::PrefixSet(prefixes));
+// This code always checks for non-zero file size.  This helper makes
+// that less verbose.
+int64 GetFileSizeOrZero(const FilePath& file_path) {
+  int64 size_64;
+  if (!file_util::GetFileSize(file_path, &size_64))
+    return 0;
+  return size_64;
 }
 
 }  // namespace
 
 // The default SafeBrowsingDatabaseFactory.
 class SafeBrowsingDatabaseFactoryImpl : public SafeBrowsingDatabaseFactory {
  public:
   virtual SafeBrowsingDatabase* CreateSafeBrowsingDatabase(
       bool enable_download_protection,
       bool enable_client_side_whitelist,
@@ skipping 45 matching lines @@
   return FilePath(db_base_filename.value() + kDownloadDBFile);
 }
 
 // static
 FilePath SafeBrowsingDatabase::BloomFilterForFilename(
     const FilePath& db_filename) {
   return FilePath(db_filename.value() + kBloomFilterFile);
 }
 
 // static
+FilePath SafeBrowsingDatabase::PrefixSetForFilename(
+    const FilePath& db_filename) {
+  return FilePath(db_filename.value() + kPrefixSetFile);
+}
+
+// static
 FilePath SafeBrowsingDatabase::CsdWhitelistDBFilename(
     const FilePath& db_filename) {
   return FilePath(db_filename.value() + kCsdWhitelistDBFile);
 }
 
 // static
 FilePath SafeBrowsingDatabase::DownloadWhitelistDBFilename(
     const FilePath& db_filename) {
   return FilePath(db_filename.value() + kDownloadWhitelistDBFile);
 }
@@ skipping 54 matching lines @@
 void SafeBrowsingDatabaseNew::Init(const FilePath& filename_base) {
   DCHECK_EQ(creation_loop_, MessageLoop::current());
   // Ensure we haven't been run before.
   DCHECK(browse_filename_.empty());
   DCHECK(download_filename_.empty());
   DCHECK(csd_whitelist_filename_.empty());
   DCHECK(download_whitelist_filename_.empty());
 
   browse_filename_ = BrowseDBFilename(filename_base);
   bloom_filter_filename_ = BloomFilterForFilename(browse_filename_);
+  prefix_set_filename_ = PrefixSetForFilename(browse_filename_);
 
   browse_store_->Init(
       browse_filename_,
       base::Bind(&SafeBrowsingDatabaseNew::HandleCorruptDatabase,
                  base::Unretained(this)));
   DVLOG(1) << "Init browse store: " << browse_filename_.value();
 
   {
     // 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_);
     full_browse_hashes_.clear();
     pending_browse_hashes_.clear();
-    LoadBloomFilter();
+    LoadBloomFilterOrPrefixSet();
   }
 
   if (download_store_.get()) {
     download_filename_ = DownloadDBFilename(filename_base);
     download_store_->Init(
         download_filename_,
         base::Bind(&SafeBrowsingDatabaseNew::HandleCorruptDatabase,
                    base::Unretained(this)));
     DVLOG(1) << "Init download store: " << download_filename_.value();
   }
@@ skipping 42 matching lines @@
   // reset.  Perhaps inline |Delete()|?
   if (!Delete())
     return false;
 
   // Reset objects in memory.
   {
     base::AutoLock locked(lookup_lock_);
     full_browse_hashes_.clear();
     pending_browse_hashes_.clear();
     prefix_miss_cache_.clear();
-    // TODO(shess): This could probably be |bloom_filter_.reset()|.
-    browse_bloom_filter_ = new BloomFilter(BloomFilter::kBloomFilterMinSize *
-                                           BloomFilter::kBloomFilterSizeRatio);
-    // TODO(shess): It is simpler for the code to assume that presence
-    // of a bloom filter always implies presence of a prefix set.
-    prefix_set_.reset(CreateEmptyPrefixSet());
+    browse_bloom_filter_ = NULL;
+    prefix_set_.reset();
   }
   // Wants to acquire the lock itself.
   WhitelistEverything(&csd_whitelist_);
   WhitelistEverything(&download_whitelist_);
 
   return true;
 }
 
 // TODO(lzheng): Remove matching_list, it is not used anywhere.
 bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
     const GURL& url,
     std::string* matching_list,
     std::vector<SBPrefix>* prefix_hits,
     std::vector<SBFullHashResult>* full_hits,
     base::Time last_update) {
   // Clear the results first.
   matching_list->clear();
   prefix_hits->clear();
   full_hits->clear();
 
   std::vector<SBFullHash> full_hashes;
   BrowseFullHashesToCheck(url, false, &full_hashes);
   if (full_hashes.empty())
     return false;
 
   // This function is called on the I/O thread, prevent changes to
-  // bloom filter and caches.
+  // filter and caches.
   base::AutoLock locked(lookup_lock_);
 
-  if (!browse_bloom_filter_.get())
+  // TODO(shess): During transition, users will have a bloom filter
+  // but no prefix set until first update, after which they'll have a
+  // prefix set but no bloom filter.
+  const bool use_prefix_set = prefix_set_.get() != NULL;
+  if (!use_prefix_set && !browse_bloom_filter_.get())
     return false;
 
   size_t miss_count = 0;
   for (size_t i = 0; i < full_hashes.size(); ++i) {
-    if (browse_bloom_filter_->Exists(full_hashes[i].prefix)) {
-      prefix_hits->push_back(full_hashes[i].prefix);
-      if (prefix_miss_cache_.count(full_hashes[i].prefix) > 0)
+    const SBPrefix prefix = full_hashes[i].prefix;
+    if ((use_prefix_set && prefix_set_->Exists(prefix)) ||
+        (!use_prefix_set && browse_bloom_filter_->Exists(prefix))) {
+      prefix_hits->push_back(prefix);
+      if (prefix_miss_cache_.count(prefix) > 0)
         ++miss_count;
     }
   }
 
   // If all the prefixes are cached as 'misses', don't issue a GetHash.
   if (miss_count == prefix_hits->size())
     return false;
 
   // Find the matching full-hash results.  |full_browse_hashes_| are from the
   // database, |pending_browse_hashes_| are from GetHash requests between
@@ skipping 239 matching lines @@
   }
 }
 
 void SafeBrowsingDatabaseNew::InsertChunks(const std::string& list_name,
                                            const SBChunkList& chunks) {
   DCHECK_EQ(creation_loop_, MessageLoop::current());
 
   if (corruption_detected_ || chunks.empty())
     return;
 
-  const base::Time insert_start = base::Time::Now();
+  const base::TimeTicks before = base::TimeTicks::Now();
 
   const int list_id = safe_browsing_util::GetListId(list_name);
   DVLOG(2) << list_name << ": " << list_id;
 
   SafeBrowsingStore* store = GetStore(list_id);
   if (!store) return;
 
   change_detected_ = true;
 
   store->BeginChunk();
   if (chunks.front().is_add) {
     InsertAddChunks(list_id, chunks);
   } else {
     InsertSubChunks(list_id, chunks);
   }
   store->FinishChunk();
 
-  UMA_HISTOGRAM_TIMES("SB2.ChunkInsert", base::Time::Now() - insert_start);
+  UMA_HISTOGRAM_TIMES("SB2.ChunkInsert", base::TimeTicks::Now() - before);
 }
 
 void SafeBrowsingDatabaseNew::DeleteChunks(
     const std::vector<SBChunkDelete>& chunk_deletes) {
   DCHECK_EQ(creation_loop_, MessageLoop::current());
 
   if (corruption_detected_ || chunk_deletes.empty())
     return;
 
   const std::string& list_name = chunk_deletes.front().list_name;
@@ skipping 227 matching lines @@
 }
 
 void SafeBrowsingDatabaseNew::UpdateDownloadStore() {
   if (!download_store_.get())
     return;
 
   // For download, we don't cache and save full hashes.
   std::vector<SBAddFullHash> empty_add_hashes;
 
   // For download, backend lookup happens only if a prefix is in add list.
-  // No need to pass in miss cache when call FinishUpdate to caculate
-  // bloomfilter false positives.
   std::set<SBPrefix> empty_miss_cache;
 
   // These results are not used after this call. Simply ignore the
   // returned value after FinishUpdate(...).
   SBAddPrefixes add_prefixes_result;
   std::vector<SBAddFullHash> add_full_hashes_result;
 
   if (!download_store_->FinishUpdate(empty_add_hashes,
                                      empty_miss_cache,
                                      &add_prefixes_result,
                                      &add_full_hashes_result))
     RecordFailure(FAILURE_DOWNLOAD_DATABASE_UPDATE_FINISH);
 
-  int64 size_64;
-  if (file_util::GetFileSize(download_filename_, &size_64)) {
-    UMA_HISTOGRAM_COUNTS("SB2.DownloadDatabaseKilobytes",
-                         static_cast<int>(size_64 / 1024));
-  }
+  int64 file_size = GetFileSizeOrZero(download_filename_);
+  UMA_HISTOGRAM_COUNTS("SB2.DownloadDatabaseKilobytes",
+                       static_cast<int>(file_size / 1024));
 
 #if defined(OS_MACOSX)
   base::mac::SetFileBackupExclusion(download_filename_);
 #endif
 }
 
 void SafeBrowsingDatabaseNew::UpdateBrowseStore() {
   // Copy out the pending add hashes.  Copy rather than swapping in
   // case |ContainsBrowseURL()| is called before the new filter is complete.
   std::vector<SBAddFullHash> pending_add_hashes;
   {
     base::AutoLock locked(lookup_lock_);
     pending_add_hashes.insert(pending_add_hashes.end(),
                               pending_browse_hashes_.begin(),
                               pending_browse_hashes_.end());
   }
 
-  // Measure the amount of IO during the bloom filter build.
+  // Measure the amount of IO during the filter build.
   base::IoCounters io_before, io_after;
   base::ProcessHandle handle = base::Process::Current().handle();
   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::Time before = base::Time::Now();
+  const base::TimeTicks before = base::TimeTicks::Now();
 
   SBAddPrefixes add_prefixes;
   std::vector<SBAddFullHash> add_full_hashes;
   if (!browse_store_->FinishUpdate(pending_add_hashes, prefix_miss_cache_,
                                    &add_prefixes, &add_full_hashes)) {
     RecordFailure(FAILURE_BROWSE_DATABASE_UPDATE_FINISH);
     return;
   }
 
-  scoped_refptr<BloomFilter> bloom_filter;
-  scoped_ptr<safe_browsing::PrefixSet> prefix_set;
-  FiltersFromAddPrefixes(add_prefixes, &bloom_filter, &prefix_set);
+  // TODO(shess): If |add_prefixes| were sorted by the prefix, it
+  // could be passed directly to |PrefixSet()|, removing the need for
+  // |prefixes|.  For now, |prefixes| is useful while debugging
+  // things.
+  std::vector<SBPrefix> prefixes;
+  prefixes.reserve(add_prefixes.size());
+  for (SBAddPrefixes::const_iterator iter = add_prefixes.begin();
+       iter != add_prefixes.end(); ++iter) {
+    prefixes.push_back(iter->prefix);
+  }
+
+  std::sort(prefixes.begin(), prefixes.end());
+  scoped_ptr<safe_browsing::PrefixSet>
+      prefix_set(new safe_browsing::PrefixSet(prefixes));
 
   // This needs to be in sorted order by prefix for efficient access.
   std::sort(add_full_hashes.begin(), add_full_hashes.end(),
             SBAddFullHashPrefixLess);
 
   // Swap in the newly built filter and cache.
   {
     base::AutoLock locked(lookup_lock_);
     full_browse_hashes_.swap(add_full_hashes);
 
     // TODO(shess): If |CacheHashResults()| is posted between the
     // earlier lock and this clear, those pending hashes will be lost.
     // It could be fixed by only removing hashes which were collected
     // at the earlier point.  I believe that is fail-safe as-is (the
     // hash will be fetched again).
     pending_browse_hashes_.clear();
     prefix_miss_cache_.clear();
-    browse_bloom_filter_.swap(bloom_filter);
+    browse_bloom_filter_ = NULL;  // Stop using the bloom filter.
     prefix_set_.swap(prefix_set);
   }
 
-  const base::TimeDelta bloom_gen = base::Time::Now() - before;
+  DVLOG(1) << "SafeBrowsingDatabaseImpl built prefix set in "
+           << (base::TimeTicks::Now() - before).InMilliseconds()
+           << " ms total.  prefix count: " << add_prefixes.size();
+  UMA_HISTOGRAM_LONG_TIMES("SB2.BuildFilter", base::TimeTicks::Now() - before);
 
-  // Persist the bloom filter to disk.  Since only this thread changes
-  // |browse_bloom_filter_|, there is no need to lock.
-  WriteBloomFilter();
+  // Persist the prefix set to disk.  Since only this thread changes
+  // |prefix_set_|, there is no need to lock.
+  WritePrefixSet();
 
   // 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));
   }
-  DVLOG(1) << "SafeBrowsingDatabaseImpl built bloom filter in "
-           << bloom_gen.InMilliseconds() << " ms total.  prefix count: "
-           << add_prefixes.size();
-  UMA_HISTOGRAM_LONG_TIMES("SB2.BuildFilter", bloom_gen);
-  UMA_HISTOGRAM_COUNTS("SB2.FilterKilobytes",
-                       browse_bloom_filter_->size() / 1024);
-  int64 size_64;
-  if (file_util::GetFileSize(browse_filename_, &size_64)) {
-    UMA_HISTOGRAM_COUNTS("SB2.BrowseDatabaseKilobytes",
-                         static_cast<int>(size_64 / 1024));
-  }
+
+  int64 file_size = GetFileSizeOrZero(prefix_set_filename_);
+  UMA_HISTOGRAM_COUNTS("SB2.PrefixSetKilobytes",
+                       static_cast<int>(file_size / 1024));
+  file_size = GetFileSizeOrZero(browse_filename_);
+  UMA_HISTOGRAM_COUNTS("SB2.BrowseDatabaseKilobytes",
+                       static_cast<int>(file_size / 1024));
 
 #if defined(OS_MACOSX)
   base::mac::SetFileBackupExclusion(browse_filename_);
 #endif
 }
 
 void SafeBrowsingDatabaseNew::HandleCorruptDatabase() {
   // Reset the database after the current task has unwound (but only
   // reset once within the scope of a given task).
   if (!reset_factory_.HasWeakPtrs()) {
     RecordFailure(FAILURE_DATABASE_CORRUPT);
     MessageLoop::current()->PostTask(FROM_HERE,
         base::Bind(&SafeBrowsingDatabaseNew::OnHandleCorruptDatabase,
                    reset_factory_.GetWeakPtr()));
   }
 }
 
 void SafeBrowsingDatabaseNew::OnHandleCorruptDatabase() {
   RecordFailure(FAILURE_DATABASE_CORRUPT_HANDLER);
   corruption_detected_ = true;  // Stop updating the database.
   ResetDatabase();
   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::LoadBloomFilter() {
+// TODO(shess): After a transition period, this can convert to just
+// giving up if the prefix set is not on disk.
+void SafeBrowsingDatabaseNew::LoadBloomFilterOrPrefixSet() {
   DCHECK_EQ(creation_loop_, MessageLoop::current());
   DCHECK(!bloom_filter_filename_.empty());
+  DCHECK(!prefix_set_filename_.empty());
 
-  // If we're missing either of the database or filter files, we wait until the
-  // next update to generate a new filter.
-  // TODO(paulg): Investigate how often the filter file is missing and how
-  // expensive it would be to regenerate it.
-  int64 size_64 = 0;
-  if (!file_util::GetFileSize(browse_filename_, &size_64) || size_64 == 0)
+  // If there is no database, the filter cannot be used.
+  base::PlatformFileInfo db_info;
+  if (!file_util::GetFileInfo(browse_filename_, &db_info) || db_info.size == 0)
     return;
 
-  if (!file_util::GetFileSize(bloom_filter_filename_, &size_64) ||
-      size_64 == 0) {
-    RecordFailure(FAILURE_DATABASE_FILTER_MISSING);
+  RecordFilterLoad(FILTER_LOAD);
+
+  // If there is no prefix set, or if the file is too old, check for a
+  // bloom filter.
+  // TODO(shess): The time check is in case this code gets reverted
+  // and re-landed.  It might be good to keep as a sanity check.
+  // Better would be to put the db's checksum in the filter file.
+  base::PlatformFileInfo prefix_set_info;
+  if (!file_util::GetFileInfo(prefix_set_filename_, &prefix_set_info) ||
+      prefix_set_info.size == 0 ||
+      prefix_set_info.last_modified < db_info.last_modified) {
+    // No prefix set.
+    prefix_set_.reset();
+
+    int64 file_size = GetFileSizeOrZero(bloom_filter_filename_);
+    if (!file_size) {
+      RecordFailure(FAILURE_DATABASE_FILTER_MISSING);
+      return;
+    }
+
+    const base::TimeTicks before = base::TimeTicks::Now();
+    browse_bloom_filter_ = BloomFilter::LoadFile(bloom_filter_filename_);
+    DVLOG(1) << "SafeBrowsingDatabaseNew read bloom filter in "
+             << (base::TimeTicks::Now() - before).InMilliseconds() << " ms";
+    UMA_HISTOGRAM_TIMES("SB2.BloomFilterLoad", base::TimeTicks::Now() - before);
+
+    if (!browse_bloom_filter_.get())
+      RecordFailure(FAILURE_DATABASE_FILTER_READ);
+    else
+      RecordFilterLoad(FILTER_LOADED_BLOOM_FILTER);
+
     return;
   }
 
+  // Once there is a prefix set stored, never use the bloom filter.
+  browse_bloom_filter_ = NULL;
+
+  // TODO(shess): The bloom filter file should have been deleted in
+  // WritePrefixSet(), unless this code is reverted and re-landed.
+  // Just paranoid.
+  file_util::Delete(bloom_filter_filename_, false);
+
   const base::TimeTicks before = base::TimeTicks::Now();
-  browse_bloom_filter_ = BloomFilter::LoadFile(bloom_filter_filename_);
-  DVLOG(1) << "SafeBrowsingDatabaseNew read bloom filter in "
+  prefix_set_.reset(safe_browsing::PrefixSet::LoadFile(prefix_set_filename_));
+  DVLOG(1) << "SafeBrowsingDatabaseNew read prefix set in "
            << (base::TimeTicks::Now() - before).InMilliseconds() << " ms";
+  UMA_HISTOGRAM_TIMES("SB2.PrefixSetLoad", base::TimeTicks::Now() - before);
 
-  if (!browse_bloom_filter_.get())
-    RecordFailure(FAILURE_DATABASE_FILTER_READ);
-
-  // Use an empty prefix set until the first update.
-  prefix_set_.reset(CreateEmptyPrefixSet());
+  if (!prefix_set_.get())
+    RecordFailure(FAILURE_DATABASE_PREFIX_SET_READ);
+  else
+    RecordFilterLoad(FILTER_LOADED_PREFIX_SET);
 }
 
 bool SafeBrowsingDatabaseNew::Delete() {
   DCHECK_EQ(creation_loop_, MessageLoop::current());
 
   const bool r1 = browse_store_->Delete();
   if (!r1)
     RecordFailure(FAILURE_DATABASE_STORE_DELETE);
 
   const bool r2 = download_store_.get() ? download_store_->Delete() : true;
   if (!r2)
     RecordFailure(FAILURE_DATABASE_STORE_DELETE);
 
   const bool r3 = csd_whitelist_store_.get() ?
       csd_whitelist_store_->Delete() : true;
   if (!r3)
     RecordFailure(FAILURE_DATABASE_STORE_DELETE);
 
   const bool r4 = download_whitelist_store_.get() ?
       download_whitelist_store_->Delete() : true;
   if (!r4)
     RecordFailure(FAILURE_DATABASE_STORE_DELETE);
 
   const bool r5 = file_util::Delete(bloom_filter_filename_, false);
   if (!r5)
     RecordFailure(FAILURE_DATABASE_FILTER_DELETE);
-  return r1 && r2 && r3 && r4 && r5;
+
+  const bool r6 = file_util::Delete(prefix_set_filename_, false);
+  if (!r6)
+    RecordFailure(FAILURE_DATABASE_PREFIX_SET_DELETE);
+  return r1 && r2 && r3 && r4 && r5 && r6;
 }
 
-void SafeBrowsingDatabaseNew::WriteBloomFilter() {
+void SafeBrowsingDatabaseNew::WritePrefixSet() {
   DCHECK_EQ(creation_loop_, MessageLoop::current());
 
-  if (!browse_bloom_filter_.get())
+  if (!prefix_set_.get())
     return;
 
   const base::TimeTicks before = base::TimeTicks::Now();
-  const bool write_ok = browse_bloom_filter_->WriteFile(bloom_filter_filename_);
-  DVLOG(1) << "SafeBrowsingDatabaseNew wrote bloom filter in "
+  const bool write_ok = prefix_set_->WriteFile(prefix_set_filename_);
+  DVLOG(1) << "SafeBrowsingDatabaseNew wrote prefix set in "
            << (base::TimeTicks::Now() - before).InMilliseconds() << " ms";
+  UMA_HISTOGRAM_TIMES("SB2.PrefixSetWrite", base::TimeTicks::Now() - before);
 
   if (!write_ok)
-    RecordFailure(FAILURE_DATABASE_FILTER_WRITE);
+    RecordFailure(FAILURE_DATABASE_PREFIX_SET_WRITE);
+
+  // Delete any stale bloom filter (checking before deleting is
+  // unlikely to be faster).
+  file_util::Delete(bloom_filter_filename_, false);
 
 #if defined(OS_MACOSX)
-  base::mac::SetFileBackupExclusion(bloom_filter_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 19 matching lines @@
   if (std::binary_search(new_whitelist.begin(), new_whitelist.end(),
                          kill_switch)) {
     // The kill switch is whitelisted hence we whitelist all URLs.
     WhitelistEverything(whitelist);
   } else {
     base::AutoLock locked(lookup_lock_);
     whitelist->second = false;
     whitelist->first.swap(new_whitelist);
   }
 }