Move DownloadHistory ownership and usage out from content to chrome. DownloadManager talks to the history system through its delegate interface. I'll move the DownloadHistoryInfo struct from chrome to content in a future change.

BUG=82782
Review URL: http://codereview.chromium.org/7704004

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

9 files changed, 124 insertions, 95 deletions

diff --git a/content/browser/download/download_file_manager.cc b/content/browser/download/download_file_manager.cc
index 09bb2ba..399f72b 100644
--- a/content/browser/download/download_file_manager.cc
+++ b/content/browser/download/download_file_manager.cc
@@ -28,8 +28,7 @@ const int kUpdatePeriodMs = 500;
 }  // namespace
 
 DownloadFileManager::DownloadFileManager(ResourceDispatcherHost* rdh)
-    : next_id_(0),
-      resource_dispatcher_host_(rdh) {
+    : resource_dispatcher_host_(rdh) {
 }
 
 DownloadFileManager::~DownloadFileManager() {
@@ -114,11 +113,8 @@ void DownloadFileManager::UpdateInProgressDownloads() {
   }
 }
 
-// Called on the IO thread once the ResourceDispatcherHost has decided that a
-// request is a download.
 int DownloadFileManager::GetNextId() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  return next_id_++;
+  return next_id_.GetNext();
 }
 
 void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
diff --git a/content/browser/download/download_file_manager.h b/content/browser/download/download_file_manager.h
index 45cef5e..0c14dca 100644
--- a/content/browser/download/download_file_manager.h
+++ b/content/browser/download/download_file_manager.h
@@ -42,6 +42,7 @@
 
 #include <map>
 
+#include "base/atomic_sequence_num.h"
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/hash_tables.h"
@@ -72,7 +73,7 @@ class DownloadFileManager
   // Called on shutdown on the UI thread.
   void Shutdown();
 
-  // Called on the IO thread
+  // Called on the IO or UI threads.
   int GetNextId();
 
   // Called on UI thread to make DownloadFileManager start the download.
@@ -153,8 +154,8 @@ class DownloadFileManager
   // it from the maps.
   void EraseDownload(int id);
 
-  // Unique ID for each DownloadFile.
-  int next_id_;
+  // Unique ID for each DownloadItem.
+  base::AtomicSequenceNumber next_id_;
 
   typedef base::hash_map<int, DownloadFile*> DownloadFileMap;
 
diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc
index 6423110..2fdf2a3 100644
--- a/content/browser/download/download_item.cc
+++ b/content/browser/download/download_item.cc
@@ -16,7 +16,6 @@
 #include "net/base/net_util.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_extensions.h"
-#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/history/download_history_info.h"
@@ -116,6 +115,13 @@ DownloadItem::DangerType GetDangerType(bool dangerous_file,
 
 }  // namespace
 
+// 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 DownloadItem::kUninitializedHandle = 0;
+
 // Constructor for reading from the history service.
 DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const DownloadHistoryInfo& info)
@@ -171,7 +177,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS),
       start_time_(info.start_time),
-      db_handle_(DownloadHistory::kUninitializedHandle),
+      db_handle_(DownloadItem::kUninitializedHandle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
@@ -190,8 +196,9 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
 DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const FilePath& path,
                            const GURL& url,
-                           bool is_otr)
-    : download_id_(download_manager->GetNextSavePageId()),
+                           bool is_otr,
+                           int download_id)
+    : download_id_(download_id),
       full_path_(path),
       url_chain_(1, url),
       referrer_url_(GURL()),
@@ -201,7 +208,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS),
       start_time_(base::Time::Now()),
-      db_handle_(DownloadHistory::kUninitializedHandle),
+      db_handle_(DownloadItem::kUninitializedHandle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h
index a2fd5d4..72046f8 100644
--- a/content/browser/download/download_item.h
+++ b/content/browser/download/download_item.h
@@ -94,6 +94,10 @@ class DownloadItem : public NotificationObserver {
     DELETE_DUE_TO_USER_DISCARD
   };
 
+  // A fake download table ID which represents a download that has started,
+  // but is not yet in the table.
+  static const int kUninitializedHandle;
+
   // Interface that observers of a particular download must implement in order
   // to receive updates to the download's status.
   class Observer {
@@ -120,7 +124,8 @@ class DownloadItem : public NotificationObserver {
   DownloadItem(DownloadManager* download_manager,
                const FilePath& path,
                const GURL& url,
-               bool is_otr);
+               bool is_otr,
+               int download_id);
 
   virtual ~DownloadItem();
 
diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc
index 4fc121f..1e20f1a 100644
--- a/content/browser/download/download_manager.cc
+++ b/content/browser/download/download_manager.cc
@@ -13,7 +13,6 @@
 #include "base/stl_util.h"
 #include "base/task.h"
 #include "build/build_config.h"
-#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_history_info.h"
 #include "chrome/browser/profiles/profile.h"
@@ -61,7 +60,6 @@ DownloadManager::DownloadManager(DownloadManagerDelegate* delegate,
       browser_context_(NULL),
       file_manager_(NULL),
       status_updater_(status_updater->AsWeakPtr()),
-      next_save_page_id_(0),
       delegate_(delegate) {
   if (status_updater_)
     status_updater_->AddDelegate(this);
@@ -114,7 +112,7 @@ void DownloadManager::Shutdown() {
       download->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
     } else if (download->IsPartialDownload()) {
       download->Cancel(false);
-      download_history_->UpdateEntry(download);
+      delegate_->UpdateItemInPersistentStore(download);
     }
   }
 
@@ -135,8 +133,7 @@ void DownloadManager::Shutdown() {
   DCHECK(save_page_downloads_.empty());
 
   file_manager_ = NULL;
-
-  download_history_.reset();
+  delegate_->Shutdown();
 
   shutdown_needed_ = false;
 }
@@ -194,10 +191,6 @@ bool DownloadManager::Init(content::BrowserContext* browser_context) {
   shutdown_needed_ = true;
 
   browser_context_ = browser_context;
-  download_history_.reset(new DownloadHistory(
-      Profile::FromBrowserContext(browser_context)));
-  download_history_->Load(
-      NewCallback(this, &DownloadManager::OnQueryDownloadEntriesComplete));
 
   // In test mode, there may be no ResourceDispatcherHost.  In this case it's
   // safe to avoid setting |file_manager_| because we only call a small set of
@@ -356,8 +349,7 @@ void DownloadManager::ContinueDownloadWithPath(DownloadItem* download,
 
   download->Rename(download_path);
 
-  download_history_->AddEntry(download,
-      NewCallback(this, &DownloadManager::OnCreateDownloadEntryComplete));
+  delegate_->AddItemToPersistentStore(download);
 }
 
 void DownloadManager::UpdateDownload(int32 download_id, int64 size) {
@@ -368,7 +360,7 @@ void DownloadManager::UpdateDownload(int32 download_id, int64 size) {
     if (download->IsInProgress()) {
       download->Update(size);
       UpdateDownloadProgress();  // Reflect size updates.
-      download_history_->UpdateEntry(download);
+      delegate_->UpdateItemInPersistentStore(download);
     }
   }
 }
@@ -422,7 +414,7 @@ void DownloadManager::AssertQueueStateConsistent(DownloadItem* download) {
   CHECK(ContainsKey(downloads_, download));
 
   // Check history_downloads_ consistency.
-  if (download->db_handle() != DownloadHistory::kUninitializedHandle) {
+  if (download->db_handle() != DownloadItem::kUninitializedHandle) {
     CHECK(ContainsKey(history_downloads_, download->db_handle()));
   } else {
     // TODO(rdsmith): Somewhat painful; make sure to disable in
@@ -458,7 +450,7 @@ bool DownloadManager::IsDownloadReadyForCompletion(DownloadItem* download) {
   // If the download hasn't been inserted into the history system
   // (which occurs strictly after file name determination, intermediate
   // file rename, and UI display) then it's not ready for completion.
-  if (download->db_handle() == DownloadHistory::kUninitializedHandle)
+  if (download->db_handle() == DownloadItem::kUninitializedHandle)
     return false;
 
   return true;
@@ -481,7 +473,7 @@ void DownloadManager::MaybeCompleteDownload(DownloadItem* download) {
   DCHECK_NE(DownloadItem::DANGEROUS, download->safety_state());
   DCHECK_EQ(1u, in_progress_.count(download->id()));
   DCHECK(download->all_data_saved());
-  DCHECK(download->db_handle() != DownloadHistory::kUninitializedHandle);
+  DCHECK(download->db_handle() != DownloadItem::kUninitializedHandle);
   DCHECK_EQ(1u, history_downloads_.count(download->db_handle()));
 
   VLOG(20) << __FUNCTION__ << "()" << " executing: download = "
@@ -491,7 +483,7 @@ void DownloadManager::MaybeCompleteDownload(DownloadItem* download) {
   in_progress_.erase(download->id());
   UpdateDownloadProgress();  // Reflect removal from in_progress_.
 
-  download_history_->UpdateEntry(download);
+  delegate_->UpdateItemInPersistentStore(download);
 
   // Finish the download.
   download->OnDownloadCompleting(file_manager_);
@@ -501,7 +493,7 @@ void DownloadManager::DownloadCompleted(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download = GetDownloadItem(download_id);
   DCHECK(download);
-  download_history_->UpdateEntry(download);
+  delegate_->UpdateItemInPersistentStore(download);
   active_downloads_.erase(download_id);
 }
 
@@ -530,7 +522,7 @@ void DownloadManager::OnDownloadRenamedToFinalName(int download_id,
     item->set_path_uniquifier(uniquifier);
 
   item->OnDownloadRenamedToFinalName(full_path);
-  download_history_->UpdateDownloadPath(item, full_path);
+  delegate_->UpdatePathForItemInPersistentStore(item, full_path);
 }
 
 void DownloadManager::DownloadCancelled(int32 download_id) {
@@ -545,11 +537,11 @@ void DownloadManager::DownloadCancelled(int32 download_id) {
 
   // Clean up will happen when the history system create callback runs if we
   // don't have a valid db_handle yet.
-  if (download->db_handle() != DownloadHistory::kUninitializedHandle) {
+  if (download->db_handle() != DownloadItem::kUninitializedHandle) {
     in_progress_.erase(it);
     active_downloads_.erase(download_id);
     UpdateDownloadProgress();  // Reflect removal from in_progress_.
-    download_history_->UpdateEntry(download);
+    delegate_->UpdateItemInPersistentStore(download);
   }
 
   DownloadCancelledInternal(download_id, download->request_handle());
@@ -589,11 +581,11 @@ void DownloadManager::OnDownloadError(int32 download_id,
   //
   // Clean up will happen when the history system create callback runs if we
   // don't have a valid db_handle yet.
-  if (download->db_handle() != DownloadHistory::kUninitializedHandle) {
+  if (download->db_handle() != DownloadItem::kUninitializedHandle) {
     in_progress_.erase(download_id);
     active_downloads_.erase(download_id);
     UpdateDownloadProgress();  // Reflect removal from in_progress_.
-    download_history_->UpdateEntry(download);
+    delegate_->UpdateItemInPersistentStore(download);
   }
 
   BrowserThread::PostTask(
@@ -638,7 +630,7 @@ void DownloadManager::RemoveDownload(int64 download_handle) {
 
   // Make history update.
   DownloadItem* download = it->second;
-  download_history_->RemoveEntry(download);
+  delegate_->RemoveItemFromPersistentStore(download);
 
   // Remove from our tables and delete.
   int downloads_count = RemoveDownloadItems(DownloadVector(1, download));
@@ -647,7 +639,7 @@ void DownloadManager::RemoveDownload(int64 download_handle) {
 
 int DownloadManager::RemoveDownloadsBetween(const base::Time remove_begin,
                                             const base::Time remove_end) {
-  download_history_->RemoveEntriesBetween(remove_begin, remove_end);
+  delegate_->RemoveItemsFromPersistentStoreBetween(remove_begin, remove_end);
 
   // All downloads visible to the user will be in the history,
   // so scan that map.
@@ -792,7 +784,7 @@ void DownloadManager::FileSelectionCanceled(void* params) {
 
 // The history service has retrieved all download entries. 'entries' contains
 // 'DownloadHistoryInfo's in sorted order (by ascending start_time).
-void DownloadManager::OnQueryDownloadEntriesComplete(
+void DownloadManager::OnPersistentStoreQueryComplete(
     std::vector<DownloadHistoryInfo>* entries) {
   for (size_t i = 0; i < entries->size(); ++i) {
     DownloadItem* download = new DownloadItem(this, entries->at(i));
@@ -810,19 +802,11 @@ void DownloadManager::AddDownloadItemToHistory(DownloadItem* download,
                                                int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // It's not immediately obvious, but HistoryBackend::CreateDownload() can
-  // call this function with an invalid |db_handle|. For instance, this can
-  // happen when the history database is offline. We cannot have multiple
-  // DownloadItems with the same invalid db_handle, so we need to assign a
-  // unique |db_handle| here.
-  if (db_handle == DownloadHistory::kUninitializedHandle)
-    db_handle = download_history_->GetNextFakeDbHandle();
-
   // TODO(rdsmith): Convert to DCHECK() when http://crbug.com/84508
   // is fixed.
-  CHECK_NE(DownloadHistory::kUninitializedHandle, db_handle);
+  CHECK_NE(DownloadItem::kUninitializedHandle, db_handle);
 
-  DCHECK(download->db_handle() == DownloadHistory::kUninitializedHandle);
+  DCHECK(download->db_handle() == DownloadItem::kUninitializedHandle);
   download->set_db_handle(db_handle);
 
   DCHECK(!ContainsKey(history_downloads_, download->db_handle()));
@@ -836,11 +820,22 @@ void DownloadManager::AddDownloadItemToHistory(DownloadItem* download,
   NotifyModelChanged();
 }
 
+
+void DownloadManager::OnItemAddedToPersistentStore(int32 download_id,
+                                                   int64 db_handle) {
+  if (save_page_downloads_.count(download_id)) {
+    OnSavePageItemAddedToPersistentStore(download_id, db_handle);
+  } else if (active_downloads_.count(download_id)) {
+    OnDownloadItemAddedToPersistentStore(download_id, db_handle);
+  }
+  // It's valid that we don't find a matching item, i.e. on shutdown.
+}
+
 // Once the new DownloadItem's creation info has been committed to the history
 // service, we associate the DownloadItem with the db handle, update our
 // 'history_downloads_' map and inform observers.
-void DownloadManager::OnCreateDownloadEntryComplete(int32 download_id,
-                                                    int64 db_handle) {
+void DownloadManager::OnDownloadItemAddedToPersistentStore(int32 download_id,
+                                                           int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download = GetActiveDownloadItem(download_id);
   if (!download)
@@ -866,7 +861,7 @@ void DownloadManager::OnCreateDownloadEntryComplete(int32 download_id,
         << " download = " << download->DebugString(true);
     in_progress_.erase(download_id);
     active_downloads_.erase(download_id);
-    download_history_->UpdateEntry(download);
+    delegate_->UpdateItemInPersistentStore(download);
     download->UpdateObservers();
   }
 }
@@ -966,16 +961,15 @@ void DownloadManager::SavePageDownloadStarted(DownloadItem* download) {
 
   // Add this entry to the history service.
   // Additionally, the UI is notified in the callback.
-  download_history_->AddEntry(download,
-      NewCallback(this, &DownloadManager::OnSavePageDownloadEntryAdded));
+  delegate_->AddItemToPersistentStore(download);
 }
 
 // SavePackage will call SavePageDownloadFinished upon completion/cancellation.
-// The history callback will call OnSavePageDownloadEntryAdded.
+// The history callback will call OnSavePageItemAddedToPersistentStore.
 // If the download finishes before the history callback,
-// OnSavePageDownloadEntryAdded calls SavePageDownloadFinished, ensuring that
-// the history event is update regardless of the order in which these two events
-// complete.
+// OnSavePageItemAddedToPersistentStore calls SavePageDownloadFinished, ensuring
+// that the history event is update regardless of the order in which these two
+// events complete.
 // If something removes the download item from the download manager (Remove,
 // Shutdown) the result will be that the SavePage system will not be able to
 // properly update the download item (which no longer exists) or the download
@@ -984,8 +978,8 @@ void DownloadManager::SavePageDownloadStarted(DownloadItem* download) {
 // Initiation -> History Callback -> Removal -> Completion), but there's no way
 // to solve that without canceling on Remove (which would then update the DB).
 
-void DownloadManager::OnSavePageDownloadEntryAdded(int32 download_id,
-                                                   int64 db_handle) {
+void DownloadManager::OnSavePageItemAddedToPersistentStore(int32 download_id,
+                                                           int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadMap::const_iterator it = save_page_downloads_.find(download_id);
@@ -1008,8 +1002,8 @@ void DownloadManager::OnSavePageDownloadEntryAdded(int32 download_id,
 }
 
 void DownloadManager::SavePageDownloadFinished(DownloadItem* download) {
-  if (download->db_handle() != DownloadHistory::kUninitializedHandle) {
-    download_history_->UpdateEntry(download);
+  if (download->db_handle() != DownloadItem::kUninitializedHandle) {
+    delegate_->UpdateItemInPersistentStore(download);
     DCHECK(ContainsKey(save_page_downloads_, download->id()));
     save_page_downloads_.erase(download->id());
 
@@ -1020,9 +1014,3 @@ void DownloadManager::SavePageDownloadFinished(DownloadItem* download) {
           Details<DownloadItem>(download));
   }
 }
-
-int32 DownloadManager::GetNextSavePageId() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  return next_save_page_id_++;
-}
-
diff --git a/content/browser/download/download_manager.h b/content/browser/download/download_manager.h
index 0720989..8c8767b 100644
--- a/content/browser/download/download_manager.h
+++ b/content/browser/download/download_manager.h
@@ -48,14 +48,12 @@
 #include "content/browser/download/download_status_updater_delegate.h"
 
 class DownloadFileManager;
-class DownloadHistory;
 class DownloadManagerDelegate;
 class DownloadStatusUpdater;
 class GURL;
 class ResourceDispatcherHost;
 class TabContents;
 struct DownloadCreateInfo;
-struct DownloadHistoryInfo;
 struct DownloadSaveInfo;
 
 namespace content {
@@ -181,10 +179,14 @@ class DownloadManager
   // Remove a download observer from ourself.
   void RemoveObserver(Observer* observer);
 
-  // Methods called on completion of a query sent to the history system.
-  void OnQueryDownloadEntriesComplete(
+  // Called by the embedder, after creating the download manager, to let it know
+  // about downloads from previous runs of the browser.
+  void OnPersistentStoreQueryComplete(
       std::vector<DownloadHistoryInfo>* entries);
-  void OnCreateDownloadEntryComplete(int32 download_id, int64 db_handle);
+
+  // Called by the embedder, in response to
+  // DownloadManagerDelegate::AddItemToPersistentStore.
+  void OnItemAddedToPersistentStore(int32 download_id, int64 db_handle);
 
   // Display a new download in the appropriate browser UI.
   void ShowDownloadInBrowser(DownloadItem* download);
@@ -196,8 +198,6 @@ class DownloadManager
 
   content::BrowserContext* browser_context() { return browser_context_; }
 
-  DownloadHistory* download_history() { return download_history_.get(); }
-
   FilePath last_download_path() { return last_download_path_; }
 
   // Creates the download item.  Must be called on the UI thread.
@@ -242,15 +242,9 @@ class DownloadManager
   // to the DownloadManager.
   void SavePageDownloadStarted(DownloadItem* download);
 
-  // Callback when Save Page As entry is commited to the history system.
-  void OnSavePageDownloadEntryAdded(int32 download_id, int64 db_handle);
-
   // Called when Save Page download is done.
   void SavePageDownloadFinished(DownloadItem* download);
 
-  // Download Id for next Save Page.
-  int32 GetNextSavePageId();
-
   // Get the download item from the active map.  Useful when the item is not
   // yet in the history map.
   DownloadItem* GetActiveDownloadItem(int id);
@@ -312,13 +306,19 @@ class DownloadManager
   // Remove from internal maps.
   int RemoveDownloadItems(const DownloadVector& pending_deletes);
 
+  // Called when a download entry is committed to the persistent store.
+  void OnDownloadItemAddedToPersistentStore(int32 download_id, int64 db_handle);
+
+  // Called when Save Page As entry is commited to the persistent store.
+  void OnSavePageItemAddedToPersistentStore(int32 download_id, int64 db_handle);
+
   // |downloads_| is the owning set for all downloads known to the
   // DownloadManager.  This includes downloads started by the user in
   // this session, downloads initialized from the history system, and
   // "save page as" downloads.  All other DownloadItem containers in
   // the DownloadManager are maps; they do not own the DownloadItems.
   // Note that this is the only place (with any functional implications;
-  // see save_page_as_downloads_ below) that "save page as" downloads are
+  // see save_page_downloads_ below) that "save page as" downloads are
   // kept, as the DownloadManager's only job is to hold onto those
   // until destruction.
   //
@@ -327,16 +327,17 @@ class DownloadManager
   // sessions.
   //
   // |active_downloads_| is a map of all downloads that are currently being
-  // processed. The key is the ID assigned by the ResourceDispatcherHost,
+  // processed. The key is the ID assigned by the DownloadFileManager,
   // which is unique for the current session.
   //
   // |in_progress_| is a map of all downloads that are in progress and that have
   // not yet received a valid history handle. The key is the ID assigned by the
-  // ResourceDispatcherHost, which is unique for the current session.
+  // DownloadFileManager, which is unique for the current session.
   //
-  // |save_page_as_downloads_| (if defined) is a collection of all the
+  // |save_page_downloads_| (if defined) is a collection of all the
   // downloads the "save page as" system has given to us to hold onto
-  // until we are destroyed.  It is only used for debugging.
+  // until we are destroyed. They key is DownloadFileManager, so it is unique
+  // compared to download item. It is only used for debugging.
   //
   // When a download is created through a user action, the corresponding
   // DownloadItem* is placed in |active_downloads_| and remains there until the
@@ -364,8 +365,6 @@ class DownloadManager
   // The current active browser context.
   content::BrowserContext* browser_context_;
 
-  scoped_ptr<DownloadHistory> download_history_;
-
   // Non-owning pointer for handling file writing on the download_thread_.
   DownloadFileManager* file_manager_;
 
@@ -376,9 +375,6 @@ class DownloadManager
   // user wants us to prompt for a save location for each download.
   FilePath last_download_path_;
 
-  // Download Id for next Save Page.
-  int32 next_save_page_id_;
-
   // Allows an embedder to control behavior. Guaranteed to outlive this object.
   DownloadManagerDelegate* delegate_;
 
diff --git a/content/browser/download/download_manager_delegate.h b/content/browser/download/download_manager_delegate.h
index b969d1a..fd2e2d6 100644
--- a/content/browser/download/download_manager_delegate.h
+++ b/content/browser/download/download_manager_delegate.h
@@ -8,7 +8,9 @@
 
 #include "base/basictypes.h"
 #include "base/memory/weak_ptr.h"
+#include "base/time.h"
 
+class DownloadItem;
 class FilePath;
 class TabContents;
 class SavePackage;
@@ -16,6 +18,9 @@ class SavePackage;
 // Browser's download manager: manages all downloads and destination view.
 class DownloadManagerDelegate {
  public:
+  // Lets the delegate know that the download manager is shutting down.
+  virtual void Shutdown() = 0;
+
   // Notifies the delegate that a download is starting. The delegate can return
   // false to delay the start of the download, in which case it should call
   // DownloadManager::RestartDownload when it's ready.
@@ -39,6 +44,31 @@ class DownloadManagerDelegate {
   // Returns true if we need to generate a binary hash for downloads.
   virtual bool GenerateFileHash() = 0;
 
+  // Notifies the embedder that a new download item is created. The
+  // DownloadManager waits for the embedder to add information about this
+  // download to its persistent store. When the embedder is done, it calls
+  // DownloadManager::OnDownloadItemAddedToPersistentStore.
+  virtual void AddItemToPersistentStore(DownloadItem* item) = 0;
+
+  // Notifies the embedder that information about the given download has change,
+  // so that it can update its persistent store.
+  virtual void UpdateItemInPersistentStore(DownloadItem* item) = 0;
+
+  // Notifies the embedder that path for the download item has changed, so that
+  // it can update its persistent store.
+  virtual void UpdatePathForItemInPersistentStore(
+      DownloadItem* item,
+      const FilePath& new_path) = 0;
+
+  // Notifies the embedder that it should remove the download item from its
+  // persistent store.
+  virtual void RemoveItemFromPersistentStore(DownloadItem* item) = 0;
+
+  // Notifies the embedder to remove downloads from the given time range.
+  virtual void RemoveItemsFromPersistentStoreBetween(
+      const base::Time remove_begin,
+      const base::Time remove_end) = 0;
+
   // Retrieve the directories to save html pages and downloads to.
   virtual void GetSaveDir(TabContents* tab_contents,
                           FilePath* website_save_dir,
diff --git a/content/browser/download/save_file_manager.h b/content/browser/download/save_file_manager.h
index fb6a9a2..5f555ed 100644
--- a/content/browser/download/save_file_manager.h
+++ b/content/browser/download/save_file_manager.h
@@ -89,7 +89,9 @@ class SaveFileManager
   // Lifetime management.
   void Shutdown();
 
-  // Called on the IO thread
+  // Called on the IO thread. This generates unique IDs for
+  // SaveFileResourceHandler objects (there's one per file in a SavePackage).
+  // Note that this is different from the SavePackage's id.
   int GetNextId();
 
   // Save the specified URL. Called on the UI thread and forwarded to the
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 80da266..5cb56df 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -21,6 +21,7 @@
 #include "content/browser/browser_context.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/content_browser_client.h"
+#include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item.h"
 #include "content/browser/download/download_manager.h"
 #include "content/browser/download/download_manager_delegate.h"
@@ -262,12 +263,15 @@ bool SavePackage::Init() {
     return false;
   }
 
-  DCHECK(download_manager_);
+  ResourceDispatcherHost* rdh =
+      content::GetContentClient()->browser()->GetResourceDispatcherHost();
+
   // Create the download item, and add ourself as an observer.
   download_ = new DownloadItem(download_manager_,
                                saved_main_file_path_,
                                page_url_,
-                               browser_context->IsOffTheRecord());
+                               browser_context->IsOffTheRecord(),
+                               rdh->download_file_manager()->GetNextId());
   download_->AddObserver(this);
 
   // Transfer ownership to the download manager.