Make DownloadHistory observe manager, items

chrome/browser/download/chrome_download_manager_delegate.h

Index: chrome/browser/download/chrome_download_manager_delegate.h
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index 2cadbe92e5b1a1d19ba4ddfb9a7caf5fcc0edfb8..f786fd23da08bc676b18a3f82aea0767c4b3dd39 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -10,8 +10,9 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "chrome/browser/safe_browsing/download_protection_service.h"
+#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_path_reservation_tracker.h"
+#include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "content/public/browser/download_danger_type.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager_delegate.h"
@@ -57,7 +58,11 @@ class ChromeDownloadManagerDelegate
 
   void SetDownloadManager(content::DownloadManager* dm);
 
-  // Should be called before the call to ShouldCompleteDownload() to
+  // Add/Remove Observers of DownloadHistory events.
+  void AddHistoryObserver(DownloadHistory::Observer* observer);
+  void RemoveHistoryObserver(DownloadHistory::Observer* observer);

[Randy Smith (Not in Mondays), 2012/11/02 23:31:25]

These appear to only be used by tests.  Any reason not to give tests some way to
get at the DownloadHistory object instead?  Or at least make these "ForTesting"?

[benjhayden, 2012/11/06 20:01:14]

Exposed DownloadService::GetDownloadHistory(). LMK if you think I should also
make DownloadHistory::Add/RemoveObserver "ForTesting".

[Randy Smith (Not in Mondays), 2012/11/07 21:10:29]

Nope, I think that'll do it--you correctly divined that the weirdness of having
the Add/Remove interfaces on CDMD was what was making me ootsy.

[benjhayden, 2012/11/08 18:57:03]

Done.

+
+  // Should be called before the first call to ShouldCompleteDownload() to
   // disable SafeBrowsing checks for |item|.
   static void DisableSafeBrowsing(content::DownloadItem* item);
 
@@ -75,17 +80,6 @@ class ChromeDownloadManagerDelegate
       const base::Closure& complete_callback) OVERRIDE;
   virtual bool ShouldOpenDownload(content::DownloadItem* item) OVERRIDE;
   virtual bool GenerateFileHash() OVERRIDE;
-  virtual void AddItemToPersistentStore(content::DownloadItem* item) OVERRIDE;
-  virtual void UpdateItemInPersistentStore(
-      content::DownloadItem* item) OVERRIDE;
-  virtual void UpdatePathForItemInPersistentStore(
-      content::DownloadItem* item,
-      const FilePath& new_path) OVERRIDE;
-  virtual void RemoveItemFromPersistentStore(
-      content::DownloadItem* item) OVERRIDE;
-  virtual void RemoveItemsFromPersistentStoreBetween(
-      base::Time remove_begin,
-      base::Time remove_end) OVERRIDE;
   virtual void GetSaveDir(content::BrowserContext* browser_context,
                           FilePath* website_save_dir,
                           FilePath* download_save_dir,
@@ -102,7 +96,6 @@ class ChromeDownloadManagerDelegate
   void ClearLastDownloadPath();
 
   DownloadPrefs* download_prefs() { return download_prefs_.get(); }
-  DownloadHistory* download_history() { return download_history_.get(); }
 
  protected:
   // So that test classes can inherit from this for override purposes.
@@ -213,9 +206,6 @@ class ChromeDownloadManagerDelegate
       content::DownloadDangerType danger_type,
       const FilePath& target_path);
 
-  // Callback from history system.
-  void OnItemAddedToPersistentStore(int32 download_id, int64 db_handle);
-
   // Check policy of whether we should open this download with a web intents
   // dispatch.
   bool ShouldOpenWithWebIntents(const content::DownloadItem* item);