[safe-browsing] Database full hash matches like prefix match.

The safe-browsing code incorrectly replicates full-hashes into the
prefix set, and treats database full hashes as a form of cached
response.  Move the database full hashes into PrefixSet in preparation
for removing the replication.  Treat full-hash matches and prefix
matches the same (as matches which may lead to a gethash request).
ContainsBrowseUrl() full hash result changed to indicate that it is
previously-cached results.

Add a unit test that a full hash is persisted by PrefixSet.  Right now
it succeeds because of the replication of full-hash prefixes into the
prefix set.

BUG=361248,172527

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

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

11 files changed, 327 insertions, 196 deletions

diff --git a/chrome/browser/safe_browsing/database_manager.cc b/chrome/browser/safe_browsing/database_manager.cc
index 1af1bf6..6af4054 100644
--- a/chrome/browser/safe_browsing/database_manager.cc
+++ b/chrome/browser/safe_browsing/database_manager.cc
@@ -359,10 +359,10 @@ bool SafeBrowsingDatabaseManager::CheckBrowseUrl(const GURL& url,
   }
 
   std::vector<SBPrefix> prefix_hits;
-  std::vector<SBFullHashResult> full_hits;
+  std::vector<SBFullHashResult> cached_hits;
 
   bool prefix_match =
-      database_->ContainsBrowseUrl(url, &prefix_hits, &full_hits,
+      database_->ContainsBrowseUrl(url, &prefix_hits, &cached_hits,
           sb_service_->protocol_manager()->last_update());
 
   UMA_HISTOGRAM_TIMES("SB2.FilterCheck", base::TimeTicks::Now() - start);
@@ -377,9 +377,9 @@ bool SafeBrowsingDatabaseManager::CheckBrowseUrl(const GURL& url,
                                                    client,
                                                    safe_browsing_util::MALWARE,
                                                    expected_threats);
-  check->need_get_hash = full_hits.empty();
+  check->need_get_hash = cached_hits.empty();
   check->prefix_hits.swap(prefix_hits);
-  check->full_hits.swap(full_hits);
+  check->full_hits.swap(cached_hits);
   checks_.insert(check);
 
   BrowserThread::PostTask(
diff --git a/chrome/browser/safe_browsing/prefix_set.cc b/chrome/browser/safe_browsing/prefix_set.cc
index 7aeb40e..007f8b8 100644
--- a/chrome/browser/safe_browsing/prefix_set.cc
+++ b/chrome/browser/safe_browsing/prefix_set.cc
@@ -23,7 +23,7 @@ static uint32 kMagic = 0x864088dd;
 
 // Version history:
 // Version 1: b6cb7cfe/r74487 by shess@chromium.org on 2011-02-10
-// version 2: 2b59b0a6/r253924 by shess@chromium.org on 2014-02-27
+// Version 2: 2b59b0a6/r253924 by shess@chromium.org on 2014-02-27
 
 // Version 2 layout is identical to version 1.  The sort order of |index_|
 // changed from |int32| to |uint32| to match the change of |SBPrefix|.
@@ -92,7 +92,7 @@ PrefixSet::PrefixSet(IndexVector* index, std::vector<uint16>* deltas) {
 
 PrefixSet::~PrefixSet() {}
 
-bool PrefixSet::Exists(SBPrefix prefix) const {
+bool PrefixSet::PrefixExists(SBPrefix prefix) const {
   if (index_.empty())
     return false;
 
@@ -124,6 +124,14 @@ bool PrefixSet::Exists(SBPrefix prefix) const {
   return current == prefix;
 }
 
+bool PrefixSet::Exists(const SBFullHash& hash) const {
+  if (std::binary_search(full_hashes_.begin(), full_hashes_.end(),
+                         hash, SBFullHashLess)) {
+    return true;
+  }
+  return PrefixExists(hash.prefix);
+}
+
 void PrefixSet::GetPrefixes(std::vector<SBPrefix>* prefixes) const {
   prefixes->reserve(index_.size() + deltas_.size());
 
@@ -230,7 +238,7 @@ scoped_ptr<PrefixSet> PrefixSet::LoadFile(const base::FilePath& filter_name) {
     std::vector<SBPrefix> prefixes;
     PrefixSet(&index, &deltas).GetPrefixes(&prefixes);
     std::sort(prefixes.begin(), prefixes.end());
-    return PrefixSetBuilder(prefixes).GetPrefixSet().Pass();
+    return PrefixSetBuilder(prefixes).GetPrefixSetNoHashes().Pass();
   }
 
   // Steals contents of |index| and |deltas| via swap().
@@ -331,7 +339,8 @@ PrefixSetBuilder::PrefixSetBuilder(const std::vector<SBPrefix>& prefixes)
 PrefixSetBuilder::~PrefixSetBuilder() {
 }
 
-scoped_ptr<PrefixSet> PrefixSetBuilder::GetPrefixSet() {
+scoped_ptr<PrefixSet> PrefixSetBuilder::GetPrefixSet(
+    const std::vector<SBFullHash>& hashes) {
   DCHECK(prefix_set_.get());
 
   // Flush runs until buffered data is gone.
@@ -343,9 +352,16 @@ scoped_ptr<PrefixSet> PrefixSetBuilder::GetPrefixSet() {
   // they're almost free.
   PrefixSet::IndexVector(prefix_set_->index_).swap(prefix_set_->index_);
 
+  prefix_set_->full_hashes_ = hashes;
+  std::sort(prefix_set_->full_hashes_.begin(), prefix_set_->full_hashes_.end(),
+            SBFullHashLess);
+
   return prefix_set_.Pass();
 }
 
+scoped_ptr<PrefixSet> PrefixSetBuilder::GetPrefixSetNoHashes() {
+  return GetPrefixSet(std::vector<SBFullHash>()).Pass();
+}
 
 void PrefixSetBuilder::EmitRun() {
   DCHECK(prefix_set_.get());
diff --git a/chrome/browser/safe_browsing/prefix_set.h b/chrome/browser/safe_browsing/prefix_set.h
index da4099e..9a4bbb07 100644
--- a/chrome/browser/safe_browsing/prefix_set.h
+++ b/chrome/browser/safe_browsing/prefix_set.h
@@ -65,8 +65,9 @@ class PrefixSet {
  public:
   ~PrefixSet();
 
-  // |true| if |prefix| was in |prefixes| passed to the constructor.
-  bool Exists(SBPrefix prefix) const;
+  // |true| if |hash| is in the hashes passed to the set's builder, or if
+  // |hash.prefix| is one of the prefixes passed to the set's builder.
+  bool Exists(const SBFullHash& hash) const;
 
   // Persist the set on disk.
   static scoped_ptr<PrefixSet> LoadFile(const base::FilePath& filter_name);
@@ -78,6 +79,8 @@ class PrefixSet {
   friend class PrefixSetTest;
   FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, AllBig);
   FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, EdgeCases);
+  FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, Empty);
+  FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, FullHashBuild);
   FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, IntMinMax);
   FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, OneElement);
   FRIEND_TEST_ALL_PREFIXES(PrefixSetTest, ReadWriteSigned);
@@ -106,6 +109,10 @@ class PrefixSet {
   void AddRun(SBPrefix index_prefix,
               const uint16* run_begin, const uint16* run_end);
 
+  // |true| if |prefix| is one of the prefixes passed to the set's builder.
+  // Provided for testing purposes.
+  bool PrefixExists(SBPrefix prefix) const;
+
   // Regenerate the vector of prefixes passed to the constructor into
   // |prefixes|.  Prefixes will be added in sorted order.  Useful for testing.
   void GetPrefixes(std::vector<SBPrefix>* prefixes) const;
@@ -128,6 +135,9 @@ class PrefixSet {
   // |index_|, or the end of |deltas_| for the last |index_| pair.
   std::vector<uint16> deltas_;
 
+  // Full hashes ordered by SBFullHashLess.
+  std::vector<SBFullHash> full_hashes_;
+
   DISALLOW_COPY_AND_ASSIGN(PrefixSet);
 };
 
@@ -145,8 +155,13 @@ class PrefixSetBuilder {
   void AddPrefix(SBPrefix prefix);
 
   // Flush any buffered prefixes, and return the final PrefixSet instance.
-  // Any call other than the destructor is illegal after this call.
-  scoped_ptr<PrefixSet> GetPrefixSet();
+  // |hashes| are sorted and stored in |full_hashes_|.  Any call other than the
+  // destructor is illegal after this call.
+  scoped_ptr<PrefixSet> GetPrefixSet(const std::vector<SBFullHash>& hashes);
+
+  // Helper for clients which only track prefixes.  Calls GetPrefixSet() with
+  // empty hash vector.
+  scoped_ptr<PrefixSet> GetPrefixSetNoHashes();
 
  private:
   // Encode a run of deltas for |AddRun()|.  The run is broken by a too-large
diff --git a/chrome/browser/safe_browsing/prefix_set_unittest.cc b/chrome/browser/safe_browsing/prefix_set_unittest.cc
index 1e8b12d..2520282 100644
--- a/chrome/browser/safe_browsing/prefix_set_unittest.cc
+++ b/chrome/browser/safe_browsing/prefix_set_unittest.cc
@@ -82,15 +82,15 @@ class PrefixSetTest : public PlatformTest {
     EXPECT_TRUE(std::equal(check.begin(), check.end(), prefixes_copy.begin()));
 
     for (size_t i = 0; i < prefixes.size(); ++i) {
-      EXPECT_TRUE(prefix_set.Exists(prefixes[i]));
+      EXPECT_TRUE(prefix_set.PrefixExists(prefixes[i]));
 
       const SBPrefix left_sibling = prefixes[i] - 1;
       if (check.count(left_sibling) == 0)
-        EXPECT_FALSE(prefix_set.Exists(left_sibling));
+        EXPECT_FALSE(prefix_set.PrefixExists(left_sibling));
 
       const SBPrefix right_sibling = prefixes[i] + 1;
       if (check.count(right_sibling) == 0)
-        EXPECT_FALSE(prefix_set.Exists(right_sibling));
+        EXPECT_FALSE(prefix_set.PrefixExists(right_sibling));
     }
   }
 
@@ -104,7 +104,7 @@ class PrefixSetTest : public PlatformTest {
     base::FilePath filename = temp_dir_.path().AppendASCII("PrefixSetTest");
 
     PrefixSetBuilder builder(shared_prefixes_);
-    if (!builder.GetPrefixSet()->WriteFile(filename))
+    if (!builder.GetPrefixSetNoHashes()->WriteFile(filename))
       return false;
 
     *filenamep = filename;
@@ -211,16 +211,16 @@ std::vector<SBPrefix> PrefixSetTest::shared_prefixes_;
 // Test that a small sparse random input works.
 TEST_F(PrefixSetTest, Baseline) {
   PrefixSetBuilder builder(shared_prefixes_);
-  CheckPrefixes(*builder.GetPrefixSet(), shared_prefixes_);
+  CheckPrefixes(*builder.GetPrefixSetNoHashes(), shared_prefixes_);
 }
 
 // Test that the empty set doesn't appear to have anything in it.
 TEST_F(PrefixSetTest, Empty) {
   const std::vector<SBPrefix> empty;
   PrefixSetBuilder builder(empty);
-  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSet();
+  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSetNoHashes();
   for (size_t i = 0; i < shared_prefixes_.size(); ++i) {
-    EXPECT_FALSE(prefix_set->Exists(shared_prefixes_[i]));
+    EXPECT_FALSE(prefix_set->PrefixExists(shared_prefixes_[i]));
   }
 }
 
@@ -228,10 +228,10 @@ TEST_F(PrefixSetTest, Empty) {
 TEST_F(PrefixSetTest, OneElement) {
   const std::vector<SBPrefix> prefixes(100, 0u);
   PrefixSetBuilder builder(prefixes);
-  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSet();
-  EXPECT_FALSE(prefix_set->Exists(static_cast<SBPrefix>(-1)));
-  EXPECT_TRUE(prefix_set->Exists(prefixes[0]));
-  EXPECT_FALSE(prefix_set->Exists(1u));
+  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSetNoHashes();
+  EXPECT_FALSE(prefix_set->PrefixExists(static_cast<SBPrefix>(-1)));
+  EXPECT_TRUE(prefix_set->PrefixExists(prefixes[0]));
+  EXPECT_FALSE(prefix_set->PrefixExists(1u));
 
   // Check that |GetPrefixes()| returns the same set of prefixes as
   // was passed in.
@@ -258,7 +258,7 @@ TEST_F(PrefixSetTest, IntMinMax) {
 
   std::sort(prefixes.begin(), prefixes.end());
   PrefixSetBuilder builder(prefixes);
-  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSet();
+  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSetNoHashes();
 
   // Check that |GetPrefixes()| returns the same set of prefixes as
   // was passed in.
@@ -281,7 +281,7 @@ TEST_F(PrefixSetTest, AllBig) {
 
   std::sort(prefixes.begin(), prefixes.end());
   PrefixSetBuilder builder(prefixes);
-  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSet();
+  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSetNoHashes();
 
   // Check that |GetPrefixes()| returns the same set of prefixes as
   // was passed in.
@@ -293,12 +293,11 @@ TEST_F(PrefixSetTest, AllBig) {
                          prefixes_copy.begin()));
 }
 
-// Use artificial inputs to test various edge cases in Exists().
-// Items before the lowest item aren't present.  Items after the
-// largest item aren't present.  Create a sequence of items with
-// deltas above and below 2^16, and make sure they're all present.
-// Create a very long sequence with deltas below 2^16 to test crossing
-// |kMaxRun|.
+// Use artificial inputs to test various edge cases in PrefixExists().  Items
+// before the lowest item aren't present.  Items after the largest item aren't
+// present.  Create a sequence of items with deltas above and below 2^16, and
+// make sure they're all present.  Create a very long sequence with deltas below
+// 2^16 to test crossing |kMaxRun|.
 TEST_F(PrefixSetTest, EdgeCases) {
   std::vector<SBPrefix> prefixes;
 
@@ -336,7 +335,7 @@ TEST_F(PrefixSetTest, EdgeCases) {
 
   std::sort(prefixes.begin(), prefixes.end());
   PrefixSetBuilder builder(prefixes);
-  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSet();
+  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSetNoHashes();
 
   // Check that |GetPrefixes()| returns the same set of prefixes as
   // was passed in.
@@ -348,17 +347,17 @@ TEST_F(PrefixSetTest, EdgeCases) {
                          prefixes_copy.begin()));
 
   // Items before and after the set are not present, and don't crash.
-  EXPECT_FALSE(prefix_set->Exists(kHighBitSet - 100));
-  EXPECT_FALSE(prefix_set->Exists(kHighBitClear + 100));
+  EXPECT_FALSE(prefix_set->PrefixExists(kHighBitSet - 100));
+  EXPECT_FALSE(prefix_set->PrefixExists(kHighBitClear + 100));
 
   // Check that the set correctly flags all of the inputs, and also
   // check items just above and below the inputs to make sure they
   // aren't present.
   for (size_t i = 0; i < prefixes.size(); ++i) {
-    EXPECT_TRUE(prefix_set->Exists(prefixes[i]));
+    EXPECT_TRUE(prefix_set->PrefixExists(prefixes[i]));
 
-    EXPECT_FALSE(prefix_set->Exists(prefixes[i] - 1));
-    EXPECT_FALSE(prefix_set->Exists(prefixes[i] + 1));
+    EXPECT_FALSE(prefix_set->PrefixExists(prefixes[i] - 1));
+    EXPECT_FALSE(prefix_set->PrefixExists(prefixes[i] + 1));
   }
 }
 
@@ -382,7 +381,7 @@ TEST_F(PrefixSetTest, ReadWrite) {
     prefixes.push_back(kHighBitSet);
 
     PrefixSetBuilder builder(prefixes);
-    ASSERT_TRUE(builder.GetPrefixSet()->WriteFile(filename));
+    ASSERT_TRUE(builder.GetPrefixSetNoHashes()->WriteFile(filename));
 
     scoped_ptr<PrefixSet> prefix_set = PrefixSet::LoadFile(filename);
     ASSERT_TRUE(prefix_set.get());
@@ -393,7 +392,7 @@ TEST_F(PrefixSetTest, ReadWrite) {
   {
     std::vector<SBPrefix> prefixes;
     PrefixSetBuilder builder(prefixes);
-    ASSERT_TRUE(builder.GetPrefixSet()->WriteFile(filename));
+    ASSERT_TRUE(builder.GetPrefixSetNoHashes()->WriteFile(filename));
 
     scoped_ptr<PrefixSet> prefix_set = PrefixSet::LoadFile(filename);
     ASSERT_TRUE(prefix_set.get());
@@ -550,6 +549,42 @@ TEST_F(PrefixSetTest, SizeTRecovery) {
   ASSERT_FALSE(prefix_set.get());
 }
 
+// Test Exists() against full hashes passed to builder.
+TEST_F(PrefixSetTest, FullHashBuild) {
+  const SBFullHash kHash1 = SBFullHashForString("one");
+  const SBFullHash kHash2 = SBFullHashForString("two");
+  const SBFullHash kHash3 = SBFullHashForString("three");
+  const SBFullHash kHash4 = SBFullHashForString("four");
+  const SBFullHash kHash5 = SBFullHashForString("five");
+  const SBFullHash kHash6 = SBFullHashForString("six");
+
+  std::vector<SBPrefix> prefixes;
+  prefixes.push_back(kHash1.prefix);
+  prefixes.push_back(kHash2.prefix);
+  std::sort(prefixes.begin(), prefixes.end());
+
+  std::vector<SBFullHash> hashes;
+  hashes.push_back(kHash4);
+  hashes.push_back(kHash5);
+
+  PrefixSetBuilder builder(prefixes);
+  scoped_ptr<PrefixSet> prefix_set = builder.GetPrefixSet(hashes);
+
+  EXPECT_TRUE(prefix_set->Exists(kHash1));
+  EXPECT_TRUE(prefix_set->Exists(kHash2));
+  EXPECT_FALSE(prefix_set->Exists(kHash3));
+  EXPECT_TRUE(prefix_set->Exists(kHash4));
+  EXPECT_TRUE(prefix_set->Exists(kHash5));
+  EXPECT_FALSE(prefix_set->Exists(kHash6));
+
+  EXPECT_TRUE(prefix_set->PrefixExists(kHash1.prefix));
+  EXPECT_TRUE(prefix_set->PrefixExists(kHash2.prefix));
+  EXPECT_FALSE(prefix_set->PrefixExists(kHash3.prefix));
+  EXPECT_FALSE(prefix_set->PrefixExists(kHash4.prefix));
+  EXPECT_FALSE(prefix_set->PrefixExists(kHash5.prefix));
+  EXPECT_FALSE(prefix_set->PrefixExists(kHash6.prefix));
+}
+
 // Test that a version 1 file is re-ordered correctly on read.
 TEST_F(PrefixSetTest, ReadWriteSigned) {
   base::FilePath filename;
@@ -594,14 +629,14 @@ TEST_F(PrefixSetTest, ReadWriteSigned) {
   scoped_ptr<PrefixSet> prefix_set = PrefixSet::LoadFile(filename);
   ASSERT_TRUE(prefix_set.get());
 
-  // |Exists()| uses |std::upper_bound()| to find a starting point, which
+  // |PrefixExists()| uses |std::upper_bound()| to find a starting point, which
   // assumes |index_| is sorted.  Depending on how |upper_bound()| is
   // implemented, if the actual list is sorted by |int32|, then one of these
   // test pairs should fail.
-  EXPECT_TRUE(prefix_set->Exists(1000u));
-  EXPECT_TRUE(prefix_set->Exists(1023u));
-  EXPECT_TRUE(prefix_set->Exists(static_cast<uint32>(-1000)));
-  EXPECT_TRUE(prefix_set->Exists(static_cast<uint32>(-1000 + 23)));
+  EXPECT_TRUE(prefix_set->PrefixExists(1000u));
+  EXPECT_TRUE(prefix_set->PrefixExists(1023u));
+  EXPECT_TRUE(prefix_set->PrefixExists(static_cast<uint32>(-1000)));
+  EXPECT_TRUE(prefix_set->PrefixExists(static_cast<uint32>(-1000 + 23)));
 
   std::vector<SBPrefix> prefixes_copy;
   prefix_set->GetPrefixes(&prefixes_copy);
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.cc b/chrome/browser/safe_browsing/safe_browsing_database.cc
index 5a43a71..e1df581 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database.cc
@@ -315,11 +315,6 @@ int64 GetFileSizeOrZero(const base::FilePath& file_path) {
   return size_64;
 }
 
-// Used to order whitelist storage in memory.
-bool SBFullHashLess(const SBFullHash& a, const SBFullHash& b) {
-  return memcmp(a.full_hash, b.full_hash, sizeof(a.full_hash)) < 0;
-}
-
 }  // namespace
 
 // The default SafeBrowsingDatabaseFactory.
@@ -524,7 +519,6 @@ void SafeBrowsingDatabaseNew::Init(const base::FilePath& filename_base) {
     // threads.  Then again, that means there is no possibility of
     // contention on the lock...
     base::AutoLock locked(lookup_lock_);
-    full_browse_hashes_.clear();
     cached_browse_hashes_.clear();
     LoadPrefixSet();
   }
@@ -648,7 +642,6 @@ bool SafeBrowsingDatabaseNew::ResetDatabase() {
   // Reset objects in memory.
   {
     base::AutoLock locked(lookup_lock_);
-    full_browse_hashes_.clear();
     cached_browse_hashes_.clear();
     prefix_miss_cache_.clear();
     browse_prefix_set_.reset();
@@ -664,11 +657,11 @@ bool SafeBrowsingDatabaseNew::ResetDatabase() {
 bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
     const GURL& url,
     std::vector<SBPrefix>* prefix_hits,
-    std::vector<SBFullHashResult>* full_hits,
+    std::vector<SBFullHashResult>* cached_hits,
     base::Time last_update) {
   // Clear the results first.
   prefix_hits->clear();
-  full_hits->clear();
+  cached_hits->clear();
 
   std::vector<SBFullHash> full_hashes;
   BrowseFullHashesToCheck(url, false, &full_hashes);
@@ -687,8 +680,8 @@ bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
 
   size_t miss_count = 0;
   for (size_t i = 0; i < full_hashes.size(); ++i) {
-    const SBPrefix prefix = full_hashes[i].prefix;
-    if (browse_prefix_set_->Exists(prefix)) {
+    if (browse_prefix_set_->Exists(full_hashes[i])) {
+      const SBPrefix prefix = full_hashes[i].prefix;
       prefix_hits->push_back(prefix);
       if (prefix_miss_cache_.count(prefix) > 0)
         ++miss_count;
@@ -699,15 +692,11 @@ bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
   if (miss_count == prefix_hits->size())
     return false;
 
-  // Find the matching full-hash results.  |full_browse_hashes_| are from the
-  // database, |cached_browse_hashes_| are from GetHash requests between
-  // updates.
+  // Find matching cached gethash responses.
   std::sort(prefix_hits->begin(), prefix_hits->end());
-
-  GetCachedFullHashesForBrowse(*prefix_hits, full_browse_hashes_,
-                               full_hits, last_update);
   GetCachedFullHashesForBrowse(*prefix_hits, cached_browse_hashes_,
-                               full_hits, last_update);
+                               cached_hits, last_update);
+
   return true;
 }
 
@@ -776,7 +765,7 @@ bool SafeBrowsingDatabaseNew::ContainsSideEffectFreeWhitelistUrl(
   if (!side_effect_free_whitelist_prefix_set_.get())
     return false;
 
-  return side_effect_free_whitelist_prefix_set_->Exists(full_hash.prefix);
+  return side_effect_free_whitelist_prefix_set_->Exists(full_hash);
 }
 
 bool SafeBrowsingDatabaseNew::ContainsMalwareIP(const std::string& ip_address) {
@@ -1321,31 +1310,27 @@ void SafeBrowsingDatabaseNew::UpdateBrowseStore() {
 
   const base::TimeTicks before = base::TimeTicks::Now();
 
+  // TODO(shess): Perhaps refactor to let builder accumulate full hashes on the
+  // fly?  Other clients use the SBAddFullHash vector, but AFAICT they only use
+  // the SBFullHash portion.  It would need an accessor on PrefixSet.
   safe_browsing::PrefixSetBuilder builder;
   std::vector<SBAddFullHash> add_full_hashes;
   if (!browse_store_->FinishUpdate(&builder, &add_full_hashes)) {
     RecordFailure(FAILURE_BROWSE_DATABASE_UPDATE_FINISH);
     return;
   }
-  scoped_ptr<safe_browsing::PrefixSet> prefix_set(builder.GetPrefixSet());
 
-  std::vector<SBFullHashCached> full_hash_results;
+  std::vector<SBFullHash> 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);
+    full_hash_results.push_back(add_full_hashes[i].full_hash);
   }
 
-  // This needs to be in sorted order by prefix for efficient access.
-  std::sort(full_hash_results.begin(), full_hash_results.end(),
-            SBFullHashCachedPrefixLess);
+  scoped_ptr<safe_browsing::PrefixSet>
+      prefix_set(builder.GetPrefixSet(full_hash_results));
 
   // Swap in the newly built filter and cache.
   {
     base::AutoLock locked(lookup_lock_);
-    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.
@@ -1399,12 +1384,12 @@ void SafeBrowsingDatabaseNew::UpdateSideEffectFreeWhitelistStore() {
   std::vector<SBAddFullHash> add_full_hashes_result;
 
   if (!side_effect_free_whitelist_store_->FinishUpdate(
-          &builder,
-          &add_full_hashes_result)) {
+          &builder, &add_full_hashes_result)) {
     RecordFailure(FAILURE_SIDE_EFFECT_FREE_WHITELIST_UPDATE_FINISH);
     return;
   }
-  scoped_ptr<safe_browsing::PrefixSet> prefix_set(builder.GetPrefixSet());
+  scoped_ptr<safe_browsing::PrefixSet>
+      prefix_set(builder.GetPrefixSetNoHashes());
 
   // Swap in the newly built prefix set.
   {
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.h b/chrome/browser/safe_browsing/safe_browsing_database.h
index 0658fea..f734514 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database.h
@@ -104,11 +104,12 @@ class SafeBrowsingDatabase {
   virtual bool ResetDatabase() = 0;
 
   // Returns false if |url| is not in the browse database.  If it returns true,
-  // then |prefix_hits| and |full_hits| contains the matching hash prefixes.
-  // This function is safe to call from threads other than the creation thread.
+  // then |prefix_hits| contains the list of prefix matches, and |cached_hits|
+  // contains the cached gethash results for those prefixes (if any).  This
+  // function is safe to call from threads other than the creation thread.
   virtual bool ContainsBrowseUrl(const GURL& url,
                                  std::vector<SBPrefix>* prefix_hits,
-                                 std::vector<SBFullHashResult>* full_hits,
+                                 std::vector<SBFullHashResult>* cached_hits,
                                  base::Time last_update) = 0;
 
   // Returns false if none of |urls| are in Download database. If it returns
@@ -299,7 +300,7 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
   virtual bool ResetDatabase() OVERRIDE;
   virtual bool ContainsBrowseUrl(const GURL& url,
                                  std::vector<SBPrefix>* prefix_hits,
-                                 std::vector<SBFullHashResult>* full_hits,
+                                 std::vector<SBFullHashResult>* cached_hits,
                                  base::Time last_update) OVERRIDE;
   virtual bool ContainsDownloadUrl(const std::vector<GURL>& urls,
                                    std::vector<SBPrefix>* prefix_hits) OVERRIDE;
@@ -404,8 +405,8 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
   base::MessageLoop* creation_loop_;
 
   // Lock for protecting access to variables that may be used on the
-  // IO thread.  This includes |prefix_set_|, |full_browse_hashes_|,
-  // |cached_browse_hashes_|, |prefix_miss_cache_|, |csd_whitelist_|.
+  // IO thread.  This includes |prefix_set_|, |cached_browse_hashes_|,
+  // |prefix_miss_cache_|, |csd_whitelist_|.
   base::Lock lookup_lock_;
 
   // Underlying persistent store for chunk data.
@@ -446,12 +447,8 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
   // The IP blacklist should be small.  At most a couple hundred IPs.
   IPBlacklist ip_blacklist_;
 
-  // Cached browse store related full-hash items, ordered by prefix for
-  // efficient scanning.
-  // |full_browse_hashes_| are items from |browse_store_|,
-  // |cached_browse_hashes_| are items from |CacheHashResults()|, which will be
-  // discarded on next update.
-  std::vector<SBFullHashCached> full_browse_hashes_;
+  // Store items from CacheHashResults(), ordered by hash for efficient
+  // scanning.  Discarded on next update.
   std::vector<SBFullHashCached> cached_browse_hashes_;
 
   // Cache of prefixes that returned empty results (no full hash
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
index 29b0dee..0f433eb 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
@@ -83,6 +83,8 @@ void InsertAddChunkHostFullHashes(SBChunk* chunk,
   chunk->hosts.push_back(host);
 }
 
+// TODO(shess): This sounds like something to insert a full-hash chunk, but it's
+// actually specific to IP blacklist.
 void InsertAddChunkFullHash(SBChunk* chunk,
                             int chunk_number,
                             const std::string& ip_str,
@@ -520,55 +522,70 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
   chunks.clear();
   chunks.push_back(chunk);
   database_->InsertChunks(safe_browsing_util::kMalwareList, chunks);
+
+  // A chunk with a full hash.
+  chunk.hosts.clear();
+  InsertAddChunkHostFullHashes(&chunk, 7, "www.evil.com/",
+                               "www.evil.com/evil.html");
+  chunks.clear();
+  chunks.push_back(chunk);
+  database_->InsertChunks(safe_browsing_util::kMalwareList, chunks);
+
   database_->UpdateFinished(true);
 
   // Make sure they were added correctly.
   GetListsInfo(&lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
-  EXPECT_EQ(lists[0].adds, "1-3");
+  EXPECT_EQ(lists[0].adds, "1-3,7");
   EXPECT_TRUE(lists[0].subs.empty());
 
   const Time now = Time::Now();
-  std::vector<SBFullHashResult> full_hashes;
+  std::vector<SBFullHashResult> cached_hashes;
   std::vector<SBPrefix> prefix_hits;
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_EQ(prefix_hits[0], SBPrefixForString("www.evil.com/phishing.html"));
   EXPECT_EQ(prefix_hits.size(), 1U);
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/notevil1.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/notevil2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/good1.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/good2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://192.168.0.1/malware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_TRUE(prefix_hits.empty());
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/robots.txt"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
+
+  EXPECT_TRUE(database_->ContainsBrowseUrl(
+      GURL("http://www.evil.com/evil.html"),
+      &prefix_hits, &cached_hashes, now));
+  ASSERT_EQ(1U, prefix_hits.size());
+  EXPECT_EQ(prefix_hits[0], SBPrefixForString("www.evil.com/evil.html"));
 
   // Attempt to re-add the first chunk (should be a no-op).
   // see bug: http://code.google.com/p/chromium/issues/detail?id=4522
@@ -584,7 +601,7 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
 
   GetListsInfo(&lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
-  EXPECT_EQ(lists[0].adds, "1-3");
+  EXPECT_EQ(lists[0].adds, "1-3,7");
   EXPECT_TRUE(lists[0].subs.empty());
 
   // Test removing a single prefix from the add chunk.
@@ -600,26 +617,26 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_EQ(prefix_hits[0], SBPrefixForString("www.evil.com/phishing.html"));
   EXPECT_EQ(prefix_hits.size(), 1U);
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/notevil1.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_TRUE(prefix_hits.empty());
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/notevil2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/good1.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/good2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   GetListsInfo(&lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
@@ -648,19 +665,19 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/notevil2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/good1.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/good2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   GetListsInfo(&lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
-  EXPECT_EQ(lists[0].adds, "1,3");
+  EXPECT_EQ(lists[0].adds, "1,3,7");
   EXPECT_EQ(lists[0].subs, "4");
 
   // The adddel command exposed a bug in the transaction code where any
@@ -684,7 +701,7 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
 
   GetListsInfo(&lists);
   EXPECT_TRUE(lists[0].name == safe_browsing_util::kMalwareList);
-  EXPECT_EQ(lists[0].adds, "1,3");
+  EXPECT_EQ(lists[0].adds, "1,3,7");
   EXPECT_EQ(lists[0].subs, "");
 
   // Test a sub command coming in before the add.
@@ -701,7 +718,7 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.notevilanymore.com/index.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   // Now insert the tardy add chunk and we don't expect them to appear
   // in database because of the previous sub chunk.
@@ -717,11 +734,34 @@ TEST_F(SafeBrowsingDatabaseTest, BrowseDatabase) {
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.notevilanymore.com/index.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.notevilanymore.com/good.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
+
+  // Reset and reload the database.  The database will rely on the prefix set.
+  database_.reset(new SafeBrowsingDatabaseNew);
+  database_->Init(database_filename_);
+
+  // Check that a prefix still hits.
+  EXPECT_TRUE(database_->ContainsBrowseUrl(
+      GURL("http://www.evil.com/phishing.html"),
+      &prefix_hits, &cached_hashes, now));
+  EXPECT_EQ(prefix_hits[0], SBPrefixForString("www.evil.com/phishing.html"));
+  EXPECT_EQ(prefix_hits.size(), 1U);
+
+  // Also check that it's not just always returning true in this case.
+  EXPECT_FALSE(database_->ContainsBrowseUrl(
+      GURL("http://www.evil.com/"),
+      &prefix_hits, &cached_hashes, now));
+
+  // Check that the full hash is still present.
+  EXPECT_TRUE(database_->ContainsBrowseUrl(
+      GURL("http://www.evil.com/evil.html"),
+      &prefix_hits, &cached_hashes, now));
+  ASSERT_EQ(1U, prefix_hits.size());
+  EXPECT_EQ(prefix_hits[0], SBPrefixForString("www.evil.com/evil.html"));
 }
 
 
@@ -793,14 +833,14 @@ TEST_F(SafeBrowsingDatabaseTest, ZeroSizeChunk) {
   database_->UpdateFinished(true);
 
   const Time now = Time::Now();
-  std::vector<SBFullHashResult> full_hashes;
+  std::vector<SBFullHashResult> cached_hashes;
   std::vector<SBPrefix> prefix_hits;
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.notempty.com/full1.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.notempty.com/full2.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   GetListsInfo(&lists);
   EXPECT_EQ(lists[0].adds, "1,10,19-22");
@@ -862,29 +902,29 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
 
   // Test the cache lookup for the first prefix.
   std::vector<SBPrefix> prefixes;
-  std::vector<SBFullHashResult> full_hashes;
+  std::vector<SBFullHashResult> cached_hashes;
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
-      &prefixes, &full_hashes, Time::Now());
-  ASSERT_EQ(1U, full_hashes.size());
+      &prefixes, &cached_hashes, Time::Now());
+  ASSERT_EQ(1U, cached_hashes.size());
   EXPECT_TRUE(
-      SBFullHashEqual(full_hashes[0].hash,
+      SBFullHashEqual(cached_hashes[0].hash,
                       SBFullHashForString("www.evil.com/phishing.html")));
 
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Test the cache lookup for the second prefix.
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefixes, &full_hashes, Time::Now());
-  ASSERT_EQ(1U, full_hashes.size());
+      &prefixes, &cached_hashes, Time::Now());
+  ASSERT_EQ(1U, cached_hashes.size());
   EXPECT_TRUE(
-      SBFullHashEqual(full_hashes[0].hash,
+      SBFullHashEqual(cached_hashes[0].hash,
                       SBFullHashForString("www.evil.com/malware.html")));
 
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Test removing a prefix via a sub chunk.
   SBChunk chunk;
@@ -901,21 +941,21 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   // This prefix should still be there, but the fullhash is gone.
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefixes, &full_hashes, Time::Now()));
+      &prefixes, &cached_hashes, Time::Now()));
   ASSERT_EQ(1U, prefixes.size());
   EXPECT_EQ(SBPrefixForString("www.evil.com/malware.html"), prefixes[0]);
-  EXPECT_TRUE(full_hashes.empty());
+  EXPECT_TRUE(cached_hashes.empty());
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // This prefix should be gone.
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
-      &prefixes, &full_hashes, Time::Now());
-  EXPECT_TRUE(full_hashes.empty());
+      &prefixes, &cached_hashes, Time::Now());
+  EXPECT_TRUE(cached_hashes.empty());
 
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Test that an AddDel for the original chunk removes the last cached entry.
   EXPECT_TRUE(database_->UpdateStarted(&lists));
@@ -923,13 +963,12 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   database_->UpdateFinished(true);
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefixes, &full_hashes, Time::Now());
-  EXPECT_TRUE(full_hashes.empty());
-  EXPECT_TRUE(database_->full_browse_hashes_.empty());
+      &prefixes, &cached_hashes, Time::Now());
+  EXPECT_TRUE(cached_hashes.empty());
   EXPECT_TRUE(database_->cached_browse_hashes_.empty());
 
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Test that the cache won't return expired values. First we have to adjust
   // the cached entries' received time to make them older, since the database
@@ -953,14 +992,14 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
 
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefixes, &full_hashes, expired);
-  EXPECT_TRUE(full_hashes.empty());
+      &prefixes, &cached_hashes, expired);
+  EXPECT_TRUE(cached_hashes.empty());
 
   // This entry should still exist.
   database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
-      &prefixes, &full_hashes, expired);
-  EXPECT_EQ(1U, full_hashes.size());
+      &prefixes, &cached_hashes, expired);
+  EXPECT_EQ(1U, cached_hashes.size());
 
   // Testing prefix miss caching. First, we clear out the existing database,
   // Since PopulateDatabaseForCacheTest() doesn't handle adding duplicate
@@ -989,17 +1028,17 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
   // in the database, it is flagged as a miss so looking up the associated URL
   // will not succeed.
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
   prefix_misses.clear();
   empty_full_hash.clear();
   prefix_misses.push_back(SBPrefixForString("www.evil.com/phishing.html"));
   database_->CacheHashResults(prefix_misses, empty_full_hash);
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing.html"),
-      &prefixes, &full_hashes, Time::Now()));
+      &prefixes, &cached_hashes, Time::Now()));
 
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Test receiving a full add chunk.
   chunk.hosts.clear();
@@ -1014,23 +1053,21 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.fullevil.com/bad1.html"),
-      &prefixes, &full_hashes, Time::Now()));
-  ASSERT_EQ(1U, full_hashes.size());
-  EXPECT_TRUE(
-      SBFullHashEqual(full_hashes[0].hash,
-                      SBFullHashForString("www.fullevil.com/bad1.html")));
+      &prefixes, &cached_hashes, Time::Now()));
+  EXPECT_TRUE(cached_hashes.empty());
+  ASSERT_EQ(1U, prefixes.size());
+  EXPECT_EQ(SBPrefixForString("www.fullevil.com/bad1.html"), prefixes[0]);
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.fullevil.com/bad2.html"),
-      &prefixes, &full_hashes, Time::Now()));
-  ASSERT_EQ(1U, full_hashes.size());
-  EXPECT_TRUE(
-      SBFullHashEqual(full_hashes[0].hash,
-                      SBFullHashForString("www.fullevil.com/bad2.html")));
+      &prefixes, &cached_hashes, Time::Now()));
+  EXPECT_TRUE(cached_hashes.empty());
+  ASSERT_EQ(1U, prefixes.size());
+  EXPECT_EQ(SBPrefixForString("www.fullevil.com/bad2.html"), prefixes[0]);
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Test receiving a full sub chunk, which will remove one of the full adds.
   chunk.hosts.clear();
@@ -1045,19 +1082,18 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.fullevil.com/bad1.html"),
-      &prefixes, &full_hashes, Time::Now()));
-  EXPECT_TRUE(full_hashes.empty());
+      &prefixes, &cached_hashes, Time::Now()));
+  EXPECT_TRUE(cached_hashes.empty());
 
   // There should be one remaining full add.
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.fullevil.com/bad2.html"),
-      &prefixes, &full_hashes, Time::Now()));
-  ASSERT_EQ(1U, full_hashes.size());
-  EXPECT_TRUE(
-      SBFullHashEqual(full_hashes[0].hash,
-                      SBFullHashForString("www.fullevil.com/bad2.html")));
+      &prefixes, &cached_hashes, Time::Now()));
+  EXPECT_TRUE(cached_hashes.empty());
+  ASSERT_EQ(1U, prefixes.size());
+  EXPECT_EQ(SBPrefixForString("www.fullevil.com/bad2.html"), prefixes[0]);
   prefixes.clear();
-  full_hashes.clear();
+  cached_hashes.clear();
 
   // Now test an AddDel for the remaining full add.
   EXPECT_TRUE(database_->UpdateStarted(&lists));
@@ -1066,10 +1102,47 @@ TEST_F(SafeBrowsingDatabaseTest, HashCaching) {
 
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.fullevil.com/bad1.html"),
-      &prefixes, &full_hashes, Time::Now()));
+      &prefixes, &cached_hashes, Time::Now()));
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.fullevil.com/bad2.html"),
-      &prefixes, &full_hashes, Time::Now()));
+      &prefixes, &cached_hashes, Time::Now()));
+
+  // Add a fullhash which has a prefix collision for a known url.
+  static const char kExampleFine[] = "www.example.com/fine.html";
+  static const char kExampleCollision[] =
+      "www.example.com/3123364814/malware.htm";
+  ASSERT_EQ(SBPrefixForString(kExampleFine),
+            SBPrefixForString(kExampleCollision));
+  EXPECT_TRUE(database_->UpdateStarted(&lists));
+  {
+    SBChunkList chunks;
+    SBChunk chunk;
+    InsertAddChunkHostPrefixUrl(&chunk, 21, "www.example.com/",
+                                kExampleCollision);
+    chunks.push_back(chunk);
+    database_->InsertChunks(safe_browsing_util::kMalwareList, chunks);
+  }
+  database_->UpdateFinished(true);
+
+  // Cache gethash response for |kExampleCollision|.
+  {
+    SBFullHashResult result;
+    result.hash = SBFullHashForString(kExampleCollision);
+    result.list_id = safe_browsing_util::MALWARE;
+    database_->CacheHashResults(std::vector<SBPrefix>(1, result.hash.prefix),
+                                std::vector<SBFullHashResult>(1, result));
+  }
+
+  // Expect a prefix hit due to the collision between |kExampleFine| and
+  // |kExampleCollision|, with the gethash showing only |kExampleCollision|.
+  EXPECT_TRUE(database_->ContainsBrowseUrl(
+      GURL(std::string("http://") + kExampleFine),
+      &prefixes, &cached_hashes, Time::Now()));
+  ASSERT_EQ(1U, prefixes.size());
+  EXPECT_EQ(SBPrefixForString(kExampleFine), prefixes[0]);
+  ASSERT_EQ(1U, cached_hashes.size());
+  EXPECT_TRUE(SBFullHashEqual(cached_hashes[0].hash,
+                              SBFullHashForString(kExampleCollision)));
 }
 
 // Test that corrupt databases are appropriately handled, even if the
@@ -1517,21 +1590,21 @@ TEST_F(SafeBrowsingDatabaseTest, SameHostEntriesOkay) {
 
   const Time now = Time::Now();
   std::vector<SBPrefix> prefixes;
-  std::vector<SBFullHashResult> full_hashes;
+  std::vector<SBFullHashResult> cached_hashes;
   std::vector<SBPrefix> prefix_hits;
 
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware1.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware2.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing1.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing2.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
 
   // Test removing a single prefix from the add chunk.
   // Remove the prefix that added first.
@@ -1557,16 +1630,16 @@ TEST_F(SafeBrowsingDatabaseTest, SameHostEntriesOkay) {
   // Verify that the database contains urls expected.
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware1.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware2.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing1.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/phishing2.html"),
-      &prefixes, &full_hashes, now));
+      &prefixes, &cached_hashes, now));
 }
 
 // Test that an empty update doesn't actually update the database.
@@ -1653,14 +1726,14 @@ TEST_F(SafeBrowsingDatabaseTest, FilterFile) {
 
   // Find the malware url in the database, don't find a good url.
   const Time now = Time::Now();
-  std::vector<SBFullHashResult> full_hashes;
+  std::vector<SBFullHashResult> cached_hashes;
   std::vector<SBPrefix> prefix_hits;
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/goodware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   base::FilePath filter_file = database_->PrefixSetForFilename(
       database_->BrowseDBFilename(database_filename_));
@@ -1672,10 +1745,10 @@ TEST_F(SafeBrowsingDatabaseTest, FilterFile) {
   database_->Init(database_filename_);
   EXPECT_TRUE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/goodware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 
   // If there is no filter file, the database cannot find malware urls.
   base::DeleteFile(filter_file, false);
@@ -1684,10 +1757,10 @@ TEST_F(SafeBrowsingDatabaseTest, FilterFile) {
   database_->Init(database_filename_);
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.evil.com/malware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
   EXPECT_FALSE(database_->ContainsBrowseUrl(
       GURL("http://www.good.com/goodware.html"),
-      &prefix_hits, &full_hashes, now));
+      &prefix_hits, &cached_hashes, now));
 }
 
 TEST_F(SafeBrowsingDatabaseTest, MalwareIpBlacklist) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 83762de..3bbea4d 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -118,12 +118,12 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
   // otherwise it returns false.
   virtual bool ContainsBrowseUrl(const GURL& url,
                                  std::vector<SBPrefix>* prefix_hits,
-                                 std::vector<SBFullHashResult>* full_hits,
+                                 std::vector<SBFullHashResult>* cached_hits,
                                  base::Time last_update) OVERRIDE {
     std::vector<GURL> urls(1, url);
     return ContainsUrl(safe_browsing_util::MALWARE,
                        safe_browsing_util::PHISH,
-                       urls, prefix_hits, full_hits);
+                       urls, prefix_hits, cached_hits);
   }
   virtual bool ContainsDownloadUrl(
       const std::vector<GURL>& urls,
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 0044cec..55437ee 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
@@ -144,7 +144,7 @@ TEST_F(SafeBrowsingStoreFileTest, Empty) {
   EXPECT_TRUE(add_full_hashes_result.empty());
 
   std::vector<SBPrefix> prefixes_result;
-  builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+  builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
   EXPECT_TRUE(prefixes_result.empty());
 }
 
@@ -181,7 +181,7 @@ TEST_F(SafeBrowsingStoreFileTest, BasicStore) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(3U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
     EXPECT_EQ(kHash5.prefix, prefixes_result[1]);
@@ -214,7 +214,7 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(4U, prefixes_result.size());
     EXPECT_EQ(kMinSBPrefix, prefixes_result[0]);
     EXPECT_EQ(kHash1.prefix, prefixes_result[1]);
@@ -236,7 +236,7 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(2U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
     EXPECT_EQ(kHash2.prefix, prefixes_result[1]);
@@ -273,7 +273,7 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
 
     // Knocked out the chunk expected.
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(1U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
 
@@ -297,7 +297,7 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(1U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
 
@@ -321,7 +321,7 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(2U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
     EXPECT_EQ(kHash3.prefix, prefixes_result[1]);
@@ -391,7 +391,7 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(1U, prefixes_result.size());
     EXPECT_EQ(kHash3.prefix, prefixes_result[0]);
 
@@ -420,7 +420,7 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     EXPECT_TRUE(prefixes_result.empty());
     EXPECT_TRUE(add_full_hashes_result.empty());
   }
@@ -439,7 +439,7 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
     EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
 
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     EXPECT_TRUE(prefixes_result.empty());
     EXPECT_TRUE(add_full_hashes_result.empty());
   }
@@ -496,7 +496,7 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
     safe_browsing::PrefixSetBuilder builder;
     ASSERT_TRUE(store_->BeginUpdate());
     EXPECT_TRUE(store_->FinishUpdate(&builder, &orig_hashes));
-    builder.GetPrefixSet()->GetPrefixes(&orig_prefixes);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&orig_prefixes);
     EXPECT_GT(orig_prefixes.size(), 0U);
     EXPECT_GT(orig_hashes.size(), 0U);
     EXPECT_FALSE(corruption_detected_);
@@ -812,7 +812,7 @@ TEST_F(SafeBrowsingStoreFileTest, Version7) {
 
     // The sub'ed prefix and hash are gone.
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(1U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
     EXPECT_TRUE(add_full_hashes_result.empty());
@@ -888,7 +888,7 @@ TEST_F(SafeBrowsingStoreFileTest, Version8) {
 
     // The sub'ed prefix and hash are gone.
     std::vector<SBPrefix> prefixes_result;
-    builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
+    builder.GetPrefixSetNoHashes()->GetPrefixes(&prefixes_result);
     ASSERT_EQ(1U, prefixes_result.size());
     EXPECT_EQ(kHash1.prefix, prefixes_result[0]);
     EXPECT_TRUE(add_full_hashes_result.empty());
diff --git a/chrome/browser/safe_browsing/safe_browsing_util.h b/chrome/browser/safe_browsing/safe_browsing_util.h
index 3304989..f2f2d0e 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util.h
+++ b/chrome/browser/safe_browsing/safe_browsing_util.h
@@ -40,6 +40,10 @@ inline bool SBFullHashEqual(const SBFullHash& a, const SBFullHash& b) {
   return !memcmp(a.full_hash, b.full_hash, sizeof(a.full_hash));
 }
 
+inline bool SBFullHashLess(const SBFullHash& a, const SBFullHash& b) {
+  return memcmp(a.full_hash, b.full_hash, sizeof(a.full_hash)) < 0;
+}
+
 // Generate full hash for the given string.
 SBFullHash SBFullHashForString(const base::StringPiece& str);
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_util_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_util_unittest.cc
index 14f0bbf..d12586f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_util_unittest.cc
@@ -341,6 +341,12 @@ TEST(SafeBrowsingUtilTest, FullHashOperators) {
   EXPECT_TRUE(SBFullHashEqual(kHash2, kHash2));
   EXPECT_FALSE(SBFullHashEqual(kHash1, kHash2));
   EXPECT_FALSE(SBFullHashEqual(kHash2, kHash1));
+
+  EXPECT_FALSE(SBFullHashLess(kHash1, kHash2));
+  EXPECT_TRUE(SBFullHashLess(kHash2, kHash1));
+
+  EXPECT_FALSE(SBFullHashLess(kHash1, kHash1));
+  EXPECT_FALSE(SBFullHashLess(kHash2, kHash2));
 }
 
 }  // namespace