Reduce memory consumption by keeping the SafeBrowsing

database closed when not processing updates.

Review URL: http://codereview.chromium.org/10643

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5381 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 182 insertions, 83 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_database.h b/chrome/browser/safe_browsing/safe_browsing_database.h
index e504054..da232d0 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database.h
@@ -90,7 +90,7 @@ class SafeBrowsingDatabase {
  protected:
   static std::wstring BloomFilterFilename(const std::wstring& db_filename);
 
-  // Load the bloom filter off disk.  Generates one if it can't find it.
+  // Load the bloom filter off disk, or generates one if it doesn't exist.
   virtual void LoadBloomFilter();
 
   // Deletes the on-disk bloom filter, i.e. because it's stale.
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
index d47b94f..77aff53 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
@@ -40,6 +40,10 @@ static const int kOnResumeHoldupMs = 5 * 60 * 1000;  // 5 minutes.
 // The maximum staleness for a cached entry.
 static const int kMaxStalenessMinutes = 45;
 
+// The bloom filter based file name suffix.
+static const wchar_t kBloomFilterFileSuffix[] = L" Bloom";
+
+
 // Implementation --------------------------------------------------------------
 
 SafeBrowsingDatabaseBloom::SafeBrowsingDatabaseBloom()
@@ -47,6 +51,7 @@ SafeBrowsingDatabaseBloom::SafeBrowsingDatabaseBloom()
       init_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(reset_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(resume_factory_(this)),
+      add_count_(0),
       did_resume_(false) {
 }
 
@@ -58,33 +63,12 @@ bool SafeBrowsingDatabaseBloom::Init(const std::wstring& filename,
                                      Callback0::Type* chunk_inserted_callback) {
   DCHECK(!init_ && filename_.empty());
 
-  filename_ = filename + L" Bloom";
-  if (!Open())
-    return false;
-
-  bool load_filter = false;
-  if (!DoesSqliteTableExist(db_, "add_prefix")) {
-    if (!CreateTables()) {
-      // Database could be corrupt, try starting from scratch.
-      if (!ResetDatabase())
-        return false;
-    }
-  } else if (!CheckCompatibleVersion()) {
-    if (!ResetDatabase())
-      return false;
-  } else {
-    load_filter = true;
-  }
-
-  add_count_ = GetAddPrefixCount();
+  filename_ = filename + kBloomFilterFileSuffix;
   bloom_filter_filename_ = BloomFilterFilename(filename_);
 
-  if (load_filter) {
-    LoadBloomFilter();
-  } else {
-    bloom_filter_ =
-        new BloomFilter(kBloomFilterMinSize * kBloomFilterSizeRatio);
-  }
+  hash_cache_.reset(new HashCache);
+
+  LoadBloomFilter();
 
   init_ = true;
   chunk_inserted_callback_.reset(chunk_inserted_callback);
@@ -92,9 +76,43 @@ bool SafeBrowsingDatabaseBloom::Init(const std::wstring& filename,
   return true;
 }
 
+void SafeBrowsingDatabaseBloom::LoadBloomFilter() {
+  DCHECK(!bloom_filter_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;
+  if (!file_util::GetFileSize(filename_, &size_64) || size_64 == 0)
+    return;
+
+  if (!file_util::GetFileSize(bloom_filter_filename_, &size_64) ||
+      size_64 == 0)
+    return;
+
+  // We have a bloom filter file, so use that as our filter.
+  int size = static_cast<int>(size_64);
+  char* data = new char[size];
+  CHECK(data);
+
+  Time before = Time::Now();
+  file_util::ReadFile(bloom_filter_filename_, data, size);
+  SB_DLOG(INFO) << "SafeBrowsingDatabase read bloom filter in "
+                << (Time::Now() - before).InMilliseconds() << " ms";
+
+  bloom_filter_ = new BloomFilter(data, size);
+}
+
 bool SafeBrowsingDatabaseBloom::Open() {
-  if (sqlite3_open(WideToUTF8(filename_).c_str(), &db_) != SQLITE_OK)
+  if (db_)
+    return true;
+
+  if (sqlite3_open(WideToUTF8(filename_).c_str(), &db_) != SQLITE_OK) {
+    sqlite3_close(db_);
+    db_ = NULL;
     return false;
+  }
 
   // Run the database in exclusive mode. Nobody else should be accessing the
   // database while we're running, and this will give somewhat improved perf.
@@ -102,7 +120,16 @@ bool SafeBrowsingDatabaseBloom::Open() {
 
   statement_cache_.reset(new SqliteStatementCache(db_));
 
-  hash_cache_.reset(new HashCache);
+  if (!DoesSqliteTableExist(db_, "add_prefix")) {
+    if (!CreateTables()) {
+      // Database could be corrupt, try starting from scratch.
+      if (!ResetDatabase())
+        return false;
+    }
+  } else if (!CheckCompatibleVersion()) {
+    if (!ResetDatabase())
+      return false;
+  }
 
   return true;
 }
@@ -220,10 +247,8 @@ bool SafeBrowsingDatabaseBloom::ResetDatabase() {
       new BloomFilter(kBloomFilterMinSize * kBloomFilterSizeRatio);
   file_util::Delete(bloom_filter_filename_, false);
 
-  if (!Open())
-    return false;
-
-  return CreateTables();
+  // TODO(paulg): Fix potential infinite recursion between Open and Reset.
+  return Open();
 }
 
 bool SafeBrowsingDatabaseBloom::CheckCompatibleVersion() {
@@ -383,10 +408,15 @@ void SafeBrowsingDatabaseBloom::InsertChunks(const std::string& list_name,
 
 bool SafeBrowsingDatabaseBloom::UpdateStarted() {
   DCHECK(insert_transaction_.get() == NULL);
+
+  if (!Open())
+    return false;
+
   insert_transaction_.reset(new SQLTransaction(db_));
   if (insert_transaction_->Begin() != SQLITE_OK) {
     DCHECK(false) << "Safe browsing database couldn't start transaction";
     insert_transaction_.reset();
+    Close();
     return false;
   }
   return true;
@@ -397,6 +427,7 @@ void SafeBrowsingDatabaseBloom::UpdateFinished(bool update_succeeded) {
     BuildBloomFilter();
 
   insert_transaction_.reset();
+  Close();
 
   // We won't need the chunk caches until the next update (which will read them
   // from the database), so free their memory as they may contain thousands of
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.h b/chrome/browser/safe_browsing/safe_browsing_database_bloom.h
index 7ecb2ab..3f3f1a2 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.h
@@ -198,6 +198,9 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase {
   // Flush in memory temporary caches.
   void ClearUpdateCaches();
 
+  // Load the bloom filter off disk.
+  virtual void LoadBloomFilter();
+
   // Encode the list id in the lower bit of the chunk.
   static inline int EncodeChunkId(int chunk, int list_id) {
     DCHECK(list_id == 0 || list_id == 1);
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
index 45161d6..b4ae5ea7 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
@@ -87,6 +87,13 @@ namespace {
     file_util::Delete(filename + L" Filter", false);
   }
 
+  void GetListsInfo(SafeBrowsingDatabase* database,
+                    std::vector<SBListChunkRanges>* lists) {
+    EXPECT_TRUE(database->UpdateStarted());
+    database->GetListsInfo(lists);
+    database->UpdateFinished(true);
+  }
+
 }  // namespace
 
 // Tests retrieving list name information.
@@ -134,8 +141,7 @@ TEST(SafeBrowsingDatabase, ListName) {
   database->UpdateFinished(true);
 
   std::vector<SBListChunkRanges> lists;
-  database->GetListsInfo(&lists);
-
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].adds, "1-3");
   EXPECT_TRUE(lists[0].subs.empty());
@@ -155,10 +161,12 @@ TEST(SafeBrowsingDatabase, ListName) {
   chunks->push_back(chunk);
 
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
-  database->GetListsInfo(&lists);
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].adds, "1-3");
   EXPECT_EQ(lists[0].subs, "7");
@@ -184,6 +192,7 @@ TEST(SafeBrowsingDatabase, ListName) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kPhishingList, chunks);
 
   // Insert some phishing sub chunks.
@@ -213,14 +222,16 @@ TEST(SafeBrowsingDatabase, ListName) {
   chunks->push_back(chunk);
   database->InsertChunks(safe_browsing_util::kPhishingList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
-  database->GetListsInfo(&lists);
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].adds, "1-3");
   EXPECT_EQ(lists[0].subs, "7");
   EXPECT_TRUE(lists[1].name == safe_browsing_util::kPhishingList);
   EXPECT_EQ(lists[1].adds, "47");
   EXPECT_EQ(lists[1].subs, "200-201");
+  lists.clear();
 
   TearDownTestDatabase(database);
 }
@@ -244,7 +255,9 @@ TEST(SafeBrowsingDatabase, Database) {
 
   std::deque<SBChunk>* chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
+  std::vector<SBListChunkRanges> lists;
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
 
   // Add another chunk with two different hostkeys.
@@ -285,13 +298,14 @@ TEST(SafeBrowsingDatabase, Database) {
   chunks->push_back(chunk);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   // Make sure they were added correctly.
-  std::vector<SBListChunkRanges> lists;
-  database->GetListsInfo(&lists);
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].adds, "1-3");
   EXPECT_TRUE(lists[0].subs.empty());
+  lists.clear();
 
   const Time now = Time::Now();
   std::vector<SBFullHashResult> full_hashes;
@@ -352,8 +366,10 @@ TEST(SafeBrowsingDatabase, Database) {
   chunks->push_back(chunk);
 
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   EXPECT_TRUE(database->ContainsUrl(GURL("http://www.evil.com/phishing.html"),
                                     &matching_list, &prefix_hits,
@@ -378,14 +394,17 @@ TEST(SafeBrowsingDatabase, Database) {
                                     &matching_list, &prefix_hits,
                                     &full_hashes, now));
 
-  database->GetListsInfo(&lists);
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].subs, "4");
+  lists.clear();
 
   // Test removing all the prefixes from an add chunk.
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   AddDelChunk(database, safe_browsing_util::kMalwareList, 2);
   database->UpdateFinished(true);
+  lists.clear();
 
   EXPECT_FALSE(database->ContainsUrl(GURL("http://www.evil.com/notevil2.html"),
                                      &matching_list, &prefix_hits,
@@ -399,10 +418,11 @@ TEST(SafeBrowsingDatabase, Database) {
                                      &matching_list, &prefix_hits,
                                      &full_hashes, now));
 
-  database->GetListsInfo(&lists);
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].adds, "1,3");
   EXPECT_EQ(lists[0].subs, "4");
+  lists.clear();
 
   // The adddel command exposed a bug in the transaction code where any
   // transaction after it would fail.  Add a dummy entry and remove it to
@@ -420,6 +440,7 @@ TEST(SafeBrowsingDatabase, Database) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
 
   // Now remove the dummy entry.  If there are any problems with the
@@ -429,11 +450,13 @@ TEST(SafeBrowsingDatabase, Database) {
   // Test the subdel command.
   SubDelChunk(database, safe_browsing_util::kMalwareList, 4);
   database->UpdateFinished(true);
+  lists.clear();
 
-  database->GetListsInfo(&lists);
+  GetListsInfo(database, &lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
   EXPECT_EQ(lists[0].adds, "1,3");
   EXPECT_EQ(lists[0].subs, "");
+  lists.clear();
 
   // Test a sub command coming in before the add.
   host.host = Sha256Prefix("www.notevilanymore.com/");
@@ -452,8 +475,10 @@ TEST(SafeBrowsingDatabase, Database) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   EXPECT_FALSE(database->ContainsUrl(
       GURL("http://www.notevilanymore.com/index.html"),
@@ -473,8 +498,10 @@ TEST(SafeBrowsingDatabase, Database) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   EXPECT_FALSE(database->ContainsUrl(
       GURL("http://www.notevilanymore.com/index.html"),
@@ -518,14 +545,17 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) {
   chunk.hosts.push_back(host);
   chunks->push_back(chunk);
 
+  std::vector<SBListChunkRanges> lists;
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   // Add an empty ADD and SUB chunk.
-  std::vector<SBListChunkRanges> list_chunks_empty;
-  database->GetListsInfo(&list_chunks_empty);
-  EXPECT_EQ(list_chunks_empty[0].adds, "1,10");
+  GetListsInfo(database, &lists);
+  EXPECT_EQ(lists[0].adds, "1,10");
+  lists.clear();
 
   SBChunk empty_chunk;
   empty_chunk.chunk_number = 19;
@@ -533,6 +563,7 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(empty_chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   chunks = new std::deque<SBChunk>;
   empty_chunk.chunk_number = 7;
@@ -540,11 +571,12 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) {
   chunks->push_back(empty_chunk);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
-  list_chunks_empty.clear();
-  database->GetListsInfo(&list_chunks_empty);
-  EXPECT_EQ(list_chunks_empty[0].adds, "1,10,19");
-  EXPECT_EQ(list_chunks_empty[0].subs, "7");
+  GetListsInfo(database, &lists);
+  EXPECT_EQ(lists[0].adds, "1,10,19");
+  EXPECT_EQ(lists[0].subs, "7");
+  lists.clear();
 
   // Add an empty chunk along with a couple that contain data. This should
   // result in the chunk range being reduced in size.
@@ -575,8 +607,10 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) {
   chunks->push_back(empty_chunk);
 
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   const Time now = Time::Now();
   std::vector<SBFullHashResult> full_hashes;
@@ -589,27 +623,33 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) {
                                     &matching_list, &prefix_hits,
                                     &full_hashes, now));
 
-  list_chunks_empty.clear();
-  database->GetListsInfo(&list_chunks_empty);
-  EXPECT_EQ(list_chunks_empty[0].adds, "1,10,19-22");
-  EXPECT_EQ(list_chunks_empty[0].subs, "7");
+  GetListsInfo(database, &lists);
+  EXPECT_EQ(lists[0].adds, "1,10,19-22");
+  EXPECT_EQ(lists[0].subs, "7");
+  lists.clear();
 
   // Handle AddDel and SubDel commands for empty chunks.
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   AddDelChunk(database, safe_browsing_util::kMalwareList, 21);
   database->UpdateFinished(true);
-  list_chunks_empty.clear();
-  database->GetListsInfo(&list_chunks_empty);
-  EXPECT_EQ(list_chunks_empty[0].adds, "1,10,19-20,22");
-  EXPECT_EQ(list_chunks_empty[0].subs, "7");
+  lists.clear();
+
+  GetListsInfo(database, &lists);
+  EXPECT_EQ(lists[0].adds, "1,10,19-20,22");
+  EXPECT_EQ(lists[0].subs, "7");
+  lists.clear();
 
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   SubDelChunk(database, safe_browsing_util::kMalwareList, 7);
   database->UpdateFinished(true);
-  list_chunks_empty.clear();
-  database->GetListsInfo(&list_chunks_empty);
-  EXPECT_EQ(list_chunks_empty[0].adds, "1,10,19-20,22");
-  EXPECT_EQ(list_chunks_empty[0].subs, "");
+  lists.clear();
+
+  GetListsInfo(database, &lists);
+  EXPECT_EQ(lists[0].adds, "1,10,19-20,22");
+  EXPECT_EQ(lists[0].subs, "");
+  lists.clear();
 
   TearDownTestDatabase(database);
 }
@@ -630,10 +670,13 @@ void PopulateDatabaseForCacheTest(SafeBrowsingDatabase* database) {
   chunk.hosts.push_back(host);
 
   std::deque<SBChunk>* chunks = new std::deque<SBChunk>;
+  std::vector<SBListChunkRanges> lists;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   // Add the GetHash results to the cache.
   SBFullHashResult full_hash;
@@ -663,11 +706,11 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   EXPECT_EQ(hash_cache->size(), 2U);
 
   // Test the cache lookup for the first prefix.
-  std::string list;
+  std::string listname;
   std::vector<SBPrefix> prefixes;
   std::vector<SBFullHashResult> full_hashes;
   database->ContainsUrl(GURL("http://www.evil.com/phishing.html"),
-                        &list, &prefixes, &full_hashes, Time::Now());
+                        &listname, &prefixes, &full_hashes, Time::Now());
   EXPECT_EQ(full_hashes.size(), 1U);
 
   SBFullHashResult full_hash;
@@ -681,7 +724,7 @@ TEST(SafeBrowsingDatabase, HashCaching) {
 
   // Test the cache lookup for the second prefix.
   database->ContainsUrl(GURL("http://www.evil.com/malware.html"),
-                        &list, &prefixes, &full_hashes, Time::Now());
+                        &listname, &prefixes, &full_hashes, Time::Now());
   EXPECT_EQ(full_hashes.size(), 1U);
   base::SHA256HashString("www.evil.com/malware.html",
                          &full_hash.hash, sizeof(SBFullHash));
@@ -706,13 +749,17 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   chunk.hosts.push_back(host);
   std::deque<SBChunk>* chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
+
+  std::vector<SBListChunkRanges> lists;
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
+  lists.clear();
 
   // This prefix should still be there.
   database->ContainsUrl(GURL("http://www.evil.com/malware.html"),
-                        &list, &prefixes, &full_hashes, Time::Now());
+                        &listname, &prefixes, &full_hashes, Time::Now());
   EXPECT_EQ(full_hashes.size(), 1U);
   base::SHA256HashString("www.evil.com/malware.html",
                          &full_hash.hash, sizeof(SBFullHash));
@@ -724,7 +771,7 @@ TEST(SafeBrowsingDatabase, HashCaching) {
 
   // This prefix should be gone.
   database->ContainsUrl(GURL("http://www.evil.com/phishing.html"),
-                        &list, &prefixes, &full_hashes, Time::Now());
+                        &listname, &prefixes, &full_hashes, Time::Now());
   EXPECT_EQ(full_hashes.size(), 0U);
 
   prefixes.clear();
@@ -732,13 +779,15 @@ TEST(SafeBrowsingDatabase, HashCaching) {
 
   // Test that an AddDel for the original chunk removes the last cached entry.
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   AddDelChunk(database, safe_browsing_util::kMalwareList, 1);
   database->UpdateFinished(true);
   database->ContainsUrl(GURL("http://www.evil.com/malware.html"),
-                        &list, &prefixes, &full_hashes, Time::Now());
+                        &listname, &prefixes, &full_hashes, Time::Now());
   EXPECT_EQ(full_hashes.size(), 0U);
   EXPECT_EQ(database->hash_cache()->size(), 0U);
 
+  lists.clear();
   prefixes.clear();
   full_hashes.clear();
 
@@ -760,12 +809,12 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   entries.push_back(entry);
 
   database->ContainsUrl(GURL("http://www.evil.com/malware.html"),
-                        &list, &prefixes, &full_hashes, expired);
+                        &listname, &prefixes, &full_hashes, expired);
   EXPECT_EQ(full_hashes.size(), 0U);
 
   // This entry should still exist.
   database->ContainsUrl(GURL("http://www.evil.com/phishing.html"),
-                        &list, &prefixes, &full_hashes, expired);
+                        &listname, &prefixes, &full_hashes, expired);
   EXPECT_EQ(full_hashes.size(), 1U);
 
 
@@ -773,8 +822,10 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   // Since PopulateDatabaseForCacheTest() doesn't handle adding duplicate
   // chunks.
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   AddDelChunk(database, safe_browsing_util::kMalwareList, 1);
   database->UpdateFinished(true);
+  lists.clear();
 
   std::vector<SBPrefix> prefix_misses;
   std::vector<SBFullHashResult> empty_full_hash;
@@ -791,7 +842,7 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   // Prefix miss cache should be cleared.
   EXPECT_EQ(database->prefix_miss_cache()->size(), 0U);
 
-  list.clear();
+  lists.clear();
   prefixes.clear();
   full_hashes.clear();
 
@@ -821,28 +872,29 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
 
   EXPECT_TRUE(database->ContainsUrl(GURL("http://www.fullevil.com/bad1.html"),
-                                    &list, &prefixes, &full_hashes, 
+                                    &listname, &prefixes, &full_hashes, 
                                     Time::Now()));
   EXPECT_EQ(full_hashes.size(), 1U);
   EXPECT_EQ(0, memcmp(full_hashes[0].hash.full_hash,
                       full_add1.full_hash,
                       sizeof(SBFullHash)));
-  list.clear();
+  lists.clear();
   prefixes.clear();
   full_hashes.clear();
 
   EXPECT_TRUE(database->ContainsUrl(GURL("http://www.fullevil.com/bad2.html"),
-                                    &list, &prefixes, &full_hashes, 
+                                    &listname, &prefixes, &full_hashes, 
                                     Time::Now()));
   EXPECT_EQ(full_hashes.size(), 1U);
   EXPECT_EQ(0, memcmp(full_hashes[0].hash.full_hash,
                       full_add2.full_hash,
                       sizeof(SBFullHash)));
-  list.clear();
+  lists.clear();
   prefixes.clear();
   full_hashes.clear();
 
@@ -863,36 +915,39 @@ TEST(SafeBrowsingDatabase, HashCaching) {
   chunks = new std::deque<SBChunk>;
   chunks->push_back(chunk);
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   database->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database->UpdateFinished(true);
 
   EXPECT_FALSE(database->ContainsUrl(GURL("http://www.fullevil.com/bad1.html"),
-                                     &list, &prefixes, &full_hashes, 
+                                     &listname, &prefixes, &full_hashes, 
                                      Time::Now()));
   EXPECT_EQ(full_hashes.size(), 0U);
 
   // There should be one remaining full add.
   EXPECT_TRUE(database->ContainsUrl(GURL("http://www.fullevil.com/bad2.html"),
-                                    &list, &prefixes, &full_hashes, 
+                                    &listname, &prefixes, &full_hashes, 
                                     Time::Now()));
   EXPECT_EQ(full_hashes.size(), 1U);
   EXPECT_EQ(0, memcmp(full_hashes[0].hash.full_hash,
                       full_add2.full_hash,
                       sizeof(SBFullHash)));
-  list.clear();
+  lists.clear();
   prefixes.clear();
   full_hashes.clear();
 
   // Now test an AddDel for the remaining full add.
   database->UpdateStarted();
+  database->GetListsInfo(&lists);
   AddDelChunk(database, safe_browsing_util::kMalwareList, 20);
   database->UpdateFinished(true);
+  lists.clear();
 
   EXPECT_FALSE(database->ContainsUrl(GURL("http://www.fullevil.com/bad1.html"),
-                                     &list, &prefixes, &full_hashes, 
+                                     &listname, &prefixes, &full_hashes, 
                                      Time::Now()));
   EXPECT_FALSE(database->ContainsUrl(GURL("http://www.fullevil.com/bad2.html"),
-                                     &list, &prefixes, &full_hashes, 
+                                     &listname, &prefixes, &full_hashes, 
                                      Time::Now()));
 
   TearDownTestDatabase(database);
@@ -958,13 +1013,15 @@ void PeformUpdate(const std::wstring& initial_db,
   CHECK(metric->GetIOCounters(&before));
 #endif
 
+  std::vector<SBListChunkRanges> lists;
   database->UpdateStarted();
-
+  database->GetListsInfo(&lists);
   database->DeleteChunks(deletes);
   for (size_t i = 0; i < chunks.size(); ++i)
     database->InsertChunks(chunks[i].listname, chunks[i].chunks);
 
   database->UpdateFinished(true);
+  lists.clear();
 
 #if defined(OS_WIN) || defined(OS_LINUX)
   CHECK(metric->GetIOCounters(&after));
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 792a4d8..c09806d 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -595,9 +595,13 @@ void SafeBrowsingService::GetAllChunksFromDatabase() {
   DCHECK(MessageLoop::current() == db_thread_->message_loop());
   bool database_error = true;
   std::vector<SBListChunkRanges> lists;
-  if (GetDatabase() && GetDatabase()->UpdateStarted()) {
-    GetDatabase()->GetListsInfo(&lists);
-    database_error = false;
+  if (GetDatabase()) {
+    if (GetDatabase()->UpdateStarted()) {
+      GetDatabase()->GetListsInfo(&lists);
+      database_error = false;
+    } else {
+      GetDatabase()->UpdateFinished(false);
+    }
   }
 
   io_loop_->PostTask(FROM_HERE, NewRunnableMethod(
@@ -658,7 +662,11 @@ void SafeBrowsingService::OnResume() {
 
 void SafeBrowsingService::HandleResume() {
   DCHECK(MessageLoop::current() == db_thread_->message_loop());
-  GetDatabase()->HandleResume();
+  // We don't call GetDatabase() here, since we want to avoid unnecessary calls
+  // to Open, Reset, etc, or reload the bloom filter while we're coming out of
+  // a suspended state.
+  if (database_)
+    database_->HandleResume();
 }
 
 void SafeBrowsingService::RunQueuedClients() {