Index: chrome/browser/history/download_database.cc |
diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc |
index c9881156db31829652ffccc9ffd972e32777b381..9d84477434259b3a05540ecba9c599b2443306b9 100644 |
--- a/chrome/browser/history/download_database.cc |
+++ b/chrome/browser/history/download_database.cc |
@@ -11,12 +11,14 @@ |
#include "base/debug/alias.h" |
#include "base/file_path.h" |
#include "base/metrics/histogram.h" |
+#include "base/stl_util.h" |
#include "base/time.h" |
#include "base/utf_string_conversions.h" |
#include "build/build_config.h" |
#include "content/public/browser/browser_thread.h" |
#include "content/public/browser/download_item.h" |
#include "content/public/browser/download_persistent_store_info.h" |
+#include "content/public/browser/download_interrupt_reasons.h" |
#include "sql/statement.h" |
using content::DownloadItem; |
@@ -28,16 +30,27 @@ namespace { |
static const char kSchema[] = |
"CREATE TABLE downloads (" |
- "id INTEGER PRIMARY KEY," // SQLite-generated primary key. |
- "full_path LONGVARCHAR NOT NULL," // Location of the download on disk. |
- "url LONGVARCHAR NOT NULL," // URL of the downloaded file. |
+ "id INTEGER PRIMARY KEY," // Primary key. |
+ "target_path LONGVARCHAR NOT NULL," // Final location of the download on disk. |
+ "current_path LONGVARCHAR NOT NULL," // Current location of the download |
+ // on disk. |
"start_time INTEGER NOT NULL," // When the download was started. |
"received_bytes INTEGER NOT NULL," // Total size downloaded. |
"total_bytes INTEGER NOT NULL," // Total size of the download. |
- "state INTEGER NOT NULL," // 1=complete, 2=cancelled, 4=interrupted |
+ "state INTEGER NOT NULL," // 1=complete, 4=interrupted |
+ "interrupt_reason INTEGER NOT NULL,"// Reason the download was interrupted. |
"end_time INTEGER NOT NULL," // When the download completed. |
"opened INTEGER NOT NULL)"; // 1 if it has ever been opened else 0 |
+static const char kUrlChainSchema[] = |
+ "CREATE TABLE downloads_url_chains (" |
+ "id INTEGER NOT NULL," // SQLite-generated primary key. |
sky
2012/11/14 00:34:31
Your comment is a bit misleading here. Don't you m
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Right. Copy and paste error. Done.
|
+ "chain_index INTEGER NOT NULL," // Index of url in chain |
+ // 0 is initial target, |
+ // MAX is target after redirects. |
+ "url LONGVARCHAR NOT NULL, " // URL. |
+ "PRIMARY KEY (id, chain_index) )"; |
+ |
// These constants and next two functions are used to allow |
// DownloadItem::DownloadState to change without breaking the database schema. |
// They guarantee that the values of the |state| field in the database are one |
@@ -67,7 +80,7 @@ DownloadItem::DownloadState IntToState(int state) { |
case kStateInProgress: return DownloadItem::IN_PROGRESS; |
case kStateComplete: return DownloadItem::COMPLETE; |
case kStateCancelled: return DownloadItem::CANCELLED; |
- // We should not need kStateBug140687 here because MigrateDownloadState() |
+ // We should not need kStateBug140687 here because MigrateDownloadsState() |
// is called in HistoryDatabase::Init(). |
case kStateInterrupted: return DownloadItem::INTERRUPTED; |
default: return DownloadItem::MAX_DOWNLOAD_STATE; |
@@ -136,6 +149,43 @@ bool DownloadDatabase::MigrateDownloadsState() { |
return statement.Run(); |
} |
+bool DownloadDatabase::MigrateReasonAndPaths() { |
+ // We need to rename the table and copy back from it because SQLite |
+ // provides no way to rename or delete a column. |
+ if (!GetDB().Execute("ALTER TABLE downloads RENAME TO downloads_tmp")) |
+ return false; |
+ |
+ // Recreate main table and populate it. |
+ if (!GetDB().Execute(kSchema)) |
+ return false; |
+ |
+ sql::Statement statement_populate(GetDB().GetUniqueStatement( |
+ "INSERT INTO downloads " |
+ "( id, target_path, current_path, start_time, received_bytes, total_bytes, " |
+ " state, interrupt_reason, end_time, opened ) " |
+ "SELECT id, full_path, full_path, start_time, received_bytes, total_bytes, " |
+ " state, ?, end_time, opened " |
+ "FROM downloads_tmp")); |
+ statement_populate.BindInt(0, content::DOWNLOAD_INTERRUPT_REASON_NONE); |
sky
2012/11/14 00:34:31
I get nervous when we start persisting enums from
benjhayden
2012/11/14 19:43:22
I agree with Scott. This is how we ran into troubl
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Fair enough. There are enough errors in the enum
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Still pushing back, Milo--keeping 20 interrupt rea
benjhayden
2012/11/15 15:57:41
Comments don't do anything for me or the compiler.
|
+ if (!statement_populate.Run()) |
+ return false; |
+ |
+ // Create new chain table and populate it. |
+ if (!GetDB().Execute(kUrlChainSchema)) |
+ return false; |
+ |
+ if (!GetDB().Execute("INSERT INTO downloads_url_chains " |
+ " ( id, chain_index, url) " |
+ " SELECT id, 0, url from downloads_tmp")) |
+ return false; |
+ |
+ // Get rid of temporary table. |
+ if (!GetDB().Execute("DROP TABLE downloads_tmp")) |
+ return false; |
+ |
+ return true; |
+} |
+ |
bool DownloadDatabase::InitDownloadTable() { |
CheckThread(); |
GetMetaTable().GetValue(kNextDownloadId, &next_id_); |
@@ -143,7 +193,7 @@ bool DownloadDatabase::InitDownloadTable() { |
return EnsureColumnExists("end_time", "INTEGER NOT NULL DEFAULT 0") && |
EnsureColumnExists("opened", "INTEGER NOT NULL DEFAULT 0"); |
} else { |
- return GetDB().Execute(kSchema); |
+ return GetDB().Execute(kSchema) && GetDB().Execute(kUrlChainSchema); |
} |
} |
@@ -160,36 +210,88 @@ void DownloadDatabase::QueryDownloads( |
next_db_handle_ = 1; |
std::set<DownloadID> db_handles; |
- sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, |
- "SELECT id, full_path, url, start_time, received_bytes, " |
- "total_bytes, state, end_time, opened " |
+ std::map<DownloadID, DownloadPersistentStoreInfo*> info_map; |
+ |
+ sql::Statement statement_main(GetDB().GetCachedStatement(SQL_FROM_HERE, |
+ "SELECT id, target_path, current_path, start_time, received_bytes, " |
+ "total_bytes, state, interrupt_reason, end_time, opened " |
"FROM downloads " |
"ORDER BY start_time")); |
- while (statement.Step()) { |
- DownloadPersistentStoreInfo info; |
- info.db_handle = statement.ColumnInt64(0); |
- info.path = ColumnFilePath(statement, 1); |
- info.url = GURL(statement.ColumnString(2)); |
- info.start_time = base::Time::FromTimeT(statement.ColumnInt64(3)); |
- info.received_bytes = statement.ColumnInt64(4); |
- info.total_bytes = statement.ColumnInt64(5); |
- int state = statement.ColumnInt(6); |
- info.state = IntToState(state); |
- info.end_time = base::Time::FromTimeT(statement.ColumnInt64(7)); |
- info.opened = statement.ColumnInt(8) != 0; |
- if (info.db_handle >= next_db_handle_) |
- next_db_handle_ = info.db_handle + 1; |
- if (!db_handles.insert(info.db_handle).second) { |
- // info.db_handle was already in db_handles. The database is corrupt. |
- base::debug::Alias(&info.db_handle); |
+ while (statement_main.Step()) { |
+ DownloadPersistentStoreInfo* info(new DownloadPersistentStoreInfo); |
+ int column = 0; |
benjhayden
2012/11/14 19:43:22
I've also been very tempted to do this, and it is
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Does that make it different than what it replaces?
benjhayden
2012/11/15 15:57:41
I think there's a philosophical subtlety here. Let
|
+ |
+ info->db_handle = statement_main.ColumnInt64(column++); |
+ info->target_path = ColumnFilePath(statement_main, column++); |
+ info->current_path = ColumnFilePath(statement_main, column++); |
+ info->start_time = base::Time::FromTimeT( |
+ statement_main.ColumnInt64(column++)); |
+ info->received_bytes = statement_main.ColumnInt64(column++); |
+ info->total_bytes = statement_main.ColumnInt64(column++); |
+ int state = statement_main.ColumnInt(column++); |
+ info->state = IntToState(state); |
+ info->interrupt_reason = static_cast<content::DownloadInterruptReason>( |
+ statement_main.ColumnInt(column++)); |
+ info->end_time = base::Time::FromTimeT( |
+ statement_main.ColumnInt64(column++)); |
+ info->opened = statement_main.ColumnInt(column++) != 0; |
+ if (info->db_handle >= next_db_handle_) |
+ next_db_handle_ = info->db_handle + 1; |
+ if (!db_handles.insert(info->db_handle).second) { |
+ // info->db_handle was already in db_handles. The database is corrupt. |
+ base::debug::Alias(&info->db_handle); |
DCHECK(false); |
} |
- if (info.state == DownloadItem::MAX_DOWNLOAD_STATE) { |
+ if (info->state == DownloadItem::MAX_DOWNLOAD_STATE) { |
UMA_HISTOGRAM_COUNTS("Download.DatabaseInvalidState", state); |
continue; |
} |
- results->push_back(info); |
+ DCHECK(!ContainsKey(info_map, info->db_handle)); |
+ info_map[info->db_handle] = info; |
+ } |
+ |
+ sql::Statement statement_chain(GetDB().GetCachedStatement( |
+ SQL_FROM_HERE, |
+ "SELECT id, chain_index, url FROM downloads_url_chains " |
+ "ORDER BY id, chain_index")); |
+ |
+ while (statement_chain.Step()) { |
+ int64 db_handle = statement_chain.ColumnInt64(0); |
+ int chain_index = statement_chain.ColumnInt(1); |
+ |
+ // Note that these DCHECKs may trip as a result of corrupted databases. |
+ // We have them because in debug builds the chances are higher there's |
+ // an actual bug than that the database is corrupt, but we handle the |
+ // DB corruption case in production code. |
+ |
+ // Confirm the handle has already been seen--if it hasn't, discard the |
+ // record. |
+ DCHECK(ContainsKey(info_map, db_handle)); |
+ if (!ContainsKey(info_map, db_handle)) |
+ continue; |
+ |
+ // Confirm all previous URLs in the chain have already been seen; |
+ // if not, fill in with null or discard record. |
+ int current_chain_size = info_map[db_handle]->url_chain.size(); |
+ std::vector<GURL>* url_chain(&info_map[db_handle]->url_chain); |
+ DCHECK_EQ(chain_index, current_chain_size); |
+ while (current_chain_size < chain_index) { |
+ url_chain->push_back(GURL()); |
+ current_chain_size++; |
+ } |
+ if (current_chain_size > chain_index) |
+ continue; |
benjhayden
2012/11/14 19:43:22
Why not overwrite?
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
I flipped a coin :-}. Do you have some reason to
benjhayden
2012/11/15 15:57:41
Ah, I missed the ORDER BY clause and thought that
|
+ |
+ // Save the record. |
+ url_chain->push_back(GURL(statement_chain.ColumnString(2))); |
+ } |
+ |
+ for (std::map<DownloadID, DownloadPersistentStoreInfo*>::iterator |
+ it = info_map.begin(); it != info_map.end(); ++it) { |
+ results->push_back(*it->second); |
+ delete it->second; |
+ it->second = NULL; |
} |
} |
@@ -203,23 +305,29 @@ bool DownloadDatabase::UpdateDownload(const DownloadPersistentStoreInfo& data) { |
} |
sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, |
"UPDATE downloads " |
- "SET received_bytes=?, state=?, end_time=?, opened=? WHERE id=?")); |
- statement.BindInt64(0, data.received_bytes); |
- statement.BindInt(1, state); |
- statement.BindInt64(2, data.end_time.ToTimeT()); |
- statement.BindInt(3, (data.opened ? 1 : 0)); |
- statement.BindInt64(4, data.db_handle); |
+ "SET received_bytes=?, state=?, interrupt_reason=?, end_time=?, " |
+ " opened=? WHERE id=?")); |
+ |
+ int column = 0; |
+ statement.BindInt64(column++, data.received_bytes); |
+ statement.BindInt(column++, state); |
+ statement.BindInt(column++, data.interrupt_reason); |
+ statement.BindInt64(column++, data.end_time.ToTimeT()); |
+ statement.BindInt(column++, (data.opened ? 1 : 0)); |
+ statement.BindInt64(column++, data.db_handle); |
return statement.Run(); |
} |
-bool DownloadDatabase::UpdateDownloadPath(const FilePath& path, |
- DownloadID db_handle) { |
+bool DownloadDatabase::UpdateDownloadPath( |
+ const FilePath& target_path, const FilePath& current_path, |
+ DownloadID db_handle) { |
CheckThread(); |
DCHECK(db_handle > 0); |
sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, |
- "UPDATE downloads SET full_path=? WHERE id=?")); |
- BindFilePath(statement, path, 0); |
+ "UPDATE downloads SET target_path=?, current_path=? WHERE id=?")); |
+ BindFilePath(statement, target_path, 0); |
+ BindFilePath(statement, current_path, 0); |
benjhayden
2012/11/14 19:43:22
Check the column index here.
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Whoops. Good catch. Done. I'll see if I can wri
|
statement.BindInt64(1, db_handle); |
return statement.Run(); |
@@ -251,31 +359,54 @@ int64 DownloadDatabase::CreateDownload( |
if (state == kStateInvalid) |
return false; |
- sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, |
- "INSERT INTO downloads " |
- "(id, full_path, url, start_time, received_bytes, total_bytes, state, " |
- "end_time, opened) " |
- "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)")); |
- |
int db_handle = next_db_handle_++; |
- statement.BindInt64(0, db_handle); |
- BindFilePath(statement, info.path, 1); |
- statement.BindString(2, info.url.spec()); |
- statement.BindInt64(3, info.start_time.ToTimeT()); |
sky
2012/11/14 00:34:31
Since you're writing migration code, could you mig
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Done.
|
- statement.BindInt64(4, info.received_bytes); |
- statement.BindInt64(5, info.total_bytes); |
- statement.BindInt(6, state); |
- statement.BindInt64(7, info.end_time.ToTimeT()); |
- statement.BindInt(8, info.opened ? 1 : 0); |
- |
- if (statement.Run()) { |
- // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;} |
- GetMetaTable().SetValue(kNextDownloadId, ++next_id_); |
- |
- return db_handle; |
+ { |
+ sql::Statement statement_insert(GetDB().GetCachedStatement( |
+ SQL_FROM_HERE, |
+ "INSERT INTO downloads " |
+ "(id, target_path, current_path, start_time, " |
+ " received_bytes, total_bytes, state, interrupt_reason, " |
+ " end_time, opened) " |
+ "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)")); |
+ |
+ int column = 0; |
+ statement_insert.BindInt64(column++, db_handle); |
+ BindFilePath(statement_insert, info.target_path, column++); |
+ BindFilePath(statement_insert, info.current_path, column++); |
+ statement_insert.BindInt64(column++, info.start_time.ToTimeT()); |
+ statement_insert.BindInt64(column++, info.received_bytes); |
+ statement_insert.BindInt64(column++, info.total_bytes); |
+ statement_insert.BindInt(column++, state); |
+ statement_insert.BindInt(column++, content::DOWNLOAD_INTERRUPT_REASON_NONE); |
+ statement_insert.BindInt64(column++, info.end_time.ToTimeT()); |
+ statement_insert.BindInt(column++, info.opened ? 1 : 0); |
+ if (!statement_insert.Run()) { |
+ LOG(WARNING) << "Main insertion for download create failed."; |
+ return 0; |
benjhayden
2012/11/14 19:43:22
When you merge with my DownloadHistory, please be
Randy Smith (Not in Mondays)
2012/11/14 22:47:34
Will do.
|
+ } |
} |
- return 0; |
+ |
+ sql::Statement statement_insert_chain( |
+ GetDB().GetCachedStatement(SQL_FROM_HERE, |
+ "INSERT INTO downloads_url_chains " |
+ "(id, chain_index, url) " |
+ "VALUES (?, ?, ?)")); |
+ for (size_t i = 0; i < info.url_chain.size(); ++i) { |
+ statement_insert_chain.BindInt64(0, db_handle); |
+ statement_insert_chain.BindInt(1, i); |
+ statement_insert_chain.BindString(2, info.url_chain[i].spec()); |
+ if (!statement_insert_chain.Run()) { |
+ LOG(WARNING) << "Url insertion for download create failed."; |
+ return 0; |
+ } |
+ statement_insert_chain.Reset(true); |
+ } |
+ |
+ // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;} |
+ GetMetaTable().SetValue(kNextDownloadId, ++next_id_); |
+ |
+ return db_handle; |
} |
void DownloadDatabase::RemoveDownload(DownloadID db_handle) { |