From 5cb7d01aa8abef3a09f70d7fe5b0139227ca31be Mon Sep 17 00:00:00 2001 From: "shess@chromium.org" Date: Fri, 19 Feb 2010 23:28:08 +0000 Subject: 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 --- chrome/browser/safe_browsing/safe_browsing_store.h | 40 +++++----- .../safe_browsing/safe_browsing_store_file.cc | 23 ++---- .../safe_browsing/safe_browsing_store_file.h | 13 +--- .../safe_browsing/safe_browsing_store_sqlite.cc | 25 ++++--- .../safe_browsing/safe_browsing_store_sqlite.h | 8 +- .../safe_browsing/safe_browsing_store_unittest.cc | 85 ++++++++++------------ .../safe_browsing_store_unittest_helper.cc | 28 +++---- 7 files changed, 97 insertions(+), 125 deletions(-) (limited to 'chrome') 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(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::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::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::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::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::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::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::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::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 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 - 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 - 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)); -- cgit v1.1