summaryrefslogtreecommitdiffstats
path: root/chrome/browser/safe_browsing
diff options
context:
space:
mode:
authorshess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-04 06:45:43 +0000
committershess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-04 06:45:43 +0000
commitb0544c44da65ac1a2b0654f97ec473c76176aea2 (patch)
treebaa67ac64a2be7bd3e0bdae932ada206ee9d5991 /chrome/browser/safe_browsing
parent7af6806afc92098dbe8194195fa0607a9e6c7490 (diff)
downloadchromium_src-b0544c44da65ac1a2b0654f97ec473c76176aea2.zip
chromium_src-b0544c44da65ac1a2b0654f97ec473c76176aea2.tar.gz
chromium_src-b0544c44da65ac1a2b0654f97ec473c76176aea2.tar.bz2
Revert "On-the-fly migration from SafeBrowsingStoreSqlite -> *File."
Broke the build. Sorry. BUG=none TEST=none Review URL: http://codereview.chromium.org/669043 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40607 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/safe_browsing')
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_store_file.cc86
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_store_file.h20
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc141
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_store_sqlite.h4
4 files changed, 9 insertions, 242 deletions
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.cc b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
index 75c7ad0..7186388 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.cc
@@ -7,9 +7,6 @@
#include "base/callback.h"
#include "base/md5.h"
-// TODO(shess): Remove after migration.
-#include "chrome/browser/safe_browsing/safe_browsing_store_sqlite.h"
-
namespace {
// NOTE(shess): kFileMagic should not be a byte-wise palindrome, so
@@ -209,17 +206,6 @@ bool SafeBrowsingStoreFile::Delete() {
return false;
}
- // Also make sure any SQLite data is deleted. This should only be
- // needed if a journal file is left from a crash and the database is
- // reset before SQLite gets a chance to straighten things out.
- // TODO(shess): Remove after migration.
- SafeBrowsingStoreSqlite old_store;
- old_store.Init(
- filename_,
- NewCallback(this, &SafeBrowsingStoreFile::HandleCorruptDatabase));
- if (!old_store.Delete())
- return false;
-
return true;
}
@@ -243,12 +229,11 @@ bool SafeBrowsingStoreFile::Close() {
// Make sure the files are closed.
file_.reset();
new_file_.reset();
- old_store_.reset();
return true;
}
bool SafeBrowsingStoreFile::BeginUpdate() {
- DCHECK(!file_.get() && !new_file_.get() && !old_store_.get());
+ DCHECK(!file_.get() && !new_file_.get());
// Structures should all be clear unless something bad happened.
DCHECK(add_chunks_cache_.empty());
@@ -282,30 +267,8 @@ bool SafeBrowsingStoreFile::BeginUpdate() {
if (!ReadArray(&header, 1, file.get(), NULL))
return OnCorruptDatabase();
- if (header.magic != kFileMagic || header.version != kFileVersion) {
- // Magic numbers didn't match, maybe it's a SQLite database.
- scoped_ptr<SafeBrowsingStoreSqlite>
- sqlite_store(new SafeBrowsingStoreSqlite());
- sqlite_store->Init(
- filename_,
- NewCallback(this, &SafeBrowsingStoreFile::HandleCorruptDatabase));
- if (!sqlite_store->BeginUpdate())
- return OnCorruptDatabase();
-
- // Pull chunks-seen data into local structures, rather than
- // optionally wiring various calls through to the SQLite store.
- std::vector<int32> chunks;
- sqlite_store->GetAddChunks(&chunks);
- add_chunks_cache_.insert(chunks.begin(), chunks.end());
-
- sqlite_store->GetSubChunks(&chunks);
- sub_chunks_cache_.insert(chunks.begin(), chunks.end());
-
- new_file_.swap(new_file);
- old_store_.swap(sqlite_store);
-
- return true;
- }
+ if (header.magic != kFileMagic || header.version != kFileVersion)
+ return OnCorruptDatabase();
// Check that the file size makes sense given the header. This is a
// cheap way to protect against header corruption while deferring
@@ -370,7 +333,7 @@ bool SafeBrowsingStoreFile::DoUpdate(
const std::vector<SBAddFullHash>& pending_adds,
std::vector<SBAddPrefix>* add_prefixes_result,
std::vector<SBAddFullHash>* add_full_hashes_result) {
- DCHECK(old_store_.get() || file_.get() || empty_);
+ DCHECK(file_.get() || empty_);
DCHECK(new_file_.get());
std::vector<SBAddPrefix> add_prefixes;
@@ -378,30 +341,8 @@ bool SafeBrowsingStoreFile::DoUpdate(
std::vector<SBAddFullHash> add_full_hashes;
std::vector<SBSubFullHash> sub_full_hashes;
- // Read |old_store_| into the vectors.
- if (old_store_.get()) {
- // Push deletions to |old_store_| so they can be applied to the
- // data being read.
- for (base::hash_set<int32>::const_iterator iter = add_del_cache_.begin();
- iter != add_del_cache_.end(); ++iter) {
- old_store_->DeleteAddChunk(*iter);
- }
- for (base::hash_set<int32>::const_iterator iter = sub_del_cache_.begin();
- iter != sub_del_cache_.end(); ++iter) {
- old_store_->DeleteSubChunk(*iter);
- }
-
- if (!old_store_->ReadAddPrefixes(&add_prefixes) ||
- !old_store_->ReadSubPrefixes(&sub_prefixes) ||
- !old_store_->ReadAddHashes(&add_full_hashes) ||
- !old_store_->ReadSubHashes(&sub_full_hashes))
- return OnCorruptDatabase();
-
- // Do not actually update the old store.
- if (!old_store_->CancelUpdate())
- return OnCorruptDatabase();
- } else if (!empty_) {
- // Read |file_| into the vectors.
+ // Read |file_| into the vectors.
+ if (!empty_) {
DCHECK(file_.get());
if (!FileRewind(file_.get()))
@@ -538,16 +479,9 @@ bool SafeBrowsingStoreFile::DoUpdate(
// Close the file handle and swizzle the file into place.
new_file_.reset();
- if (old_store_.get()) {
- const bool deleted = old_store_->Delete();
- old_store_.reset();
- if (!deleted)
- return false;
- } else {
- if (!file_util::Delete(filename_, false) &&
- file_util::PathExists(filename_))
- return false;
- }
+ if (!file_util::Delete(filename_, false) &&
+ file_util::PathExists(filename_))
+ return false;
const FilePath new_filename = TemporaryFileForFilename(filename_);
if (!file_util::Move(new_filename, filename_))
@@ -574,12 +508,10 @@ bool SafeBrowsingStoreFile::FinishUpdate(
DCHECK(!new_file_.get());
DCHECK(!file_.get());
- DCHECK(!old_store_.get());
return Close();
}
bool SafeBrowsingStoreFile::CancelUpdate() {
- old_store_.reset();
return Close();
}
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file.h b/chrome/browser/safe_browsing/safe_browsing_store_file.h
index 3996442..4e0cda9 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file.h
@@ -13,12 +13,6 @@
#include "base/callback.h"
#include "base/file_util.h"
-// TODO(shess): Data is migrated from SafeBrowsingStoreSqlite as part
-// of the next update. That way everyone doesn't pull a new database
-// when the code rolls out (and the migration code isn't gnarly).
-// After substantially everyone has migrated, the migration code can
-// be removed. Make sure that it deletes any journal file.
-
// Implement SafeBrowsingStore in terms of a flat file. The file
// format is pretty literal:
//
@@ -109,9 +103,6 @@
// often, consider retaining the last known-good file for recovery
// purposes, rather than deleting it.
-// TODO(shess): Remove after migration.
-class SafeBrowsingStoreSqlite;
-
class SafeBrowsingStoreFile : public SafeBrowsingStore {
public:
SafeBrowsingStoreFile();
@@ -198,12 +189,6 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
// a convenience to the caller.
bool OnCorruptDatabase();
- // Helper for creating a corruption callback for |old_store_|.
- // TODO(shess): Remove after migration.
- void HandleCorruptDatabase() {
- OnCorruptDatabase();
- }
-
// Clear temporary buffers used to accumulate chunk data.
bool ClearChunkBuffers() {
// NOTE: .clear() doesn't release memory.
@@ -246,11 +231,6 @@ class SafeBrowsingStoreFile : public SafeBrowsingStore {
file_util::ScopedFILE new_file_;
bool empty_;
- // If the main file existed, but was an SQLite store, this is a
- // handle to it.
- // TODO(shess): Remove this (and all references) after migration.
- scoped_ptr<SafeBrowsingStoreSqlite> old_store_;
-
// Cache of chunks which have been seen. Loaded from the database
// on BeginUpdate() so that it can be queried during the
// transaction.
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 411ce8a..8e6ee61 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
@@ -3,7 +3,6 @@
// found in the LICENSE file.
#include "chrome/browser/safe_browsing/safe_browsing_store_file.h"
-#include "chrome/browser/safe_browsing/safe_browsing_store_sqlite.h"
#include "base/callback.h"
#include "chrome/browser/safe_browsing/safe_browsing_store_unittest_helper.h"
@@ -145,144 +144,4 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
EXPECT_TRUE(corruption_detected_);
}
-// Info to build a trivial store for migration testing.
-const int kAddChunk1 = 1;
-const int kAddChunk2 = 3;
-const int kAddChunk3 = 5;
-const int kSubChunk1 = 2;
-const int kSubChunk2 = 4;
-const SBFullHash kHash1 = SBFullHashFromString("one");
-const SBFullHash kHash2 = SBFullHashFromString("two");
-const SBPrefix kPrefix1 = kHash1.prefix;
-const SBPrefix kPrefix2 = kHash2.prefix;
-const SBPrefix kPrefix3 = SBFullHashFromString("three").prefix;
-const SBPrefix kPrefix4 = SBFullHashFromString("four").prefix;
-const SBPrefix kPrefix5 = SBFullHashFromString("five").prefix;
-const base::Time kNow = base::Time::Now();
-
-// Load the store with some data.
-void LoadStore(SafeBrowsingStore* store) {
- EXPECT_TRUE(store->BeginUpdate());
- EXPECT_TRUE(store->BeginChunk());
-
- store->SetAddChunk(kAddChunk1);
- store->SetSubChunk(kSubChunk1);
- store->SetAddChunk(kAddChunk2);
-
- EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kPrefix1));
- EXPECT_TRUE(store->WriteAddPrefix(kAddChunk1, kPrefix2));
- EXPECT_TRUE(store->WriteAddPrefix(kAddChunk2, kPrefix3));
- EXPECT_TRUE(store->WriteSubPrefix(kSubChunk1, kAddChunk3, kPrefix4));
- EXPECT_TRUE(store->WriteAddHash(kAddChunk1, kNow, kHash1));
- EXPECT_TRUE(store->WriteSubHash(kSubChunk1, kAddChunk1, kHash2));
-
- EXPECT_TRUE(store->FinishChunk());
-
- std::vector<SBAddFullHash> pending_adds;
- std::vector<SBAddPrefix> add_prefixes;
- std::vector<SBAddFullHash> add_hashes;
- EXPECT_TRUE(store->FinishUpdate(pending_adds, &add_prefixes, &add_hashes));
- EXPECT_EQ(3U, add_prefixes.size());
-}
-
-// Verify that the store looks like what results from LoadStore(), and
-// update it.
-void UpdateStore(SafeBrowsingStore* store) {
- EXPECT_TRUE(store->BeginUpdate());
- EXPECT_TRUE(store->BeginChunk());
-
- // The chunks in the database should be the same.
- std::vector<int> add_chunks;
- store->GetAddChunks(&add_chunks);
- ASSERT_EQ(2U, add_chunks.size());
- EXPECT_EQ(kAddChunk1, add_chunks[0]);
- EXPECT_EQ(kAddChunk2, add_chunks[1]);
-
- std::vector<int> sub_chunks;
- store->GetSubChunks(&sub_chunks);
- ASSERT_EQ(1U, sub_chunks.size());
- EXPECT_EQ(kSubChunk1, sub_chunks[0]);
-
- EXPECT_TRUE(store->CheckAddChunk(kAddChunk1));
- EXPECT_TRUE(store->CheckSubChunk(kSubChunk1));
- EXPECT_TRUE(store->CheckAddChunk(kAddChunk2));
-
- EXPECT_FALSE(store->CheckAddChunk(kAddChunk3));
- store->SetAddChunk(kAddChunk3);
- // This one already has a sub.
- EXPECT_TRUE(store->WriteAddPrefix(kAddChunk3, kPrefix4));
- EXPECT_TRUE(store->WriteAddPrefix(kAddChunk3, kPrefix5));
-
- EXPECT_FALSE(store->CheckSubChunk(kSubChunk2));
- store->SetSubChunk(kSubChunk2);
- EXPECT_TRUE(store->WriteSubPrefix(kSubChunk2, kAddChunk1, kPrefix1));
-
- EXPECT_TRUE(store->FinishChunk());
-
- store->DeleteAddChunk(kAddChunk2);
-
- std::vector<SBAddFullHash> pending_adds;
- std::vector<SBAddPrefix> add_prefixes;
- std::vector<SBAddFullHash> add_hashes;
- EXPECT_TRUE(store->FinishUpdate(pending_adds, &add_prefixes, &add_hashes));
- EXPECT_EQ(2U, add_prefixes.size());
-}
-
-// Verify that the expected UpdateStore() data is present.
-void CheckStore(SafeBrowsingStore* store) {
- EXPECT_TRUE(store->BeginUpdate());
-
- // The chunks in the database should be the same.
- std::vector<int> add_chunks;
- store->GetAddChunks(&add_chunks);
- ASSERT_EQ(2U, add_chunks.size());
- EXPECT_EQ(kAddChunk1, add_chunks[0]);
- EXPECT_EQ(kAddChunk3, add_chunks[1]);
-
- std::vector<int> sub_chunks;
- store->GetSubChunks(&sub_chunks);
- ASSERT_EQ(2U, sub_chunks.size());
- EXPECT_EQ(kSubChunk1, sub_chunks[0]);
- EXPECT_EQ(kSubChunk2, sub_chunks[1]);
-
- EXPECT_TRUE(store->CancelUpdate());
-}
-
-// Verify that the migration sequence works as expected in the
-// non-migration cases.
-TEST_F(SafeBrowsingStoreFileTest, MigrateBaselineFile) {
- LoadStore(store_.get());
- UpdateStore(store_.get());
- CheckStore(store_.get());
-}
-TEST_F(SafeBrowsingStoreFileTest, MigrateBaselineSqlite) {
- SafeBrowsingStoreSqlite sqlite_store;
- sqlite_store.Init(filename_, NULL);
-
- LoadStore(&sqlite_store);
- UpdateStore(&sqlite_store);
- CheckStore(&sqlite_store);
-}
-
-// The sequence should work exactly the same when we migrate from
-// SQLite to file.
-TEST_F(SafeBrowsingStoreFileTest, Migrate) {
- // No existing store.
- EXPECT_FALSE(file_util::PathExists(filename_));
-
- {
- SafeBrowsingStoreSqlite sqlite_store;
- sqlite_store.Init(filename_, NULL);
-
- LoadStore(&sqlite_store);
- }
-
- // At this point |filename_| references a SQLite store.
- EXPECT_TRUE(file_util::PathExists(filename_));
-
- // Update and check using a file store.
- UpdateStore(store_.get());
- CheckStore(store_.get());
-}
-
} // namespace
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h b/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h
index e72073b..5e03d50 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h
+++ b/chrome/browser/safe_browsing/safe_browsing_store_sqlite.h
@@ -102,10 +102,6 @@ class SafeBrowsingStoreSqlite : public SafeBrowsingStore {
}
private:
- // For on-the-fly migration.
- // TODO(shess): Remove (entire class) after migration.
- friend class SafeBrowsingStoreFile;
-
// The following routines return true on success, or false on
// failure. Failure is presumed to be persistent, so the caller
// should stop trying and unwind the transaction.