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

content/browser/download/download_manager.h

 // 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.
 //
 // The DownloadManager object manages the process of downloading, including
 // updates to the history system and providing the information for displaying
 // the downloads view in the Destinations tab. There is one DownloadManager per
 // active browser context in Chrome.
 //
 // Download observers:
@@ skipping 245 matching lines @@
   virtual int64 GetTotalDownloadBytes();
 
   // Called by the delegate after the save as dialog is closed.
   void FileSelected(const FilePath& path, void* params);
   void FileSelectionCanceled(void* params);
 
   // Called by the delegate if it delayed the download in
   // DownloadManagerDelegate::ShouldStartDownload and now is ready.
   void RestartDownload(int32 download_id);
 
+  // Mark the download opened in the persistent store.
+  void MarkDownloadOpened(DownloadItem* download);

[Randy Smith (Not in Mondays), 2011/09/28 19:41:55]

I thought this was being removed?

[benjhayden, 2011/09/28 20:29:39]

I removed DownloadDatabase::MarkDownloadRemoved and
HistoryService::MarkDownloadRemoved, but there isn't a DownloadManager method
that just calls delegate->UpdateEntry(). All the other methods that call
delegate->UpdateEntry() also do a bunch of other stuff, depending on who calls
them. MarkDownloadOpened() will also implement DownloadOpensOutstanding in the
next patch set.

[Randy Smith (Not in Mondays), 2011/09/30 15:57:21]

Hmmm.  Ok.  From a pure functionality approach, I'd rather have a blunt hammer
(when something changes in the DownloadItem, it calls
DownloadManager::DownloadUpdated(), which passes the change along to the history
system.  Or even better, the DM is an observer of the DI, and just does the
automatic pass along if the DI is a persistent DI).  But that makes collecting
the opens outstanding statistic hard.  Still, if you'd be willing, keep an eye
out for ways to make this type of transformation; I think it would simplify the
DI->DM interface and reduce the size of the DM interface.  

(I'm tempted to request a "RecordOpensOustanding" method on the DM that the DI
calls on this transition, and then just switching the actual functionality over
to one of the sketches above, and if you want to that, feel free.  Of you can
leave it like this and keep a lookout for future improvements--your call.)

[benjhayden, 2011/10/03 20:54:39]

The DI could implement RecordOpensOutstanding() inline using DM->Search(). This
feels just a little dirty.
DM::MarkDownloadOpened() fits with DM's current style, right? I think I'd rather
handle big refactors like this in batch CLs.

I can think of a few different ways of reorganizing the DM/DI interfaces. Want
to have a meeting at some point? Is there something already scheduled for
tomorrow's team meeting?

[Randy Smith (Not in Mondays), 2011/10/04 18:34:31]

Agreed.
Fair enough.  I agree that we should try and keep interfaces internally
self-consistent and change them all at once.  

(OKRs is the primary thing on the agenda for tomorrow's meeting :-}.  Maybe next
week.)

+
   // 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.
   void CheckForHistoryFilesRemoval();
 
   // Checks whether a downloaded file still exists and updates the file's state
   // if the file is already removed. The check runs in the background and may
   // finish asynchronously after this method returns.
   void CheckForFileRemoval(DownloadItem* download_item);
 
@@ skipping 151 matching lines @@
   DownloadManagerDelegate* delegate_;
 
   // TODO(rdsmith): Remove when http://crbug.com/84508 is fixed.
   // For debugging only.
   int64 largest_db_handle_in_history_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadManager);
 };
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_