diff options
author | rdsmith@chromium.org <rdsmith@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-19 20:36:13 +0000 |
---|---|---|
committer | rdsmith@chromium.org <rdsmith@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-19 20:36:13 +0000 |
commit | e5107ce3463cf71c72ddab762029c3597937cbee (patch) | |
tree | c0e2ceb8fee7210dd9058adc919b11af6fe2bb18 | |
parent | ed34899180b46a71f4f1209b3b7d3aaa2b4636c9 (diff) | |
download | chromium_src-e5107ce3463cf71c72ddab762029c3597937cbee.zip chromium_src-e5107ce3463cf71c72ddab762029c3597937cbee.tar.gz chromium_src-e5107ce3463cf71c72ddab762029c3597937cbee.tar.bz2 |
Some checks to narrow in on duplicate history ids from history DB.
R=benjhayden@chromium.org
BUG=96627
Review URL: http://codereview.chromium.org/7904025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@101814 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/history/download_database.cc | 49 | ||||
-rw-r--r-- | chrome/browser/history/download_database.h | 8 | ||||
-rw-r--r-- | content/browser/download/download_manager.cc | 5 |
3 files changed, 57 insertions, 5 deletions
diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc index 906ac83..10e078e 100644 --- a/chrome/browser/history/download_database.cc +++ b/chrome/browser/history/download_database.cc @@ -10,6 +10,7 @@ #include "base/file_path.h" #include "base/utf_string_conversions.h" #include "build/build_config.h" +#include "content/browser/browser_thread.h" #include "content/browser/download/download_item.h" #include "content/browser/download/download_persistent_store_info.h" #include "sql/statement.h" @@ -54,7 +55,8 @@ FilePath ColumnFilePath(sql::Statement& statement, int col) { } // namespace -DownloadDatabase::DownloadDatabase() { +DownloadDatabase::DownloadDatabase() + : owning_thread_set_(false) { } DownloadDatabase::~DownloadDatabase() { @@ -147,6 +149,13 @@ bool DownloadDatabase::CleanUpInProgressEntries() { int64 DownloadDatabase::CreateDownload( const DownloadPersistentStoreInfo& info) { + if (owning_thread_set_) { + CHECK_EQ(owning_thread_, base::PlatformThread::CurrentId()); + } else { + owning_thread_ = base::PlatformThread::CurrentId(); + owning_thread_set_ = true; + } + sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, "INSERT INTO downloads " "(full_path, url, start_time, received_bytes, total_bytes, state) " @@ -161,8 +170,14 @@ int64 DownloadDatabase::CreateDownload( statement.BindInt64(4, info.total_bytes); statement.BindInt(5, info.state); - if (statement.Run()) - return GetDB().GetLastInsertRowId(); + if (statement.Run()) { + int64 id = GetDB().GetLastInsertRowId(); + + CHECK_EQ(0u, returned_ids_.count(id)); + returned_ids_.insert(id); + + return id; + } return 0; } @@ -174,10 +189,36 @@ void DownloadDatabase::RemoveDownload(DownloadID db_handle) { statement.BindInt64(0, db_handle); statement.Run(); + + returned_ids_.erase(db_handle); } void DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin, base::Time delete_end) { + time_t start_time = delete_begin.ToTimeT(); + time_t end_time = delete_end.ToTimeT(); + + // TODO(rdsmith): Remove when http://crbug.com/96627 is resolved. + { + sql::Statement dbg_statement(GetDB().GetCachedStatement( + SQL_FROM_HERE, + "SELECT id FROM downloads WHERE start_time >= ? AND start_time < ? " + "AND (State = ? OR State = ? OR State = ?)")); + if (!dbg_statement) + return; + dbg_statement.BindInt64(0, start_time); + dbg_statement.BindInt64( + 1, + end_time ? end_time : std::numeric_limits<int64>::max()); + dbg_statement.BindInt(2, DownloadItem::COMPLETE); + dbg_statement.BindInt(3, DownloadItem::CANCELLED); + dbg_statement.BindInt(4, DownloadItem::INTERRUPTED); + while (dbg_statement.Step()) { + int64 id_to_delete = dbg_statement.ColumnInt64(0); + returned_ids_.erase(id_to_delete); + } + } + // This does not use an index. We currently aren't likely to have enough // downloads where an index by time will give us a lot of benefit. sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, @@ -186,8 +227,6 @@ void DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin, if (!statement) return; - time_t start_time = delete_begin.ToTimeT(); - time_t end_time = delete_end.ToTimeT(); statement.BindInt64(0, start_time); statement.BindInt64( 1, diff --git a/chrome/browser/history/download_database.h b/chrome/browser/history/download_database.h index c77500c..85c6722 100644 --- a/chrome/browser/history/download_database.h +++ b/chrome/browser/history/download_database.h @@ -6,7 +6,10 @@ #define CHROME_BROWSER_HISTORY_DOWNLOAD_DATABASE_H_ #pragma once +#include <set> + #include "chrome/browser/history/history_types.h" +#include "base/threading/platform_thread.h" struct DownloadPersistentStoreInfo; class FilePath; @@ -63,6 +66,11 @@ class DownloadDatabase { bool DropDownloadTable(); private: + // TODO(rdsmith): Remove after http://crbug.com/96627 has been resolved. + std::set<int64> returned_ids_; + bool owning_thread_set_; + base::PlatformThreadId owning_thread_; + DISALLOW_COPY_AND_ASSIGN(DownloadDatabase); }; diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc index 3a2a7ee..f1c52d4 100644 --- a/content/browser/download/download_manager.cc +++ b/content/browser/download/download_manager.cc @@ -848,6 +848,11 @@ void DownloadManager::OnDownloadItemAddedToPersistentStore(int32 download_id, // TODO(rdsmith): Remove after http://crbug.com/85408 resolved. int64 largest_handle = largest_db_handle_in_history_; base::debug::Alias(&largest_handle); + int32 matching_item_download_id + = (ContainsKey(history_downloads_, db_handle) ? + history_downloads_[db_handle]->id() : 0); + base::debug::Alias(&matching_item_download_id); + CHECK(!ContainsKey(history_downloads_, db_handle)); AddDownloadItemToHistory(download, db_handle); |