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

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 66be07371507416f322051f0f1ade8ec9f6d37ef..15de86e63bf693b9abefa8e2dcae9896e963e874 100644
--- a/chrome/browser/history/download_database.cc
+++ b/chrome/browser/history/download_database.cc
@@ -26,6 +26,8 @@
 //   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 {
 
@@ -57,19 +59,23 @@ FilePath ColumnFilePath(sql::Statement& statement, int col) {
 // 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,"
@@ -77,11 +83,50 @@ bool DownloadDatabase::InitDownloadTable() {
         "start_time INTEGER NOT NULL,"
         "received_bytes INTEGER NOT NULL,"
         "total_bytes INTEGER NOT NULL,"
-        "state INTEGER NOT NULL)"))
+        "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;
 }
 
@@ -93,9 +138,10 @@ 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)
@@ -111,26 +157,41 @@ void DownloadDatabase::QueryDownloads(
     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);
@@ -163,10 +224,12 @@ int64 DownloadDatabase::CreateDownload(
     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;
 
@@ -176,6 +239,8 @@ int64 DownloadDatabase::CreateDownload(
   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();