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

chrome/browser/history/download_database.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/history/download_database.h"
 
 #include <limits>
 #include <string>
 #include <vector>
 
 #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 "chrome/browser/history/download_row.h"
+#include "chrome/browser/history/history_types.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_item.h"
 #include "sql/statement.h"
 
 using content::DownloadItem;
 
 namespace history {
 
 // static
 const int64 DownloadDatabase::kUninitializedHandle = -1;
 
 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.
+  "current_path LONGVARCHAR NOT NULL," // Current location of the download
+                                       // on disk.
+  "target_path LONGVARCHAR NOT NULL," // Final 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
 // of the values returned by StateToInt, and that the values of the |state|
 // field of the DownloadRows returned by QueryDownloads() are one of the values
 // returned by IntToState().
 static const int kStateInvalid = -1;
 static const int kStateInProgress = 0;
 static const int kStateComplete = 1;
 static const int kStateCancelled = 2;
 static const int kStateBug140687 = 3;
 static const int kStateInterrupted = 4;
 
 int StateToInt(DownloadItem::DownloadState state) {
   switch (state) {
     case DownloadItem::IN_PROGRESS: return kStateInProgress;
     case DownloadItem::COMPLETE: return kStateComplete;
     case DownloadItem::CANCELLED: return kStateCancelled;
     case DownloadItem::INTERRUPTED: return kStateInterrupted;
     case DownloadItem::MAX_DOWNLOAD_STATE: return kStateInvalid;
     default: return kStateInvalid;
   }
 }
 
 DownloadItem::DownloadState IntToState(int state) {
   switch (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;
   }
 }
 
 #if defined(OS_POSIX)
 
 // Binds/reads the given file path to the given column of the given statement.
 void BindFilePath(sql::Statement& statement, const FilePath& path, int col) {
@@ skipping 39 matching lines @@
 }
 
 bool DownloadDatabase::MigrateDownloadsState() {
   sql::Statement statement(GetDB().GetUniqueStatement(
         "UPDATE downloads SET state=? WHERE state=?"));
   statement.BindInt(0, kStateInterrupted);
   statement.BindInt(1, kStateBug140687);
   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

[benjhayden, 2012/12/10 16:38:42]

dependent on

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

Done.

+  // implementation of base::Time and needs to change if that changes.

[benjhayden, 2012/12/10 16:38:42]

What parts of base::Time specifically?

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

Updated comment.

+  sql::Statement statement_populate(GetDB().GetUniqueStatement(
+    "INSERT INTO downloads "
+    "( id, current_path, target_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() {
   GetMetaTable().GetValue(kNextDownloadId, &next_id_);
   if (GetDB().DoesTableExist("downloads")) {
     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_url_chain") &&
+            GetDB().Execute(kSchema) && GetDB().Execute(kUrlChainSchema));
   }
 }
 
 bool DownloadDatabase::DropDownloadTable() {
   return GetDB().Execute("DROP TABLE downloads");
 }
 
 void DownloadDatabase::QueryDownloads(
     std::vector<DownloadRow>* results) {
   results->clear();
   if (next_db_handle_ < 1)
     next_db_handle_ = 1;
   std::set<int64> 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, DownloadRow*> info_map;
+
+  sql::Statement statement_main(GetDB().GetCachedStatement(SQL_FROM_HERE,
+      "SELECT id, current_path, target_path, start_time, received_bytes, "
+        "total_bytes, state, interrupt_reason, end_time, opened "
       "FROM downloads "
       "ORDER BY start_time"));
 
-  while (statement.Step()) {
-    DownloadRow 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()) {
+    DownloadRow* info(new DownloadRow());
+    int column = 0;
+
+    info->db_handle = statement_main.ColumnInt64(column++);
+    info->current_path = ColumnFilePath(statement_main, column++);
+    info->target_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;

[benjhayden, 2012/12/10 16:38:42]

Does this leak info?

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

D'oh!  Thanks.  Switched to scoped_ptr<>.

     }
-    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);

[benjhayden, 2012/12/10 16:38:42]

Why not use the column++ trick here?

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

Didn't seem as necessary as there were only two items, so the "Is this item at
this index in the above select statement" check was pretty much equivalent to
the "Is this item after this other item in the above select statement" check. 
However, I certainly don't mind using it, and it has the advantage of
consistency.  Done.

+    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, DownloadRow*>::iterator
+           it = info_map.begin(); it != info_map.end(); ++it) {
+    results->push_back(*it->second);

[benjhayden, 2012/12/10 16:38:42]

Took me a second to see that this copies the info. Comment?

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

Done.

+    delete it->second;
+    it->second = NULL;
   }
 }
 
 bool DownloadDatabase::UpdateDownload(const DownloadRow& data) {
   DCHECK(data.db_handle > 0);
   int state = StateToInt(data.state);
   if (state == kStateInvalid) {
     // TODO(benjhayden) [D]CHECK instead.
     return false;
   }
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads "
-      "SET full_path=?, received_bytes=?, state=?, end_time=?, total_bytes=?, "
-      "opened=? WHERE id=?"));
-  BindFilePath(statement, data.path, 0);
-  statement.BindInt64(1, data.received_bytes);
-  statement.BindInt(2, state);
-  statement.BindInt64(3, data.end_time.ToTimeT());
-  statement.BindInt(4, data.total_bytes);
-  statement.BindInt(5, (data.opened ? 1 : 0));
-  statement.BindInt64(6, data.db_handle);
+      "SET current_path=?, target_path=?, received_bytes=?, state=?, "
+      "interrupt_reason=?, end_time=?, total_bytes=?, opened=? WHERE id=?"));
+  int column = 0;
+  BindFilePath(statement, data.current_path, column++);
+  BindFilePath(statement, data.target_path, column++);
+  statement.BindInt64(column++, data.received_bytes);
+  statement.BindInt(column++, state);
+  statement.BindInt(column++, static_cast<int>(data.interrupt_reason));
+  statement.BindInt64(column++, data.end_time.ToInternalValue());
+  statement.BindInt(column++, data.total_bytes);
+  statement.BindInt(column++, (data.opened ? 1 : 0));
+  statement.BindInt64(column++, data.db_handle);
 
   return statement.Run();
 }
 
 bool DownloadDatabase::CleanUpInProgressEntries() {
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads SET state=? WHERE state=?"));
   statement.BindInt(0, kStateCancelled);
   statement.BindInt(1, kStateInProgress);
 
   return statement.Run();
 }
 
 int64 DownloadDatabase::CreateDownload(
     const DownloadRow& info) {
   if (next_db_handle_ == 0) {
     // This is unlikely. All current known tests and users already call
     // QueryDownloads() before CreateDownload().
     std::vector<DownloadRow> results;
     QueryDownloads(&results);
     CHECK_NE(0, next_db_handle_);
   }
 
   int state = StateToInt(info.state);
   if (state == kStateInvalid)
     return kUninitializedHandle;
 
-  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);
+  {
+    sql::Statement statement_insert(GetDB().GetCachedStatement(
+        SQL_FROM_HERE,
+        "INSERT INTO downloads "
+        "(id, current_path, target_path, start_time, "
+        " received_bytes, total_bytes, state, interrupt_reason, "
+        " end_time, opened) "
+        "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
 
-  if (statement.Run()) {
-    // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
-    GetMetaTable().SetValue(kNextDownloadId, ++next_id_);
+    int column = 0;
+    statement_insert.BindInt64(column++, db_handle);
+    BindFilePath(statement_insert, info.current_path, column++);
+    BindFilePath(statement_insert, info.target_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());

[benjhayden, 2012/12/10 16:38:42]

I'm going to trust that you've talked to the appropriate people about using
ToInternalValue, because I don't know it.

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

It was Scott in c#2 and c#4 above, and I believe he reviewed this file, so we
should be ok.

+    statement_insert.BindInt(column++, info.opened ? 1 : 0);
+    if (!statement_insert.Run()) {
+      LOG(WARNING) << "Main insertion for download create failed.";
+      return 0;

[benjhayden, 2012/12/10 16:38:42]

return kUninitializedHandle;

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

Done.

+    }
+  }
 
-    return db_handle;
+  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;

[benjhayden, 2012/12/10 16:38:42]

return kUninitializedHandle;

[Randy Smith (Not in Mondays), 2012/12/11 18:17:30]

Done.

+    }
+    statement_insert_chain.Reset(true);
   }
-  return kUninitializedHandle;
+
+  // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
+  GetMetaTable().SetValue(kNextDownloadId, ++next_id_);
+
+  return db_handle;
 }
 
 void DownloadDatabase::RemoveDownload(int64 handle) {
-  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, handle);
-  statement.Run();
+  downloads_statement.BindInt64(0, handle);
+  downloads_statement.Run();
+
+  sql::Statement urlchain_statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
+      "DELETE FROM downloads_url_chains WHERE id=?"));
+  urlchain_statement.BindInt64(0, handle);
+  urlchain_statement.Run();
 }
 
 int DownloadDatabase::CountDownloads() {
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "SELECT count(*) from downloads"));
   statement.Step();
   return statement.ColumnInt(0);
 }
 
 }  // namespace history