Download code cleanup patch of death:

Separate all interactions with HistoryService out of DownloadManager
to new class DownloadHistory owned by the DownloadManager.

The goal is to create more smaller classes with clearly defined
responsibilities.

TEST=unit_tests, browser_tests, ui_tests
BUG=48913

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

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

chrome/browser/automation/automation_provider.cc

@@ -2099,8 +2099,6 @@ void AutomationProvider::AddHistoryItem(Browser* browser,
 void AutomationProvider::GetDownloadsInfo(Browser* browser,
                                           DictionaryValue* args,
                                           IPC::Message* reply_message) {
-  AutomationProviderDownloadManagerObserver observer;
-  std::vector<DownloadItem*> downloads;
   scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
   AutomationJSONReply reply(this, reply_message);
 
@@ -2108,11 +2106,9 @@ void AutomationProvider::GetDownloadsInfo(Browser* browser,
     reply.SendError("no download manager");
     return;
   }
-  // Use DownloadManager's GetDownloads() method and not GetCurrentDownloads()
-  // since that would be transient; a download might enter and empty out
-  // the current download queue too soon to be noticed.
-  profile_->GetDownloadManager()->GetDownloads(&observer, L"");
-  downloads = observer.Downloads();
+
+  std::vector<DownloadItem*> downloads;
+  profile_->GetDownloadManager()->GetAllDownloads(FilePath(), &downloads);
 
   std::map<DownloadItem::DownloadState, std::string> state_to_string;
   state_to_string[DownloadItem::IN_PROGRESS] = std::string("IN_PROGRESS");
@@ -2160,8 +2156,6 @@ void AutomationProvider::WaitForDownloadsToComplete(
     Browser* browser,
     DictionaryValue* args,
     IPC::Message* reply_message) {
-  AutomationProviderDownloadManagerObserver observer;
-  std::vector<DownloadItem*> downloads;
   AutomationJSONReply reply(this, reply_message);
 
   // Look for a quick return.
@@ -2169,9 +2163,9 @@ void AutomationProvider::WaitForDownloadsToComplete(
     reply.SendSuccess(NULL);  // No download manager.
     return;
   }
-  profile_->GetDownloadManager()->GetCurrentDownloads(&observer, FilePath());
-  downloads = observer.Downloads();
-  if (downloads.size() == 0) {
+  std::vector<DownloadItem*> downloads;
+  profile_->GetDownloadManager()->GetCurrentDownloads(FilePath(), &downloads);
+  if (downloads.empty()) {
     reply.SendSuccess(NULL);
     return;
   }

chrome/browser/automation/automation_provider_observers.h

@@ -14,6 +14,7 @@
 #include "chrome/browser/browsing_data_remover.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_manager.h"
+#include "chrome/browser/history/history.h"
 #include "chrome/browser/importer/importer.h"
 #include "chrome/browser/importer/importer_data_types.h"
 #include "chrome/browser/password_manager/password_store.h"
@@ -596,27 +597,6 @@ class AutomationProviderBookmarkModelObserver : BookmarkModelObserver {
   DISALLOW_COPY_AND_ASSIGN(AutomationProviderBookmarkModelObserver);
 };
 
-// When asked for pending downloads, the DownloadManager places
-// results in a DownloadManager::Observer.
-class AutomationProviderDownloadManagerObserver
-    : public DownloadManager::Observer {
- public:
-  AutomationProviderDownloadManagerObserver() : DownloadManager::Observer()  {}
-  virtual ~AutomationProviderDownloadManagerObserver() {}
-  virtual void ModelChanged() {}
-  virtual void SetDownloads(std::vector<DownloadItem*>& downloads) {
-    downloads_ = downloads;
-  }
-  std::vector<DownloadItem*> Downloads() {
-    return downloads_;
-  }
- private:
-  std::vector<DownloadItem*> downloads_;
-
-  DISALLOW_COPY_AND_ASSIGN(AutomationProviderDownloadManagerObserver);
-};
-
-
 // Allows the automation provider to wait for all downloads to finish.
 class AutomationProviderDownloadItemObserver : public DownloadItem::Observer {
  public:
@@ -679,8 +659,8 @@ class AutomationProviderImportSettingsObserver
 };
 
 // Allows automation provider to wait for getting passwords to finish.
-class AutomationProviderGetPasswordsObserver :
-    public PasswordStoreConsumer {
+class AutomationProviderGetPasswordsObserver
+    : public PasswordStoreConsumer {
  public:
   AutomationProviderGetPasswordsObserver(
       AutomationProvider* provider,

chrome/browser/dom_ui/downloads_dom_handler.cc

@@ -9,11 +9,13 @@
 #include "base/singleton.h"
 #include "base/string_piece.h"
 #include "base/thread.h"
+#include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/dom_ui/chrome_url_data_manager.h"
 #include "chrome/browser/dom_ui/fileicon_source.h"
+#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/metrics/user_metrics.h"
@@ -110,15 +112,16 @@ void DownloadsDOMHandler::OnDownloadUpdated(DownloadItem* download) {
 }
 
 // A download has started or been deleted. Query our DownloadManager for the
-// current set of downloads, which will call us back in SetDownloads once it
-// has retrieved them.
+// current set of downloads.
 void DownloadsDOMHandler::ModelChanged() {
   ClearDownloadItems();
-  download_manager_->GetDownloads(this, search_text_);
+  download_manager_->download_history()->Search(
+      WideToUTF16(search_text_),
+      NewCallback(this, &DownloadsDOMHandler::OnSearchDownloadsComplete));
 }
 
-void DownloadsDOMHandler::SetDownloads(
-    std::vector<DownloadItem*>& downloads) {
+void DownloadsDOMHandler::OnSearchDownloadsComplete(
+    std::vector<DownloadItem*> downloads) {
   ClearDownloadItems();
 
   // Swap new downloads in.
@@ -149,7 +152,9 @@ void DownloadsDOMHandler::HandleGetDownloads(const Value* value) {
   if (search_text_.compare(new_search) != 0) {
     search_text_ = new_search;
     ClearDownloadItems();
-    download_manager_->GetDownloads(this, search_text_);
+    download_manager_->download_history()->Search(
+        WideToUTF16(search_text_),
+        NewCallback(this, &DownloadsDOMHandler::OnSearchDownloadsComplete));
   } else {
     SendCurrentDownloads();
   }

chrome/browser/dom_ui/downloads_dom_handler.h

@@ -35,7 +35,8 @@ class DownloadsDOMHandler : public DOMMessageHandler,
 
   // DownloadManager::Observer interface
   virtual void ModelChanged();
-  virtual void SetDownloads(std::vector<DownloadItem*>& downloads);
+
+  void OnSearchDownloadsComplete(std::vector<DownloadItem*> downloads);
 
   // Callback for the "getDownloads" message.
   void HandleGetDownloads(const Value* value);

chrome/browser/dom_ui/filebrowse_ui.cc

@@ -126,7 +126,8 @@ class FilebrowseHandler : public net::DirectoryLister::DirectoryListerDelegate,
 
   // DownloadManager::Observer interface
   virtual void ModelChanged();
-  virtual void SetDownloads(std::vector<DownloadItem*>& downloads);
+
+  void OnSearchDownloadsComplete(std::vector<DownloadItem*> downloads);
 
   // Callback for the "getRoots" message.
   void HandleGetRoots(const Value* value);
@@ -969,7 +970,8 @@ void FilebrowseHandler::ModelChanged() {
   download_manager_->GetAllDownloads(this, FilePath());
 }
 
-void FilebrowseHandler::SetDownloads(std::vector<DownloadItem*>& downloads) {
+void FilebrowseHandler::OnSearchDownloadsComplete(
+    std::vector<DownloadItem*> downloads) {
   ClearDownloadItems();
   std::vector<DownloadItem*> new_downloads;
   // Scan for any in progress downloads and add ourself to them as an observer.

chrome/browser/download/download_history.cc

+// Copyright (c) 2010 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.
+
+#include "chrome/browser/download/download_history.h"
+
+#include "base/logging.h"
+#include "chrome/browser/history/download_types.h"
+#include "chrome/browser/history/history_marshaling.h"
+#include "chrome/browser/download/download_item.h"
+#include "chrome/browser/profile.h"
+
+// Our download table ID starts at 1, so we use 0 to represent a download that
+// has started, but has not yet had its data persisted in the table. We use fake
+// database handles in incognito mode starting at -1 and progressively getting
+// more negative.
+// static
+const int DownloadHistory::kUninitializedHandle = 0;
+
+DownloadHistory::DownloadHistory(Profile* profile, DownloadItemMapper* mapper)
+    : profile_(profile),
+      next_fake_db_handle_(kUninitializedHandle - 1),
+      download_item_mapper_(mapper) {
+  DCHECK(profile);
+  DCHECK(mapper);
+}
+
+void DownloadHistory::Load(HistoryService::DownloadQueryCallback* callback) {
+  DCHECK(callback);
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (!hs) {
+    delete callback;
+    return;
+  }
+  hs->QueryDownloads(&history_consumer_, callback);
+
+  // This is the initial load, so do a cleanup of corrupt in-progress entries.
+  hs->CleanUpInProgressEntries();
+}
+
+void DownloadHistory::Search(const string16& query,
+                             DownloadSearchCallback* callback) {
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (!hs) {
+    delete callback;
+    return;
+  }
+
+  HistoryService::Handle handle = hs->SearchDownloads(
+      query,
+      &history_consumer_,
+      NewCallback(this, &DownloadHistory::OnSearchDownloadsComplete));
+  history_consumer_.SetClientData(hs, handle, callback);
+}
+
+void DownloadHistory::AddEntry(
+    const DownloadCreateInfo& info,
+    DownloadItem* download_item,
+    HistoryService::DownloadCreateCallback* callback) {
+  // Do not store the download in the history database for a few special cases:
+  // - incognito mode (that is the point of this mode)
+  // - extensions (users don't think of extension installation as 'downloading')
+  // - temporary download, like in drag-and-drop
+  // - history service is not available (e.g. in tests)
+  // We have to make sure that these handles don't collide with normal db
+  // handles, so we use a negative value. Eventually, they could overlap, but
+  // you'd have to do enough downloading that your ISP would likely stab you in
+  // the neck first. YMMV.
+  // FIXME(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (download_item->is_otr() || download_item->is_extension_install() ||
+      download_item->is_temporary() || !hs) {
+    callback->RunWithParams(
+        history::DownloadCreateRequest::TupleType(info, GetNextFakeDbHandle()));
+    delete callback;
+    return;
+  }
+
+  hs->CreateDownload(info, &history_consumer_, callback);
+}
+
+void DownloadHistory::UpdateEntry(DownloadItem* download_item) {
+  // Don't store info in the database if the download was initiated while in
+  // incognito mode or if it hasn't been initialized in our database table.
+  if (download_item->db_handle() <= kUninitializedHandle)
+    return;
+
+  // FIXME(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (!hs)
+    return;
+
+  hs->UpdateDownload(download_item->received_bytes(),
+                     download_item->state(),
+                     download_item->db_handle());
+}
+
+void DownloadHistory::UpdateDownloadPath(DownloadItem* download_item,
+                                         const FilePath& new_path) {
+  // No update necessary if the download was initiated while in incognito mode.
+  if (download_item->db_handle() <= kUninitializedHandle)
+    return;
+
+  // FIXME(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (hs)
+    hs->UpdateDownloadPath(new_path, download_item->db_handle());
+}
+
+void DownloadHistory::RemoveEntry(DownloadItem* download_item) {
+  // No update necessary if the download was initiated while in incognito mode.
+  if (download_item->db_handle() <= kUninitializedHandle)
+    return;
+
+  // FIXME(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (hs)
+    hs->RemoveDownload(download_item->db_handle());
+}
+
+void DownloadHistory::RemoveEntriesBetween(const base::Time remove_begin,
+                                           const base::Time remove_end) {
+  // FIXME(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (hs)
+    hs->RemoveDownloadsBetween(remove_begin, remove_end);
+}
+
+void DownloadHistory::OnSearchDownloadsComplete(HistoryService::Handle handle,
+                                                std::vector<int64>* results) {
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  DownloadSearchCallback* callback =
+      history_consumer_.GetClientData(hs, handle);
+  if (!callback)
+    return;
+
+  std::vector<DownloadItem*> download_items;
+  for (std::vector<int64>::iterator i = results->begin();
+       i != results->end(); ++i) {
+    DownloadItem* download_item =
+        download_item_mapper_->GetDownloadItemFromDbHandle(*i);
+    if (download_item)
+      download_items.push_back(download_item);
+  }
+
+  callback->RunWithParams(MakeTuple(download_items));
+}
+
+int64 DownloadHistory::GetNextFakeDbHandle() {
+  return next_fake_db_handle_--;
+}

chrome/browser/download/download_history.h

+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_
+#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_
+#pragma once
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "chrome/browser/cancelable_request.h"
+#include "chrome/browser/history/history.h"
+
+class DownloadItem;
+class Profile;
+
+namespace base {
+class Time;
+}
+
+// Interacts with the HistoryService on behalf of the download subsystem.
+class DownloadHistory {
+ public:
+  // Converts history database handles to download items.
+  class DownloadItemMapper {
+   public:
+    virtual ~DownloadItemMapper() {}
+
+    // Returns a DownloadItem* corresponding to |db_handle|, or NULL if no such
+    // DownloadItem exists.
+    virtual DownloadItem* GetDownloadItemFromDbHandle(int64 db_handle) = 0;
+  };
+
+  // A fake download table ID which represents a download that has started,
+  // but is not yet in the table.
+  static const int kUninitializedHandle;
+
+  typedef Callback1<std::vector<DownloadItem*> >::Type DownloadSearchCallback;
+
+  DownloadHistory(Profile* profile, DownloadItemMapper* mapper);
+
+  // Retrieves DownloadCreateInfos saved in the history.
+  void Load(HistoryService::DownloadQueryCallback* callback);
+
+  // Starts a history search for downloads and invokes |callback| when done.
+  void Search(const string16& query,
+              DownloadSearchCallback* callback);
+
+  // Adds a new entry for a download to the history database.
+  void AddEntry(const DownloadCreateInfo& info,
+                DownloadItem* download_item,
+                HistoryService::DownloadCreateCallback* callback);
+
+  // Updates the history entry for |download_item|.
+  void UpdateEntry(DownloadItem* download_item);
+
+  // Updates the download path for |download_item| to |new_path|.
+  void UpdateDownloadPath(DownloadItem* download_item,
+                          const FilePath& new_path);
+
+  // Removes |download_item| from the history database.
+  void RemoveEntry(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();
+
+ private:
+  void OnSearchDownloadsComplete(HistoryService::Handle handle,
+                                 std::vector<int64>* results);
+
+  Profile* profile_;
+
+  // 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_;
+
+  DownloadItemMapper* download_item_mapper_;
+
+  CancelableRequestConsumerTSimple<DownloadSearchCallback*> history_consumer_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadHistory);
+};
+
+#endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_

chrome/browser/download/download_item.cc

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "base/timer.h"
+#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
@@ -68,7 +69,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS),
       start_time_(info.start_time),
-      db_handle_(DownloadManager::kUninitializedHandle),
+      db_handle_(DownloadHistory::kUninitializedHandle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
@@ -103,7 +104,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS),
       start_time_(base::Time::Now()),
-      db_handle_(DownloadManager::kUninitializedHandle),
+      db_handle_(DownloadHistory::kUninitializedHandle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),

chrome/browser/download/download_item.h

@@ -171,7 +171,6 @@ class DownloadItem {
   void set_db_handle(int64 handle) { db_handle_ = handle; }
   int64 db_handle() const { return db_handle_; }
   bool is_paused() const { return is_paused_; }
-  void set_is_paused(bool pause) { is_paused_ = pause; }
   bool open_when_complete() const { return open_when_complete_; }
   void set_open_when_complete(bool open) { open_when_complete_ = open; }
   int render_process_id() const { return render_process_id_; }

chrome/browser/download/download_manager.h

@@ -37,9 +37,9 @@
 #include "base/file_path.h"
 #include "base/observer_list.h"
 #include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "base/time.h"
-#include "chrome/browser/cancelable_request.h"
-#include "chrome/browser/history/history.h"
+#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/pref_member.h"
 #include "chrome/browser/shell_dialogs.h"
 
@@ -51,20 +51,18 @@ class Profile;
 class ResourceDispatcherHost;
 class URLRequestContextGetter;
 class TabContents;
+struct DownloadCreateInfo;
 struct DownloadSaveInfo;
 
 // Browser's download manager: manages all downloads and destination view.
 class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
+                        public DownloadHistory::DownloadItemMapper,
                         public SelectFileDialog::Listener {
   // For testing.
   friend class DownloadManagerTest;
   friend class MockDownloadManager;
 
  public:
-  // A fake download table ID which representas a download that has started,
-  // but is not yet in the table.
-  static const int kUninitializedHandle;
-
   DownloadManager();
 
   static void RegisterUserPrefs(PrefService* prefs);
@@ -77,11 +75,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
     // of downloads.
     virtual void ModelChanged() = 0;
 
-    // A callback once the DownloadManager has retrieved the requested set of
-    // downloads. The DownloadManagerObserver must copy the vector, but does not
-    // own the individual DownloadItems, when this call is made.
-    virtual void SetDownloads(std::vector<DownloadItem*>& downloads) = 0;
-
     // Called when the DownloadManager is being destroyed to prevent Observers
     // from calling back to a stale pointer.
     virtual void ManagerGoingDown() {}
@@ -90,44 +83,23 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
     virtual ~Observer() {}
   };
 
-  // Public API
-
-  // If this download manager has an incognito profile, find all incognito
-  // downloads and pass them along to the parent profile's download manager
-  // via DoGetDownloads. Otherwise, just call DoGetDownloads().
-  void GetDownloads(Observer* observer,
-                    const std::wstring& search_text);
-
-  // Begin a search for all downloads matching 'search_text'. If 'search_text'
-  // is empty, return all known downloads. The results are returned in the
-  // 'SetDownloads' observer callback.
-  void DoGetDownloads(Observer* observer,
-                      const std::wstring& search_text,
-                      std::vector<DownloadItem*>& otr_downloads);
-
   // Return all temporary downloads that reside in the specified directory.
-  void GetTemporaryDownloads(Observer* observer,
-                             const FilePath& dir_path);
+  void GetTemporaryDownloads(const FilePath& dir_path,
+                             std::vector<DownloadItem*>* result);
 
   // Return all non-temporary downloads in the specified directory that are
   // are in progress or have finished.
-  void GetAllDownloads(Observer* observer, const FilePath& dir_path);
+  void GetAllDownloads(const FilePath& dir_path,
+                       std::vector<DownloadItem*>* result);
 
   // Return all non-temporary downloads in the specified directory that are
   // either in-progress or finished but still waiting for user confirmation.
-  void GetCurrentDownloads(Observer* observer, const FilePath& dir_path);
+  void GetCurrentDownloads(const FilePath& dir_path,
+                           std::vector<DownloadItem*>* result);
 
   // Returns true if initialized properly.
   bool Init(Profile* profile);
 
-  // Schedule a query of the history service to retrieve all downloads.
-  void QueryHistoryForDownloads();
-
-  // Cleans up IN_PROGRESS history entries as these entries are corrupt because
-  // of the sudden exit. Changes them to CANCELED. Executed only when called
-  // first time, subsequent calls a no op.
-  void CleanUpInProgressHistoryEntries();
-
   // Notifications sent from the download thread to the UI thread
   void StartDownload(DownloadCreateInfo* info);
   void UpdateDownload(int32 download_id, int64 size);
@@ -181,8 +153,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   void OnQueryDownloadEntriesComplete(
       std::vector<DownloadCreateInfo>* entries);
   void OnCreateDownloadEntryComplete(DownloadCreateInfo info, int64 db_handle);
-  void OnSearchComplete(HistoryService::Handle handle,
-                        std::vector<int64>* results);
 
   // Display a new download in the appropriate browser UI.
   void ShowDownloadInBrowser(const DownloadCreateInfo& info,
@@ -204,6 +174,8 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
 
   FilePath download_path() { return *download_path_; }
 
+  DownloadHistory* download_history() { return download_history_.get(); }
+
   // Clears the last download path, used to initialize "save as" dialogs.
   void ClearLastDownloadPath();
 
@@ -231,6 +203,9 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // types.
   bool HasAutoOpenFileTypesRegistered() const;
 
+  // Overridden from DownloadHistory::DownloadItemMapper:
+  virtual DownloadItem* GetDownloadItemFromDbHandle(int64 db_handle);
+
   // Overridden from SelectFileDialog::Listener:
   virtual void FileSelected(const FilePath& path, int index, void* params);
   virtual void FileSelectionCanceled(void* params);
@@ -255,15 +230,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
                                FilePath* generated_name);
 
  private:
-  class FakeDbHandleGenerator {
-   public:
-    explicit FakeDbHandleGenerator(int64 start_value) : value_(start_value) {}
-
-    int64 GetNext() { return value_--; }
-   private:
-    int64 value_;
-  };
-
   // This class is used to let an incognito DownloadManager observe changes to
   // a normal DownloadManager, to propagate ModelChanged() calls from the parent
   // DownloadManager to the observers of the incognito DownloadManager.
@@ -275,7 +241,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
 
     // Observer interface.
     virtual void ModelChanged();
-    virtual void SetDownloads(std::vector<DownloadItem*>& downloads);
     virtual void ManagerGoingDown();
 
    private:
@@ -319,13 +284,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   void ContinueStartDownload(DownloadCreateInfo* info,
                              const FilePath& target_path);
 
-  // Update the history service for a particular download.
-  // Marked virtual for testing.
-  virtual void UpdateHistoryForDownload(DownloadItem* download);
-  void RemoveDownloadFromHistory(DownloadItem* download);
-  void RemoveDownloadsFromHistoryBetween(const base::Time remove_begin,
-                                         const base::Time remove_before);
-
   // Create an extension based on the file name and mime type.
   static void GenerateExtension(const FilePath& file_name,
                                 const std::string& mime_type,
@@ -425,8 +383,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   Profile* profile_;
   scoped_refptr<URLRequestContextGetter> request_context_getter_;
 
-  // Used for history service request management.
-  CancelableRequestConsumerTSimple<Observer*> cancelable_consumer_;
+  scoped_ptr<DownloadHistory> download_history_;
 
   // Non-owning pointer for handling file writing on the download_thread_.
   DownloadFileManager* file_manager_;
@@ -461,12 +418,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // saved.
   scoped_refptr<SelectFileDialog> select_file_dialog_;
 
-  // 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 FakeDbHandleGenerator
-  // automatically decrement the handle value on every use.
-  FakeDbHandleGenerator fake_db_handle_;
-
   scoped_ptr<OtherDownloadManagerObserver> other_download_manager_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadManager);

chrome/browser/download/drag_download_file.cc

@@ -151,17 +151,13 @@ void DragDownloadFile::DownloadCompleted(bool is_successful) {
 void DragDownloadFile::ModelChanged() {
   AssertCurrentlyOnUIThread();
 
-  download_manager_->GetTemporaryDownloads(this, file_path_.DirName());
-}
-
-void DragDownloadFile::SetDownloads(std::vector<DownloadItem*>& downloads) {
-  AssertCurrentlyOnUIThread();
-
-  std::vector<DownloadItem*>::const_iterator it = downloads.begin();
-  for (; it != downloads.end(); ++it) {
-    if (!download_item_observer_added_ && (*it)->url() == url_) {
+  std::vector<DownloadItem*> downloads;
+  download_manager_->GetTemporaryDownloads(file_path_.DirName(), &downloads);
+  for (std::vector<DownloadItem*>::const_iterator i = downloads.begin();
+       i != downloads.end(); ++i) {
+    if (!download_item_observer_added_ && (*i)->url() == url_) {
       download_item_observer_added_ = true;
-      (*it)->AddObserver(this);
+      (*i)->AddObserver(this);
     }
   }
 }

chrome/browser/download/drag_download_file.h

@@ -52,7 +52,6 @@ class DragDownloadFile : public DownloadFileProvider,
   // DownloadManager::Observer methods.
   // Called on UI thread.
   virtual void ModelChanged();
-  virtual void SetDownloads(std::vector<DownloadItem*>& downloads);
 
   // DownloadItem::Observer methods.
   // Called on UI thread.

chrome/chrome_browser.gypi

@@ -1185,6 +1185,8 @@
         'browser/download/download_file.h',
         'browser/download/download_file_manager.cc',
         'browser/download/download_file_manager.h',
+        'browser/download/download_history.cc',
+        'browser/download/download_history.h',
         'browser/download/download_item.cc',
         'browser/download/download_item.h',
         'browser/download/download_item_model.cc',

chrome/test/ui_test_utils.cc

@@ -168,7 +168,7 @@ class DownloadsCompleteObserver : public DownloadManager::Observer,
 
     download_manager_->RemoveObserver(this);
     // waiting_ will have been set if not all downloads were complete on first
-    // pass below in SetDownloads().
+    // pass below in OnSearchDownloadsComplete().
     if (waiting_)
       MessageLoopForUI::current()->Quit();
     return true;
@@ -186,10 +186,13 @@ class DownloadsCompleteObserver : public DownloadManager::Observer,
 
   // DownloadManager::Observer
   virtual void ModelChanged() {
-    download_manager_->GetDownloads(this, L"");
+    download_manager_->download_history()->Search(
+        string16(),
+        NewCallback(this,
+                    &DownloadsCompleteObserver::OnSearchDownloadsComplete));
   }
 
-  virtual void SetDownloads(std::vector<DownloadItem*>& downloads) {
+  void OnSearchDownloadsComplete(std::vector<DownloadItem*> downloads) {
     downloads_ = downloads;
     if (CheckAllDownloadsComplete())
       return;