diff options
author | shess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-08 21:43:16 +0000 |
---|---|---|
committer | shess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-08 21:43:16 +0000 |
commit | 7b1e3710535c1bc6945ad170f667e0a1e9a88ef9 (patch) | |
tree | 2bd669956a242ac48e30093aeacf2d4a57f612da | |
parent | 16e70aeca7951680226d7ad461d5c21dadccc728 (diff) | |
download | chromium_src-7b1e3710535c1bc6945ad170f667e0a1e9a88ef9.zip chromium_src-7b1e3710535c1bc6945ad170f667e0a1e9a88ef9.tar.gz chromium_src-7b1e3710535c1bc6945ad170f667e0a1e9a88ef9.tar.bz2 |
Refactor chunk ownership in SafeBrowsing code.
This attempts to clean up ownership so that it's more clear who
creates and deletes the chunk data between parsing and storage in the
database. SBChunkList replaces std::deque<SBChunk>, mostly to add
correct cleanup in the destructor rather than requiring FreeChunks()
to be called manually. Additionally remove deletion of chunk info
from the database layer, now it processes const data.
Additionally remove the need for SafeBrowsingDatabase to have a
chunk-inserted callback by having the caller handle it directly.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/668123
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40942 0039d316-1c4b-4281-b951-d872f2087c98
12 files changed, 212 insertions, 206 deletions
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc index d47d4d1..31a093a 100644 --- a/chrome/browser/safe_browsing/protocol_manager.cc +++ b/chrome/browser/safe_browsing/protocol_manager.cc @@ -329,13 +329,12 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url, int next_update_sec = -1; bool re_key = false; bool reset = false; - std::vector<SBChunkDelete>* chunk_deletes = - new std::vector<SBChunkDelete>; + scoped_ptr<std::vector<SBChunkDelete> > chunk_deletes( + new std::vector<SBChunkDelete>); std::vector<ChunkUrl> chunk_urls; if (!parser.ParseUpdate(data, length, client_key_, &next_update_sec, &re_key, - &reset, chunk_deletes, &chunk_urls)) { - delete chunk_deletes; + &reset, chunk_deletes.get(), &chunk_urls)) { return false; } @@ -368,15 +367,13 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url, // database. if (reset) { sb_service_->ResetDatabase(); - delete chunk_deletes; return true; } - // Chunks to delete from our storage. + // Chunks to delete from our storage. Pass ownership of + // |chunk_deletes|. if (!chunk_deletes->empty()) - sb_service_->HandleChunkDelete(chunk_deletes); - else - delete chunk_deletes; + sb_service_->HandleChunkDelete(chunk_deletes.release()); break; } @@ -386,12 +383,12 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url, const ChunkUrl chunk_url = chunk_request_urls_.front(); bool re_key = false; - std::deque<SBChunk>* chunks = new std::deque<SBChunk>; + scoped_ptr<SBChunkList> chunks(new SBChunkList); UMA_HISTOGRAM_COUNTS("SB2.ChunkSize", length); update_size_ += length; if (!parser.ParseChunk(data, length, client_key_, chunk_url.mac, - &re_key, chunks)) { + &re_key, chunks.get())) { #ifndef NDEBUG std::string data_str; data_str.assign(data, length); @@ -404,19 +401,16 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url, << ", Base64Encode(data): " << encoded_chunk << ", length: " << length; #endif - safe_browsing_util::FreeChunks(chunks); - delete chunks; return false; } if (re_key) HandleReKey(); - if (chunks->empty()) { - delete chunks; - } else { + // Chunks to add to storage. Pass ownership of |chunks|. + if (!chunks->empty()) { chunk_pending_to_write_ = true; - sb_service_->HandleChunk(chunk_url.list_name, chunks); + sb_service_->HandleChunk(chunk_url.list_name, chunks.release()); } break; diff --git a/chrome/browser/safe_browsing/protocol_parser.cc b/chrome/browser/safe_browsing/protocol_parser.cc index 3dfb4ba1..4590e56 100644 --- a/chrome/browser/safe_browsing/protocol_parser.cc +++ b/chrome/browser/safe_browsing/protocol_parser.cc @@ -249,7 +249,7 @@ bool SafeBrowsingProtocolParser::ParseChunk(const char* data, const std::string& key, const std::string& mac, bool* re_key, - std::deque<SBChunk>* chunks) { + SBChunkList* chunks) { int remaining = length; const char* chunk_data = data; diff --git a/chrome/browser/safe_browsing/protocol_parser.h b/chrome/browser/safe_browsing/protocol_parser.h index 4d77e50..097eedb72 100644 --- a/chrome/browser/safe_browsing/protocol_parser.h +++ b/chrome/browser/safe_browsing/protocol_parser.h @@ -79,7 +79,7 @@ class SafeBrowsingProtocolParser { const std::string& key, const std::string& mac, bool* re_key, - std::deque<SBChunk>* chunks); + SBChunkList* chunks); // Parse the result of a GetHash request, returning the list of full hashes. // If we are checking for valid MACs, the caller should populate 'key'. diff --git a/chrome/browser/safe_browsing/protocol_parser_unittest.cc b/chrome/browser/safe_browsing/protocol_parser_unittest.cc index 1796bd3..3aa5bd1 100644 --- a/chrome/browser/safe_browsing/protocol_parser_unittest.cc +++ b/chrome/browser/safe_browsing/protocol_parser_unittest.cc @@ -17,7 +17,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddChunk) { // Run the parse. SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(add_chunk.data(), static_cast<int>(add_chunk.length()), "", "", &re_key, &chunks); @@ -49,8 +49,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddChunk) { EXPECT_EQ(entry->prefix_count(), 2); EXPECT_EQ(entry->PrefixAt(0), 0x38383838); EXPECT_EQ(entry->PrefixAt(1), 0x39393939); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing one add chunk with full hashes. @@ -70,7 +68,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddFullChunk) { // Run the parse. SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(add_chunk.data(), static_cast<int>(add_chunk.length()), "", "", &re_key, &chunks); @@ -87,8 +85,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddFullChunk) { EXPECT_EQ(entry->prefix_count(), 2); EXPECT_TRUE(entry->FullHashAt(0) == full_hash1); EXPECT_TRUE(entry->FullHashAt(1) == full_hash2); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing multiple add chunks. We'll use the same chunk as above, and add @@ -101,7 +97,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddChunks) { // Run the parse. SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(add_chunk.data(), static_cast<int>(add_chunk.length()), "", "", &re_key, &chunks); @@ -145,8 +141,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddChunks) { EXPECT_EQ(entry->prefix_count(), 2); EXPECT_EQ(entry->PrefixAt(0), 0x70707070); EXPECT_EQ(entry->PrefixAt(1), 0x67676767); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing one add chunk where a hostkey spans several entries. @@ -163,7 +157,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddBigChunk) { SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(add_chunk.data(), static_cast<int>(add_chunk.length()), "", "", &re_key, &chunks); @@ -177,8 +171,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestAddBigChunk) { const SBChunkHost& host = chunks[0].hosts[0]; EXPECT_EQ(host.host, 0x61616161); EXPECT_EQ(host.entry->prefix_count(), 260); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing one sub chunk. @@ -191,7 +183,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestSubChunk) { // Run the parse. SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(sub_chunk.data(), static_cast<int>(sub_chunk.length()), "", "", &re_key, &chunks); @@ -229,8 +221,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestSubChunk) { EXPECT_EQ(entry->PrefixAt(0), 0x38383838); EXPECT_EQ(entry->ChunkIdAtPrefix(1), 0x79797979); EXPECT_EQ(entry->PrefixAt(1), 0x39393939); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing one sub chunk with full hashes. @@ -252,7 +242,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestSubFullChunk) { // Run the parse. SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(sub_chunk.data(), static_cast<int>(sub_chunk.length()), "", "", &re_key, &chunks); @@ -271,8 +261,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestSubFullChunk) { EXPECT_TRUE(entry->FullHashAt(0) == full_hash1); EXPECT_EQ(entry->ChunkIdAtPrefix(1), 0x7a7a7a7a); EXPECT_TRUE(entry->FullHashAt(1) == full_hash2); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing the SafeBrowsing update response. @@ -609,7 +597,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestZeroSizeAddChunk) { std::string add_chunk("a:1:4:0\n"); SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(add_chunk.data(), static_cast<int>(add_chunk.length()), @@ -619,8 +607,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestZeroSizeAddChunk) { EXPECT_EQ(chunks[0].chunk_number, 1); EXPECT_EQ(chunks[0].hosts.size(), static_cast<size_t>(0)); - safe_browsing_util::FreeChunks(&chunks); - // Now test a zero size chunk in between normal chunks. chunks.clear(); std::string add_chunks("a:1:4:18\n1234\001abcd5678\001wxyz" @@ -647,8 +633,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestZeroSizeAddChunk) { EXPECT_EQ(chunks[2].hosts.size(), static_cast<size_t>(1)); EXPECT_EQ(chunks[2].hosts[0].host, 0x65666163); EXPECT_EQ(chunks[2].hosts[0].entry->PrefixAt(0), 0x66656562); - - safe_browsing_util::FreeChunks(&chunks); } // Test parsing a zero sized sub chunk. @@ -656,7 +640,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestZeroSizeSubChunk) { std::string sub_chunk("s:9:4:0\n"); SafeBrowsingProtocolParser parser; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; bool result = parser.ParseChunk(sub_chunk.data(), static_cast<int>(sub_chunk.length()), @@ -665,8 +649,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestZeroSizeSubChunk) { EXPECT_EQ(chunks.size(), static_cast<size_t>(1)); EXPECT_EQ(chunks[0].chunk_number, 9); EXPECT_EQ(chunks[0].hosts.size(), static_cast<size_t>(0)); - - safe_browsing_util::FreeChunks(&chunks); chunks.clear(); // Test parsing a zero sized sub chunk mixed in with content carrying chunks. @@ -698,8 +680,6 @@ TEST(SafeBrowsingProtocolParsingTest, TestZeroSizeSubChunk) { EXPECT_EQ(chunks[2].hosts[1].entry->prefix_count(), 1); EXPECT_EQ(chunks[2].hosts[1].entry->PrefixAt(0), 0x6f6e6d6c); EXPECT_EQ(chunks[2].hosts[1].entry->ChunkIdAtPrefix(0), 0x35363738); - - safe_browsing_util::FreeChunks(&chunks); } TEST(SafeBrowsingProtocolParsingTest, TestVerifyUpdateMac) { @@ -765,7 +745,7 @@ TEST(SafeBrowsingProtocolParsingTest, TestVerifyChunkMac) { }; bool re_key = false; - std::deque<SBChunk> chunks; + SBChunkList chunks; const std::string key("v_aDSz6jI92WeHCOoZ07QA=="); const std::string mac("W9Xp2fUcQ9V66If6Cvsrstpa4Kk="); @@ -773,6 +753,4 @@ TEST(SafeBrowsingProtocolParsingTest, TestVerifyChunkMac) { sizeof(chunk), key, mac, &re_key, &chunks)); EXPECT_FALSE(re_key); - - safe_browsing_util::FreeChunks(&chunks); } diff --git a/chrome/browser/safe_browsing/safe_browsing_database.h b/chrome/browser/safe_browsing/safe_browsing_database.h index 1fc5756..c188f25 100644 --- a/chrome/browser/safe_browsing/safe_browsing_database.h +++ b/chrome/browser/safe_browsing/safe_browsing_database.h @@ -10,8 +10,8 @@ #include <set> #include <vector> -#include "base/callback.h" #include "base/file_path.h" +#include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/time.h" #include "chrome/browser/safe_browsing/safe_browsing_util.h" @@ -30,10 +30,8 @@ class SafeBrowsingDatabase { static SafeBrowsingDatabase* Create(); virtual ~SafeBrowsingDatabase(); - // Initializes the database with the given filename. The callback is - // executed after finishing a chunk. - virtual void Init(const FilePath& filename, - Callback0::Type* chunk_inserted_callback) = 0; + // Initializes the database with the given filename. + virtual void Init(const FilePath& filename) = 0; // Deletes the current database and creates a new one. virtual bool ResetDatabase() = 0; @@ -49,11 +47,12 @@ class SafeBrowsingDatabase { // Processes add/sub commands. Database will free the chunks when it's done. virtual void InsertChunks(const std::string& list_name, - std::deque<SBChunk>* chunks) = 0; + const SBChunkList& chunks) = 0; // Processs adddel/subdel commands. Database will free chunk_deletes when // it's done. - virtual void DeleteChunks(std::vector<SBChunkDelete>* chunk_deletes) = 0; + virtual void DeleteChunks( + const std::vector<SBChunkDelete>& chunk_deletes) = 0; // Returns the lists and their add/sub chunks. virtual void GetListsInfo(std::vector<SBListChunkRanges>* lists) = 0; diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc index a0e7fcc..d1ceb37 100644 --- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc +++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc @@ -5,7 +5,6 @@ #include "chrome/browser/safe_browsing/safe_browsing_database_bloom.h" #include "base/auto_reset.h" -#include "base/callback.h" #include "base/file_util.h" #include "base/message_loop.h" #include "base/process_util.h" @@ -44,8 +43,7 @@ SafeBrowsingDatabaseBloom::~SafeBrowsingDatabaseBloom() { Close(); } -void SafeBrowsingDatabaseBloom::Init(const FilePath& filename, - Callback0::Type* chunk_inserted_callback) { +void SafeBrowsingDatabaseBloom::Init(const FilePath& filename) { DCHECK(filename_.empty()); // Ensure we haven't been run before. filename_ = FilePath(filename.value() + kBloomFilterFileSuffix); @@ -56,8 +54,6 @@ void SafeBrowsingDatabaseBloom::Init(const FilePath& filename, hash_cache_.reset(new HashCache); LoadBloomFilter(); - - chunk_inserted_callback_.reset(chunk_inserted_callback); } bool SafeBrowsingDatabaseBloom::ResetDatabase() { @@ -157,37 +153,33 @@ bool SafeBrowsingDatabaseBloom::ContainsUrl( } void SafeBrowsingDatabaseBloom::InsertChunks(const std::string& list_name, - std::deque<SBChunk>* chunks) { - if (chunks->empty()) + const SBChunkList& chunks) { + if (chunks.empty()) return; base::Time insert_start = base::Time::Now(); int list_id = safe_browsing_util::GetListId(list_name); - ChunkType chunk_type = chunks->front().is_add ? ADD_CHUNK : SUB_CHUNK; + ChunkType chunk_type = chunks.front().is_add ? ADD_CHUNK : SUB_CHUNK; - while (!chunks->empty()) { - SBChunk& chunk = chunks->front(); - chunk.list_id = list_id; + for (SBChunkList::const_iterator iter = chunks.begin(); + iter != chunks.end(); ++iter) { + const SBChunk& chunk = *iter; int chunk_id = chunk.chunk_number; // The server can give us a chunk that we already have because it's part of // a range. Don't add it again. if (!ChunkExists(list_id, chunk_type, chunk_id)) { - while (!chunk.hosts.empty()) { + for (std::deque<SBChunkHost>::const_iterator hiter = chunk.hosts.begin(); + hiter != chunk.hosts.end(); ++hiter) { // Read the existing record for this host, if it exists. - SBPrefix host = chunk.hosts.front().host; - SBEntry* entry = chunk.hosts.front().entry; - entry->set_list_id(list_id); + const SBPrefix host = hiter->host; + const SBEntry* entry = hiter->entry; if (chunk_type == ADD_CHUNK) { - entry->set_chunk_id(chunk_id); - InsertAdd(host, entry); + InsertAdd(chunk_id, host, entry, list_id); } else { - InsertSub(chunk_id, host, entry); + InsertSub(chunk_id, host, entry, list_id); } - - entry->Destroy(); - chunk.hosts.pop_front(); } int encoded = EncodeChunkId(chunk_id, list_id); @@ -195,33 +187,21 @@ void SafeBrowsingDatabaseBloom::InsertChunks(const std::string& list_name, add_chunk_cache_.insert(encoded); else sub_chunk_cache_.insert(encoded); - } else { - while (!chunk.hosts.empty()) { - chunk.hosts.front().entry->Destroy(); - chunk.hosts.pop_front(); - } } - - chunks->pop_front(); } UMA_HISTOGRAM_TIMES("SB2.ChunkInsert", base::Time::Now() - insert_start); - - delete chunks; - - if (chunk_inserted_callback_.get()) - chunk_inserted_callback_->Run(); } void SafeBrowsingDatabaseBloom::DeleteChunks( - std::vector<SBChunkDelete>* chunk_deletes) { - if (chunk_deletes->empty()) + const std::vector<SBChunkDelete>& chunk_deletes) { + if (chunk_deletes.empty()) return; - int list_id = safe_browsing_util::GetListId(chunk_deletes->front().list_name); + int list_id = safe_browsing_util::GetListId(chunk_deletes.front().list_name); - for (size_t i = 0; i < chunk_deletes->size(); ++i) { - const SBChunkDelete& chunk = (*chunk_deletes)[i]; + for (size_t i = 0; i < chunk_deletes.size(); ++i) { + const SBChunkDelete& chunk = chunk_deletes[i]; std::vector<int> chunk_numbers; RangesToChunks(chunk.chunk_del, &chunk_numbers); for (size_t del = 0; del < chunk_numbers.size(); ++del) { @@ -232,8 +212,6 @@ void SafeBrowsingDatabaseBloom::DeleteChunks( add_del_cache_.insert(encoded_chunk); } } - - delete chunk_deletes; } void SafeBrowsingDatabaseBloom::GetListsInfo( @@ -1100,9 +1078,10 @@ void SafeBrowsingDatabaseBloom::OnHandleCorruptDatabase() { DCHECK(false) << "SafeBrowsing database was corrupt and reset"; } -void SafeBrowsingDatabaseBloom::InsertAdd(SBPrefix host, SBEntry* entry) { +void SafeBrowsingDatabaseBloom::InsertAdd( + int chunk_id, SBPrefix host, const SBEntry* entry, int list_id) { STATS_COUNTER("SB.HostInsert", 1); - int encoded = EncodeChunkId(entry->chunk_id(), entry->list_id()); + int encoded = EncodeChunkId(chunk_id, list_id); DCHECK(entry->IsAdd()); if (!entry->IsPrefix()) { @@ -1177,9 +1156,9 @@ void SafeBrowsingDatabaseBloom::InsertAddFullHash(SBPrefix prefix, } void SafeBrowsingDatabaseBloom::InsertSub( - int chunk_id, SBPrefix host, SBEntry* entry) { + int chunk_id, SBPrefix host, const SBEntry* entry, int list_id) { STATS_COUNTER("SB.HostDelete", 1); - int encoded = EncodeChunkId(chunk_id, entry->list_id()); + int encoded = EncodeChunkId(chunk_id, list_id); int encoded_add; DCHECK(entry->IsSub()); @@ -1187,7 +1166,7 @@ void SafeBrowsingDatabaseBloom::InsertSub( for (int i = 0; i < entry->prefix_count(); ++i) { SBFullHash full_hash = entry->FullHashAt(i); SBPrefix prefix = full_hash.prefix; - encoded_add = EncodeChunkId(entry->ChunkIdAtPrefix(i), entry->list_id()); + encoded_add = EncodeChunkId(entry->ChunkIdAtPrefix(i), list_id); InsertSubPrefix(prefix, encoded, encoded_add); InsertSubFullHash(prefix, encoded, encoded_add, full_hash, false); } @@ -1195,13 +1174,12 @@ void SafeBrowsingDatabaseBloom::InsertSub( // We have prefixes. int count = entry->prefix_count(); if (count == 0) { - encoded_add = EncodeChunkId(entry->chunk_id(), entry->list_id()); + encoded_add = EncodeChunkId(entry->chunk_id(), list_id); InsertSubPrefix(host, encoded, encoded_add); } else { for (int i = 0; i < count; i++) { SBPrefix prefix = entry->PrefixAt(i); - encoded_add = EncodeChunkId(entry->ChunkIdAtPrefix(i), - entry->list_id()); + encoded_add = EncodeChunkId(entry->ChunkIdAtPrefix(i), list_id); InsertSubPrefix(prefix, encoded, encoded_add); } } diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.h b/chrome/browser/safe_browsing/safe_browsing_database_bloom.h index d8154dc..91b3b9c 100644 --- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.h +++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.h @@ -10,7 +10,6 @@ #include <string> #include <vector> -#include "base/callback.h" #include "base/lock.h" #include "base/task.h" #include "chrome/browser/safe_browsing/safe_browsing_database.h" @@ -31,8 +30,7 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase { virtual ~SafeBrowsingDatabaseBloom(); // SafeBrowsingDatabase interface: - virtual void Init(const FilePath& filename, - Callback0::Type* chunk_inserted_callback); + virtual void Init(const FilePath& filename); virtual bool ResetDatabase(); virtual bool ContainsUrl(const GURL& url, std::string* matching_list, @@ -40,8 +38,8 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase { std::vector<SBFullHashResult>* full_hits, base::Time last_update); virtual void InsertChunks(const std::string& list_name, - std::deque<SBChunk>* chunks); - virtual void DeleteChunks(std::vector<SBChunkDelete>* chunk_deletes); + const SBChunkList& chunks); + virtual void DeleteChunks(const std::vector<SBChunkDelete>& chunk_deletes); virtual void GetListsInfo(std::vector<SBListChunkRanges>* lists); virtual void CacheHashResults( const std::vector<SBPrefix>& prefixes, @@ -120,7 +118,7 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase { void OnHandleCorruptDatabase(); // Adding add entries to the database. - void InsertAdd(SBPrefix host, SBEntry* entry); + void InsertAdd(int chunk, SBPrefix host, const SBEntry* entry, int list_id); void InsertAddPrefix(SBPrefix prefix, int encoded_chunk); void InsertAddFullHash(SBPrefix prefix, int encoded_chunk, @@ -128,7 +126,7 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase { SBFullHash full_prefix); // Adding sub entries to the database. - void InsertSub(int chunk, SBPrefix host, SBEntry* entry); + void InsertSub(int chunk, SBPrefix host, const SBEntry* entry, int list_id); void InsertSubPrefix(SBPrefix prefix, int encoded_chunk, int encoded_add_chunk); @@ -173,9 +171,6 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase { // Cache of compiled statements for our database. scoped_ptr<SqliteStatementCache> statement_cache_; - // Called after an add/sub chunk is processed. - scoped_ptr<Callback0::Type> chunk_inserted_callback_; - // Used to schedule resetting the database because of corruption. ScopedRunnableMethodFactory<SafeBrowsingDatabaseBloom> reset_factory_; diff --git a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc index 2e43d17..38c5f28 100644 --- a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc +++ b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc @@ -39,12 +39,12 @@ namespace { const std::string& list, int chunk_id, bool is_sub_del) { - std::vector<SBChunkDelete>* deletes = new std::vector<SBChunkDelete>; + std::vector<SBChunkDelete> deletes; SBChunkDelete chunk_delete; chunk_delete.list_name = list; chunk_delete.is_sub_del = is_sub_del; chunk_delete.chunk_del.push_back(ChunkRange(chunk_id)); - deletes->push_back(chunk_delete); + deletes.push_back(chunk_delete); db->DeleteChunks(deletes); } @@ -82,7 +82,7 @@ namespace { file_util::Delete(filename, false); SafeBrowsingDatabase* database = SafeBrowsingDatabase::Create(); - database->Init(filename, NULL); + database->Init(filename); return database; } @@ -111,6 +111,7 @@ class SafeBrowsingDatabasePlatformTest : public PlatformTest { TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { FileAutoDeleter file_deleter(CreateTestDirectory()); SafeBrowsingDatabase* database = SetupTestDatabase(file_deleter.path()); + SBChunkList chunks; // Insert some malware add chunks. SBChunkHost host; @@ -122,8 +123,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.chunk_number = 1; chunk.is_add = true; chunk.hosts.push_back(host); - std::deque<SBChunk>* chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -135,8 +136,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.is_add = true; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); host.host = Sha256Prefix("www.whatever.com/"); @@ -147,8 +148,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.is_add = true; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); database->UpdateFinished(true); @@ -169,8 +170,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.is_add = false; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); @@ -201,8 +202,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.is_add = true; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kPhishingList, chunks); @@ -217,8 +218,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.is_add = false; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->InsertChunks(safe_browsing_util::kPhishingList, chunks); host.host = Sha256Prefix("www.phishy2.com/"); @@ -230,8 +231,8 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { chunk.is_add = false; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->InsertChunks(safe_browsing_util::kPhishingList, chunks); database->UpdateFinished(true); lists.clear(); @@ -252,6 +253,7 @@ TEST_F(SafeBrowsingDatabasePlatformTest, ListName) { TEST(SafeBrowsingDatabase, Database) { FileAutoDeleter file_deleter(CreateTestDirectory()); SafeBrowsingDatabase* database = SetupTestDatabase(file_deleter.path()); + SBChunkList chunks; // Add a simple chunk with one hostkey. SBChunkHost host; @@ -266,8 +268,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.is_add = true; chunk.hosts.push_back(host); - std::deque<SBChunk>* chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); std::vector<SBListChunkRanges> lists; database->UpdateStarted(); database->GetListsInfo(&lists); @@ -292,8 +294,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -307,8 +309,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); database->UpdateFinished(true); lists.clear(); @@ -378,8 +380,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -405,8 +407,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); @@ -455,8 +457,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); @@ -508,8 +510,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -543,8 +545,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -566,8 +568,8 @@ TEST(SafeBrowsingDatabase, Database) { chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -590,6 +592,7 @@ TEST(SafeBrowsingDatabase, Database) { TEST(SafeBrowsingDatabase, ZeroSizeChunk) { FileAutoDeleter file_deleter(CreateTestDirectory()); SafeBrowsingDatabase* database = SetupTestDatabase(file_deleter.path()); + SBChunkList chunks; // Populate with a couple of normal chunks. SBChunkHost host; @@ -604,8 +607,8 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) { chunk.chunk_number = 1; chunk.hosts.push_back(host); - std::deque<SBChunk>* chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); host.host = Sha256Prefix("www.random.com/"); host.entry = SBEntry::Create(SBEntry::ADD_PREFIX, 2); @@ -615,7 +618,7 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) { chunk.chunk_number = 10; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks->push_back(chunk); + chunks.push_back(chunk); std::vector<SBListChunkRanges> lists; database->UpdateStarted(); @@ -632,15 +635,15 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) { SBChunk empty_chunk; empty_chunk.chunk_number = 19; empty_chunk.is_add = true; - chunks = new std::deque<SBChunk>; - chunks->push_back(empty_chunk); + chunks.clear(); + chunks.push_back(empty_chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); - chunks = new std::deque<SBChunk>; + chunks.clear(); empty_chunk.chunk_number = 7; empty_chunk.is_add = false; - chunks->push_back(empty_chunk); + chunks.push_back(empty_chunk); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); database->UpdateFinished(true); lists.clear(); @@ -660,13 +663,13 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) { empty_chunk.is_add = true; empty_chunk.hosts.clear(); empty_chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(empty_chunk); + chunks.clear(); + chunks.push_back(empty_chunk); empty_chunk.chunk_number = 21; empty_chunk.is_add = true; empty_chunk.hosts.clear(); - chunks->push_back(empty_chunk); + chunks.push_back(empty_chunk); host.host = Sha256Prefix("www.notempty.com/"); host.entry = SBEntry::Create(SBEntry::ADD_PREFIX, 1); @@ -676,7 +679,7 @@ TEST(SafeBrowsingDatabase, ZeroSizeChunk) { empty_chunk.hosts.push_back(host); empty_chunk.chunk_number = 22; empty_chunk.is_add = true; - chunks->push_back(empty_chunk); + chunks.push_back(empty_chunk); database->UpdateStarted(); database->GetListsInfo(&lists); @@ -741,9 +744,9 @@ void PopulateDatabaseForCacheTest(SafeBrowsingDatabase* database) { chunk.is_add = true; chunk.hosts.push_back(host); - std::deque<SBChunk>* chunks = new std::deque<SBChunk>; + SBChunkList chunks; std::vector<SBListChunkRanges> lists; - chunks->push_back(chunk); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -820,8 +823,8 @@ TEST(SafeBrowsingDatabase, HashCaching) { chunk.is_add = false; chunk.hosts.clear(); chunk.hosts.push_back(host); - std::deque<SBChunk>* chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + SBChunkList chunks; + chunks.push_back(chunk); std::vector<SBListChunkRanges> lists; database->UpdateStarted(); @@ -949,8 +952,8 @@ TEST(SafeBrowsingDatabase, HashCaching) { chunk.is_add = true; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -992,8 +995,8 @@ TEST(SafeBrowsingDatabase, HashCaching) { chunk.is_add = false; chunk.hosts.clear(); chunk.hosts.push_back(host); - chunks = new std::deque<SBChunk>; - chunks->push_back(chunk); + chunks.clear(); + chunks.push_back(chunk); database->UpdateStarted(); database->GetListsInfo(&lists); database->InsertChunks(safe_browsing_util::kMalwareList, chunks); @@ -1050,14 +1053,17 @@ FilePath GetFullSBDataPath(const FilePath& path) { return full_path; } +// TODO(shess): The clients of this structure manually manage +// |chunks|. Improve this code to apply the RAII idiom to manage +// |chunks|. struct ChunksInfo { - std::deque<SBChunk>* chunks; + SBChunkList* chunks; // weak std::string listname; }; void PerformUpdate(const FilePath& initial_db, const std::vector<ChunksInfo>& chunks, - std::vector<SBChunkDelete>* deletes) { + const std::vector<SBChunkDelete>& deletes) { IoCounters before, after; FilePath path; @@ -1073,7 +1079,7 @@ void PerformUpdate(const FilePath& initial_db, } SafeBrowsingDatabase* database = SafeBrowsingDatabase::Create(); - database->Init(path, NULL); + database->Init(path); Time before_time = Time::Now(); base::ProcessHandle handle = base::Process::Current().handle(); @@ -1091,7 +1097,7 @@ void PerformUpdate(const FilePath& initial_db, database->GetListsInfo(&lists); database->DeleteChunks(deletes); for (size_t i = 0; i < chunks.size(); ++i) - database->InsertChunks(chunks[i].listname, chunks[i].chunks); + database->InsertChunks(chunks[i].listname, *chunks[i].chunks); database->UpdateFinished(true); lists.clear(); @@ -1147,7 +1153,7 @@ void UpdateDatabase(const FilePath& initial_db, file_util::ReadFile(file, data.get(), size); ChunksInfo info; - info.chunks = new std::deque<SBChunk>; + info.chunks = new SBChunkList; bool re_key; result = parser.ParseChunk(data.get(), size, "", "", @@ -1163,7 +1169,7 @@ void UpdateDatabase(const FilePath& initial_db, } } - std::vector<SBChunkDelete>* deletes = new std::vector<SBChunkDelete>; + std::vector<SBChunkDelete> deletes; if (!response_path.empty()) { std::string update; FilePath full_response_path = GetFullSBDataPath(response_path); @@ -1177,7 +1183,7 @@ void UpdateDatabase(const FilePath& initial_db, &next_update, &rekey, &reset, - deletes, + &deletes, &urls); DCHECK(result); if (!updates_path.empty()) @@ -1186,6 +1192,13 @@ void UpdateDatabase(const FilePath& initial_db, } PerformUpdate(initial_db, chunks, deletes); + + // TODO(shess): Make ChunksInfo handle this via scoping. + for (std::vector<ChunksInfo>::iterator iter = chunks.begin(); + iter != chunks.end(); ++iter) { + delete iter->chunks; + iter->chunks = NULL; + } } namespace { @@ -1249,11 +1262,11 @@ TEST(SafeBrowsingDatabase, DISABLED_DatabaseOldUpdatesIO) { // Does a a lot of addel's on very large chunks. TEST(SafeBrowsingDatabase, DISABLED_DatabaseOldLotsofDeletesIO) { std::vector<ChunksInfo> chunks; - std::vector<SBChunkDelete>* deletes = new std::vector<SBChunkDelete>; + std::vector<SBChunkDelete> deletes; SBChunkDelete del; del.is_sub_del = false; del.list_name = safe_browsing_util::kMalwareList; del.chunk_del.push_back(ChunkRange(3539, 3579)); - deletes->push_back(del); + deletes.push_back(del); PerformUpdate(GetOldSafeBrowsingPath(), chunks, deletes); } diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc index 43a2e27..be02dba 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.cc +++ b/chrome/browser/safe_browsing/safe_browsing_service.cc @@ -192,7 +192,7 @@ void SafeBrowsingService::HandleGetHashResults( } void SafeBrowsingService::HandleChunk(const std::string& list, - std::deque<SBChunk>* chunks) { + SBChunkList* chunks) { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); DCHECK(enabled_); safe_browsing_thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod( @@ -428,9 +428,7 @@ SafeBrowsingDatabase* SafeBrowsingService::GetDatabase() { Time before = Time::Now(); SafeBrowsingDatabase* database = SafeBrowsingDatabase::Create(); - Callback0::Type* chunk_callback = - NewCallback(this, &SafeBrowsingService::ChunkInserted); - database->Init(path, chunk_callback); + database->Init(path); { // Acquiring the lock here guarantees correct ordering between the writes to // the new database object above, and the setting of |databse_| below. @@ -514,13 +512,6 @@ void SafeBrowsingService::OnGetAllChunksFromDatabase( protocol_manager_->OnGetChunksComplete(lists, database_error); } -void SafeBrowsingService::ChunkInserted() { - DCHECK(MessageLoop::current() == safe_browsing_thread_->message_loop()); - ChromeThread::PostTask( - ChromeThread::IO, FROM_HERE, - NewRunnableMethod(this, &SafeBrowsingService::OnChunkInserted)); -} - void SafeBrowsingService::OnChunkInserted() { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); if (enabled_) @@ -553,15 +544,24 @@ void SafeBrowsingService::DatabaseLoadComplete() { void SafeBrowsingService::HandleChunkForDatabase( const std::string& list_name, - std::deque<SBChunk>* chunks) { + SBChunkList* chunks) { DCHECK(MessageLoop::current() == safe_browsing_thread_->message_loop()); - GetDatabase()->InsertChunks(list_name, chunks); + if (chunks) { + GetDatabase()->InsertChunks(list_name, *chunks); + delete chunks; + } + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &SafeBrowsingService::OnChunkInserted)); } void SafeBrowsingService::DeleteChunks( std::vector<SBChunkDelete>* chunk_deletes) { DCHECK(MessageLoop::current() == safe_browsing_thread_->message_loop()); - GetDatabase()->DeleteChunks(chunk_deletes); + if (chunk_deletes) { + GetDatabase()->DeleteChunks(*chunk_deletes); + delete chunk_deletes; + } } SafeBrowsingService::UrlCheckResult SafeBrowsingService::GetResultFromListname( diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h index 5fb31a7..873dcb3 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.h +++ b/chrome/browser/safe_browsing/safe_browsing_service.h @@ -116,7 +116,7 @@ class SafeBrowsingService bool can_cache); // Called on the IO thread. - void HandleChunk(const std::string& list, std::deque<SBChunk>* chunks); + void HandleChunk(const std::string& list, SBChunkList* chunks); void HandleChunkDelete(std::vector<SBChunkDelete>* chunk_deletes); // Update management. Called on the IO thread. @@ -213,9 +213,6 @@ class SafeBrowsingService void OnGetAllChunksFromDatabase(const std::vector<SBListChunkRanges>& lists, bool database_error); - // Called on the db thread when a chunk insertion is complete. - void ChunkInserted(); - // Called on the IO thread after the database reports that it added a chunk. void OnChunkInserted(); @@ -227,7 +224,7 @@ class SafeBrowsingService // Called on the database thread to add/remove chunks and host keys. // Callee will free the data when it's done. void HandleChunkForDatabase(const std::string& list, - std::deque<SBChunk>* chunks); + SBChunkList* chunks); void DeleteChunks(std::vector<SBChunkDelete>* chunk_deletes); diff --git a/chrome/browser/safe_browsing/safe_browsing_util.cc b/chrome/browser/safe_browsing/safe_browsing_util.cc index 5386f27..4d7c7ae 100644 --- a/chrome/browser/safe_browsing/safe_browsing_util.cc +++ b/chrome/browser/safe_browsing/safe_browsing_util.cc @@ -27,6 +27,22 @@ static const char kContinueUrlFormat[] = static const char kReportParams[] = "?tpl=%s&continue=%s&url=%s"; +// SBChunkList ----------------------------------------------------------------- + +void SBChunkList::clear() { + for (std::vector<SBChunk>::iterator citer = chunks_.begin(); + citer != chunks_.end(); ++citer) { + for (std::deque<SBChunkHost>::iterator hiter = citer->hosts.begin(); + hiter != citer->hosts.end(); ++hiter) { + if (hiter->entry) { + hiter->entry->Destroy(); + hiter->entry = NULL; + } + } + } + chunks_.clear(); +} + // SBEntry --------------------------------------------------------------------- // static @@ -145,13 +161,6 @@ std::string GetListName(int list_id) { return (list_id == PHISH) ? kPhishingList : std::string(); } -void FreeChunks(std::deque<SBChunk>* chunks) { - for (; !chunks->empty(); chunks->pop_front()) { - for (; !chunks->front().hosts.empty(); chunks->front().hosts.pop_front()) - chunks->front().hosts.front().entry->Destroy(); - } -} - void GenerateHostsToCheck(const GURL& url, std::vector<std::string>* hosts) { hosts->clear(); const std::string host = url.host(); // const sidesteps GCC bugs below! diff --git a/chrome/browser/safe_browsing/safe_browsing_util.h b/chrome/browser/safe_browsing/safe_browsing_util.h index 21fd7e3..ea58891 100644 --- a/chrome/browser/safe_browsing/safe_browsing_util.h +++ b/chrome/browser/safe_browsing/safe_browsing_util.h @@ -12,6 +12,7 @@ #include <string> #include <vector> +#include "base/basictypes.h" #include "base/scoped_ptr.h" #include "chrome/browser/safe_browsing/chunk_range.h" @@ -59,6 +60,50 @@ struct SBChunk { std::deque<SBChunkHost> hosts; }; +// Container for a set of chunks. Interim wrapper to replace use of +// |std::deque<SBChunk>| with something having safer memory semantics. +// management. +// TODO(shess): |SBEntry| is currently a very roundabout way to hold +// things pending storage. It could be replaced with the structures +// used in SafeBrowsingStore, then lots of bridging code could +// dissappear. +class SBChunkList { + public: + SBChunkList() {} + ~SBChunkList() { + clear(); + } + + // Implement that subset of the |std::deque<>| interface which + // callers expect. + bool empty() const { return chunks_.empty(); } + size_t size() { return chunks_.size(); } + + void push_back(const SBChunk& chunk) { chunks_.push_back(chunk); } + SBChunk& back() { return chunks_.back(); } + SBChunk& front() { return chunks_.front(); } + const SBChunk& front() const { return chunks_.front(); } + + typedef std::vector<SBChunk>::const_iterator const_iterator; + const_iterator begin() const { return chunks_.begin(); } + const_iterator end() const { return chunks_.end(); } + + typedef std::vector<SBChunk>::iterator iterator; + iterator begin() { return chunks_.begin(); } + iterator end() { return chunks_.end(); } + + SBChunk& operator[](size_t n) { return chunks_[n]; } + const SBChunk& operator[](size_t n) const { return chunks_[n]; } + + // Calls |SBEvent::Destroy()| before clearing |chunks_|. + void clear(); + + private: + std::vector<SBChunk> chunks_; + + DISALLOW_COPY_AND_ASSIGN(SBChunkList); +}; + // Used when we get a gethash response. struct SBFullHashResult { SBFullHash hash; @@ -229,8 +274,6 @@ enum ListType { int GetListId(const std::string& name); std::string GetListName(int list_id); -void FreeChunks(std::deque<SBChunk>* chunks); - // Given a URL, returns all the hosts we need to check. They are returned // in order of size (i.e. b.c is first, then a.b.c). void GenerateHostsToCheck(const GURL& url, std::vector<std::string>* hosts); |