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

Download.OpenTime: number of milliseconds between a download completing and when it is opened 

Download.FirstOpenTime: same, but is only counted the first time that a download is opened (even across restarts) 

Download.HistorySize: number of items in the history when a download begins. A user who downloads 5 times per minute, Clears All, then repeats will look exactly like a user who downloads 5 times per week then Clears All and repeats, which is a flat line that goes to 5 then stops. People like me who Clear All after every download will be a dirac delta in the 0 bucket, whereas people who never Clear All will be a flat line all the way out. This should look more like an exponential distribution, with bumps at db sizes when chrome updated. The expected value of this distribution will be the most interesting number.

Download.ShelfSizeOnAutoClose: number of items displayed in the shelf when it autocloses

Download.ShelfSizeOnUserClose: same as ShelfSizeOnAutoClose, but either when the user opens chrome://downloads or clicks the shelf's |x|.

Download.ShelfInProgressSizeOnAutoClose: same as ShelfSizeOnAutoClose, but only counts in-progress items.

Download.ShelfInProgressSizeOnUserClose: same as ShelfSizeOnUserClose, but only counts in-progress items.

Download.ClearAllSize: number of items removed by 'Clear All'

Download.OpensOutstanding: number of completed unopened items when an item is opened.

FirstOpenTime required adding |end_time| and |opened| to the downloads table.
ScheduleAndForget only goes up to 4 args, so I changed UpdateEntry to use DownloadPersistentStoreInfo.

Review URL: http://codereview.chromium.org/8008021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105871 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 136 insertions, 11 deletions

diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc
index 6a6301f..4ffe6c7 100644
--- a/content/browser/download/download_item.cc
+++ b/content/browser/download/download_item.cc
@@ -130,6 +130,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       start_tick_(base::TimeTicks()),
       state_(static_cast<DownloadState>(info.state)),
       start_time_(info.start_time),
+      end_time_(info.end_time),
       db_handle_(info.db_handle),
       download_manager_(download_manager),
       is_paused_(false),
@@ -140,7 +141,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       is_otr_(false),
       is_temporary_(false),
       all_data_saved_(false),
-      opened_(false),
+      opened_(info.opened),
       open_enabled_(true),
       delegate_delayed_complete_(false) {
   if (IsInProgress())
@@ -285,8 +286,10 @@ void DownloadItem::OpenDownload() {
   // program that opens the file.  So instead we spawn a check to update
   // the UI if the file has been deleted in parallel with the open.
   download_manager_->CheckForFileRemoval(this);
+  download_stats::RecordOpen(end_time(), !opened());
   opened_ = true;
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadOpened(this));
+  download_manager_->MarkDownloadOpened(this);
 
   // For testing: If download opening is disabled on this item,
   // make the rest of the routine a no-op.
@@ -390,6 +393,7 @@ void DownloadItem::MarkAsComplete() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DCHECK(all_data_saved_);
+  end_time_ = base::Time::Now();
   TransitionTo(COMPLETE);
 }
 
@@ -420,6 +424,7 @@ void DownloadItem::Completed() {
   VLOG(20) << __FUNCTION__ << "() " << DebugString(false);
 
   DCHECK(all_data_saved_);
+  end_time_ = base::Time::Now();
   TransitionTo(COMPLETE);
   download_manager_->DownloadCompleted(id());
   download_stats::RecordDownloadCompleted(start_tick_, received_bytes_);
@@ -688,10 +693,12 @@ DownloadPersistentStoreInfo DownloadItem::GetPersistentStoreInfo() const {
                                      GetURL(),
                                      referrer_url(),
                                      start_time(),
+                                     end_time(),
                                      received_bytes(),
                                      total_bytes(),
                                      state(),
-                                     db_handle());
+                                     db_handle(),
+                                     opened());
 }
 
 FilePath DownloadItem::GetTargetFilePath() const {
diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h
index 6e04cdc..baf3ca1 100644
--- a/content/browser/download/download_item.h
+++ b/content/browser/download/download_item.h
@@ -279,6 +279,7 @@ class CONTENT_EXPORT DownloadItem {
   int32 id() const { return download_id_; }
   DownloadId global_id() const;
   base::Time start_time() const { return start_time_; }
+  base::Time end_time() const { return end_time_; }
   void set_db_handle(int64 handle) { db_handle_ = handle; }
   int64 db_handle() const { return db_handle_; }
   DownloadManager* download_manager() { return download_manager_; }
@@ -433,6 +434,9 @@ class CONTENT_EXPORT DownloadItem {
   // Time the download was started
   base::Time start_time_;
 
+  // Time the download completed
+  base::Time end_time_;
+
   // Our persistent store handle
   int64 db_handle_;
 
diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc
index 0a9337c..e8e9050 100644
--- a/content/browser/download/download_manager.cc
+++ b/content/browser/download/download_manager.cc
@@ -25,6 +25,7 @@
 #include "content/browser/download/download_item.h"
 #include "content/browser/download/download_manager_delegate.h"
 #include "content/browser/download/download_persistent_store_info.h"
+#include "content/browser/download/download_stats.h"
 #include "content/browser/download/download_status_updater.h"
 #include "content/browser/download/interrupt_reasons.h"
 #include "content/browser/renderer_host/render_process_host.h"
@@ -689,6 +690,7 @@ int DownloadManager::RemoveDownloads(const base::Time remove_begin) {
 }
 
 int DownloadManager::RemoveAllDownloads() {
+  download_stats::RecordClearAllSize(history_downloads_.size());
   // The null times make the date range unbounded.
   return RemoveDownloadsBetween(base::Time(), base::Time());
 }
@@ -848,6 +850,8 @@ void DownloadManager::AddDownloadItemToHistory(DownloadItem* download,
   // is fixed.
   CHECK_NE(DownloadItem::kUninitializedHandle, db_handle);
 
+  download_stats::RecordHistorySize(history_downloads_.size());
+
   DCHECK(download->db_handle() == DownloadItem::kUninitializedHandle);
   download->set_db_handle(db_handle);
 
@@ -1075,3 +1079,14 @@ void DownloadManager::SavePageDownloadFinished(DownloadItem* download) {
           Details<DownloadItem>(download));
   }
 }
+
+void DownloadManager::MarkDownloadOpened(DownloadItem* download) {
+  delegate_->UpdateItemInPersistentStore(download);
+  int num_unopened = 0;
+  for (DownloadMap::iterator it = history_downloads_.begin();
+       it != history_downloads_.end(); ++it) {
+    if (it->second->IsComplete() && !it->second->opened())
+      ++num_unopened;
+  }
+  download_stats::RecordOpensOutstanding(num_unopened);
+}
diff --git a/content/browser/download/download_manager.h b/content/browser/download/download_manager.h
index 096b1e2..8d1a161 100644
--- a/content/browser/download/download_manager.h
+++ b/content/browser/download/download_manager.h
@@ -265,6 +265,9 @@ class CONTENT_EXPORT DownloadManager
   // DownloadManagerDelegate::ShouldStartDownload and now is ready.
   void RestartDownload(int32 download_id);
 
+  // Mark the download opened in the persistent store.
+  void MarkDownloadOpened(DownloadItem* download);
+
   // Checks whether downloaded files still exist. Updates state of downloads
   // that refer to removed files. The check runs in the background and may
   // finish asynchronously after this method returns.
diff --git a/content/browser/download/download_manager_delegate.h b/content/browser/download/download_manager_delegate.h
index 55d25f6..1f7c173 100644
--- a/content/browser/download/download_manager_delegate.h
+++ b/content/browser/download/download_manager_delegate.h
@@ -74,6 +74,8 @@ class DownloadManagerDelegate {
 
   // Notifies the delegate that information about the given download has change,
   // so that it can update its persistent store.
+  // Does not update |url|, |start_time|, |total_bytes|; uses |db_handle| only
+  // to select the row in the database table to update.
   virtual void UpdateItemInPersistentStore(DownloadItem* item) = 0;
 
   // Notifies the delegate that path for the download item has changed, so that
diff --git a/content/browser/download/download_persistent_store_info.cc b/content/browser/download/download_persistent_store_info.cc
index 8ca92b1..a15f1b7 100644
--- a/content/browser/download/download_persistent_store_info.cc
+++ b/content/browser/download/download_persistent_store_info.cc
@@ -10,7 +10,8 @@ DownloadPersistentStoreInfo::DownloadPersistentStoreInfo()
     : received_bytes(0),
       total_bytes(0),
       state(0),
-      db_handle(0) {
+      db_handle(0),
+      opened(false) {
 }
 
 DownloadPersistentStoreInfo::DownloadPersistentStoreInfo(
@@ -18,18 +19,22 @@ DownloadPersistentStoreInfo::DownloadPersistentStoreInfo(
     const GURL& url,
     const GURL& referrer,
     const base::Time& start,
+    const base::Time& end,
     int64 received,
     int64 total,
     int32 download_state,
-    int64 handle)
+    int64 handle,
+    bool download_opened)
     : path(path),
       url(url),
       referrer_url(referrer),
       start_time(start),
+      end_time(end),
       received_bytes(received),
       total_bytes(total),
       state(download_state),
-      db_handle(handle) {
+      db_handle(handle),
+      opened(download_opened) {
 }
 
 DownloadPersistentStoreInfo::~DownloadPersistentStoreInfo() {
diff --git a/content/browser/download/download_persistent_store_info.h b/content/browser/download/download_persistent_store_info.h
index 948d048..84a2b99 100644
--- a/content/browser/download/download_persistent_store_info.h
+++ b/content/browser/download/download_persistent_store_info.h
@@ -19,7 +19,14 @@
 class DownloadItem;
 
 // Contains the information that is stored in the download system's persistent
-// store (or refers to it).  Managed by the DownloadItem.
+// store (or refers to it).  Managed by the DownloadItem.  When used to create a
+// history entry, all fields except for |db_handle| are set by DownloadItem and
+// read by DownloadDatabase.  When used to update a history entry, DownloadItem
+// sets all fields, but DownloadDatabase only reads |end_time|,
+// |received_bytes|, |state|, and |opened|, and uses |db_handle| to select the
+// row in the database table to update.  When used to load DownloadItems from
+// the history, all fields except |referrer_url| are set by the DownloadDatabase
+// and read by the DownloadItem.
 struct CONTENT_EXPORT DownloadPersistentStoreInfo {
   // TODO(ahendrickson) -- Reduce the number of constructors.
   DownloadPersistentStoreInfo();
@@ -27,36 +34,45 @@ struct CONTENT_EXPORT DownloadPersistentStoreInfo {
                               const GURL& url,
                               const GURL& referrer,
                               const base::Time& start,
+                              const base::Time& end,
                               int64 received,
                               int64 total,
                               int32 download_state,
-                              int64 handle);
+                              int64 handle,
+                              bool download_opened);
   ~DownloadPersistentStoreInfo();  // For linux-clang.
 
   // The final path where the download is saved.
   FilePath path;
 
   // The URL from which we are downloading. This is the final URL after any
-  // redirection by the server for |url_chain|.
+  // redirection by the server for |url_chain|. Is not changed by UpdateEntry().
   GURL url;
 
   // The URL that referred us.
   GURL referrer_url;
 
-  // The time when the download started.
+  // The time when the download started. Is not changed by UpdateEntry().
   base::Time start_time;
 
+  // The time when the download completed.
+  base::Time end_time;
+
   // The number of bytes received (so far).
   int64 received_bytes;
 
-  // The total number of bytes in the download.
+  // The total number of bytes in the download. Is not changed by UpdateEntry().
   int64 total_bytes;
 
   // The current state of the download.
   int32 state;
 
-  // The handle of the download in the database.
+  // The handle of the download in the database. Is not changed by
+  // UpdateEntry().
   int64 db_handle;
+
+  // Whether this download has ever been opened from the browser.
+  bool opened;
 };
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_PERSISTENT_STORE_INFO_H_
diff --git a/content/browser/download/download_stats.cc b/content/browser/download/download_stats.cc
index be43ba0..856e62a 100644
--- a/content/browser/download/download_stats.cc
+++ b/content/browser/download/download_stats.cc
@@ -177,4 +177,57 @@ void RecordDownloadMimeType(const std::string& mime_type_string) {
                             DOWNLOAD_CONTENT_MAX);
 }
 
+void RecordOpen(const base::Time& end, bool first) {
+  if (!end.is_null()) {
+    UMA_HISTOGRAM_LONG_TIMES("Download.OpenTime", (base::Time::Now() - end));
+    if (first) {
+      UMA_HISTOGRAM_LONG_TIMES("Download.FirstOpenTime",
+                              (base::Time::Now() - end));
+    }
+  }
+}
+
+void RecordHistorySize(int size) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Download.HistorySize",
+                              size,
+                              0/*min*/,
+                              (1 << 10)/*max*/,
+                              32/*num_buckets*/);
+}
+
+void RecordShelfClose(int size, int in_progress, bool autoclose) {
+  static const int kMaxShelfSize = 16;
+  if (autoclose) {
+    UMA_HISTOGRAM_ENUMERATION("Download.ShelfSizeOnAutoClose",
+                              size,
+                              kMaxShelfSize);
+    UMA_HISTOGRAM_ENUMERATION("Download.ShelfInProgressSizeOnAutoClose",
+                              in_progress,
+                              kMaxShelfSize);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("Download.ShelfSizeOnUserClose",
+                              size,
+                              kMaxShelfSize);
+    UMA_HISTOGRAM_ENUMERATION("Download.ShelfInProgressSizeOnUserClose",
+                              in_progress,
+                              kMaxShelfSize);
+  }
+}
+
+void RecordClearAllSize(int size) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Download.ClearAllSize",
+                              size,
+                              0/*min*/,
+                              (1 << 10)/*max*/,
+                              32/*num_buckets*/);
+}
+
+void RecordOpensOutstanding(int size) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Download.OpensOutstanding",
+                              size,
+                              0/*min*/,
+                              (1 << 10)/*max*/,
+                              64/*num_buckets*/);
+}
+
 }  // namespace download_stats
diff --git a/content/browser/download/download_stats.h b/content/browser/download/download_stats.h
index 6e55d07..bfb244c 100644
--- a/content/browser/download/download_stats.h
+++ b/content/browser/download/download_stats.h
@@ -15,6 +15,7 @@
 #include "content/browser/download/interrupt_reasons.h"
 
 namespace base {
+class Time;
 class TimeTicks;
 }
 
@@ -91,6 +92,25 @@ void RecordDownloadWriteSize(size_t data_len);
 // Record WRITE_LOOP_COUNT and number of loops.
 void RecordDownloadWriteLoopCount(int count);
 
+// Record the time of both the first open and all subsequent opens since the
+// download completed.
+void RecordOpen(const base::Time& end, bool first);
+
+// Record the number of items that are in the history at the time that a
+// new download is added to the history.
+void RecordHistorySize(int size);
+
+// Record the total number of items and the number of in-progress items showing
+// in the shelf when it closes.  Set |autoclose| to true when the shelf is
+// closing itself, false when the user explicitly closed it.
+void RecordShelfClose(int size, int in_progress, bool autoclose);
+
+// Record the number of downloads removed by ClearAll.
+void RecordClearAllSize(int size);
+
+// Record the number of completed unopened downloads when a download is opened.
+void RecordOpensOutstanding(int size);
+
 }  // namespace download_stats
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_STATS_H_