Make DownloadHistory observe manager, items

chrome/browser/download/download_history.h

Index: chrome/browser/download/download_history.h
diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h
index b1f827b4c0f64bf7eb3bcf224b45800ae46bbc86..39f3addc0866fbe501469deff73ed19f8d8e5653 100644
--- a/chrome/browser/download/download_history.h
+++ b/chrome/browser/download/download_history.h
@@ -6,34 +6,50 @@
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_
 
 #include <map>
+#include <set>
+#include <vector>
 
-#include "base/basictypes.h"
-#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/cancelable_request.h"
 #include "chrome/browser/history/history.h"
-
-class Profile;
-
-namespace base {
-class Time;
-}
+#include "content/public/browser/download_item.h"
+#include "content/public/browser/download_manager.h"
 
 namespace content {
-class DownloadItem;
+struct DownloadPersistenStoreInfo;
 }
 
-// Interacts with the HistoryService on behalf of the download subsystem.
-class DownloadHistory {
+// Wrap HistoryService for easy mocking and to hide CancelableRequestConsumer.
+class HistoryServiceDownloadInterface {
+ public:
+  virtual void QueryDownloads(const HistoryService::DownloadQueryCallback&) = 0;
+  virtual HistoryService::Handle GetVisibleVisitCountToHost(
+      const GURL& referrer_url,
+      const HistoryService::GetVisibleVisitCountToHostCallback& callback) = 0;
+  virtual void CreateDownload(
+      int32 id,
+      const content::DownloadPersistentStoreInfo& info,
+      const HistoryService::DownloadCreateCallback& callback) = 0;
+  virtual void UpdateDownload(const content::DownloadPersistentStoreInfo&) = 0;
+  virtual void RemoveDownloads(const std::set<int64>& handles) = 0;
+  virtual void OnDownloadHistoryDestroyed() = 0;
+};
+
+// Observes a single DownloadManager and all its DownloadItems, keeping the
+// DownloadDatabase up to date.
+class DownloadHistory: public content::DownloadManager::Observer,
+                       public content::DownloadItem::Observer,
+                       public base::SupportsWeakPtr<DownloadHistory> {
  public:
   typedef base::Callback<void(bool)> VisitedBeforeDoneCallback;
+  typedef std::vector<content::DownloadPersistentStoreInfo> InfoVector;
+  typedef std::set<int64> HandleSet;
 
-  explicit DownloadHistory(Profile* profile);
-  ~DownloadHistory();
+  DownloadHistory(
+      content::DownloadManager* manager,
+      HistoryServiceDownloadInterface* history);
 
-  // Retrieves the next_id counter from the sql meta_table.
-  // Should be much faster than Load so that we may delay downloads until after
-  // this call with minimal performance penalty.
-  void GetNextId(const HistoryService::DownloadNextIdCallback& callback);
+  virtual ~DownloadHistory();
 
   // Retrieves DownloadCreateInfos saved in the history.
   void Load(const HistoryService::DownloadQueryCallback& callback);
@@ -44,49 +60,57 @@ class DownloadHistory {
                                   const GURL& referrer_url,
                                   const VisitedBeforeDoneCallback& callback);
 
-  // Adds a new entry for a download to the history database.
-  void AddEntry(content::DownloadItem* download_item,
-                const HistoryService::DownloadCreateCallback& callback);
-
-  // Updates the history entry for |download_item|.
-  void UpdateEntry(content::DownloadItem* download_item);
-
-  // Updates the download path for |download_item| to |new_path|.
-  void UpdateDownloadPath(content::DownloadItem* download_item,
-                          const FilePath& new_path);
+  // content::DownloadManager::Observer
+  virtual void OnDownloadCreated(
+      content::DownloadManager* manager,
+      content::DownloadItem* item) OVERRIDE;
+  virtual void ManagerGoingDown(content::DownloadManager* manager) OVERRIDE;
 
-  // Removes |download_item| from the history database.
-  void RemoveEntry(content::DownloadItem* download_item);
-
-  // Removes download-related history entries in the given time range.
-  void RemoveEntriesBetween(const base::Time remove_begin,
-                            const base::Time remove_end);
-
-  // Returns a new unique database handle which will not collide with real ones.
-  int64 GetNextFakeDbHandle();
+  // content::DownloadItem::Observer
+  virtual void OnDownloadUpdated(content::DownloadItem* item) OVERRIDE;
+  virtual void OnDownloadRemoved(content::DownloadItem* item) OVERRIDE;
+  virtual void OnDownloadDestroyed(content::DownloadItem* item) OVERRIDE;
 
  private:
+  typedef std::set<int32> IdSet;
+  typedef std::map<int32, content::DownloadPersistentStoreInfo> InfoMap;

[Randy Smith (Not in Mondays), 2012/08/02 22:47:56]

Maybe put in a UMA to track how often we get an OnDownloadUpdated that we ignore
because there's no change we care about.  If it's small enough, we could get rid
of the maps and unilaterally update the history.

[Randy Smith (Not in Mondays), 2012/08/02 22:47:56]

Evict persistent infos from the map on completion and matched up with its DB
handle (avoid having space requirements proportional to number of historical
downloads).

[benjhayden, 2012/08/14 21:31:47]

Done.

[benjhayden, 2012/08/14 21:31:47]

Done.

   typedef std::map<HistoryService::Handle, VisitedBeforeDoneCallback>
       VisitedBeforeRequestsMap;
 
+  void MaybeAddToHistory(content::DownloadItem* item);
+  void ItemAdded(int32 id, int64 db_handle);
+  void RemoveDownloadsBatch();
   void OnGotVisitCountToHost(HistoryService::Handle handle,
                              bool found_visits,
                              int count,
                              base::Time first_visit);
+  void LoadCallback(const HistoryService::DownloadQueryCallback& callback,
+                    InfoVector* infos);
 
-  Profile* profile_;
+  content::DownloadManager* manager_;
 
-  // In case we don't have a valid db_handle, we use |fake_db_handle_| instead.
-  // This is useful for incognito mode or when the history database is offline.
-  // Downloads are expected to have unique handles, so we decrement the next
-  // fake handle value on every use.
-  int64 next_fake_db_handle_;
-
-  CancelableRequestConsumer history_consumer_;
+  HistoryServiceDownloadInterface* history_;
 
   // The outstanding requests made by CheckVisitedReferrerBefore().
   VisitedBeforeRequestsMap visited_before_requests_;
 
+  // Ids of the downloads that are currently being added to the database.
+  IdSet adding_;
+
+  // |db_handles| of items that are scheduled for removal from history.
+  HandleSet removing_;
+
+  // This allows OnDownloadUpdated() to see what changed in a DownloadItem if
+  // anything, in order to prevent writing to the database unnecessarily.
+  // All downloads for |manager_| are in |infos_| and are observed by |this|.
+  InfoMap infos_;
+
+  // Items loaded from the history, to be matched up with OnDownloadCreated.
+  InfoVector loaded_infos_;
+
+  // Count the number of items in the history for UMA.
+  int64 history_size_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadHistory);
 };