Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(395)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc

Issue 220493003: Safebrowsing: change gethash caching to match api 2.3 rules, fix some corner cases. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: changes for review #2 Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/browser/safe_browsing/safe_browsing_database_unittest.cc ('K') | « chrome/browser/safe_browsing/safe_browsing_store_file.cc ('k') | chrome/browser/safe_browsing/safe_browsing_store_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
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 e56b97420a195e6ee40da4d19041cbcb93c21cd4..e397a4ca8f038765a11eb13c851a82408b640ecf 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
@@ -62,7 +62,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());
@@ -70,7 +70,7 @@ class SafeBrowsingStoreFileTest : public PlatformTest {
EXPECT_TRUE(store_->CheckAddChunk(kAddChunk1));
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash1.prefix));
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash2.prefix));
- EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, now, kHash2));
+ EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, kHash2));
store_->SetSubChunk(kSubChunk1);
EXPECT_TRUE(store_->CheckSubChunk(kSubChunk1));
@@ -83,13 +83,10 @@ class SafeBrowsingStoreFileTest : public PlatformTest {
EXPECT_FALSE(store_->CheckAddChunk(kAddChunk3));
EXPECT_FALSE(store_->CheckSubChunk(kAddChunk1));
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
}
// Manually read the shard stride info from the file.
@@ -127,13 +124,10 @@ TEST_F(SafeBrowsingStoreFileTest, Empty) {
EXPECT_FALSE(store_->CheckSubChunk(1));
EXPECT_FALSE(store_->CheckSubChunk(-1));
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
EXPECT_TRUE(add_full_hashes_result.empty());
std::vector<SBPrefix> prefixes_result;
@@ -144,8 +138,7 @@ TEST_F(SafeBrowsingStoreFileTest, Empty) {
// Write some prefix data to the store and verify that it looks like
// it is still there after the transaction completes.
TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
- const base::Time now = base::Time::Now();
- PopulateStore(now);
+ PopulateStore();
ASSERT_TRUE(store_->BeginUpdate());
@@ -159,12 +152,9 @@ TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
EXPECT_EQ(kSubChunk1, chunks[0]);
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -174,8 +164,6 @@ TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
ASSERT_EQ(1U, add_full_hashes_result.size());
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(SBFullHashEqual(kHash2, add_full_hashes_result[0].full_hash));
}
@@ -194,12 +182,9 @@ TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
EXPECT_TRUE(store_->CheckSubChunk(kSubChunk1));
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
// Still has the expected contents.
std::vector<SBPrefix> prefixes_result;
@@ -210,15 +195,13 @@ TEST_F(SafeBrowsingStoreFileTest, StorePrefix) {
ASSERT_EQ(1U, add_full_hashes_result.size());
EXPECT_EQ(kAddChunk1, add_full_hashes_result[0].chunk_id);
- EXPECT_EQ(now.ToTimeT(), add_full_hashes_result[0].received);
EXPECT_TRUE(SBFullHashEqual(kHash2, 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());
@@ -229,12 +212,9 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
EXPECT_TRUE(store_->FinishChunk());
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -254,12 +234,9 @@ TEST_F(SafeBrowsingStoreFileTest, PrefixMinMax) {
EXPECT_TRUE(store_->FinishChunk());
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -273,13 +250,11 @@ 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));
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash2.prefix));
- EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, now, kHash2));
+ EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, kHash2));
store_->SetSubChunk(kSubChunk1);
EXPECT_TRUE(store_->WriteSubPrefix(kSubChunk1, kAddChunk3, kHash3.prefix));
@@ -287,12 +262,9 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
EXPECT_TRUE(store_->FinishChunk());
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
// Knocked out the chunk expected.
std::vector<SBPrefix> prefixes_result;
@@ -311,12 +283,9 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
EXPECT_TRUE(store_->FinishChunk());
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -334,12 +303,9 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
EXPECT_TRUE(store_->FinishChunk());
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -354,15 +320,13 @@ TEST_F(SafeBrowsingStoreFileTest, SubKnockout) {
TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
ASSERT_TRUE(store_->BeginUpdate());
- const base::Time now = base::Time::Now();
-
// A chunk which will be deleted.
EXPECT_FALSE(store_->CheckAddChunk(kAddChunk1));
store_->SetAddChunk(kAddChunk1);
EXPECT_TRUE(store_->BeginChunk());
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash1.prefix));
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash2.prefix));
- EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, now, kHash2));
+ EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, kHash2));
EXPECT_TRUE(store_->FinishChunk());
// Another which won't.
@@ -370,7 +334,7 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
store_->SetAddChunk(kAddChunk2);
EXPECT_TRUE(store_->BeginChunk());
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk2, kHash3.prefix));
- EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, now, kHash3));
+ EXPECT_TRUE(store_->WriteAddHash(kAddChunk2, kHash3));
EXPECT_TRUE(store_->FinishChunk());
// A sub chunk to delete.
@@ -399,12 +363,9 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
EXPECT_TRUE(store_->CheckSubChunk(kSubChunk2));
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -413,7 +374,6 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
EXPECT_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(kHash3, add_full_hashes_result[0].full_hash));
}
@@ -429,12 +389,9 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
store_->DeleteSubChunk(kSubChunk2);
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
}
// Expect no more chunks.
@@ -445,12 +402,9 @@ TEST_F(SafeBrowsingStoreFileTest, DeleteChunks) {
EXPECT_FALSE(store_->CheckSubChunk(kSubChunk2));
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
std::vector<SBPrefix> prefixes_result;
builder.GetPrefixSet()->GetPrefixes(&prefixes_result);
@@ -466,7 +420,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());
@@ -501,16 +455,15 @@ 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.
{
- std::vector<SBAddFullHash> pending_adds;
std::vector<SBPrefix> orig_prefixes;
std::vector<SBAddFullHash> orig_hashes;
safe_browsing::PrefixSetBuilder builder;
ASSERT_TRUE(store_->BeginUpdate());
- EXPECT_TRUE(store_->FinishUpdate(pending_adds, &builder, &orig_hashes));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &orig_hashes));
builder.GetPrefixSet()->GetPrefixes(&orig_prefixes);
EXPECT_GT(orig_prefixes.size(), 0U);
EXPECT_GT(orig_hashes.size(), 0U);
@@ -533,10 +486,9 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
std::vector<SBAddFullHash> add_hashes;
corruption_detected_ = false;
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
ASSERT_TRUE(store_->BeginUpdate());
- EXPECT_FALSE(store_->FinishUpdate(pending_adds, &builder, &add_hashes));
+ EXPECT_FALSE(store_->FinishUpdate(&builder, &add_hashes));
EXPECT_TRUE(corruption_detected_);
}
@@ -565,7 +517,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_);
@@ -576,7 +528,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
@@ -594,7 +546,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
@@ -616,7 +568,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.
@@ -637,14 +589,12 @@ 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));
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash1.prefix));
EXPECT_TRUE(store_->WriteAddPrefix(kAddChunk1, kHash2.prefix));
- EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, now, kHash2));
+ EXPECT_TRUE(store_->WriteAddHash(kAddChunk1, kHash2));
store_->SetSubChunk(kSubChunk1);
EXPECT_TRUE(store_->CheckSubChunk(kSubChunk1));
@@ -668,9 +618,7 @@ TEST_F(SafeBrowsingStoreFileTest, GetAddPrefixesAndHashes) {
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(std::vector<SBAddFullHash>(),
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
SBAddPrefixes add_prefixes;
EXPECT_TRUE(store_->GetAddPrefixes(&add_prefixes));
@@ -715,12 +663,9 @@ TEST_F(SafeBrowsingStoreFileTest, Resharding) {
}
EXPECT_TRUE(store_->FinishChunk());
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
SBAddPrefixes add_prefixes;
EXPECT_TRUE(store_->GetAddPrefixes(&add_prefixes));
@@ -745,12 +690,9 @@ TEST_F(SafeBrowsingStoreFileTest, Resharding) {
EXPECT_FALSE(store_->CheckAddChunk(chunk_id + 1));
store_->DeleteAddChunk(chunk_id);
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
// New stride should be the same, or shifted one left.
const uint32 new_shard_stride = ReadStride();
@@ -823,12 +765,9 @@ TEST_F(SafeBrowsingStoreFileTest, Version7) {
EXPECT_TRUE(store_->FinishChunk());
{
- std::vector<SBAddFullHash> pending_adds;
safe_browsing::PrefixSetBuilder builder;
std::vector<SBAddFullHash> add_full_hashes_result;
- EXPECT_TRUE(store_->FinishUpdate(pending_adds,
- &builder,
- &add_full_hashes_result));
+ EXPECT_TRUE(store_->FinishUpdate(&builder, &add_full_hashes_result));
// The sub'ed prefix and hash are gone.
std::vector<SBPrefix> prefixes_result;
« chrome/browser/safe_browsing/safe_browsing_database_unittest.cc ('K') | « chrome/browser/safe_browsing/safe_browsing_store_file.cc ('k') | chrome/browser/safe_browsing/safe_browsing_store_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698