Add new UMA stats to get a handle on Downloads UI Usage

chrome/browser/history/download_database.cc

 // Copyright (c) 2011 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 <vector>
 
 #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"
 
 // Download schema:
 //
 //   id             SQLite-generated primary key.
 //   full_path      Location of the download on disk.
 //   url            URL of the downloaded file.
 //   start_time     When the download was started.
 //   received_bytes Total size downloaded.
 //   total_bytes    Total size of the download.
 //   state          Identifies if this download is completed or not. Not used
 //                  directly by the history system. See DownloadItem's
 //                  DownloadState for where this is used.
+//   end_time       When the download completed.
+//   opened         1 if the download has ever been opened else 0
 
 namespace history {
 
 namespace {
 
 #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) {
   statement.BindString(col, path.value());
@@ skipping 11 matching lines @@
 FilePath ColumnFilePath(sql::Statement& statement, int col) {
   return FilePath(UTF8ToUTF16(statement.ColumnString(col)));
 }
 
 #endif
 
 // Key in the meta_table containing the next id to use for a new download in
 // this profile.
 static const char kNextDownloadId[] = "next_download_id";
 
+// Key in the meta_table containing the active schema version of the downloads
+// table.
+static const char kDownloadsSchemaVersion[] = "downloads_schema_version";
+
 }  // namespace
 
 DownloadDatabase::DownloadDatabase()
     : owning_thread_set_(false),
-      next_id_(0) {
+      next_id_(0),
+      schema_version_(DOWNLOADS_SCHEMA_VERSION_UNINITIALIZED) {
 }
 
 DownloadDatabase::~DownloadDatabase() {
 }
 
-bool DownloadDatabase::InitDownloadTable() {
-  if (!GetDB().DoesTableExist("downloads")) {
-    if (!GetDB().Execute(
+bool DownloadDatabase::CreateDownloadsTable() {
+  if (!GetDB().Execute(
         "CREATE TABLE downloads ("
         "id INTEGER PRIMARY KEY,"
         "full_path LONGVARCHAR NOT NULL,"
         "url LONGVARCHAR NOT NULL,"
         "start_time INTEGER NOT NULL,"
         "received_bytes INTEGER NOT NULL,"
         "total_bytes INTEGER NOT NULL,"
-        "state INTEGER NOT NULL)"))
-      return false;
-  }
-  meta_table_.Init(&GetDB(), 0, 0);
-  meta_table_.GetValue(kNextDownloadId, &next_id_);
+        "state INTEGER NOT NULL,"
+        "end_time INTEGER NOT NULL,"
+        "opened INTEGER NOT NULL)"))
+    return false;
+  schema_version_ = kCurrentDownloadsSchemaVersion;
   return true;
 }
 
+bool DownloadDatabase::MaybeUpgradeDownloadsSchema() {
+  if (schema_version_ == kCurrentDownloadsSchemaVersion)
+    return true;
+  // Don't rely on the schema always being backwards-compatible.
+  if (schema_version_ > kCurrentDownloadsSchemaVersion)
+    return false;
+  if (schema_version_ == DOWNLOADS_SCHEMA_VERSION_BASE) {
+    if (!GetDB().Execute(
+          "ALTER TABLE downloads ADD COLUMN "
+          "end_time INTEGER NOT NULL DEFAULT 0") ||
+        !GetDB().Execute(
+          "ALTER TABLE downloads ADD COLUMN "
+          "opened INTEGER NOT NULL DEFAULT 0")) {
+      return false;
+    }
+    schema_version_ = DOWNLOADS_SCHEMA_VERSION_OPENED;
+  }
+  return true;
+}
+
+bool DownloadDatabase::InitDownloadTable() {
+  meta_table_.Init(&GetDB(), 0, 0);
+  meta_table_.GetValue(kNextDownloadId, &next_id_);
+  int version = schema_version_;

[Randy Smith (Not in Mondays), 2011/09/26 18:27:26]

Does initializing version do anything here?  It looks like it's overwritten on
all paths.  Why not just use schema_version_ directly?

[benjhayden, 2011/09/28 17:35:23]

Rewritten to use DoesColumnExist, which is easier and precedented.

+  if (!meta_table_.GetValue(kDownloadsSchemaVersion, &version)) {
+    version = DOWNLOADS_SCHEMA_VERSION_BASE;
+  }
+  schema_version_ = static_cast<DownloadsSchemaVersion>(version);
+  if (GetDB().DoesTableExist("downloads")) {
+    if (!MaybeUpgradeDownloadsSchema())
+      return false;
+  } else {
+    if (!CreateDownloadsTable())
+      return false;
+  }
+  meta_table_.SetValue(kDownloadsSchemaVersion, schema_version_);
+  return true;
+}
+
 bool DownloadDatabase::DropDownloadTable() {
   return GetDB().Execute("DROP TABLE downloads");
 }
 
 void DownloadDatabase::QueryDownloads(
     std::vector<DownloadPersistentStoreInfo>* results) {
   results->clear();
 
+  CHECK(schema_version_ >= DOWNLOADS_SCHEMA_VERSION_OPENED);
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "SELECT id, full_path, url, start_time, received_bytes, "
-        "total_bytes, state "
+        "total_bytes, state, end_time, opened "
       "FROM downloads "
       "ORDER BY start_time"));
   if (!statement)
     return;
 
   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);
     info.state = statement.ColumnInt(6);
+    info.end_time = base::Time::FromTimeT(statement.ColumnInt64(7));
+    info.opened = statement.ColumnInt(8) != 0;
     results->push_back(info);
   }
 }
 
 bool DownloadDatabase::UpdateDownload(int64 received_bytes,
                                       int32 state,
+                                      const base::Time& end_time,
                                       DownloadID db_handle) {
   DCHECK(db_handle > 0);
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads "
-      "SET received_bytes=?, state=? WHERE id=?"));
+      "SET received_bytes=?, state=?, end_time=? WHERE id=?"));
   if (!statement)
     return false;
 
   statement.BindInt64(0, received_bytes);
   statement.BindInt(1, state);
-  statement.BindInt64(2, db_handle);
+  statement.BindInt64(2, end_time.ToTimeT());
+  statement.BindInt64(3, db_handle);
   return statement.Run();
 }
 
+void DownloadDatabase::MarkDownloadOpened(DownloadID db_handle) {
+  CHECK(schema_version_ >= DOWNLOADS_SCHEMA_VERSION_OPENED);
+  sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
+      "UPDATE downloads SET opened=? WHERE id=?"));
+  if (!statement)
+    return;
+  statement.BindInt(0, 1);
+  statement.BindInt64(1, db_handle);
+  statement.Run();
+}
+
 bool DownloadDatabase::UpdateDownloadPath(const FilePath& path,
                                           DownloadID db_handle) {
   DCHECK(db_handle > 0);
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads SET full_path=? WHERE id=?"));
   if (!statement)
     return false;
 
   BindFilePath(statement, path, 0);
   statement.BindInt64(1, db_handle);
@@ skipping 12 matching lines @@
 
 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;
   }
 
+  CHECK(schema_version_ >= DOWNLOADS_SCHEMA_VERSION_OPENED);
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "INSERT INTO downloads "
-      "(full_path, url, start_time, received_bytes, total_bytes, state) "
-      "VALUES (?, ?, ?, ?, ?, ?)"));
+      "(full_path, url, start_time, received_bytes, total_bytes, state, "
+      "end_time, opened) "
+      "VALUES (?, ?, ?, ?, ?, ?, ?, ?)"));
   if (!statement)
     return 0;
 
   BindFilePath(statement, info.path, 0);
   statement.BindString(1, info.url.spec());
   statement.BindInt64(2, info.start_time.ToTimeT());
   statement.BindInt64(3, info.received_bytes);
   statement.BindInt64(4, info.total_bytes);
   statement.BindInt(5, info.state);
+  statement.BindInt64(6, info.end_time.ToTimeT());
+  statement.BindInt(7, info.opened ? 1 : 0);
 
   if (statement.Run()) {
     int64 id = GetDB().GetLastInsertRowId();
 
     CHECK_EQ(0u, returned_ids_.count(id));
     returned_ids_.insert(id);
 
     // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
     meta_table_.SetValue(kNextDownloadId, ++next_id_);
 
@@ skipping 52 matching lines @@
   statement.BindInt64(
       1,
       end_time ? end_time : std::numeric_limits<int64>::max());
   statement.BindInt(2, DownloadItem::COMPLETE);
   statement.BindInt(3, DownloadItem::CANCELLED);
   statement.BindInt(4, DownloadItem::INTERRUPTED);
   statement.Run();
 }
 
 }  // namespace history