add a download slices table into history download db

components/history/core/browser/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 "components/history/core/browser/download_database.h"
 
 #include <inttypes.h>
 
 #include <limits>
 #include <memory>
@@ skipping 294 matching lines @@
 
   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) )";
 
+  const char kJobsSchema[] =
+      "CREATE TABLE downloads_jobs ("
+      "download_id INTEGER NOT NULL,"       // downloads.id.
+      "job_id INTEGER NOT NULL,"            // Download job id.
+      "start_position INTEGER NOT NULL,"    // The start request position of the
+                                            // download job.
+      "length INTEGER NOT NULL,"            // length of the request, -1
+                                            // if not specified
+      "received_bytes INTEGER NOT NULL,"    // Total bytes downloaded.
+      "state INTEGER NOT NULL,"             // To be determined.

[sky, 2017/02/03 16:02:33]

Is this comment right? It seems like you're using it.

[qinmin, 2017/02/04 00:06:56]

Done.

+      "interrupt_reason INTEGER NOT NULL, " // DownloadInterruptReason
+      "PRIMARY KEY (download_id, job_id) )";
+
+  bool ret;
   if (GetDB().DoesTableExist("downloads")) {
-    return EnsureColumnExists("end_time", "INTEGER NOT NULL DEFAULT 0") &&
-           EnsureColumnExists("opened", "INTEGER NOT NULL DEFAULT 0");
+    ret = EnsureColumnExists("end_time", "INTEGER NOT NULL DEFAULT 0") &&
+        EnsureColumnExists("opened", "INTEGER NOT NULL DEFAULT 0");
   } else {
     // 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));
+    ret = (!GetDB().DoesTableExist("downloads_url_chain") &&
+        GetDB().Execute(kSchema) && GetDB().Execute(kUrlChainSchema));
   }
+
+  return ret && (GetDB().DoesTableExist("downloads_jobs") ||
+      GetDB().Execute(kJobsSchema));
 }
 
 uint32_t DownloadDatabase::GetNextDownloadId() {
   sql::Statement select_max_id(GetDB().GetUniqueStatement(
       "SELECT max(id) FROM downloads"));
   bool result = select_max_id.Step();
   DCHECK(result);
   // If there are zero records in the downloads table, then max(id) will
   // return 0 = kInvalidDownloadId, so GetNextDownloadId() will set
   // *id = kInvalidDownloadId + 1.
@@ skipping 132 matching lines @@
       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)));
   }
 
+  sql::Statement statement_download_job(GetDB().GetCachedStatement(

[sky, 2017/02/03 16:02:33]

wnloadDatabase::QueryDownload is incredibly long. Can you move querying the jobs
into its own function? QueryDownload can still call it, but it's easier to
reason about smaller functions.

[qinmin, 2017/02/04 00:06:56]

Done.

+      SQL_FROM_HERE,
+      "SELECT download_id, job_id, start_position, length, received_bytes, "
+      "state, interrupt_reason FROM downloads_jobs "
+      "ORDER BY download_id, job_id"));
+
+  while (statement_download_job.Step()) {
+    int column = 0;
+    // See the comment above about SQLITE lacking unsigned integers.
+    int64_t signed_id = statement_download_job.ColumnInt64(column++);
+    if (signed_id <= static_cast<int64_t>(kInvalidDownloadId))
+      continue;

[sky, 2017/02/03 16:02:33]

Wouldn't this indicate corruption? Should we log it?

[qinmin, 2017/02/04 00:06:56]

Done.

+    int download_id = IntToDownloadId(signed_id);
+    // Confirm the download_id has already been seen--if it hasn't, discard the
+    // record.
+    bool found = base::ContainsKey(info_map, download_id);
+    UMA_HISTOGRAM_BOOLEAN(
+        "Download.DatabaseDownloadExistsForDownloadJob", found);
+    DCHECK(found);
+    if (!found)
+      continue;
+
+    DownloadJobInfo info;
+    info.download_id = download_id;
+    info.job_id = statement_download_job.ColumnInt(column++);
+    info.start_position = statement_download_job.ColumnInt64(column++);
+    info.length = statement_download_job.ColumnInt64(column++);
+    info.received_bytes = statement_download_job.ColumnInt64(column++);
+    info.state = IntToDownloadState(
+        statement_download_job.ColumnInt(column++));
+    info.interrupt_reason = IntToDownloadInterruptReason(
+        statement_download_job.ColumnInt(column++));
+    info_map[download_id]->download_job_info.push_back(info);
+  }
+
   for (std::map<uint32_t, DownloadRow*>::iterator it = info_map.begin();
        it != info_map.end(); ++it) {
     DownloadRow* row = it->second;
     bool empty_url_chain = row->url_chain.empty();
     UMA_HISTOGRAM_BOOLEAN("Download.DatabaseEmptyUrlChain", empty_url_chain);
     if (empty_url_chain) {
       RemoveDownload(row->id);
     } else {
       // Copy the contents of the stored info.
       results->push_back(*row);
@@ skipping 40 matching lines @@
   statement.BindBlob(column++, data.hash.data(), data.hash.size());
   statement.BindInt64(column++, data.end_time.ToInternalValue());
   statement.BindInt64(column++, data.total_bytes);
   statement.BindInt(column++, (data.opened ? 1 : 0));
   statement.BindString(column++, data.by_ext_id);
   statement.BindString(column++, data.by_ext_name);
   statement.BindString(column++, data.etag);
   statement.BindString(column++, data.last_modified);
   statement.BindInt(column++, DownloadIdToInt(data.id));
 
-  return statement.Run();
+  if (!statement.Run())
+    return false;
+
+  for (size_t i = 0; i < data.download_job_info.size(); ++i) {
+    // Create a new download job if it doesn't exist in the downloads_jobs
+    // table.
+    if (!UpdateDownloadJob(data.download_job_info[i]) &&
+        !CreateDownloadJob(data.download_job_info[i])) {
+      return false;

[sky, 2017/02/03 16:02:33]

Is it possible to get test coverage of this code path?

[qinmin, 2017/02/04 00:06:56]

Done. Added HistoryBackendDBTest.UpdateDownloadWithNewJob() test

+    }
+  }
+
+  return true;
 }
 
 void DownloadDatabase::EnsureInProgressEntriesCleanedUp() {
   if (in_progress_entry_cleanup_completed_)
     return;
 
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads SET state=?, interrupt_reason=? WHERE state=?"));
   statement.BindInt(0, DownloadStateToInt(DownloadState::INTERRUPTED));
   statement.BindInt(
       1, DownloadInterruptReasonToInt(download_interrupt_reason_crash_));
   statement.BindInt(2, DownloadStateToInt(DownloadState::IN_PROGRESS));
 
   statement.Run();
+
+  sql::Statement statement_download_job(GetDB().GetCachedStatement(

[sky, 2017/02/03 16:02:33]

Is there test coverage of this code path?

[qinmin, 2017/02/04 00:06:56]

Done. HistoryBackendDBTest.ConfirmDownloadInProgressCleanup() now tests this

+      SQL_FROM_HERE,
+      "UPDATE downloads_jobs SET state=?, interrupt_reason=? WHERE state=?"));
+  statement_download_job.BindInt(
+      0, DownloadStateToInt(DownloadState::INTERRUPTED));
+  statement_download_job.BindInt(
+      1, DownloadInterruptReasonToInt(download_interrupt_reason_crash_));
+  statement_download_job.BindInt(
+      2, DownloadStateToInt(DownloadState::IN_PROGRESS));
+  statement_download_job.Run();
   in_progress_entry_cleanup_completed_ = true;
 }
 
 bool DownloadDatabase::CreateDownload(const DownloadRow& info) {
   DCHECK_NE(kInvalidDownloadId, info.id);
   DCHECK(!info.guid.empty());
   SCOPED_UMA_HISTOGRAM_TIMER("Download.Database.CreateDownloadDuration");
   EnsureInProgressEntriesCleanedUp();
 
   if (info.url_chain.empty())
@@ skipping 82 matching lines @@
     statement_insert_chain.BindInt(1, static_cast<int>(i));
     statement_insert_chain.BindString(2, info.url_chain[i].spec());
     if (!statement_insert_chain.Run()) {
       UMA_HISTOGRAM_ENUMERATION("Download.DatabaseURLChainInsertError",
                                 GetDB().GetErrorCode() & 0xff, 50);
       RemoveDownload(info.id);
       return false;
     }
     statement_insert_chain.Reset(true);
   }
+
+  for (size_t i = 0; i < info.download_job_info.size(); ++i) {
+    if (!CreateDownloadJob(info.download_job_info[i])) {
+      RemoveDownload(info.id);
+      return false;
+    }
+  }
+
   return true;
 }
 
 void DownloadDatabase::RemoveDownload(uint32_t id) {
   EnsureInProgressEntriesCleanedUp();
 
   sql::Statement downloads_statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM downloads WHERE id=?"));
   downloads_statement.BindInt(0, id);
   if (!downloads_statement.Run()) {
     UMA_HISTOGRAM_ENUMERATION("Download.DatabaseMainDeleteError",
                               GetDB().GetErrorCode() & 0xff, 50);
     return;
   }
   RemoveDownloadURLs(id);
+  RemoveDownloadJobs(id);

[sky, 2017/02/03 16:02:33]

Is there test coverage of this?

[qinmin, 2017/02/04 00:06:56]

Done. HistoryBackendDBTest.ConfirmDownloadRowCreateAndDelete() now tests this

 }
 
 void DownloadDatabase::RemoveDownloadURLs(uint32_t id) {
   sql::Statement urlchain_statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM downloads_url_chains WHERE id=?"));
   urlchain_statement.BindInt(0, id);
   if (!urlchain_statement.Run()) {
     UMA_HISTOGRAM_ENUMERATION("Download.DatabaseURLChainDeleteError",
                               GetDB().GetErrorCode() & 0xff, 50);
   }
 }
 
 size_t DownloadDatabase::CountDownloads() {
   EnsureInProgressEntriesCleanedUp();
 
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "SELECT count(*) from downloads"));
   statement.Step();
   return statement.ColumnInt(0);
 }
 
+bool DownloadDatabase::CreateDownloadJob(const DownloadJobInfo& info) {
+  sql::Statement statement_insert(GetDB().GetCachedStatement(
+      SQL_FROM_HERE,
+      "INSERT INTO downloads_jobs "
+      "(download_id, job_id, start_position, length, received_bytes, state, "
+      "interrupt_reason) "
+      "VALUES (?, ?, ?, ?, ?, ?, ?)"));
+  int column = 0;
+  statement_insert.BindInt(column++, info.download_id);
+  statement_insert.BindInt(column++, info.job_id);
+  statement_insert.BindInt64(column++, info.start_position);
+  statement_insert.BindInt64(column++, info.length);
+  statement_insert.BindInt64(column++, info.received_bytes);
+  statement_insert.BindInt(column++, DownloadStateToInt(info.state));
+  statement_insert.BindInt(
+      column++, DownloadInterruptReasonToInt(info.interrupt_reason));
+  return statement_insert.Run();
+}
+
+bool DownloadDatabase::UpdateDownloadJob(const DownloadJobInfo& info) {
+  sql::Statement statement_update(GetDB().GetCachedStatement(
+      SQL_FROM_HERE,
+      "UPDATE downloads_jobs "
+      "SET start_position=?, length=?, received_bytes=?, state=?, "
+      "interrupt_reason=? "
+      "WHERE download_id=? AND job_id=?"));
+  int column = 0;
+  statement_update.BindInt64(column++, info.start_position);
+  statement_update.BindInt64(column++, info.length);
+  statement_update.BindInt64(column++, info.received_bytes);
+  statement_update.BindInt(column++, DownloadStateToInt(info.state));
+  statement_update.BindInt(
+      column++, DownloadInterruptReasonToInt(info.interrupt_reason));
+  statement_update.BindInt(column++, info.download_id);
+  statement_update.BindInt(column++, info.job_id);
+  return statement_update.Run();
+}
+
+void DownloadDatabase::RemoveDownloadJobs(uint32_t id) {
+  sql::Statement statement_delete(GetDB().GetCachedStatement(SQL_FROM_HERE,
+      "DELETE FROM downloads_jobs WHERE download_id=?"));
+  statement_delete.BindInt(0, id);
+  statement_delete.Run();
+}
+
 }  // namespace history