[safe browsing] Remove received-time from SBAddFullHash.

The database receive_time is no longer used.  Setting it to zero going
forward so that future code can detect older possibly-suspect data (for
instance from gethash results that used to be stored in the database).

BUG=357763

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

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

6 files changed, 46 insertions, 79 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_database.cc b/chrome/browser/safe_browsing/safe_browsing_database.cc
index 09418a7..d77433c 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database.cc
@@ -856,12 +856,11 @@ void SafeBrowsingDatabaseNew::InsertAdd(int chunk_id, SBPrefix host,
     }
   } else {
     // Full hashes only.
-    const base::Time receive_time = base::Time::Now();
     for (int i = 0; i < count; ++i) {
       const SBFullHash full_hash = entry->FullHashAt(i);
 
       STATS_COUNTER("SB.PrefixAddFull", 1);
-      store->WriteAddHash(encoded_chunk_id, receive_time, full_hash);
+      store->WriteAddHash(encoded_chunk_id, full_hash);
     }
   }
 }
diff --git a/chrome/browser/safe_browsing/safe_browsing_store.h b/chrome/browser/safe_browsing/safe_browsing_store.h
index 0abe90b..2cbd30e 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store.h
@@ -74,21 +74,15 @@ typedef std::deque<SBSubPrefix> SBSubPrefixes;
 
 struct SBAddFullHash {
   int32 chunk_id;
-  int32 received;  // TODO(shess): Deprecate and remove.
+  // Received field is not used anymore, but is kept for DB compatability.
+  // TODO(shess): Deprecate and remove.
+  int32 deprecated_received;
   SBFullHash full_hash;
 
-  SBAddFullHash(int32 id, base::Time r, const SBFullHash& h)
-      : chunk_id(id),
-        received(static_cast<int32>(r.ToTimeT())),
-        full_hash(h) {
-  }
+  SBAddFullHash(int32 id, const SBFullHash& h)
+      : chunk_id(id), deprecated_received(), full_hash(h) {}
 
-  // Provided for ReadAddHashes() implementations, which already have
-  // an int32 for the time.
-  SBAddFullHash(int32 id, int32 r, const SBFullHash& h)
-      : chunk_id(id), received(r), full_hash(h) {}
-
-  SBAddFullHash() : chunk_id(), received(), full_hash() {}
+  SBAddFullHash() : chunk_id(), deprecated_received(), full_hash() {}
 
   int32 GetAddChunkId() const { return chunk_id; }
   SBPrefix GetAddPrefix() const { return full_hash.prefix; }
@@ -184,7 +178,6 @@ class SafeBrowsingStore {
 
   virtual bool WriteAddPrefix(int32 chunk_id, SBPrefix prefix) = 0;
   virtual bool WriteAddHash(int32 chunk_id,
-                            base::Time receive_time,
                             const SBFullHash& full_hash) = 0;
   virtual bool WriteSubPrefix(int32 chunk_id,
                               int32 add_chunk_id, SBPrefix prefix) = 0;
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.cc b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
index 0c8b2cd..01a5dc7 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
@@ -765,9 +765,8 @@ bool SafeBrowsingStoreFile::GetAddFullHashes(
 }
 
 bool SafeBrowsingStoreFile::WriteAddHash(int32 chunk_id,
-                                         base::Time receive_time,
                                          const SBFullHash& full_hash) {
-  add_hashes_.push_back(SBAddFullHash(chunk_id, receive_time, full_hash));
+  add_hashes_.push_back(SBAddFullHash(chunk_id, full_hash));
   return true;
 }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.h b/chrome/browser/safe_browsing/safe_browsing_store_file.h
index f54e39b..a890559 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.h
@@ -143,7 +143,6 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
 
   virtual bool WriteAddPrefix(int32 chunk_id, SBPrefix prefix) OVERRIDE;
   virtual bool WriteAddHash(int32 chunk_id,
-                            base::Time receive_time,
                             const SBFullHash& full_hash) OVERRIDE;
   virtual bool WriteSubPrefix(int32 chunk_id,
                               int32 add_chunk_id, SBPrefix prefix) OVERRIDE;
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 99e6587..b028446 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
@@ -67,7 +67,7 @@ class SafeBrowsingStoreFileTest : public PlatformTest {
   }
 
   // Populate the store with some testing data.
-  void PopulateStore(const base::Time& now) {
+  void PopulateStore() {
     ASSERT_TRUE(store_->BeginUpdate());
 
     EXPECT_TRUE(store_->BeginChunk());
@@ -87,7 +87,7 @@ class SafeBrowsingStoreFileTest : public PlatformTest {
     EXPECT_TRUE(store_->BeginChunk());
     store_->SetAddChunk(kAddChunk2);
     EXPECT_TRUE(store_->CheckAddChunk(kAddChunk2));
-    EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, now, kHash4));
+    EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, kHash4));
     EXPECT_TRUE(store_->FinishChunk());
 
     // Chunk numbers shouldn't leak over.
@@ -151,8 +151,7 @@ TEST_F(SafeBrowsingStoreFileTest, Empty) {
 // Write some prefix and hash data to the store, add more data in another
 // transaction, then verify that the union of all the data is present.
 TEST_F(SafeBrowsingStoreFileTest, BasicStore) {
-  const base::Time now = base::Time::Now();
-  PopulateStore(now);
+  PopulateStore();
 
   ASSERT_TRUE(store_->BeginUpdate());
 
@@ -189,16 +188,13 @@ TEST_F(SafeBrowsingStoreFileTest, BasicStore) {
 
     ASSERT_EQ(1U, add_full_hashes_result.size());
     EXPECT_EQ(kAddChunk2, 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(SBFullHashEqual(kHash4, add_full_hashes_result[0].full_hash));
   }
 }
 
 // Verify that the min and max prefixes are stored and operated on.
 TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
-  const base::Time now = base::Time::Now();
-  PopulateStore(now);
+  PopulateStore();
 
   ASSERT_TRUE(store_->BeginUpdate());
 
@@ -247,8 +243,6 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
 TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
   ASSERT_TRUE(store_->BeginUpdate());
 
-  const base::Time now = base::Time::Now();
-
   EXPECT_TRUE(store_->BeginChunk());
   store_->SetAddChunk(kAddChunk1);
   EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash1.prefix));
@@ -257,7 +251,7 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
 
   EXPECT_TRUE(store_->BeginChunk());
   store_->SetAddChunk(kAddChunk2);
-  EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, now, kHash4));
+  EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, kHash4));
   EXPECT_TRUE(store_->FinishChunk());
 
   EXPECT_TRUE(store_->BeginChunk());
@@ -279,7 +273,6 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
 
     ASSERT_EQ(1U, add_full_hashes_result.size());
     EXPECT_EQ(kAddChunk2, add_full_hashes_result[0].chunk_id);
-    EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
     EXPECT_TRUE(SBFullHashEqual(kHash4, add_full_hashes_result[0].full_hash));
   }
 
@@ -303,7 +296,6 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
 
     ASSERT_EQ(1U, add_full_hashes_result.size());
     EXPECT_EQ(kAddChunk2, add_full_hashes_result[0].chunk_id);
-    EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
     EXPECT_TRUE(SBFullHashEqual(kHash4, add_full_hashes_result[0].full_hash));
   }
 
@@ -328,7 +320,6 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
 
     ASSERT_EQ(1U, add_full_hashes_result.size());
     EXPECT_EQ(kAddChunk2, add_full_hashes_result[0].chunk_id);
-    EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
     EXPECT_TRUE(SBFullHashEqual(kHash4, add_full_hashes_result[0].full_hash));
   }
 }
@@ -337,8 +328,6 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
 TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
   ASSERT_TRUE(store_->BeginUpdate());
 
-  const base::Time now = base::Time::Now();
-
   // A prefix chunk which will be deleted.
   EXPECT_FALSE(store_->CheckAddChunk(kAddChunk1));
   store_->SetAddChunk(kAddChunk1);
@@ -358,7 +347,7 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
   EXPECT_FALSE(store_->CheckAddChunk(kAddChunk3));
   store_->SetAddChunk(kAddChunk3);
   EXPECT_TRUE(store_->BeginChunk());
-  EXPECT_TRUE(store_->WriteAddHash(kAddChunk3, now, kHash6));
+  EXPECT_TRUE(store_->WriteAddHash(kAddChunk3, kHash6));
   EXPECT_TRUE(store_->FinishChunk());
 
   // A sub chunk to delete.
@@ -397,7 +386,6 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
 
     ASSERT_EQ(1U, add_full_hashes_result.size());
     EXPECT_EQ(kAddChunk3, add_full_hashes_result[0].chunk_id);
-    EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
     EXPECT_TRUE(SBFullHashEqual(kHash6, add_full_hashes_result[0].full_hash));
   }
 
@@ -452,7 +440,7 @@ TEST_F(SafeBrowsingStoreFileTest, Delete) {
   EXPECT_TRUE(store_->Delete());
 
   // Create a store file.
-  PopulateStore(base::Time::Now());
+  PopulateStore();
 
   EXPECT_TRUE(base::PathExists(filename_));
   EXPECT_TRUE(store_->Delete());
@@ -487,7 +475,7 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteTemp) {
 // Test basic corruption-handling.
 TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
   // Load a store with some data.
-  PopulateStore(base::Time::Now());
+  PopulateStore();
 
   // Can successfully open and read the store.
   {
@@ -549,7 +537,7 @@ TEST_F(SafeBrowsingStoreFileTest, CheckValidity) {
 
   // A store with some data is valid.
   EXPECT_FALSE(base::PathExists(filename_));
-  PopulateStore(base::Time::Now());
+  PopulateStore();
   EXPECT_TRUE(base::PathExists(filename_));
   ASSERT_TRUE(store_->BeginUpdate());
   EXPECT_FALSE(corruption_detected_);
@@ -560,7 +548,7 @@ TEST_F(SafeBrowsingStoreFileTest, CheckValidity) {
 
 // Corrupt the header.
 TEST_F(SafeBrowsingStoreFileTest, CheckValidityHeader) {
-  PopulateStore(base::Time::Now());
+  PopulateStore();
   EXPECT_TRUE(base::PathExists(filename_));
 
   // 37 is the most random prime number.  It's also past the initial header
@@ -578,7 +566,7 @@ TEST_F(SafeBrowsingStoreFileTest, CheckValidityHeader) {
 
 // Corrupt the prefix payload.
 TEST_F(SafeBrowsingStoreFileTest, CheckValidityPayload) {
-  PopulateStore(base::Time::Now());
+  PopulateStore();
   EXPECT_TRUE(base::PathExists(filename_));
 
   // 137 is the second most random prime number.  It's also past the header and
@@ -600,7 +588,7 @@ TEST_F(SafeBrowsingStoreFileTest, CheckValidityPayload) {
 
 // Corrupt the checksum.
 TEST_F(SafeBrowsingStoreFileTest, CheckValidityChecksum) {
-  PopulateStore(base::Time::Now());
+  PopulateStore();
   EXPECT_TRUE(base::PathExists(filename_));
 
   // An offset from the end of the file which is in the checksum.
@@ -621,8 +609,6 @@ TEST_F(SafeBrowsingStoreFileTest, CheckValidityChecksum) {
 TEST_F(SafeBrowsingStoreFileTest, GetAddPrefixesAndHashes) {
   ASSERT_TRUE(store_->BeginUpdate());
 
-  const base::Time now = base::Time::Now();
-
   EXPECT_TRUE(store_->BeginChunk());
   store_->SetAddChunk(kAddChunk1);
   EXPECT_TRUE(store_->CheckAddChunk(kAddChunk1));
@@ -633,7 +619,7 @@ TEST_F(SafeBrowsingStoreFileTest, GetAddPrefixesAndHashes) {
   EXPECT_TRUE(store_->BeginChunk());
   store_->SetAddChunk(kAddChunk2);
   EXPECT_TRUE(store_->CheckAddChunk(kAddChunk2));
-  EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, now, kHash4));
+  EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, kHash4));
   EXPECT_TRUE(store_->FinishChunk());
 
   store_->SetSubChunk(kSubChunk1);
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc
index 3e2494d..2c336ab 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_unittest.cc
@@ -81,29 +81,27 @@ TEST(SafeBrowsingStoreTest, SBAddPrefixHashLess) {
   onetwo.full_hash[sizeof(SBPrefix)] = 2;
   two.prefix = 2;
 
-  const base::Time now = base::Time::Now();
-
   // prefix dominates.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(11, now, one),
-                                  SBAddFullHash(10, now, two)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(11, now, two),
-                                   SBAddFullHash(10, now, one)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(11, one),
+                                  SBAddFullHash(10, two)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(11, two),
+                                   SBAddFullHash(10, one)));
 
   // After prefix, add_id.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, now, one),
-                                  SBAddFullHash(11, now, onetwo)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(11, now, one),
-                                   SBAddFullHash(10, now, onetwo)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, one),
+                                  SBAddFullHash(11, onetwo)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(11, one),
+                                   SBAddFullHash(10, onetwo)));
 
   // After add_id, full hash.
-  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, now, one),
-                                  SBAddFullHash(10, now, onetwo)));
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, now, onetwo),
-                                   SBAddFullHash(10, now, one)));
+  EXPECT_TRUE(SBAddPrefixHashLess(SBAddFullHash(10, one),
+                                  SBAddFullHash(10, onetwo)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, onetwo),
+                                   SBAddFullHash(10, one)));
 
   // Equal is not less-than.
-  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, now, one),
-                                   SBAddFullHash(10, now, one)));
+  EXPECT_FALSE(SBAddPrefixHashLess(SBAddFullHash(10, one),
+                                   SBAddFullHash(10, one)));
 }
 
 TEST(SafeBrowsingStoreTest, SBSubPrefixLess) {
@@ -174,8 +172,6 @@ TEST(SafeBrowsingStoreTest, SBProcessSubsEmpty) {
 
 // Test that subs knock out adds.
 TEST(SafeBrowsingStoreTest, SBProcessSubsKnockout) {
-  const base::Time kNow = base::Time::Now();
-
   // A full hash which shares prefix with another.
   const SBFullHash kHash1mod = ModifyHashAfterPrefix(kHash1, 1);
 
@@ -192,13 +188,13 @@ TEST(SafeBrowsingStoreTest, SBProcessSubsKnockout) {
   sub_prefixes.push_back(SBSubPrefix(kSubChunk1, kAddChunk1, kHash5.prefix));
 
   // Add hashes with same prefix, plus subs to knock them out.
-  add_hashes.push_back(SBAddFullHash(kAddChunk2, kNow, kHash1));
-  add_hashes.push_back(SBAddFullHash(kAddChunk2, kNow, kHash1mod));
+  add_hashes.push_back(SBAddFullHash(kAddChunk2, kHash1));
+  add_hashes.push_back(SBAddFullHash(kAddChunk2, kHash1mod));
   sub_hashes.push_back(SBSubFullHash(kSubChunk2, kAddChunk2, kHash1));
   sub_hashes.push_back(SBSubFullHash(kSubChunk2, kAddChunk2, kHash1mod));
 
   // Adds with no corresponding sub.  Both items should be retained.
-  add_hashes.push_back(SBAddFullHash(kAddChunk6, kNow, kHash6));
+  add_hashes.push_back(SBAddFullHash(kAddChunk6, kHash6));
   add_prefixes.push_back(SBAddPrefix(kAddChunk7, kHash2.prefix));
 
   // Subs with no corresponding add.  Both items should be retained.
@@ -207,8 +203,8 @@ TEST(SafeBrowsingStoreTest, SBProcessSubsKnockout) {
 
   // Add hashes with the same prefix, with a sub that will knock one of them
   // out.
-  add_hashes.push_back(SBAddFullHash(kAddChunk5, kNow, kHash4));
-  add_hashes.push_back(SBAddFullHash(kAddChunk5, kNow, kHash4mod));
+  add_hashes.push_back(SBAddFullHash(kAddChunk5, kHash4));
+  add_hashes.push_back(SBAddFullHash(kAddChunk5, kHash4mod));
   sub_hashes.push_back(SBSubFullHash(kSubChunk5, kAddChunk5, kHash4mod));
 
   const base::hash_set<int32> no_deletions;
@@ -239,8 +235,6 @@ TEST(SafeBrowsingStoreTest, SBProcessSubsKnockout) {
 // Test chunk deletions, and ordering of deletions WRT subs knocking
 // out adds.
 TEST(SafeBrowsingStoreTest, SBProcessSubsDeleteChunk) {
-  const base::Time kNow = base::Time::Now();
-
   // A full hash which shares prefix with another.
   const SBFullHash kHash1mod = ModifyHashAfterPrefix(kHash1, 1);
 
@@ -249,19 +243,18 @@ TEST(SafeBrowsingStoreTest, SBProcessSubsDeleteChunk) {
   SBSubPrefixes sub_prefixes;
   std::vector<SBSubFullHash> sub_hashes;
 
-
   // An add prefix plus a sub to knock it out.
   add_prefixes.push_back(SBAddPrefix(kAddChunk1, kHash5.prefix));
   sub_prefixes.push_back(SBSubPrefix(kSubChunk1, kAddChunk1, kHash5.prefix));
 
   // Add hashes with same prefix, plus subs to knock them out.
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash1));
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash1mod));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash1));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash1mod));
   sub_hashes.push_back(SBSubFullHash(kSubChunk1, kAddChunk1, kHash1));
   sub_hashes.push_back(SBSubFullHash(kSubChunk1, kAddChunk1, kHash1mod));
 
   // Adds with no corresponding sub.  Both items should be retained.
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash6));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash6));
   add_prefixes.push_back(SBAddPrefix(kAddChunk1, kHash2.prefix));
 
   // Subs with no corresponding add.  Both items should be retained.
@@ -310,8 +303,6 @@ TEST(SafeBrowsingStoreTest, Y2K38) {
 // Test that prefixes which were injected from full hashes are being removed.
 // This was a mistake in earlier versions of the code.
 TEST(SafeBrowsingStoreTest, KnockoutPrefixVolunteers) {
-  const base::Time kNow = base::Time::Now();
-
   // Construct some full hashes which share prefix with another.
   const SBFullHash kHash1mod1 = ModifyHashAfterPrefix(kHash1, 1);
   const SBFullHash kHash2mod1 = ModifyHashAfterPrefix(kHash2, 1);
@@ -322,12 +313,12 @@ TEST(SafeBrowsingStoreTest, KnockoutPrefixVolunteers) {
   std::vector<SBSubFullHash> sub_hashes;
 
   // Full hashes for an add chunk will have had the prefix injected.
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash1));
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash1mod1));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash1));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash1mod1));
   add_prefixes.push_back(SBAddPrefix(kAddChunk1, kHash1.prefix));
 
   // Other full hashes or prefixes are not affected.
-  add_hashes.push_back(SBAddFullHash(kAddChunk1, kNow, kHash3));
+  add_hashes.push_back(SBAddFullHash(kAddChunk1, kHash3));
   add_prefixes.push_back(SBAddPrefix(kAddChunk2, kHash4.prefix));
 
   // Full hashes for a sub chunk will have had the prefix injected.