Persist download interrupt reason, both target and current paths, and url_chain.

chrome/browser/history/download_database.cc

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..11e1063adad6f5f830ddb98005b7af3975a65ceb 100644
--- a/chrome/browser/history/download_database.cc
+++ b/chrome/browser/history/download_database.cc
@@ -11,12 +11,15 @@
 #include "base/debug/alias.h"
 #include "base/file_path.h"
 #include "base/metrics/histogram.h"
+#include "base/stl_util.h"
+#include "base/stringprintf.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 +31,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,"           // downloads.id.
+    "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 +81,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 +150,53 @@ bool DownloadDatabase::MigrateDownloadsState() {
   return statement.Run();
 }
 
+bool DownloadDatabase::MigrateDownloadsReasonAndPaths() {
+  // 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.
+  if (!GetDB().Execute(kSchema))
+    return false;
+
+  // Populate it.  As we do so, we transform the time values from time_t
+  // (seconds since 1/1/1970 UTC), to our internal measure (microseconds
+  // since the Windows Epoch).  Note that this is dependent in the
+  // implementation of base::Time and needs to change if that changes.
+  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, "
+    "       CASE start_time WHEN 0 THEN 0 ELSE "
+    "            (start_time + 11644473600) * 1000000 END, "
+    "       received_bytes, total_bytes, "
+    "       state, ?, "
+    "       CASE end_time WHEN 0 THEN 0 ELSE "
+    "            (end_time + 11644473600) * 1000000 END, "
+    "       opened "
+    "FROM downloads_tmp"));
+  statement_populate.BindInt(0, content::DOWNLOAD_INTERRUPT_REASON_NONE);
+  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 +204,10 @@ bool DownloadDatabase::InitDownloadTable() {
     return EnsureColumnExists("end_time", "INTEGER NOT NULL DEFAULT 0") &&
            EnsureColumnExists("opened", "INTEGER NOT NULL DEFAULT 0");
   } else {
-    return GetDB().Execute(kSchema);
+    // If the "downloads" table doesn't exist, the downloads_url_chain
+    // table better not.
+    return (!GetDB().DoesTableExist("downloads") &&

[benjhayden, 2012/11/15 21:57:54]

I don't think this is quite right.
I think a more exact translation of what we discussed offline might be something
like this:

bool DownloadDatabase::InitDownloadTable() {
  GetMetaTable().GetValue(kNextDownloadId, &next_id_);
  bool downloads_exists = GetDB().DoesTableExist("downloads");
  if (!downloads_exists &&
      GetDB().DoesTableExist("downloads_url_chain"))
    return false;  // WTF, downloads_url_chain but no downloads?
  if (downloads_exists)
    return true;
  return GetDB().Execute(kSchema) &&
         GetDB().Execute(kUrlChainSchema);
}

+            GetDB().Execute(kSchema) && GetDB().Execute(kUrlChainSchema));
   }
 }
 
@@ -160,36 +224,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;
+
+    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::FromInternalValue(
+        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::FromInternalValue(
+        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;
+
+    // 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,24 +319,30 @@ 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.ToInternalValue());
+  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);
-  statement.BindInt64(1, db_handle);
+      "UPDATE downloads SET target_path=?, current_path=? WHERE id=?"));
+  BindFilePath(statement, target_path, 0);
+  BindFilePath(statement, current_path, 1);
+  statement.BindInt64(2, db_handle);
 
   return statement.Run();
 }
@@ -251,48 +373,75 @@ 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());
-  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.ToInternalValue());
+    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.ToInternalValue());
+    statement_insert.BindInt(column++, info.opened ? 1 : 0);
+    if (!statement_insert.Run()) {
+      LOG(WARNING) << "Main insertion for download create failed.";
+      return 0;
+    }
   }
-  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) {
   CheckThread();
 
-  sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
+  sql::Statement downloads_statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM downloads WHERE id=?"));
-  statement.BindInt64(0, db_handle);
+  downloads_statement.BindInt64(0, db_handle);
+  downloads_statement.Run();
 
-  statement.Run();
+  sql::Statement urlchain_statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
+      "DELETE FROM downloads_url_chains WHERE id=?"));
+  urlchain_statement.BindInt64(0, db_handle);
+  urlchain_statement.Run();
 }
 
 bool DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin,
                                               base::Time delete_end) {
   CheckThread();
-  time_t start_time = delete_begin.ToTimeT();
-  time_t end_time = delete_end.ToTimeT();
+  time_t start_time = delete_begin.ToInternalValue();
+  time_t end_time = delete_end.ToInternalValue();
 
   int num_downloads_deleted = -1;
   {
@@ -310,24 +459,41 @@ bool DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin,
       num_downloads_deleted = count.ColumnInt(0);
   }
 
-
   bool success = false;
   base::TimeTicks started_removing = base::TimeTicks::Now();
   {
     // 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,
-        "DELETE FROM downloads WHERE start_time >= ? AND start_time < ? "
-        "AND (State = ? OR State = ? OR State = ?)"));
-    statement.BindInt64(0, start_time);
-    statement.BindInt64(
+    const char* deletion_conditional =
+        "WHERE start_time >= ? AND start_time < ? AND "
+        "      (State = ? OR STATE = ? OR State = ?)";
+
+    sql::Statement statement_url_chains(GetDB().GetCachedStatement(
+        SQL_FROM_HERE,
+        base::StringPrintf("DELETE FROM downloads_url_chains WHERE id IN "
+                           "  (SELECT id FROM downloads %s)",
+                           deletion_conditional).c_str()));
+    statement_url_chains.BindInt64(0, start_time);
+    statement_url_chains.BindInt64(
+        1,
+        end_time ? end_time : std::numeric_limits<int64>::max());
+    statement_url_chains.BindInt(2, kStateComplete);
+    statement_url_chains.BindInt(3, kStateCancelled);
+    statement_url_chains.BindInt(4, kStateInterrupted);
+    success = statement_url_chains.Run();
+
+    sql::Statement statement_downloads(GetDB().GetCachedStatement(
+        SQL_FROM_HERE, base::StringPrintf("DELETE FROM downloads %s",
+                                          deletion_conditional).c_str()));
+    statement_downloads.BindInt64(0, start_time);
+    statement_downloads.BindInt64(
         1,
         end_time ? end_time : std::numeric_limits<int64>::max());
-    statement.BindInt(2, kStateComplete);
-    statement.BindInt(3, kStateCancelled);
-    statement.BindInt(4, kStateInterrupted);
+    statement_downloads.BindInt(2, kStateComplete);
+    statement_downloads.BindInt(3, kStateCancelled);
+    statement_downloads.BindInt(4, kStateInterrupted);
 
-    success = statement.Run();
+    success = statement_downloads.Run() && success;
   }
 
   base::TimeTicks finished_removing = base::TimeTicks::Now();