Drop duplicate SBPrefix from full-hash elements.

SafeBrowsingDatabaseBloom stores a integer prefix column and a blob
hash column, where the first four bytes of the hash are the prefix.
This isn't necessary, so getting rid of the duplication rather than
propagating it into the future.

[Previously this was left alone because it wasn't too harmful because
the code could always drop the duplication from the file format.  A
future change for SafeBrowsingStoreFile will write an array of hashes,
making the duplication unavoidable.]

BUG=none
TEST=none

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

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

7 files changed, 97 insertions, 125 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_store.h b/chrome/browser/safe_browsing/safe_browsing_store.h
index 8ef8574..34aa4b2 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store.h
@@ -50,48 +50,46 @@ struct SBAddPrefix {
 
 struct SBSubPrefix {
   int32 chunk_id;
-  SBAddPrefix add_prefix;
+  int32 add_chunk_id;
+  SBPrefix add_prefix;
 
   SBSubPrefix(int32 id, int32 add_id, int prefix)
-      : chunk_id(id), add_prefix(add_id, prefix) {}
+      : chunk_id(id), add_chunk_id(add_id), add_prefix(prefix) {}
 
-  int32 GetAddChunkId() const { return add_prefix.chunk_id; }
-  SBPrefix GetAddPrefix() const { return add_prefix.prefix; }
+  int32 GetAddChunkId() const { return add_chunk_id; }
+  SBPrefix GetAddPrefix() const { return add_prefix; }
 };
 
-// TODO(shess): The full_hash includes the prefix, so the prefix could
-// be dropped.  But SBAddPrefix is convenient for comparing across
-// different structs, and there aren't many full hashes.  Hmm.
 struct SBAddFullHash {
-  SBAddPrefix add_prefix;
+  int32 chunk_id;
   int32 received;
   SBFullHash full_hash;
 
-  SBAddFullHash(int32 id, SBPrefix p, base::Time r, SBFullHash h)
-      : add_prefix(id, p),
+  SBAddFullHash(int32 id, base::Time r, SBFullHash h)
+      : chunk_id(id),
         received(static_cast<int32>(r.ToTimeT())),
         full_hash(h) {
   }
 
   // Provided for ReadAddHashes() implementations, which already have
   // an int32 for the time.
-  SBAddFullHash(int32 id, SBPrefix p, int32 r, SBFullHash h)
-      : add_prefix(id, p), received(r), full_hash(h) {}
+  SBAddFullHash(int32 id, int32 r, SBFullHash h)
+      : chunk_id(id), received(r), full_hash(h) {}
 
-  int32 GetAddChunkId() const { return add_prefix.chunk_id; }
-  SBPrefix GetAddPrefix() const { return add_prefix.prefix; }
+  int32 GetAddChunkId() const { return chunk_id; }
+  SBPrefix GetAddPrefix() const { return full_hash.prefix; }
 };
 
 struct SBSubFullHash {
   int32 chunk_id;
-  SBAddPrefix add_prefix;
+  int32 add_chunk_id;
   SBFullHash full_hash;
 
-  SBSubFullHash(int32 id, int32 add_id, SBPrefix p, SBFullHash h)
-      : chunk_id(id), add_prefix(add_id, p), full_hash(h) {}
+  SBSubFullHash(int32 id, int32 add_id, SBFullHash h)
+      : chunk_id(id), add_chunk_id(add_id), full_hash(h) {}
 
-  int32 GetAddChunkId() const { return add_prefix.chunk_id; }
-  SBPrefix GetAddPrefix() const { return add_prefix.prefix; }
+  int32 GetAddChunkId() const { return add_chunk_id; }
+  SBPrefix GetAddPrefix() const { return full_hash.prefix; }
 };
 
 // Determine less-than based on add chunk and prefix.
@@ -173,12 +171,12 @@ class SafeBrowsingStore {
   virtual bool BeginChunk() = 0;
 
   virtual bool WriteAddPrefix(int32 chunk_id, SBPrefix prefix) = 0;
-  virtual bool WriteAddHash(int32 chunk_id, SBPrefix prefix,
+  virtual bool WriteAddHash(int32 chunk_id,
                             base::Time receive_time, SBFullHash full_hash) = 0;
   virtual bool WriteSubPrefix(int32 chunk_id,
                               int32 add_chunk_id, SBPrefix prefix) = 0;
   virtual bool WriteSubHash(int32 chunk_id, int32 add_chunk_id,
-                            SBPrefix prefix, SBFullHash full_hash) = 0;
+                            SBFullHash full_hash) = 0;
 
   // Collect the chunk data and preferrably store it on disk to
   // release memory.  Shoul not modify the data in-place.
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.cc b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
index a901e33..2e90f4f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
@@ -204,8 +204,8 @@ bool SafeBrowsingStoreFile::WriteSubPrefixes(
   for (std::vector<SBSubPrefix>::const_iterator iter = sub_prefixes.begin();
        iter != sub_prefixes.end(); ++iter) {
     if (!WriteInt32(new_file_.get(), iter->chunk_id) ||
-        !WriteInt32(new_file_.get(), iter->add_prefix.chunk_id) ||
-        !WriteInt32(new_file_.get(), iter->add_prefix.prefix))
+        !WriteInt32(new_file_.get(), iter->add_chunk_id) ||
+        !WriteInt32(new_file_.get(), iter->add_prefix))
       return false;
   }
   return true;
@@ -219,13 +219,10 @@ bool SafeBrowsingStoreFile::ReadAddHashes(
 
   for (int i = 0; i < count; ++i) {
     int32 chunk_id;
-    SBPrefix prefix;
     int32 received;
     SBFullHash full_hash;
-    DCHECK_EQ(sizeof(int32), sizeof(prefix));
 
     if (!ReadInt32(fp, &chunk_id) ||
-        !ReadInt32(fp, &prefix) ||
         !ReadInt32(fp, &received) ||
         !ReadHash(fp, &full_hash))
       return false;
@@ -233,7 +230,7 @@ bool SafeBrowsingStoreFile::ReadAddHashes(
     if (add_del_cache_.count(chunk_id) > 0)
       continue;
 
-    add_hashes->push_back(SBAddFullHash(chunk_id, prefix, received, full_hash));
+    add_hashes->push_back(SBAddFullHash(chunk_id, received, full_hash));
   }
 
   return true;
@@ -245,8 +242,7 @@ bool SafeBrowsingStoreFile::WriteAddHashes(
 
   for (std::vector<SBAddFullHash>::const_iterator iter = add_hashes.begin();
        iter != add_hashes.end(); ++iter) {
-    if (!WriteInt32(new_file_.get(), iter->add_prefix.chunk_id) ||
-        !WriteInt32(new_file_.get(), iter->add_prefix.prefix) ||
+    if (!WriteInt32(new_file_.get(), iter->chunk_id) ||
         !WriteInt32(new_file_.get(), iter->received) ||
         !WriteHash(new_file_.get(), iter->full_hash))
       return false;
@@ -263,21 +259,17 @@ bool SafeBrowsingStoreFile::ReadSubHashes(
   for (int i = 0; i < count; ++i) {
     int32 chunk_id;
     int32 add_chunk_id;
-    SBPrefix add_prefix;
     SBFullHash add_full_hash;
-    DCHECK_EQ(sizeof(int32), sizeof(add_prefix));
 
     if (!ReadInt32(fp, &chunk_id) ||
         !ReadInt32(fp, &add_chunk_id) ||
-        !ReadInt32(fp, &add_prefix) ||
         !ReadHash(fp, &add_full_hash))
       return false;
 
     if (sub_del_cache_.count(chunk_id) > 0)
       continue;
 
-    sub_hashes->push_back(
-        SBSubFullHash(chunk_id, add_chunk_id, add_prefix, add_full_hash));
+    sub_hashes->push_back(SBSubFullHash(chunk_id, add_chunk_id, add_full_hash));
   }
 
   return true;
@@ -290,8 +282,7 @@ bool SafeBrowsingStoreFile::WriteSubHashes(
   for (std::vector<SBSubFullHash>::const_iterator iter = sub_hashes.begin();
        iter != sub_hashes.end(); ++iter) {
     if (!WriteInt32(new_file_.get(), iter->chunk_id) ||
-        !WriteInt32(new_file_.get(), iter->add_prefix.chunk_id) ||
-        !WriteInt32(new_file_.get(), iter->add_prefix.prefix) ||
+        !WriteInt32(new_file_.get(), iter->add_chunk_id) ||
         !WriteHash(new_file_.get(), iter->full_hash))
       return false;
   }
@@ -461,7 +452,7 @@ bool SafeBrowsingStoreFile::DoUpdate(
   // Add the pending adds which haven't since been deleted.
   for (std::vector<SBAddFullHash>::const_iterator iter = pending_adds.begin();
        iter != pending_adds.end(); ++iter) {
-    if (add_del_cache_.count(iter->add_prefix.chunk_id) == 0)
+    if (add_del_cache_.count(iter->chunk_id) == 0)
       add_full_hashes.push_back(*iter);
   }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.h b/chrome/browser/safe_browsing/safe_browsing_store_file.h
index 6ab5940..2ee9bdf 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.h
@@ -44,8 +44,6 @@
 // }
 // array[add_hash_count] {
 //   int32 chunk_id;
-//   // TODO(shess): This duplicates first four bytes of full_hash!
-//   int32 prefix;
 //   // From base::Time::ToTimeT().  This data should never last long
 //   // enough for 32 bits to be a problem.
 //   int32 received_time;
@@ -53,7 +51,6 @@
 // array[sub_hash_count] {
 //   int32 chunk_id;
 //   int32 add_chunk_id;
-//   int32 add_prefix;
 //   char[32] add_full_hash;
 // }
 // TODO(shess): Would a checksum be worthwhile?  If so, check at open,
@@ -141,10 +138,9 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
     add_prefixes_.push_back(SBAddPrefix(chunk_id, prefix));
     return true;
   }
-  virtual bool WriteAddHash(int32 chunk_id, SBPrefix prefix,
+  virtual bool WriteAddHash(int32 chunk_id,
                             base::Time receive_time, SBFullHash full_hash) {
-    add_hashes_.push_back(
-        SBAddFullHash(chunk_id, prefix, receive_time, full_hash));
+    add_hashes_.push_back(SBAddFullHash(chunk_id, receive_time, full_hash));
     return true;
   }
   virtual bool WriteSubPrefix(int32 chunk_id,
@@ -153,9 +149,8 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
     return true;
   }
   virtual bool WriteSubHash(int32 chunk_id, int32 add_chunk_id,
-                            SBPrefix prefix, SBFullHash full_hash) {
-    sub_hashes_.push_back(
-        SBSubFullHash(chunk_id, add_chunk_id, prefix, full_hash));
+                            SBFullHash full_hash) {
+    sub_hashes_.push_back(SBSubFullHash(chunk_id, add_chunk_id, full_hash));
     return true;
   }
   virtual bool FinishChunk();
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_sqlite.cc b/chrome/browser/safe_browsing/safe_browsing_store_sqlite.cc
index 91de566..98fc2d0 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_sqlite.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_sqlite.cc
@@ -426,8 +426,8 @@ bool SafeBrowsingStoreSqlite::WriteSubPrefixes(
   for (std::vector<SBSubPrefix>::const_iterator iter = sub_prefixes.begin();
        iter != sub_prefixes.end(); ++iter) {
     statement->bind_int(0, iter->chunk_id);
-    statement->bind_int(1, iter->add_prefix.chunk_id);
-    statement->bind_int(2, iter->add_prefix.prefix);
+    statement->bind_int(1, iter->add_chunk_id);
+    statement->bind_int(2, iter->add_prefix);
     int rv = statement->step();
     if (rv == SQLITE_CORRUPT)
       return OnCorruptDatabase();
@@ -455,10 +455,12 @@ bool SafeBrowsingStoreSqlite::ReadAddHashes(
     if (add_del_cache_.count(chunk_id) > 0)
       continue;
 
+    // NOTE: Legacy format duplicated |hash.prefix| in column 1.
     const SBPrefix prefix = statement->column_int(1);
     const int32 received = statement->column_int(2);
     const SBFullHash full_hash = ReadFullHash(&statement, 3);
-    add_hashes->push_back(SBAddFullHash(chunk_id, prefix, received, full_hash));
+    DCHECK_EQ(prefix, full_hash.prefix);
+    add_hashes->push_back(SBAddFullHash(chunk_id, received, full_hash));
   }
   if (rv == SQLITE_CORRUPT)
     return OnCorruptDatabase();
@@ -481,8 +483,9 @@ bool SafeBrowsingStoreSqlite::WriteAddHashes(
 
   for (std::vector<SBAddFullHash>::const_iterator iter = add_hashes.begin();
        iter != add_hashes.end(); ++iter) {
-    statement->bind_int(0, iter->add_prefix.chunk_id);
-    statement->bind_int(1, iter->add_prefix.prefix);
+    // NOTE: Legacy format duplicated |hash.prefix| in column 1.
+    statement->bind_int(0, iter->chunk_id);
+    statement->bind_int(1, iter->full_hash.prefix);
     statement->bind_int(2, iter->received);
     statement->bind_blob(3, iter->full_hash.full_hash,
                          sizeof(iter->full_hash.full_hash));
@@ -513,11 +516,12 @@ bool SafeBrowsingStoreSqlite::ReadSubHashes(
     if (sub_del_cache_.count(chunk_id) > 0)
       continue;
 
+    // NOTE: Legacy format duplicated |hash.prefix| in column 2.
     const int32 add_chunk_id = statement->column_int(1);
     const SBPrefix add_prefix = statement->column_int(2);
     const SBFullHash full_hash = ReadFullHash(&statement, 3);
-    sub_hashes->push_back(
-        SBSubFullHash(chunk_id, add_chunk_id, add_prefix, full_hash));
+    DCHECK_EQ(add_prefix, full_hash.prefix);
+    sub_hashes->push_back(SBSubFullHash(chunk_id, add_chunk_id, full_hash));
   }
   if (rv == SQLITE_CORRUPT)
     return OnCorruptDatabase();
@@ -540,9 +544,10 @@ bool SafeBrowsingStoreSqlite::WriteSubHashes(
 
   for (std::vector<SBSubFullHash>::const_iterator iter = sub_hashes.begin();
        iter != sub_hashes.end(); ++iter) {
+    // NOTE: Legacy format duplicated |hash.prefix| in column 2.
     statement->bind_int(0, iter->chunk_id);
-    statement->bind_int(1, iter->add_prefix.chunk_id);
-    statement->bind_int(2, iter->add_prefix.prefix);
+    statement->bind_int(1, iter->add_chunk_id);
+    statement->bind_int(2, iter->full_hash.prefix);
     statement->bind_blob(3, iter->full_hash.full_hash,
                          sizeof(iter->full_hash.full_hash));
     int rv = statement->step();
@@ -621,7 +626,7 @@ bool SafeBrowsingStoreSqlite::DoUpdate(
   // Add the pending adds which haven't since been deleted.
   for (std::vector<SBAddFullHash>::const_iterator iter = pending_adds.begin();
        iter != pending_adds.end(); ++iter) {
-    if (add_del_cache_.count(iter->add_prefix.chunk_id) == 0)
+    if (add_del_cache_.count(iter->chunk_id) == 0)
       add_full_hashes.push_back(*iter);
   }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h b/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h
index 6c00aed..5e03d50 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h
@@ -34,10 +34,10 @@ class SafeBrowsingStoreSqlite : public SafeBrowsingStore {
     const std::vector<SBAddPrefix> prefixes(1, SBAddPrefix(chunk_id, prefix));
     return WriteAddPrefixes(prefixes);
   }
-  virtual bool WriteAddHash(int32 chunk_id, SBPrefix prefix,
+  virtual bool WriteAddHash(int32 chunk_id,
                             base::Time receive_time, SBFullHash full_hash) {
     const std::vector<SBAddFullHash>
-        hashes(1, SBAddFullHash(chunk_id, prefix, receive_time, full_hash));
+        hashes(1, SBAddFullHash(chunk_id, receive_time, full_hash));
     return WriteAddHashes(hashes);
   }
   virtual bool WriteSubPrefix(int32 chunk_id,
@@ -47,9 +47,9 @@ class SafeBrowsingStoreSqlite : public SafeBrowsingStore {
     return WriteSubPrefixes(prefixes);
   }
   virtual bool WriteSubHash(int32 chunk_id, int32 add_chunk_id,
-                            SBPrefix prefix, SBFullHash full_hash) {
+                            SBFullHash full_hash) {
     const std::vector<SBSubFullHash>
-        hashes(1, SBSubFullHash(chunk_id, add_chunk_id, prefix, full_hash));
+        hashes(1, SBSubFullHash(chunk_id, add_chunk_id, full_hash));
     return WriteSubHashes(hashes);
   }
   virtual bool FinishChunk() {
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc
index 5ba0149..76f0e91 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc
@@ -38,28 +38,26 @@ TEST(SafeBrowsingStoreTest, SBAddPrefixHashLess) {
   const base::Time now = base::Time::Now();
 
   // add_id dominates.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, two.prefix, now, two),
-                                  SBAddFullHash(11, one.prefix, now, one)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(11, two.prefix, now, two),
-                                   SBAddFullHash(10, one.prefix, now, one)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, now, two),
+                                  SBAddFullHash(11, now, one)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(11, now, two),
+                                   SBAddFullHash(10, now, one)));
 
   // After add_id, prefix.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, one.prefix, now, two),
-                                  SBAddFullHash(10, two.prefix, now, one)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, two.prefix, now, one),
-                                   SBAddFullHash(10, one.prefix, now, two)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, now, one),
+                                  SBAddFullHash(10, now, two)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, now, two),
+                                   SBAddFullHash(10, now, one)));
 
   // After prefix, full hash.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, one.prefix, now, one),
-                                  SBAddFullHash(10, onetwo.prefix,
-                                                now, onetwo)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, onetwo.prefix,
-                                                 now, onetwo),
-                                   SBAddFullHash(10, one.prefix, now, one)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, now, one),
+                                  SBAddFullHash(10, now, onetwo)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, now, onetwo),
+                                   SBAddFullHash(10, now, one)));
 
   // Equal is not less-than.
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, one.prefix, now, one),
-                                   SBAddFullHash(10, one.prefix, now, one)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, now, one),
+                                   SBAddFullHash(10, now, one)));
 }
 
 TEST(SafeBrowsingStoreTest, SBSubPrefixLess) {
@@ -89,26 +87,26 @@ TEST(SafeBrowsingStoreTest, SBSubFullHashLess) {
   two.prefix = 2;
 
   // add_id dominates.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBSubFullHash(12, 10, two.prefix, two),
-                                  SBSubFullHash(9, 11, one.prefix, one)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 11, two.prefix, two),
-                                   SBSubFullHash(9, 10, one.prefix, one)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBSubFullHash(12, 10, two),
+                                  SBSubFullHash(9, 11, one)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 11, two),
+                                   SBSubFullHash(9, 10, one)));
 
   // After add_id, prefix.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBSubFullHash(12, 10, one.prefix, two),
-                                  SBSubFullHash(9, 10, two.prefix, one)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 10, two.prefix, one),
-                                   SBSubFullHash(9, 10, one.prefix, two)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBSubFullHash(12, 10, one),
+                                  SBSubFullHash(9, 10, two)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 10, two),
+                                   SBSubFullHash(9, 10, one)));
 
   // After prefix, full_hash.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBSubFullHash(12, 10, one.prefix, one),
-                                  SBSubFullHash(9, 10, onetwo.prefix, onetwo)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 10, onetwo.prefix, onetwo),
-                                   SBSubFullHash(9, 10, one.prefix, one)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBSubFullHash(12, 10, one),
+                                  SBSubFullHash(9, 10, onetwo)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 10, onetwo),
+                                   SBSubFullHash(9, 10, one)));
 
   // Equal is not less-than.
-  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 10, one.prefix, one),
-                                   SBSubFullHash(9, 10, one.prefix, one)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBSubFullHash(12, 10, one),
+                                   SBSubFullHash(9, 10, one)));
 }
 
 TEST(SafeBrowsingStoreTest, SBProcessSubs) {
@@ -143,23 +141,18 @@ TEST(SafeBrowsingStoreTest, SBProcessSubs) {
   // An add with prefix and a couple hashes, plus a sub for the prefix
   // and a couple sub hashes.  The sub should knock all of them out.
   add_prefixes.push_back(SBAddPrefix(kAddChunk1, kHash1.prefix));
-  add_hashes.push_back(
-      SBAddFullHash(kAddChunk1, kHash1.prefix, kNow, kHash1));
-  add_hashes.push_back(
-      SBAddFullHash(kAddChunk1, kHash1mod1.prefix, kNow, kHash1mod1));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash1));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash1mod1));
   sub_prefixes.push_back(SBSubPrefix(kSubChunk1, kAddChunk1, kHash1.prefix));
-  sub_hashes.push_back(
-      SBSubFullHash(kSubChunk1, kAddChunk1, kHash1mod2.prefix, kHash1mod2));
-  sub_hashes.push_back(
-      SBSubFullHash(kSubChunk1, kAddChunk1, kHash1mod3.prefix, kHash1mod3));
+  sub_hashes.push_back(SBSubFullHash(kSubChunk1, kAddChunk1, kHash1mod2));
+  sub_hashes.push_back(SBSubFullHash(kSubChunk1, kAddChunk1, kHash1mod3));
 
   // An add with no corresponding sub.  Both items should be retained.
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash2.prefix, kNow, kHash2));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash2));
   add_prefixes.push_back(SBAddPrefix(kAddChunk1, kHash2.prefix));
 
   // A sub with no corresponding add.  Both items should be retained.
-  sub_hashes.push_back(
-      SBSubFullHash(kSubChunk1, kAddChunk1, kHash3.prefix, kHash3));
+  sub_hashes.push_back(SBSubFullHash(kSubChunk1, kAddChunk1, kHash3));
   sub_prefixes.push_back(SBSubPrefix(kSubChunk1, kAddChunk1, kHash3.prefix));
 
   SBProcessSubs(&add_prefixes, &sub_prefixes, &add_hashes, &sub_hashes);
@@ -169,19 +162,17 @@ TEST(SafeBrowsingStoreTest, SBProcessSubs) {
   EXPECT_EQ(kHash2.prefix, add_prefixes[0].prefix);
 
   EXPECT_EQ(1U, add_hashes.size());
-  EXPECT_EQ(kAddChunk1, add_hashes[0].add_prefix.chunk_id);
-  EXPECT_EQ(kHash2.prefix, add_hashes[0].add_prefix.prefix);
+  EXPECT_EQ(kAddChunk1, add_hashes[0].chunk_id);
   EXPECT_TRUE(SBFullHashEq(kHash2, add_hashes[0].full_hash));
 
   EXPECT_EQ(1U, sub_prefixes.size());
   EXPECT_EQ(kSubChunk1, sub_prefixes[0].chunk_id);
-  EXPECT_EQ(kAddChunk1, sub_prefixes[0].add_prefix.chunk_id);
-  EXPECT_EQ(kHash3.prefix, sub_prefixes[0].add_prefix.prefix);
+  EXPECT_EQ(kAddChunk1, sub_prefixes[0].add_chunk_id);
+  EXPECT_EQ(kHash3.prefix, sub_prefixes[0].add_prefix);
 
   EXPECT_EQ(1U, sub_hashes.size());
   EXPECT_EQ(kSubChunk1, sub_hashes[0].chunk_id);
-  EXPECT_EQ(kAddChunk1, sub_hashes[0].add_prefix.chunk_id);
-  EXPECT_EQ(kHash3.prefix, sub_hashes[0].add_prefix.prefix);
+  EXPECT_EQ(kAddChunk1, sub_hashes[0].add_chunk_id);
   EXPECT_TRUE(SBFullHashEq(kHash3, sub_hashes[0].full_hash));
 }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_unittest_helper.cc b/chrome/browser/safe_browsing/safe_browsing_store_unittest_helper.cc
index 2e333fb..d8ae136 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_unittest_helper.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_unittest_helper.cc
@@ -64,13 +64,12 @@ void SafeBrowsingStoreTestStorePrefix(SafeBrowsingStore* store) {
   EXPECT_TRUE(store->CheckAddChunk(kAddChunk1));
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kHash1.prefix));
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kHash2.prefix));
-  EXPECT_TRUE(store->WriteAddHash(kAddChunk1, kHash2.prefix, now, kHash2));
+  EXPECT_TRUE(store->WriteAddHash(kAddChunk1, now, kHash2));
 
   store->SetSubChunk(kSubChunk1);
   EXPECT_TRUE(store->CheckSubChunk(kSubChunk1));
   EXPECT_TRUE(store->WriteSubPrefix(kSubChunk1, kAddChunk3, kHash3.prefix));
-  EXPECT_TRUE(store->WriteSubHash(kSubChunk1,
-                                  kAddChunk3, kHash3.prefix, kHash3));
+  EXPECT_TRUE(store->WriteSubHash(kSubChunk1, kAddChunk3, kHash3));
   EXPECT_TRUE(store->FinishChunk());
 
   // Chunk numbers shouldn't leak over.
@@ -102,10 +101,7 @@ void SafeBrowsingStoreTestStorePrefix(SafeBrowsingStore* store) {
   EXPECT_EQ(kHash2.prefix, add_prefixes_result[1].prefix);
 
   ASSERT_EQ(1U, add_full_hashes_result.size());
-  EXPECT_EQ(kAddChunk1, add_full_hashes_result[0].add_prefix.chunk_id);
-  EXPECT_EQ(kHash2.prefix, add_full_hashes_result[0].add_prefix.prefix);
-  EXPECT_EQ(add_full_hashes_result[0].add_prefix.prefix,
-            add_full_hashes_result[0].full_hash.prefix);
+  EXPECT_EQ(kAddChunk1, add_full_hashes_result[0].chunk_id);
   // EXPECT_TRUE(add_full_hashes_result[0].received == now)?
   EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
   EXPECT_TRUE(SBFullHashEq(kHash2, add_full_hashes_result[0].full_hash));
@@ -139,8 +135,7 @@ void SafeBrowsingStoreTestStorePrefix(SafeBrowsingStore* store) {
   EXPECT_EQ(kHash2.prefix, add_prefixes_result[1].prefix);
 
   ASSERT_EQ(1U, add_full_hashes_result.size());
-  EXPECT_EQ(kAddChunk1, add_full_hashes_result[0].add_prefix.chunk_id);
-  EXPECT_EQ(kHash2.prefix, add_full_hashes_result[0].add_prefix.prefix);
+  EXPECT_EQ(kAddChunk1, add_full_hashes_result[0].chunk_id);
   EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
   EXPECT_TRUE(SBFullHashEq(kHash2, add_full_hashes_result[0].full_hash));
 }
@@ -154,7 +149,7 @@ void SafeBrowsingStoreTestSubKnockout(SafeBrowsingStore* store) {
   store->SetAddChunk(kAddChunk1);
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kHash1.prefix));
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kHash2.prefix));
-  EXPECT_TRUE(store->WriteAddHash(kAddChunk1, kHash2.prefix, now, kHash2));
+  EXPECT_TRUE(store->WriteAddHash(kAddChunk1, now, kHash2));
 
   store->SetSubChunk(kSubChunk1);
   EXPECT_TRUE(store->WriteSubPrefix(kSubChunk1, kAddChunk3, kHash3.prefix));
@@ -225,7 +220,7 @@ void SafeBrowsingStoreTestDeleteChunks(SafeBrowsingStore* store) {
   EXPECT_TRUE(store->BeginChunk());
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kHash1.prefix));
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kHash2.prefix));
-  EXPECT_TRUE(store->WriteAddHash(kAddChunk1, kHash2.prefix, now, kHash2));
+  EXPECT_TRUE(store->WriteAddHash(kAddChunk1, now, kHash2));
   EXPECT_TRUE(store->FinishChunk());
 
   // Another which won't.
@@ -233,7 +228,7 @@ void SafeBrowsingStoreTestDeleteChunks(SafeBrowsingStore* store) {
   store->SetAddChunk(kAddChunk2);
   EXPECT_TRUE(store->BeginChunk());
   EXPECT_TRUE(store->WriteAddPrefix(kAddChunk2, kHash3.prefix));
-  EXPECT_TRUE(store->WriteAddHash(kAddChunk2, kHash3.prefix, now, kHash3));
+  EXPECT_TRUE(store->WriteAddHash(kAddChunk2, now, kHash3));
   EXPECT_TRUE(store->FinishChunk());
 
   // A sub chunk to delete.
@@ -241,8 +236,7 @@ void SafeBrowsingStoreTestDeleteChunks(SafeBrowsingStore* store) {
   store->SetSubChunk(kSubChunk1);
   EXPECT_TRUE(store->BeginChunk());
   EXPECT_TRUE(store->WriteSubPrefix(kSubChunk1, kAddChunk3, kHash4.prefix));
-  EXPECT_TRUE(store->WriteSubHash(kSubChunk1,
-                                  kAddChunk3, kHash4.prefix, kHash4));
+  EXPECT_TRUE(store->WriteSubHash(kSubChunk1, kAddChunk3, kHash4));
   EXPECT_TRUE(store->FinishChunk());
 
   // A sub chunk to keep.
@@ -250,8 +244,7 @@ void SafeBrowsingStoreTestDeleteChunks(SafeBrowsingStore* store) {
   store->SetSubChunk(kSubChunk2);
   EXPECT_TRUE(store->BeginChunk());
   EXPECT_TRUE(store->WriteSubPrefix(kSubChunk2, kAddChunk4, kHash5.prefix));
-  EXPECT_TRUE(store->WriteSubHash(kSubChunk2,
-                                  kAddChunk4, kHash5.prefix, kHash5));
+  EXPECT_TRUE(store->WriteSubHash(kSubChunk2, kAddChunk4, kHash5));
   EXPECT_TRUE(store->FinishChunk());
 
   store->DeleteAddChunk(kAddChunk1);
@@ -275,8 +268,7 @@ void SafeBrowsingStoreTestDeleteChunks(SafeBrowsingStore* store) {
   EXPECT_EQ(kAddChunk2, add_prefixes_result[0].chunk_id);
   EXPECT_EQ(kHash3.prefix, add_prefixes_result[0].prefix);
   EXPECT_EQ(1U, add_full_hashes_result.size());
-  EXPECT_EQ(kAddChunk2, add_full_hashes_result[0].add_prefix.chunk_id);
-  EXPECT_EQ(kHash3.prefix, add_full_hashes_result[0].add_prefix.prefix);
+  EXPECT_EQ(kAddChunk2, add_full_hashes_result[0].chunk_id);
   EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
   EXPECT_TRUE(SBFullHashEq(kHash3, add_full_hashes_result[0].full_hash));