Refactor to simplify safe-browsing gethash cache.

Stop propagating cached gethash values to the database, as
mandated by protocol v2.3.  The timing of the cache clear may not
be precisely what v2.3 requests.

Convert SBFullHashResult.list_name to list_id, the name wasn't
needed.  Remove add_chunk_id, it was not used.

Add SBFullHashCached to store cached and database hashes in memory.

BUG=172527

Review URL: https://codereview.chromium.org/239743002

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

15 files changed, 173 insertions, 305 deletions

diff --git a/chrome/browser/safe_browsing/database_manager.cc b/chrome/browser/safe_browsing/database_manager.cc
index f1b921b..ba22281 100644
--- a/chrome/browser/safe_browsing/database_manager.cc
+++ b/chrome/browser/safe_browsing/database_manager.cc
@@ -68,6 +68,28 @@ bool IsExpectedThreat(
                                              threat_type);
 }
 
+// |list_id| is from |safe_browsing_util::ListType|.
+SBThreatType GetThreatTypeFromListId(int list_id) {
+  if (list_id == safe_browsing_util::PHISH) {
+    return SB_THREAT_TYPE_URL_PHISHING;
+  }
+
+  if (list_id == safe_browsing_util::MALWARE) {
+    return SB_THREAT_TYPE_URL_MALWARE;
+  }
+
+  if (list_id == safe_browsing_util::BINURL) {
+    return SB_THREAT_TYPE_BINARY_MALWARE_URL;
+  }
+
+  if (list_id == safe_browsing_util::EXTENSIONBLACKLIST) {
+    return SB_THREAT_TYPE_EXTENSION;
+  }
+
+  DVLOG(1) << "Unknown safe browsing list id " << list_id;
+  return SB_THREAT_TYPE_SAFE;
+}
+
 }  // namespace
 
 SafeBrowsingDatabaseManager::SafeBrowsingCheck::SafeBrowsingCheck(
@@ -784,28 +806,6 @@ void SafeBrowsingDatabaseManager::DeleteDatabaseChunks(
   }
 }
 
-SBThreatType SafeBrowsingDatabaseManager::GetThreatTypeFromListname(
-    const std::string& list_name) {
-  if (safe_browsing_util::IsPhishingList(list_name)) {
-    return SB_THREAT_TYPE_URL_PHISHING;
-  }
-
-  if (safe_browsing_util::IsMalwareList(list_name)) {
-    return SB_THREAT_TYPE_URL_MALWARE;
-  }
-
-  if (safe_browsing_util::IsBadbinurlList(list_name)) {
-    return SB_THREAT_TYPE_BINARY_MALWARE_URL;
-  }
-
-  if (safe_browsing_util::IsExtensionList(list_name)) {
-    return SB_THREAT_TYPE_EXTENSION;
-  }
-
-  DVLOG(1) << "Unknown safe browsing list " << list_name;
-  return SB_THREAT_TYPE_SAFE;
-}
-
 void SafeBrowsingDatabaseManager::DatabaseUpdateFinished(
     bool update_succeeded) {
   DCHECK_EQ(base::MessageLoop::current(),
@@ -891,7 +891,7 @@ bool SafeBrowsingDatabaseManager::HandleOneCheck(
     if (index == -1)
       continue;
     SBThreatType threat =
-        GetThreatTypeFromListname(full_hashes[index].list_name);
+        GetThreatTypeFromListId(full_hashes[index].list_id);
     if (threat != SB_THREAT_TYPE_SAFE &&
         IsExpectedThreat(threat, check->expected_threats)) {
       check->url_results[i] = threat;
@@ -905,7 +905,7 @@ bool SafeBrowsingDatabaseManager::HandleOneCheck(
     if (index == -1)
       continue;
     SBThreatType threat =
-        GetThreatTypeFromListname(full_hashes[index].list_name);
+        GetThreatTypeFromListId(full_hashes[index].list_id);
     if (threat != SB_THREAT_TYPE_SAFE &&
         IsExpectedThreat(threat, check->expected_threats)) {
       check->full_hash_results[i] = threat;
diff --git a/chrome/browser/safe_browsing/database_manager.h b/chrome/browser/safe_browsing/database_manager.h
index a8d8bad..8f2b219 100644
--- a/chrome/browser/safe_browsing/database_manager.h
+++ b/chrome/browser/safe_browsing/database_manager.h
@@ -277,8 +277,6 @@ class SafeBrowsingDatabaseManager
 
   void DeleteDatabaseChunks(std::vector<SBChunkDelete>* chunk_deletes);
 
-  static SBThreatType GetThreatTypeFromListname(const std::string& list_name);
-
   void NotifyClientBlockingComplete(Client* client, bool proceed);
 
   void DatabaseUpdateFinished(bool update_succeeded);
diff --git a/chrome/browser/safe_browsing/database_manager_unittest.cc b/chrome/browser/safe_browsing/database_manager_unittest.cc
index f45fb10..af7fda4 100644
--- a/chrome/browser/safe_browsing/database_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/database_manager_unittest.cc
@@ -47,8 +47,7 @@ bool SafeBrowsingDatabaseManagerTest::RunSBHashTest(
 
   const SBFullHashResult full_hash_result = {
       same_full_hash,
-      result_list,
-      0
+      safe_browsing_util::GetListId(result_list)
   };
 
   std::vector<SBFullHashResult> fake_results(1, full_hash_result);
diff --git a/chrome/browser/safe_browsing/protocol_parser.cc b/chrome/browser/safe_browsing/protocol_parser.cc
index 1fb64ef..db550e3 100644
--- a/chrome/browser/safe_browsing/protocol_parser.cc
+++ b/chrome/browser/safe_browsing/protocol_parser.cc
@@ -65,12 +65,13 @@ bool SafeBrowsingProtocolParser::ParseGetHash(
       return false;
 
     SBFullHashResult full_hash;
-    full_hash.list_name = cmd_parts[0];
-    full_hash.add_chunk_id = atoi(cmd_parts[1].c_str());
+    full_hash.list_id = safe_browsing_util::GetListId(cmd_parts[0]);
+    // Ignore cmd_parts[1] (add_chunk_id), as we no longer use it with SB 2.3
+    // caching rules.
     int full_hash_len = atoi(cmd_parts[2].c_str());
 
     // Ignore hash results from lists we don't recognize.
-    if (safe_browsing_util::GetListId(full_hash.list_name) < 0) {
+    if (full_hash.list_id < 0) {
       data += full_hash_len;
       length -= full_hash_len;
       continue;
diff --git a/chrome/browser/safe_browsing/protocol_parser_unittest.cc b/chrome/browser/safe_browsing/protocol_parser_unittest.cc
index c20c4ce..08d958e 100644
--- a/chrome/browser/safe_browsing/protocol_parser_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_parser_unittest.cc
@@ -439,15 +439,15 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHash) {
   EXPECT_EQ(memcmp(&full_hashes[0].hash,
                    "00112233445566778899aabbccddeeff",
                    sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[0].list_name, "goog-phish-shavar");
+  EXPECT_EQ(full_hashes[0].list_id, safe_browsing_util::PHISH);
   EXPECT_EQ(memcmp(&full_hashes[1].hash,
                    "00001111222233334444555566667777",
                    sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[1].list_name, "goog-phish-shavar");
+  EXPECT_EQ(full_hashes[1].list_id, safe_browsing_util::PHISH);
   EXPECT_EQ(memcmp(&full_hashes[2].hash,
                    "ffffeeeeddddccccbbbbaaaa99998888",
                    sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[2].list_name, "goog-phish-shavar");
+  EXPECT_EQ(full_hashes[2].list_id, safe_browsing_util::PHISH);
 
   // Test multiple lists in the GetHash results.
   std::string get_hash2("goog-phish-shavar:19:32\n"
@@ -463,15 +463,15 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHash) {
   EXPECT_EQ(memcmp(&full_hashes[0].hash,
                    "00112233445566778899aabbccddeeff",
                    sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[0].list_name, "goog-phish-shavar");
+  EXPECT_EQ(full_hashes[0].list_id, safe_browsing_util::PHISH);
   EXPECT_EQ(memcmp(&full_hashes[1].hash,
                    "cafebeefcafebeefdeaddeaddeaddead",
                    sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[1].list_name, "goog-malware-shavar");
+  EXPECT_EQ(full_hashes[1].list_id, safe_browsing_util::MALWARE);
   EXPECT_EQ(memcmp(&full_hashes[2].hash,
                    "zzzzyyyyxxxxwwwwvvvvuuuuttttssss",
                    sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[2].list_name, "goog-malware-shavar");
+  EXPECT_EQ(full_hashes[2].list_id, safe_browsing_util::MALWARE);
 }
 
 TEST(SafeBrowsingProtocolParsingTest, TestGetHashWithUnknownList) {
@@ -488,8 +488,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHashWithUnknownList) {
   EXPECT_EQ(full_hashes.size(), 1U);
   EXPECT_EQ(memcmp("12345678901234567890123456789012",
                    &full_hashes[0].hash, sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[0].list_name, "goog-phish-shavar");
-  EXPECT_EQ(full_hashes[0].add_chunk_id, 1);
+  EXPECT_EQ(full_hashes[0].list_id, safe_browsing_util::PHISH);
 
   hash_response += "goog-malware-shavar:7:32\n"
                    "abcdefghijklmnopqrstuvwxyz123457";
@@ -501,12 +500,10 @@ TEST(SafeBrowsingProtocolParsingTest, TestGetHashWithUnknownList) {
   EXPECT_EQ(full_hashes.size(), 2U);
   EXPECT_EQ(memcmp("12345678901234567890123456789012",
                    &full_hashes[0].hash, sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[0].list_name, "goog-phish-shavar");
-  EXPECT_EQ(full_hashes[0].add_chunk_id, 1);
+  EXPECT_EQ(full_hashes[0].list_id, safe_browsing_util::PHISH);
   EXPECT_EQ(memcmp("abcdefghijklmnopqrstuvwxyz123457",
                    &full_hashes[1].hash, sizeof(SBFullHash)), 0);
-  EXPECT_EQ(full_hashes[1].list_name, "goog-malware-shavar");
-  EXPECT_EQ(full_hashes[1].add_chunk_id, 7);
+  EXPECT_EQ(full_hashes[1].list_id, safe_browsing_util::MALWARE);
 }
 
 TEST(SafeBrowsingProtocolParsingTest, TestFormatHash) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.cc b/chrome/browser/safe_browsing/safe_browsing_database.cc
index 2b5b2e2..d783d12e 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database.cc
@@ -193,34 +193,28 @@ bool MatchAddPrefixes(SafeBrowsingStore* store,
 //
 // For efficiency reasons the code walks |prefix_hits| and
 // |full_hashes| in parallel, so they must be sorted by prefix.
-void GetCachedFullHashesForBrowse(const std::vector<SBPrefix>& prefix_hits,
-                                  const std::vector<SBAddFullHash>& full_hashes,
-                                  std::vector<SBFullHashResult>* full_hits,
-                                  base::Time last_update) {
+void GetCachedFullHashesForBrowse(
+    const std::vector<SBPrefix>& prefix_hits,
+    const std::vector<SBFullHashCached>& full_hashes,
+    std::vector<SBFullHashResult>* full_hits,
+    base::Time last_update) {
   const base::Time expire_time =
       base::Time::Now() - base::TimeDelta::FromMinutes(kMaxStalenessMinutes);
 
   std::vector<SBPrefix>::const_iterator piter = prefix_hits.begin();
-  std::vector<SBAddFullHash>::const_iterator hiter = full_hashes.begin();
+  std::vector<SBFullHashCached>::const_iterator hiter = full_hashes.begin();
 
   while (piter != prefix_hits.end() && hiter != full_hashes.end()) {
-    if (*piter < hiter->full_hash.prefix) {
+    if (*piter < hiter->hash.prefix) {
       ++piter;
-    } else if (hiter->full_hash.prefix < *piter) {
+    } else if (hiter->hash.prefix < *piter) {
       ++hiter;
     } else {
       if (expire_time < last_update ||
           expire_time.ToTimeT() < hiter->received) {
         SBFullHashResult result;
-        const int list_bit = GetListIdBit(hiter->chunk_id);
-        DCHECK(list_bit == safe_browsing_util::MALWARE ||
-               list_bit == safe_browsing_util::PHISH);
-        const safe_browsing_util::ListType list_id =
-            static_cast<safe_browsing_util::ListType>(list_bit);
-        if (!safe_browsing_util::GetListName(list_id, &result.list_name))
-          continue;
-        result.add_chunk_id = DecodeChunkId(hiter->chunk_id);
-        result.hash = hiter->full_hash;
+        result.list_id = hiter->list_id;
+        result.hash = hiter->hash;
         full_hits->push_back(result);
       }
 
@@ -306,10 +300,10 @@ void UpdateChunkRangesForList(SafeBrowsingStore* store,
   UpdateChunkRanges(store, std::vector<std::string>(1, listname), lists);
 }
 
-// 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;
+// Order |SBFullHashCached| items on the prefix part.
+bool SBFullHashCachedPrefixLess(const SBFullHashCached& a,
+                                const SBFullHashCached& b) {
+  return a.hash.prefix < b.hash.prefix;
 }
 
 // This code always checks for non-zero file size.  This helper makes
@@ -531,7 +525,7 @@ void SafeBrowsingDatabaseNew::Init(const base::FilePath& filename_base) {
     // contention on the lock...
     base::AutoLock locked(lookup_lock_);
     full_browse_hashes_.clear();
-    pending_browse_hashes_.clear();
+    cached_browse_hashes_.clear();
     LoadPrefixSet();
   }
 
@@ -655,7 +649,7 @@ bool SafeBrowsingDatabaseNew::ResetDatabase() {
   {
     base::AutoLock locked(lookup_lock_);
     full_browse_hashes_.clear();
-    pending_browse_hashes_.clear();
+    cached_browse_hashes_.clear();
     prefix_miss_cache_.clear();
     browse_prefix_set_.reset();
     side_effect_free_whitelist_prefix_set_.reset();
@@ -709,13 +703,13 @@ bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
     return false;
 
   // Find the matching full-hash results.  |full_browse_hashes_| are from the
-  // database, |pending_browse_hashes_| are from GetHash requests between
+  // database, |cached_browse_hashes_| are from GetHash requests between
   // updates.
   std::sort(prefix_hits->begin(), prefix_hits->end());
 
   GetCachedFullHashesForBrowse(*prefix_hits, full_browse_hashes_,
                                full_hits, last_update);
-  GetCachedFullHashesForBrowse(*prefix_hits, pending_browse_hashes_,
+  GetCachedFullHashesForBrowse(*prefix_hits, cached_browse_hashes_,
                                full_hits, last_update);
   return true;
 }
@@ -1062,28 +1056,27 @@ void SafeBrowsingDatabaseNew::CacheHashResults(
     return;
   }
 
-  // TODO(shess): SBFullHashResult and SBAddFullHash are very similar.
-  // Refactor to make them identical.
   const base::Time now = base::Time::Now();
-  const size_t orig_size = pending_browse_hashes_.size();
+  const size_t orig_size = cached_browse_hashes_.size();
   for (std::vector<SBFullHashResult>::const_iterator iter = full_hits.begin();
        iter != full_hits.end(); ++iter) {
-    const int list_id = safe_browsing_util::GetListId(iter->list_name);
-    if (list_id == safe_browsing_util::MALWARE ||
-        list_id == safe_browsing_util::PHISH) {
-      int encoded_chunk_id = EncodeChunkId(iter->add_chunk_id, list_id);
-      SBAddFullHash add_full_hash(encoded_chunk_id, now, iter->hash);
-      pending_browse_hashes_.push_back(add_full_hash);
+    if (iter->list_id == safe_browsing_util::MALWARE ||
+        iter->list_id == safe_browsing_util::PHISH) {
+      SBFullHashCached cached_hash;
+      cached_hash.hash = iter->hash;
+      cached_hash.list_id = iter->list_id;
+      cached_hash.received = static_cast<int>(now.ToTimeT());
+      cached_browse_hashes_.push_back(cached_hash);
     }
   }
 
   // Sort new entries then merge with the previously-sorted entries.
-  std::vector<SBAddFullHash>::iterator
-      orig_end = pending_browse_hashes_.begin() + orig_size;
-  std::sort(orig_end, pending_browse_hashes_.end(), SBAddFullHashPrefixLess);
-  std::inplace_merge(pending_browse_hashes_.begin(),
-                     orig_end, pending_browse_hashes_.end(),
-                     SBAddFullHashPrefixLess);
+  std::vector<SBFullHashCached>::iterator
+      orig_end = cached_browse_hashes_.begin() + orig_size;
+  std::sort(orig_end, cached_browse_hashes_.end(), SBFullHashCachedPrefixLess);
+  std::inplace_merge(cached_browse_hashes_.begin(),
+                     orig_end, cached_browse_hashes_.end(),
+                     SBFullHashCachedPrefixLess);
 }
 
 bool SafeBrowsingDatabaseNew::UpdateStarted(
@@ -1275,15 +1268,11 @@ void SafeBrowsingDatabaseNew::UpdateWhitelistStore(
   if (!store)
     return;
 
-  // For the whitelists, we don't cache and save full hashes since all
-  // hashes are already full.
-  std::vector<SBAddFullHash> empty_add_hashes;
-
   // Note: |builder| will not be empty.  The current data store implementation
   // stores all full-length hashes as both full and prefix hashes.
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> full_hashes;
-  if (!store->FinishUpdate(empty_add_hashes, &builder, &full_hashes)) {
+  if (!store->FinishUpdate(&builder, &full_hashes)) {
     RecordFailure(FAILURE_WHITELIST_DATABASE_UPDATE_FINISH);
     WhitelistEverything(whitelist);
     return;
@@ -1300,19 +1289,13 @@ int64 SafeBrowsingDatabaseNew::UpdateHashPrefixStore(
     const base::FilePath& store_filename,
     SafeBrowsingStore* store,
     FailureType failure_type) {
-  // We don't cache and save full hashes.
-  std::vector<SBAddFullHash> empty_add_hashes;
-
   // These results are not used after this call. Simply ignore the
   // returned value after FinishUpdate(...).
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes_result;
 
-  if (!store->FinishUpdate(empty_add_hashes,
-                           &builder,
-                           &add_full_hashes_result)) {
+  if (!store->FinishUpdate(&builder, &add_full_hashes_result))
     RecordFailure(failure_type);
-  }
 
 #if defined(OS_MACOSX)
   base::mac::SetFileBackupExclusion(store_filename);
@@ -1322,16 +1305,6 @@ int64 SafeBrowsingDatabaseNew::UpdateHashPrefixStore(
 }
 
 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 filter build.
   base::IoCounters io_before, io_after;
   base::ProcessHandle handle = base::Process::Current().handle();
@@ -1353,28 +1326,36 @@ void SafeBrowsingDatabaseNew::UpdateBrowseStore() {
 
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes;
-  if (!browse_store_->FinishUpdate(pending_add_hashes,
-                                   &builder, &add_full_hashes)) {
+  if (!browse_store_->FinishUpdate(&builder, &add_full_hashes)) {
     RecordFailure(FAILURE_BROWSE_DATABASE_UPDATE_FINISH);
     return;
   }
   scoped_ptr<safe_browsing::PrefixSet> prefix_set(builder.GetPrefixSet());
 
+  std::vector<SBFullHashCached> full_hash_results;
+  for (size_t i = 0; i < add_full_hashes.size(); ++i) {
+    SBFullHashCached result;
+    result.hash = add_full_hashes[i].full_hash;
+    result.list_id = GetListIdBit(add_full_hashes[i].chunk_id);
+    result.received = add_full_hashes[i].received;
+    full_hash_results.push_back(result);
+  }
+
   // This needs to be in sorted order by prefix for efficient access.
-  std::sort(add_full_hashes.begin(), add_full_hashes.end(),
-            SBAddFullHashPrefixLess);
+  std::sort(full_hash_results.begin(), full_hash_results.end(),
+            SBFullHashCachedPrefixLess);
 
   // Swap in the newly built filter and cache.
   {
     base::AutoLock locked(lookup_lock_);
-    full_browse_hashes_.swap(add_full_hashes);
+    full_browse_hashes_.swap(full_hash_results);
 
     // 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();
+    cached_browse_hashes_.clear();
     prefix_miss_cache_.clear();
     browse_prefix_set_.swap(prefix_set);
   }
@@ -1417,12 +1398,10 @@ void SafeBrowsingDatabaseNew::UpdateBrowseStore() {
 }
 
 void SafeBrowsingDatabaseNew::UpdateSideEffectFreeWhitelistStore() {
-  std::vector<SBAddFullHash> empty_add_hashes;
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes_result;
 
   if (!side_effect_free_whitelist_store_->FinishUpdate(
-          empty_add_hashes,
           &builder,
           &add_full_hashes_result)) {
     RecordFailure(FAILURE_SIDE_EFFECT_FREE_WHITELIST_UPDATE_FINISH);
@@ -1465,16 +1444,11 @@ void SafeBrowsingDatabaseNew::UpdateSideEffectFreeWhitelistStore() {
 }
 
 void SafeBrowsingDatabaseNew::UpdateIpBlacklistStore() {
-  // For the IP blacklist, we don't cache and save full hashes since all
-  // hashes are already full.
-  std::vector<SBAddFullHash> empty_add_hashes;
-
   // Note: prefixes will not be empty.  The current data store implementation
   // stores all full-length hashes as both full and prefix hashes.
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> full_hashes;
-  if (!ip_blacklist_store_->FinishUpdate(empty_add_hashes,
-                                         &builder, &full_hashes)) {
+  if (!ip_blacklist_store_->FinishUpdate(&builder, &full_hashes)) {
     RecordFailure(FAILURE_IP_BLACKLIST_UPDATE_FINISH);
     LoadIpBlacklist(std::vector<SBAddFullHash>());  // Clear the list.
     return;
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.h b/chrome/browser/safe_browsing/safe_browsing_database.h
index 9122160..7ed95a3 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database.h
@@ -47,6 +47,16 @@ class SafeBrowsingDatabaseFactory {
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingDatabaseFactory);
 };
 
+// Contains full_hash elements which are cached in memory.  Differs from
+// SBAddFullHash in deriving |list_id| from |chunk_id|.  Differs from
+// SBFullHashResult in adding |received| for later expiration.
+// TODO(shess): Remove/refactor this as part of converting to v2.3 caching
+// semantics.
+struct SBFullHashCached {
+  SBFullHash hash;
+  int list_id;  // TODO(shess): Use safe_browsing_util::ListType.
+  int received;  // time_t like SBAddFullHash.
+};
 
 // Encapsulates on-disk databases that for safebrowsing. There are
 // four databases: browse, download, download whitelist and
@@ -399,7 +409,7 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
 
   // Lock for protecting access to variables that may be used on the
   // IO thread.  This includes |prefix_set_|, |full_browse_hashes_|,
-  // |pending_browse_hashes_|, |prefix_miss_cache_|, |csd_whitelist_|.
+  // |cached_browse_hashes_|, |prefix_miss_cache_|, |csd_whitelist_|.
   base::Lock lookup_lock_;
 
   // Underlying persistent store for chunk data.
@@ -443,10 +453,10 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
   // Cached browse store related full-hash items, ordered by prefix for
   // efficient scanning.
   // |full_browse_hashes_| are items from |browse_store_|,
-  // |pending_browse_hashes_| are items from |CacheHashResults()|, which
-  // will be pushed to the store on the next update.
-  std::vector<SBAddFullHash> full_browse_hashes_;
-  std::vector<SBAddFullHash> pending_browse_hashes_;
+  // |cached_browse_hashes_| are items from |CacheHashResults()|, which will be
+  // discarded on next update.
+  std::vector<SBFullHashCached> full_browse_hashes_;
+  std::vector<SBFullHashCached> cached_browse_hashes_;
 
   // Cache of prefixes that returned empty results (no full hash
   // match) to |CacheHashResults()|.  Cached to prevent asking for
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
index 70d2b01..ed2aec8 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
@@ -863,8 +863,7 @@ void SafeBrowsingDatabaseTest::PopulateDatabaseForCacheTest() {
   // Add the GetHash results to the cache.
   SBFullHashResult full_hash;
   full_hash.hash = SBFullHashForString("www.evil.com/phishing.html");
-  full_hash.list_name = safe_browsing_util::kMalwareList;
-  full_hash.add_chunk_id = 1;
+  full_hash.list_id = safe_browsing_util::MALWARE;
 
   std::vector<SBFullHashResult> results;
   results.push_back(full_hash);
@@ -880,7 +879,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   PopulateDatabaseForCacheTest();
 
   // We should have both full hashes in the cache.
-  EXPECT_EQ(database_->pending_browse_hashes_.size(), 2U);
+  EXPECT_EQ(2U, database_->cached_browse_hashes_.size());
 
   // Test the cache lookup for the first prefix.
   std::string listname;
@@ -889,7 +888,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
       &listname, &prefixes, &full_hashes, Time::Now());
-  EXPECT_EQ(full_hashes.size(), 1U);
+  ASSERT_EQ(1U, full_hashes.size());
   EXPECT_TRUE(
       SBFullHashEqual(full_hashes[0].hash,
                       SBFullHashForString("www.evil.com/phishing.html")));
@@ -901,7 +900,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
       &listname, &prefixes, &full_hashes, Time::Now());
-  EXPECT_EQ(full_hashes.size(), 1U);
+  ASSERT_EQ(1U, full_hashes.size());
   EXPECT_TRUE(
       SBFullHashEqual(full_hashes[0].hash,
                       SBFullHashForString("www.evil.com/malware.html")));
@@ -921,14 +920,13 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   database_->InsertChunks(safe_browsing_util::kMalwareList, chunks);
   database_->UpdateFinished(true);
 
-  // This prefix should still be there.
-  database_->ContainsBrowseUrl(
+  // This prefix should still be there, but the fullhash is gone.
+  EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &listname, &prefixes, &full_hashes, Time::Now());
-  EXPECT_EQ(full_hashes.size(), 1U);
-  EXPECT_TRUE(
-      SBFullHashEqual(full_hashes[0].hash,
-                      SBFullHashForString("www.evil.com/malware.html")));
+      &listname, &prefixes, &full_hashes, Time::Now()));
+  ASSERT_EQ(1U, prefixes.size());
+  EXPECT_EQ(SBPrefixForString("www.evil.com/malware.html"), prefixes[0]);
+  EXPECT_TRUE(full_hashes.empty());
   prefixes.clear();
   full_hashes.clear();
 
@@ -950,7 +948,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
       &listname, &prefixes, &full_hashes, Time::Now());
   EXPECT_TRUE(full_hashes.empty());
   EXPECT_TRUE(database_->full_browse_hashes_.empty());
-  EXPECT_TRUE(database_->pending_browse_hashes_.empty());
+  EXPECT_TRUE(database_->cached_browse_hashes_.empty());
 
   prefixes.clear();
   full_hashes.clear();
@@ -960,15 +958,15 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   // cache insert uses Time::Now(). First, store some entries.
   PopulateDatabaseForCacheTest();
 
-  std::vector<SBAddFullHash>* hash_cache = &database_->pending_browse_hashes_;
-  EXPECT_EQ(hash_cache->size(), 2U);
+  std::vector<SBFullHashCached>* hash_cache = &database_->cached_browse_hashes_;
+  EXPECT_EQ(2U, hash_cache->size());
 
   // Now adjust one of the entries times to be in the past.
   base::Time expired = base::Time::Now() - base::TimeDelta::FromMinutes(60);
   const SBPrefix key = SBPrefixForString("www.evil.com/malware.html");
-  std::vector<SBAddFullHash>::iterator iter;
+  std::vector<SBFullHashCached>::iterator iter;
   for (iter = hash_cache->begin(); iter != hash_cache->end(); ++iter) {
-    if (iter->full_hash.prefix == key) {
+    if (iter->hash.prefix == key) {
       iter->received = static_cast<int32>(expired.ToTimeT());
       break;
     }
@@ -984,8 +982,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
       &listname, &prefixes, &full_hashes, expired);
-  EXPECT_EQ(full_hashes.size(), 1U);
-
+  EXPECT_EQ(1U, full_hashes.size());
 
   // Testing prefix miss caching. First, we clear out the existing database,
   // Since PopulateDatabaseForCacheTest() doesn't handle adding duplicate
@@ -1002,7 +999,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   database_->CacheHashResults(prefix_misses, empty_full_hash);
 
   // Prefixes with no full results are misses.
-  EXPECT_EQ(database_->prefix_miss_cache_.size(), 2U);
+  EXPECT_EQ(2U, database_->prefix_miss_cache_.size());
 
   // Update the database.
   PopulateDatabaseForCacheTest();
@@ -1042,7 +1039,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
       GURL("http://www.fullevil.com/bad1.html"),
       &listname, &prefixes, &full_hashes,
       Time::Now()));
-  EXPECT_EQ(full_hashes.size(), 1U);
+  ASSERT_EQ(1U, full_hashes.size());
   EXPECT_TRUE(
       SBFullHashEqual(full_hashes[0].hash,
                       SBFullHashForString("www.fullevil.com/bad1.html")));
@@ -1053,7 +1050,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
       GURL("http://www.fullevil.com/bad2.html"),
       &listname, &prefixes, &full_hashes,
       Time::Now()));
-  EXPECT_EQ(full_hashes.size(), 1U);
+  ASSERT_EQ(1U, full_hashes.size());
   EXPECT_TRUE(
       SBFullHashEqual(full_hashes[0].hash,
                       SBFullHashForString("www.fullevil.com/bad2.html")));
@@ -1082,7 +1079,7 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
       GURL("http://www.fullevil.com/bad2.html"),
       &listname, &prefixes, &full_hashes,
       Time::Now()));
-  EXPECT_EQ(full_hashes.size(), 1U);
+  ASSERT_EQ(1U, full_hashes.size());
   EXPECT_TRUE(
       SBFullHashEqual(full_hashes[0].hash,
                       SBFullHashForString("www.fullevil.com/bad2.html")));
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 3e5cbce..f8f4441 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -122,16 +122,16 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
                                  std::vector<SBFullHashResult>* full_hits,
                                  base::Time last_update) OVERRIDE {
     std::vector<GURL> urls(1, url);
-    return ContainsUrl(safe_browsing_util::kMalwareList,
-                       safe_browsing_util::kPhishingList,
+    return ContainsUrl(safe_browsing_util::MALWARE,
+                       safe_browsing_util::PHISH,
                        urls, prefix_hits, full_hits);
   }
   virtual bool ContainsDownloadUrl(
       const std::vector<GURL>& urls,
       std::vector<SBPrefix>* prefix_hits) OVERRIDE {
     std::vector<SBFullHashResult> full_hits;
-    bool found = ContainsUrl(safe_browsing_util::kBinUrlList,
-                             safe_browsing_util::kBinUrlList,
+    bool found = ContainsUrl(safe_browsing_util::BINURL,
+                             safe_browsing_util::BINURL,
                              urls, prefix_hits, &full_hits);
     if (!found)
       return false;
@@ -184,10 +184,10 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
 
   // Fill up the database with test URL.
   void AddUrl(const GURL& url,
-              const std::string& list_name,
+              int list_id,
               const std::vector<SBPrefix>& prefix_hits,
               const std::vector<SBFullHashResult>& full_hits) {
-    badurls_[url.spec()].list_name = list_name;
+    badurls_[url.spec()].list_id = list_id;
     badurls_[url.spec()].prefix_hits = prefix_hits;
     badurls_[url.spec()].full_hits = full_hits;
   }
@@ -199,13 +199,13 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
 
  private:
   struct Hits {
-    std::string list_name;
+    int list_id;
     std::vector<SBPrefix> prefix_hits;
     std::vector<SBFullHashResult> full_hits;
   };
 
-  bool ContainsUrl(const std::string& list_name0,
-                   const std::string& list_name1,
+  bool ContainsUrl(int list_id0,
+                   int list_id1,
                    const std::vector<GURL>& urls,
                    std::vector<SBPrefix>* prefix_hits,
                    std::vector<SBFullHashResult>* full_hits) {
@@ -218,8 +218,8 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
       if (badurls_it == badurls_.end())
         continue;
 
-      if (badurls_it->second.list_name == list_name0 ||
-          badurls_it->second.list_name == list_name1) {
+      if (badurls_it->second.list_id == list_id0 ||
+          badurls_it->second.list_id == list_id1) {
         prefix_hits->insert(prefix_hits->end(),
                             badurls_it->second.prefix_hits.begin(),
                             badurls_it->second.prefix_hits.end());
@@ -368,24 +368,13 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
   }
 
   static void GenUrlFullhashResult(const GURL& url,
-                                   const std::string& list_name,
-                                   int add_chunk_id,
+                                   int list_id,
                                    SBFullHashResult* full_hash) {
     std::string host;
     std::string path;
     safe_browsing_util::CanonicalizeUrl(url, &host, &path, NULL);
     full_hash->hash = SBFullHashForString(host + path);
-    full_hash->list_name = list_name;
-    full_hash->add_chunk_id = add_chunk_id;
-  }
-
-  static void GenDigestFullhashResult(const std::string& full_digest,
-                                      const std::string& list_name,
-                                      int add_chunk_id,
-                                      SBFullHashResult* full_hash) {
-    full_hash->hash = safe_browsing_util::StringToSBFullHash(full_digest);
-    full_hash->list_name = list_name;
-    full_hash->add_chunk_id = add_chunk_id;
+    full_hash->list_id = list_id;
   }
 
   virtual void SetUp() {
@@ -423,7 +412,7 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
     // full hash is hit in database's local cache.
     std::vector<SBFullHashResult> empty_full_hits;
     TestSafeBrowsingDatabase* db = db_factory_.GetDb();
-    db->AddUrl(url, full_hash.list_name, prefix_hits, empty_full_hits);
+    db->AddUrl(url, full_hash.list_id, prefix_hits, empty_full_hits);
 
     TestProtocolManager* pm = pm_factory_.GetProtocolManager();
     pm->SetGetFullHashResponse(full_hash);
@@ -523,9 +512,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Malware) {
   // After adding the url to safebrowsing database and getfullhash result,
   // we should see the interstitial page.
   SBFullHashResult malware_full_hash;
-  int chunk_id = 0;
-  GenUrlFullhashResult(url, safe_browsing_util::kMalwareList, chunk_id,
-                       &malware_full_hash);
+  GenUrlFullhashResult(url, safe_browsing_util::MALWARE, &malware_full_hash);
   EXPECT_CALL(observer_,
               OnSafeBrowsingMatch(IsUnsafeResourceFor(url))).Times(1);
   EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(url))).Times(1);
@@ -544,9 +531,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, DISABLED_MalwareWithWhitelist) {
   // After adding the url to safebrowsing database and getfullhash result,
   // we should see the interstitial page.
   SBFullHashResult malware_full_hash;
-  int chunk_id = 0;
-  GenUrlFullhashResult(url, safe_browsing_util::kMalwareList, chunk_id,
-                       &malware_full_hash);
+  GenUrlFullhashResult(url, safe_browsing_util::MALWARE, &malware_full_hash);
   EXPECT_CALL(observer_,
               OnSafeBrowsingMatch(IsUnsafeResourceFor(url))).Times(1);
   EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(url))).Times(1)
@@ -605,9 +590,8 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Prefetch) {
   // getfullhash result, we should not see the interstitial page since the
   // only malware was a prefetch target.
   SBFullHashResult malware_full_hash;
-  int chunk_id = 0;
-  GenUrlFullhashResult(malware_url, safe_browsing_util::kMalwareList,
-                       chunk_id, &malware_full_hash);
+  GenUrlFullhashResult(malware_url, safe_browsing_util::MALWARE,
+                       &malware_full_hash);
   SetupResponseForUrl(malware_url, malware_full_hash);
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(ShowingInterstitialPage());
@@ -691,9 +675,8 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrl) {
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, client->GetThreatType());
 
   SBFullHashResult full_hash_result;
-  int chunk_id = 0;
-  GenUrlFullhashResult(badbin_url, safe_browsing_util::kBinUrlList,
-                       chunk_id, &full_hash_result);
+  GenUrlFullhashResult(badbin_url, safe_browsing_util::BINURL,
+                       &full_hash_result);
   SetupResponseForUrl(badbin_url, full_hash_result);
 
   client->CheckDownloadUrl(badbin_urls);
@@ -718,9 +701,8 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrlRedirects) {
   EXPECT_EQ(SB_THREAT_TYPE_SAFE, client->GetThreatType());
 
   SBFullHashResult full_hash_result;
-  int chunk_id = 0;
-  GenUrlFullhashResult(badbin_url, safe_browsing_util::kBinUrlList,
-                       chunk_id, &full_hash_result);
+  GenUrlFullhashResult(badbin_url, safe_browsing_util::BINURL,
+                       &full_hash_result);
   SetupResponseForUrl(badbin_url, full_hash_result);
 
   client->CheckDownloadUrl(badbin_urls);
@@ -735,9 +717,8 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrlTimedOut) {
 
   scoped_refptr<TestSBClient> client(new TestSBClient);
   SBFullHashResult full_hash_result;
-  int chunk_id = 0;
-  GenUrlFullhashResult(badbin_url, safe_browsing_util::kBinUrlList,
-                       chunk_id, &full_hash_result);
+  GenUrlFullhashResult(badbin_url, safe_browsing_util::BINURL,
+                       &full_hash_result);
   SetupResponseForUrl(badbin_url, full_hash_result);
   client->CheckDownloadUrl(badbin_urls);
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_store.h b/chrome/browser/safe_browsing/safe_browsing_store.h
index b381743..0abe90b 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store.h
@@ -74,7 +74,7 @@ typedef std::deque<SBSubPrefix> SBSubPrefixes;
 
 struct SBAddFullHash {
   int32 chunk_id;
-  int32 received;
+  int32 received;  // TODO(shess): Deprecate and remove.
   SBFullHash full_hash;
 
   SBAddFullHash(int32 id, base::Time r, const SBFullHash& h)
@@ -220,12 +220,7 @@ class SafeBrowsingStore {
   // Pass the collected chunks through SBPRocessSubs() and commit to
   // permanent storage.  The resulting add prefixes and hashes will be
   // stored in |add_prefixes_result| and |add_full_hashes_result|.
-  // |pending_adds| is the set of full hashes which have been received
-  // since the previous update, and is provided as a convenience
-  // (could be written via WriteAddHash(), but that would flush the
-  // chunk to disk).
   virtual bool FinishUpdate(
-      const std::vector<SBAddFullHash>& pending_adds,
       safe_browsing::PrefixSetBuilder* builder,
       std::vector<SBAddFullHash>* add_full_hashes_result) = 0;
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.cc b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
index e89d412..0c8b2cd 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
@@ -434,12 +434,6 @@ bool prefix_bounder(SBPrefix val, const T& elt) {
 // aggregate operations on same.
 class StateInternal {
  public:
-  explicit StateInternal(const std::vector<SBAddFullHash>& pending_adds)
-    : add_full_hashes_(pending_adds.begin(), pending_adds.end()) {
-  }
-
-  StateInternal() {}
-
   // Append indicated amount of data from |fp|.
   bool AppendData(size_t add_prefix_count, size_t sub_prefix_count,
                   size_t add_hash_count, size_t sub_hash_count,
@@ -907,7 +901,6 @@ bool SafeBrowsingStoreFile::FinishChunk() {
 }
 
 bool SafeBrowsingStoreFile::DoUpdate(
-    const std::vector<SBAddFullHash>& pending_adds,
     safe_browsing::PrefixSetBuilder* builder,
     std::vector<SBAddFullHash>* add_full_hashes_result) {
   DCHECK(file_.get() || empty_);
@@ -931,7 +924,7 @@ bool SafeBrowsingStoreFile::DoUpdate(
                        std::max(static_cast<int>(update_size / 1024), 1));
 
   // Chunk updates to integrate.
-  StateInternal new_state(pending_adds);
+  StateInternal new_state;
 
   // Read update chunks.
   for (int i = 0; i < chunks_written_; ++i) {
@@ -1173,13 +1166,12 @@ bool SafeBrowsingStoreFile::DoUpdate(
 }
 
 bool SafeBrowsingStoreFile::FinishUpdate(
-    const std::vector<SBAddFullHash>& pending_adds,
     safe_browsing::PrefixSetBuilder* builder,
     std::vector<SBAddFullHash>* add_full_hashes_result) {
   DCHECK(builder);
   DCHECK(add_full_hashes_result);
 
-  if (!DoUpdate(pending_adds, builder, add_full_hashes_result)) {
+  if (!DoUpdate(builder, add_full_hashes_result)) {
     CancelUpdate();
     return false;
   }
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.h b/chrome/browser/safe_browsing/safe_browsing_store_file.h
index 22863f2..f54e39b 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.h
@@ -152,10 +152,7 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
   virtual bool FinishChunk() OVERRIDE;
 
   virtual bool BeginUpdate() OVERRIDE;
-  // Store updates with pending add full hashes in file store and
-  // return |add_prefixes_result| and |add_full_hashes_result|.
   virtual bool FinishUpdate(
-      const std::vector<SBAddFullHash>& pending_adds,
       safe_browsing::PrefixSetBuilder* builder,
       std::vector<SBAddFullHash>* add_full_hashes_result) OVERRIDE;
   virtual bool CancelUpdate() OVERRIDE;
@@ -185,9 +182,9 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
   static bool DeleteStore(const base::FilePath& basename);
 
  private:
-  // Update store file with pending full hashes.
-  virtual bool DoUpdate(const std::vector<SBAddFullHash>& pending_adds,
-                        safe_browsing::PrefixSetBuilder* builder,
+  // Does the actual update for FinishUpdate(), so that FinishUpdate() can clean
+  // up correctly in case of error.
+  virtual bool DoUpdate(safe_browsing::PrefixSetBuilder* builder,
                         std::vector<SBAddFullHash>* add_full_hashes_result);
 
   // Some very lucky users have an original-format file still in their
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
index e17f7e18..864367f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
@@ -83,13 +83,10 @@ class SafeBrowsingStoreFileTest : public PlatformTest {
     EXPECT_FALSE(store_->CheckAddChunk(kAddChunk3));
     EXPECT_FALSE(store_->CheckSubChunk(kAddChunk1));
 
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
 
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
   }
 
   // Manually read the shard stride info from the file.
@@ -127,13 +124,10 @@ TEST_F(SafeBrowsingStoreFileTest, Empty) {
   EXPECT_FALSE(store_->CheckSubChunk(1));
   EXPECT_FALSE(store_->CheckSubChunk(-1));
 
-  std::vector<SBAddFullHash> pending_adds;
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes_result;
 
-  EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                   &builder,
-                                   &add_full_hashes_result));
+  EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
   EXPECT_TRUE(add_full_hashes_result.empty());
 
   std::vector<SBPrefix> prefixes_result;
@@ -159,12 +153,9 @@ TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
   EXPECT_EQ(kSubChunk1, chunks[0]);
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -194,12 +185,9 @@ TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
   EXPECT_TRUE(store_->CheckSubChunk(kSubChunk1));
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     // Still has the expected contents.
     std::vector<SBPrefix> prefixes_result;
@@ -229,12 +217,9 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -254,12 +239,9 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -287,12 +269,9 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     // Knocked out the chunk expected.
     std::vector<SBPrefix> prefixes_result;
@@ -311,12 +290,9 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -334,12 +310,9 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -399,12 +372,9 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
   EXPECT_TRUE(store_->CheckSubChunk(kSubChunk2));
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -429,12 +399,9 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
   store_->DeleteSubChunk(kSubChunk2);
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
   }
 
   // Expect no more chunks.
@@ -445,12 +412,9 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
   EXPECT_FALSE(store_->CheckSubChunk(kSubChunk2));
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
     builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -505,12 +469,11 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
 
   // Can successfully open and read the store.
   {
-    std::vector<SBAddFullHash> pending_adds;
     std::vector<SBPrefix> orig_prefixes;
     std::vector<SBAddFullHash> orig_hashes;
     safe_browsing::PrefixSetBuilder builder;
     ASSERT_TRUE(store_->BeginUpdate());
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds, &builder, &orig_hashes));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &orig_hashes));
     builder.GetPrefixSet()->GetPrefixes(&orig_prefixes);
     EXPECT_GT(orig_prefixes.size(), 0U);
     EXPECT_GT(orig_hashes.size(), 0U);
@@ -533,10 +496,9 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
   std::vector<SBAddFullHash> add_hashes;
   corruption_detected_ = false;
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     ASSERT_TRUE(store_->BeginUpdate());
-    EXPECT_FALSE(store_->FinishUpdate(pending_adds, &builder, &add_hashes));
+    EXPECT_FALSE(store_->FinishUpdate(&builder, &add_hashes));
     EXPECT_TRUE(corruption_detected_);
   }
 
@@ -668,9 +630,7 @@ TEST_F(SafeBrowsingStoreFileTest, GetAddPrefixesAndHashes) {
 
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes_result;
-  EXPECT_TRUE(store_->FinishUpdate(std::vector<SBAddFullHash>(),
-                                   &builder,
-                                   &add_full_hashes_result));
+  EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
   SBAddPrefixes add_prefixes;
   EXPECT_TRUE(store_->GetAddPrefixes(&add_prefixes));
@@ -715,12 +675,9 @@ TEST_F(SafeBrowsingStoreFileTest, Resharding) {
     }
     EXPECT_TRUE(store_->FinishChunk());
 
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     SBAddPrefixes add_prefixes;
     EXPECT_TRUE(store_->GetAddPrefixes(&add_prefixes));
@@ -745,12 +702,9 @@ TEST_F(SafeBrowsingStoreFileTest, Resharding) {
     EXPECT_FALSE(store_->CheckAddChunk(chunk_id + 1));
     store_->DeleteAddChunk(chunk_id);
 
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     // New stride should be the same, or shifted one left.
     const uint32 new_shard_stride = ReadStride();
@@ -823,12 +777,9 @@ TEST_F(SafeBrowsingStoreFileTest, Version7) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     // The sub'ed prefix and hash are gone.
     std::vector<SBPrefix> prefixes_result;
@@ -902,12 +853,9 @@ TEST_F(SafeBrowsingStoreFileTest, Version8) {
   EXPECT_TRUE(store_->FinishChunk());
 
   {
-    std::vector<SBAddFullHash> pending_adds;
     safe_browsing::PrefixSetBuilder builder;
     std::vector<SBAddFullHash> add_full_hashes_result;
-    EXPECT_TRUE(store_->FinishUpdate(pending_adds,
-                                     &builder,
-                                     &add_full_hashes_result));
+    EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     // The sub'ed prefix and hash are gone.
     std::vector<SBPrefix> prefixes_result;
diff --git a/chrome/browser/safe_browsing/safe_browsing_util.cc b/chrome/browser/safe_browsing/safe_browsing_util.cc
index 40732d6..d375f90 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_util.cc
@@ -494,22 +494,6 @@ int GetUrlHashIndex(const GURL& url,
   return -1;
 }
 
-bool IsPhishingList(const std::string& list_name) {
-  return list_name.compare(kPhishingList) == 0;
-}
-
-bool IsMalwareList(const std::string& list_name) {
-  return list_name.compare(kMalwareList) == 0;
-}
-
-bool IsBadbinurlList(const std::string& list_name) {
-  return list_name.compare(kBinUrlList) == 0;
-}
-
-bool IsExtensionList(const std::string& list_name) {
-  return list_name.compare(kExtensionBlacklist) == 0;
-}
-
 GURL GeneratePhishingReportUrl(const std::string& report_page,
                                const std::string& url_to_report,
                                bool is_client_side_detection) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_util.h b/chrome/browser/safe_browsing/safe_browsing_util.h
index 783369e..3304989 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util.h
+++ b/chrome/browser/safe_browsing/safe_browsing_util.h
@@ -105,8 +105,8 @@ class SBChunkList {
 // Used when we get a gethash response.
 struct SBFullHashResult {
   SBFullHash hash;
-  std::string list_name;
-  int add_chunk_id;
+  // TODO(shess): Refactor to allow ListType here.
+  int list_id;
 };
 
 // Contains information about a list in the database.
@@ -353,11 +353,6 @@ int GetHashIndex(const SBFullHash& hash,
 int GetUrlHashIndex(const GURL& url,
                     const std::vector<SBFullHashResult>& full_hashes);
 
-bool IsPhishingList(const std::string& list_name);
-bool IsMalwareList(const std::string& list_name);
-bool IsBadbinurlList(const std::string& list_name);
-bool IsExtensionList(const std::string& list_name);
-
 GURL GeneratePhishingReportUrl(const std::string& report_page,
                                const std::string& url_to_report,
                                bool is_client_side_detection);