Make a new integer field in sql::MetaTable (a per-profile db) containing a counter for the next download id, so that this id is unique across sessions. This id will allow us to merge download id with db_handle and merge most/all of the maps in DownloadManager in future CLs.

(Retry of r98656 again. That CL included <iostream> in a "promiscuous" header file, dramatically increasing the number of static initializers, and was reverted. This is the same CL, but includes <iosfwd> instead of <iostream>.)

Make DownloadManager read this field to initialize its next_id_ counter in Init().

Put a fine-grained mutex in DownloadManager::GetNextId() so that it can be called directly from any thread.
Define a thunk wrapping DM::GNI() to be passed around between threads to guard against other threads calling any other DM methods.

This thunk owns a scoped_refptr<DM> to manage life-time issues. This pattern is implemented for DM elsewhere.
Store this thunk in ResourceContext to be called by ResourceDispatchHost/DownloadThrottlingResourceHandler on the IO thread. Pass the returned DownloadId into DownloadResourceHandler.

The alternative way to obtain ids on the IO thread is to jump over to the UI thread and back. This way would add significant latency to a critical path. GetNextId() should be fast and easily accessible from any thread.

Now that ids are per-profile, define a class DownloadId containing a per-profile id and an indication of which profile, currently the DownloadManager*. DownloadIds are hashable, comparable, globally unique, not persistent, and are used by DownloadFileManager.

When the download is added to the history, MetaTable.next_download_id will be set to the new download's id +1 if that number is greater than MT.next_download_id. Increasing this counter at the same time as the download is added to the db prevents the counter from desyncing from the db, which was the primary concern re storing the counter in the BrowserPrefs.

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

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

32 files changed, 391 insertions, 112 deletions

diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 2fd34ca..3e992ba 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -60,6 +60,8 @@ void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) {
   download_history_.reset(new DownloadHistory(profile_));
   download_history_->Load(
       NewCallback(dm, &DownloadManager::OnPersistentStoreQueryComplete));
+  download_history_->GetNextId(
+      NewCallback(dm, &DownloadManager::OnPersistentStoreGetNextId));
 }
 
 void ChromeDownloadManagerDelegate::Shutdown() {
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index 3da198b..426a434 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -27,6 +27,17 @@ DownloadHistory::~DownloadHistory() {
     delete i->second.second;
 }
 
+void DownloadHistory::GetNextId(
+    HistoryService::DownloadNextIdCallback* callback) {
+  DCHECK(callback);
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (!hs) {
+    delete callback;
+    return;
+  }
+  hs->GetNextDownloadId(&history_consumer_, callback);
+}
+
 void DownloadHistory::Load(HistoryService::DownloadQueryCallback* callback) {
   DCHECK(callback);
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h
index 6d6b237..45f978b 100644
--- a/chrome/browser/download/download_history.h
+++ b/chrome/browser/download/download_history.h
@@ -27,6 +27,11 @@ class DownloadHistory {
   explicit DownloadHistory(Profile* profile);
   ~DownloadHistory();
 
+  // 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(HistoryService::DownloadNextIdCallback* callback);
+
   // Retrieves DownloadCreateInfos saved in the history.
   void Load(HistoryService::DownloadQueryCallback* callback);
 
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index 3380046..5c578a8 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -10,8 +10,8 @@
 #include "base/i18n/rtl.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
-#include "base/string_util.h"
 #include "base/string16.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
@@ -66,7 +66,8 @@ class DownloadManagerTest : public testing::Test {
   }
 
   void AddDownloadToFileManager(int id, DownloadFile* download_file) {
-    file_manager()->downloads_[id] = download_file;
+    file_manager()->downloads_[DownloadId(download_manager_.get(), id)] =
+      download_file;
   }
 
   void OnResponseCompleted(int32 download_id, int64 size,
@@ -100,7 +101,7 @@ class DownloadManagerTest : public testing::Test {
         BrowserThread::FILE, FROM_HERE,
         NewRunnableMethod(file_manager_.get(),
                           &DownloadFileManager::UpdateDownload,
-                          id,
+                          DownloadId(download_manager_.get(), id),
                           &download_buffer_));
 
     message_loop_.RunAllPending();
diff --git a/chrome/browser/download/download_throttling_resource_handler.cc b/chrome/browser/download/download_throttling_resource_handler.cc
index 403306f..3478d54 100644
--- a/chrome/browser/download/download_throttling_resource_handler.cc
+++ b/chrome/browser/download/download_throttling_resource_handler.cc
@@ -5,8 +5,12 @@
 #include "chrome/browser/download/download_throttling_resource_handler.h"
 
 #include "base/logging.h"
+#include "content/browser/download/download_id.h"
+#include "content/browser/download/download_resource_handler.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
+#include "content/browser/renderer_host/resource_dispatcher_host_request_info.h"
+#include "content/browser/resource_context.h"
 #include "content/common/resource_response.h"
 #include "net/base/io_buffer.h"
 #include "net/base/mime_sniffer.h"
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index 05533d7..f71590a 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -23,7 +23,6 @@
 class BaseDownloadItemModel;
 class CrxInstaller;
 class DownloadItem;
-class DownloadManager;
 class GURL;
 class Profile;
 class ResourceDispatcherHost;
diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc
index 906ac83..ccaed51 100644
--- a/chrome/browser/history/download_database.cc
+++ b/chrome/browser/history/download_database.cc
@@ -52,9 +52,14 @@ FilePath ColumnFilePath(sql::Statement& statement, int col) {
 
 #endif
 
+// Key in the meta_table containing the next id to use for a new download in
+// this profile.
+static const char kNextDownloadId[] = "next_download_id";
+
 }  // namespace
 
-DownloadDatabase::DownloadDatabase() {
+DownloadDatabase::DownloadDatabase()
+  : next_id_(0) {
 }
 
 DownloadDatabase::~DownloadDatabase() {
@@ -73,6 +78,8 @@ bool DownloadDatabase::InitDownloadTable() {
         "state INTEGER NOT NULL)"))
       return false;
   }
+  meta_table_.Init(&GetDB(), 0, 0);
+  meta_table_.GetValue(kNextDownloadId, &next_id_);
   return true;
 }
 
@@ -161,8 +168,12 @@ int64 DownloadDatabase::CreateDownload(
   statement.BindInt64(4, info.total_bytes);
   statement.BindInt(5, info.state);
 
-  if (statement.Run())
-    return GetDB().GetLastInsertRowId();
+  if (statement.Run()) {
+    int64 db_handle = GetDB().GetLastInsertRowId();
+    // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
+    meta_table_.SetValue(kNextDownloadId, ++next_id_);
+    return db_handle;
+  }
   return 0;
 }
 
diff --git a/chrome/browser/history/download_database.h b/chrome/browser/history/download_database.h
index c77500c..8f97e55 100644
--- a/chrome/browser/history/download_database.h
+++ b/chrome/browser/history/download_database.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include "chrome/browser/history/history_types.h"
+#include "sql/meta_table.h"
 
 struct DownloadPersistentStoreInfo;
 class FilePath;
@@ -24,6 +25,8 @@ class DownloadDatabase {
   DownloadDatabase();
   virtual ~DownloadDatabase();
 
+  int next_download_id() const { return next_id_; }
+
   // Get all the downloads from the database.
   void QueryDownloads(std::vector<DownloadPersistentStoreInfo>* results);
 
@@ -63,6 +66,9 @@ class DownloadDatabase {
   bool DropDownloadTable();
 
  private:
+  int next_id_;
+  sql::MetaTable meta_table_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadDatabase);
 };
 
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index cdab352..6b62b89 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -511,6 +511,13 @@ HistoryService::Handle HistoryService::CreateDownload(
                   create_info);
 }
 
+HistoryService::Handle HistoryService::GetNextDownloadId(
+    CancelableRequestConsumerBase* consumer,
+    DownloadNextIdCallback* callback) {
+  return Schedule(PRIORITY_NORMAL, &HistoryBackend::GetNextDownloadId, consumer,
+                  new history::DownloadNextIdRequest(callback));
+}
+
 // Handle queries for a list of all downloads in the history database's
 // 'downloads' table.
 HistoryService::Handle HistoryService::QueryDownloads(
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index 120d764..0a09a6a 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -423,6 +423,13 @@ class HistoryService : public CancelableRequestProvider,
                         CancelableRequestConsumerBase* consumer,
                         DownloadCreateCallback* callback);
 
+  // Implemented by the caller of 'GetNextDownloadId' below.
+  typedef Callback1<int/*next_download_id*/>::Type DownloadNextIdCallback;
+
+  // Runs the callback with the next available download id.
+  Handle GetNextDownloadId(CancelableRequestConsumerBase* consumer,
+                           DownloadNextIdCallback* callback);
+
   // Implemented by the caller of 'QueryDownloads' below, and is called when the
   // history service has retrieved a list of all download state. The call
   typedef Callback1<std::vector<DownloadPersistentStoreInfo>*>::Type
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index f1ff383..4ca4e0f 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -1076,6 +1076,17 @@ void HistoryBackend::GetMostRecentKeywordSearchTerms(
 
 // Downloads -------------------------------------------------------------------
 
+void HistoryBackend::GetNextDownloadId(
+    scoped_refptr<DownloadNextIdRequest> request) {
+  if (request->canceled()) return;
+  if (db_.get()) {
+    request->value = db_->next_download_id();
+  } else {
+    request->value = 0;
+  }
+  request->ForwardResult(DownloadNextIdRequest::TupleType(request->value));
+}
+
 // Get all the download entries from the database.
 void HistoryBackend::QueryDownloads(
     scoped_refptr<DownloadQueryRequest> request) {
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index a7c7317..c9c3096 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -236,6 +236,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
 
   // Downloads -----------------------------------------------------------------
 
+  void GetNextDownloadId(scoped_refptr<DownloadNextIdRequest> request);
   void QueryDownloads(scoped_refptr<DownloadQueryRequest> request);
   void CleanUpInProgressEntries();
   void UpdateDownload(int64 received_bytes, int32 state, int64 db_handle);
diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h
index f0f1073..845ea9c 100644
--- a/chrome/browser/history/history_marshaling.h
+++ b/chrome/browser/history/history_marshaling.h
@@ -55,6 +55,11 @@ typedef CancelableRequest<FaviconService::FaviconDataCallback>
 
 // Downloads ------------------------------------------------------------------
 
+typedef CancelableRequest1<HistoryService::DownloadNextIdCallback,
+                           int/*next_id*/>
+    DownloadNextIdRequest;
+
+
 typedef CancelableRequest1<HistoryService::DownloadQueryCallback,
                            std::vector<DownloadPersistentStoreInfo> >
     DownloadQueryRequest;
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index 59dd64b..2562d21 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -55,9 +55,9 @@
 #include "net/url_request/url_request.h"
 #include "webkit/blob/blob_data.h"
 #include "webkit/blob/blob_url_request_job_factory.h"
+#include "webkit/database/database_tracker.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_url_request_job_factory.h"
-#include "webkit/database/database_tracker.h"
 #include "webkit/quota/quota_manager.h"
 
 #if defined(OS_CHROMEOS)
@@ -193,6 +193,8 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   PrefService* pref_service = profile->GetPrefs();
 
+  next_download_id_thunk_ = profile->GetDownloadManager()->GetNextIdThunk();
+
   scoped_ptr<ProfileParams> params(new ProfileParams);
   params->is_incognito = profile->IsOffTheRecord();
   params->clear_local_state_on_exit =
@@ -497,6 +499,7 @@ void ProfileIOData::LazyInitialize() const {
   resource_context_.SetUserData(NULL, const_cast<ProfileIOData*>(this));
   resource_context_.set_media_observer(
       io_thread_globals->media.media_internals.get());
+  resource_context_.set_next_download_id_thunk(next_download_id_thunk_);
 
   LazyInitializeInternal(profile_params_.get());
 
diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h
index 760a06d..9504926 100644
--- a/chrome/browser/profiles/profile_io_data.h
+++ b/chrome/browser/profiles/profile_io_data.h
@@ -16,6 +16,7 @@
 #include "base/synchronization/lock.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/prefs/pref_member.h"
+#include "content/browser/download/download_manager.h"
 #include "content/browser/resource_context.h"
 #include "net/base/cookie_monster.h"
 
@@ -281,6 +282,7 @@ class ProfileIOData {
   mutable scoped_refptr<fileapi::FileSystemContext> file_system_context_;
   mutable scoped_refptr<quota::QuotaManager> quota_manager_;
   mutable scoped_refptr<HostZoomMap> host_zoom_map_;
+  mutable DownloadManager::GetNextIdThunkType next_download_id_thunk_;
 
   // TODO(willchan): Remove from ResourceContext.
   mutable scoped_refptr<ExtensionInfoMap> extension_info_map_;
diff --git a/content/browser/download/download_file_manager.cc b/content/browser/download/download_file_manager.cc
index c1dda48..49b2501 100644
--- a/content/browser/download/download_file_manager.cc
+++ b/content/browser/download/download_file_manager.cc
@@ -65,9 +65,9 @@ void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info,
     return;
   }
 
-  int32 id = info->download_id;
-  DCHECK(GetDownloadFile(id) == NULL);
-  downloads_[id] = download_file.release();
+  DownloadId global_id(download_manager, info->download_id);
+  DCHECK(GetDownloadFile(global_id) == NULL);
+  downloads_[global_id] = download_file.release();
 
   // The file is now ready, we can un-pause the request and start saving data.
   info->request_handle.ResumeRequest();
@@ -77,11 +77,11 @@ void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info,
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(download_manager,
-                        &DownloadManager::StartDownload, id));
+                        &DownloadManager::StartDownload, info->download_id));
 }
 
-DownloadFile* DownloadFileManager::GetDownloadFile(int id) {
-  DownloadFileMap::iterator it = downloads_.find(id);
+DownloadFile* DownloadFileManager::GetDownloadFile(DownloadId global_id) {
+  DownloadFileMap::iterator it = downloads_.find(global_id);
   return it == downloads_.end() ? NULL : it->second;
 }
 
@@ -103,21 +103,17 @@ void DownloadFileManager::UpdateInProgressDownloads() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   for (DownloadFileMap::iterator i = downloads_.begin();
        i != downloads_.end(); ++i) {
-    int id = i->first;
+    DownloadId global_id = i->first;
     DownloadFile* download_file = i->second;
     DownloadManager* manager = download_file->GetDownloadManager();
     if (manager) {
       BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
           NewRunnableMethod(manager, &DownloadManager::UpdateDownload,
-                            id, download_file->bytes_so_far()));
+                            global_id.local(), download_file->bytes_so_far()));
     }
   }
 }
 
-int DownloadFileManager::GetNextId() {
-  return next_id_.GetNext();
-}
-
 void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(info);
@@ -145,7 +141,8 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
 // download (in the UI thread), we may receive a few more updates before the IO
 // thread gets the cancel message: we just delete the data since the
 // DownloadFile has been deleted.
-void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
+void DownloadFileManager::UpdateDownload(
+    DownloadId global_id, DownloadBuffer* buffer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   std::vector<DownloadBuffer::Contents> contents;
   {
@@ -153,7 +150,7 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
     contents.swap(buffer->contents);
   }
 
-  DownloadFile* download_file = GetDownloadFile(id);
+  DownloadFile* download_file = GetDownloadFile(global_id);
   bool had_error = false;
   for (size_t i = 0; i < contents.size(); ++i) {
     net::IOBuffer* data = contents[i].first;
@@ -170,7 +167,7 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
         // Calling this here in case we get more data, to avoid
         // processing data after an error.  That could lead to
         // files that are corrupted if the later processing succeeded.
-        CancelDownload(id);
+        CancelDownload(global_id);
         download_file = NULL;  // Was deleted in |CancelDownload|.
 
         if (download_manager) {
@@ -180,7 +177,7 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
               NewRunnableMethod(
                   download_manager,
                   &DownloadManager::OnDownloadError,
-                  id,
+                  global_id.local(),
                   bytes_downloaded,
                   write_result));
         }
@@ -191,16 +188,16 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
 }
 
 void DownloadFileManager::OnResponseCompleted(
-    int id,
+    DownloadId global_id,
     DownloadBuffer* buffer,
     net::Error net_error,
     const std::string& security_info) {
-  VLOG(20) << __FUNCTION__ << "()" << " id = " << id
+  VLOG(20) << __FUNCTION__ << "()" << " id = " << global_id
            << " net_error = " << net_error
            << " security_info = \"" << security_info << "\"";
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   delete buffer;
-  DownloadFile* download_file = GetDownloadFile(id);
+  DownloadFile* download_file = GetDownloadFile(global_id);
   if (!download_file)
     return;
 
@@ -208,7 +205,7 @@ void DownloadFileManager::OnResponseCompleted(
 
   DownloadManager* download_manager = download_file->GetDownloadManager();
   if (!download_manager) {
-    CancelDownload(id);
+    CancelDownload(global_id);
     return;
   }
 
@@ -228,7 +225,7 @@ void DownloadFileManager::OnResponseCompleted(
         NewRunnableMethod(
             download_manager,
             &DownloadManager::OnResponseCompleted,
-            id,
+            global_id.local(),
             download_file->bytes_so_far(),
             hash));
   } else {
@@ -238,7 +235,7 @@ void DownloadFileManager::OnResponseCompleted(
         NewRunnableMethod(
             download_manager,
             &DownloadManager::OnDownloadError,
-            id,
+            global_id.local(),
             download_file->bytes_so_far(),
             net_error));
   }
@@ -249,10 +246,10 @@ void DownloadFileManager::OnResponseCompleted(
 // This method will be sent via a user action, or shutdown on the UI thread, and
 // run on the download thread. Since this message has been sent from the UI
 // thread, the download may have already completed and won't exist in our map.
-void DownloadFileManager::CancelDownload(int id) {
-  VLOG(20) << __FUNCTION__ << "()" << " id = " << id;
+void DownloadFileManager::CancelDownload(DownloadId global_id) {
+  VLOG(20) << __FUNCTION__ << "()" << " id = " << global_id;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DownloadFileMap::iterator it = downloads_.find(id);
+  DownloadFileMap::iterator it = downloads_.find(global_id);
   if (it == downloads_.end())
     return;
 
@@ -261,24 +258,24 @@ void DownloadFileManager::CancelDownload(int id) {
            << " download_file = " << download_file->DebugString();
   download_file->Cancel();
 
-  EraseDownload(id);
+  EraseDownload(global_id);
 }
 
-void DownloadFileManager::CompleteDownload(int id) {
+void DownloadFileManager::CompleteDownload(DownloadId global_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  if (!ContainsKey(downloads_, id))
+  if (!ContainsKey(downloads_, global_id))
     return;
 
-  DownloadFile* download_file = downloads_[id];
+  DownloadFile* download_file = downloads_[global_id];
 
   VLOG(20) << " " << __FUNCTION__ << "()"
-           << " id = " << id
+           << " id = " << global_id
            << " download_file = " << download_file->DebugString();
 
   download_file->Detach();
 
-  EraseDownload(id);
+  EraseDownload(global_id);
 }
 
 void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
@@ -298,7 +295,7 @@ void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
 
   for (std::set<DownloadFile*>::iterator i = to_remove.begin();
        i != to_remove.end(); ++i) {
-    downloads_.erase((*i)->id());
+    downloads_.erase(DownloadId((*i)->GetDownloadManager(), (*i)->id()));
     delete *i;
   }
 }
@@ -312,12 +309,12 @@ void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
 // 1. tmp -> foo.crdownload (not final, safe)
 // 2. tmp-> Unconfirmed.xxx.crdownload (not final, dangerous)
 void DownloadFileManager::RenameInProgressDownloadFile(
-    int id, const FilePath& full_path) {
-  VLOG(20) << __FUNCTION__ << "()" << " id = " << id
+    DownloadId global_id, const FilePath& full_path) {
+  VLOG(20) << __FUNCTION__ << "()" << " id = " << global_id
            << " full_path = \"" << full_path.value() << "\"";
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  DownloadFile* download_file = GetDownloadFile(id);
+  DownloadFile* download_file = GetDownloadFile(global_id);
   if (!download_file)
     return;
 
@@ -328,7 +325,7 @@ void DownloadFileManager::RenameInProgressDownloadFile(
   if (net::OK != rename_error) {
     // Error. Between the time the UI thread generated 'full_path' to the time
     // this code runs, something happened that prevents us from renaming.
-    CancelDownloadOnRename(id, rename_error);
+    CancelDownloadOnRename(global_id, rename_error);
   }
 }
 
@@ -340,13 +337,15 @@ void DownloadFileManager::RenameInProgressDownloadFile(
 // 1. foo.crdownload -> foo (final, safe)
 // 2. Unconfirmed.xxx.crdownload -> xxx (final, validated)
 void DownloadFileManager::RenameCompletingDownloadFile(
-    int id, const FilePath& full_path, bool overwrite_existing_file) {
-  VLOG(20) << __FUNCTION__ << "()" << " id = " << id
+    DownloadId global_id,
+    const FilePath& full_path,
+    bool overwrite_existing_file) {
+  VLOG(20) << __FUNCTION__ << "()" << " id = " << global_id
            << " overwrite_existing_file = " << overwrite_existing_file
            << " full_path = \"" << full_path.value() << "\"";
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  DownloadFile* download_file = GetDownloadFile(id);
+  DownloadFile* download_file = GetDownloadFile(global_id);
   if (!download_file)
     return;
 
@@ -377,7 +376,7 @@ void DownloadFileManager::RenameCompletingDownloadFile(
   if (net::OK != rename_error) {
     // Error. Between the time the UI thread generated 'full_path' to the time
     // this code runs, something happened that prevents us from renaming.
-    CancelDownloadOnRename(id, rename_error);
+    CancelDownloadOnRename(global_id, rename_error);
     return;
   }
 
@@ -390,17 +389,17 @@ void DownloadFileManager::RenameCompletingDownloadFile(
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(
-          download_manager, &DownloadManager::OnDownloadRenamedToFinalName, id,
-          new_path, uniquifier));
+          download_manager, &DownloadManager::OnDownloadRenamedToFinalName,
+          global_id.local(), new_path, uniquifier));
 }
 
 // Called only from RenameInProgressDownloadFile and
 // RenameCompletingDownloadFile on the FILE thread.
-void DownloadFileManager::CancelDownloadOnRename(int id,
-                                                 net::Error rename_error) {
+void DownloadFileManager::CancelDownloadOnRename(
+    DownloadId global_id, net::Error rename_error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  DownloadFile* download_file = GetDownloadFile(id);
+  DownloadFile* download_file = GetDownloadFile(global_id);
   if (!download_file)
     return;
 
@@ -417,24 +416,24 @@ void DownloadFileManager::CancelDownloadOnRename(int id,
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(download_manager,
                         &DownloadManager::OnDownloadError,
-                        id,
+                        global_id.local(),
                         download_file->bytes_so_far(),
                         rename_error));
 }
 
-void DownloadFileManager::EraseDownload(int id) {
+void DownloadFileManager::EraseDownload(DownloadId global_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  if (!ContainsKey(downloads_, id))
+  if (!ContainsKey(downloads_, global_id))
     return;
 
-  DownloadFile* download_file = downloads_[id];
+  DownloadFile* download_file = downloads_[global_id];
 
   VLOG(20) << " " << __FUNCTION__ << "()"
-           << " id = " << id
+           << " id = " << global_id
            << " download_file = " << download_file->DebugString();
 
-  downloads_.erase(id);
+  downloads_.erase(global_id);
 
   delete download_file;
 
diff --git a/content/browser/download/download_file_manager.h b/content/browser/download/download_file_manager.h
index 5b237b1..fec51da 100644
--- a/content/browser/download/download_file_manager.h
+++ b/content/browser/download/download_file_manager.h
@@ -48,6 +48,7 @@
 #include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
 #include "base/timer.h"
+#include "content/browser/download/download_id.h"
 #include "content/browser/download/download_request_handle.h"
 #include "net/base/net_errors.h"
 #include "ui/gfx/native_widget_types.h"
@@ -74,21 +75,18 @@ class DownloadFileManager
   // Called on shutdown on the UI thread.
   void Shutdown();
 
-  // Called on the IO or UI threads.
-  int GetNextId();
-
   // Called on UI thread to make DownloadFileManager start the download.
   void StartDownload(DownloadCreateInfo* info);
 
   // Handlers for notifications sent from the IO thread and run on the
   // FILE thread.
-  void UpdateDownload(int id, DownloadBuffer* buffer);
+  void UpdateDownload(DownloadId global_id, DownloadBuffer* buffer);
   // |net_error| is 0 for normal completions, and non-0 for errors.
   // |security_info| contains SSL information (cert_id, cert_status,
   // security_bits, ssl_connection_status), which can be used to
   // fine-tune the error message.  It is empty if the transaction
   // was not performed securely.
-  void OnResponseCompleted(int id,
+  void OnResponseCompleted(DownloadId global_id,
                            DownloadBuffer* buffer,
                            net::Error net_error,
                            const std::string& security_info);
@@ -98,22 +96,22 @@ class DownloadFileManager
   // download file, as far as the DownloadFileManager is concerned -- if
   // anything happens to the download file after they are called, it will
   // be ignored.
-  void CancelDownload(int id);
-  void CompleteDownload(int id);
+  void CancelDownload(DownloadId id);
+  void CompleteDownload(DownloadId id);
 
   // Called on FILE thread by DownloadManager at the beginning of its shutdown.
   void OnDownloadManagerShutdown(DownloadManager* manager);
 
   // The DownloadManager in the UI thread has provided an intermediate
   // .crdownload name for the download specified by |id|.
-  void RenameInProgressDownloadFile(int id, const FilePath& full_path);
+  void RenameInProgressDownloadFile(DownloadId id, const FilePath& full_path);
 
   // The DownloadManager in the UI thread has provided a final name for the
   // download specified by |id|.
   // |overwrite_existing_file| prevents uniquification, and is used for SAFE
   // downloads, as the user may have decided to overwrite the file.
   // Sent from the UI thread and run on the FILE thread.
-  void RenameCompletingDownloadFile(int id,
+  void RenameCompletingDownloadFile(DownloadId id,
                                     const FilePath& full_path,
                                     bool overwrite_existing_file);
 
@@ -145,21 +143,18 @@ class DownloadFileManager
                           bool hash_needed);
 
   // Called only on the download thread.
-  DownloadFile* GetDownloadFile(int id);
+  DownloadFile* GetDownloadFile(DownloadId global_id);
 
   // Called only from RenameInProgressDownloadFile and
   // RenameCompletingDownloadFile on the FILE thread.
   // |rename_error| indicates what error caused the cancel.
-  void CancelDownloadOnRename(int id, net::Error rename_error);
+  void CancelDownloadOnRename(DownloadId global_id, net::Error rename_error);
 
   // Erases the download file with the given the download |id| and removes
   // it from the maps.
-  void EraseDownload(int id);
-
-  // Unique ID for each DownloadItem.
-  base::AtomicSequenceNumber next_id_;
+  void EraseDownload(DownloadId global_id);
 
-  typedef base::hash_map<int, DownloadFile*> DownloadFileMap;
+  typedef base::hash_map<DownloadId, DownloadFile*> DownloadFileMap;
 
   // A map of all in progress downloads.  It owns the download files.
   DownloadFileMap downloads_;
diff --git a/content/browser/download/download_id.cc b/content/browser/download/download_id.cc
new file mode 100644
index 0000000..bef4964
--- /dev/null
+++ b/content/browser/download/download_id.cc
@@ -0,0 +1,9 @@
+// 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.
+
+#include "content/browser/download/download_id.h"
+
+std::ostream& operator<<(std::ostream& out, const DownloadId& global_id) {
+  return out << global_id.manager_ << ":" << global_id.local();
+}
diff --git a/content/browser/download/download_id.h b/content/browser/download/download_id.h
new file mode 100644
index 0000000..ca04de1
--- /dev/null
+++ b/content/browser/download/download_id.h
@@ -0,0 +1,87 @@
+// 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.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_
+#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_
+#pragma once
+
+#include <iosfwd>
+
+#include "base/hash_tables.h"
+
+class DownloadManager;
+
+// DownloadId combines per-profile Download ids with an indication of which
+// profile in order to be globally unique. DownloadIds are not persistent across
+// sessions, but their local() field is.
+class DownloadId {
+ public:
+  DownloadId(const DownloadManager* manager, int32 local_id)
+    : manager_(manager),
+      local_id_(local_id) {
+  }
+
+  // Return the per-profile and persistent part of this DownloadId.
+  int32 local() const { return local_id_; }
+
+  // Returns true if this DownloadId has been allocated and could possibly refer
+  // to a DownloadItem that exists.
+  bool IsValid() const { return ((manager_ != NULL) && (local_id_ >= 0)); }
+
+  // The following methods (operator==, hash(), copy, and assign) provide
+  // support for STL containers such as hash_map.
+
+  bool operator==(const DownloadId& that) const {
+    return ((that.local_id_ == local_id_) &&
+            (that.manager_ == manager_));
+  }
+  bool operator<(const DownloadId& that) const {
+    // Even though DownloadManager* < DownloadManager* is not well defined and
+    // GCC does not require it for hash_map, MSVC requires operator< for
+    // hash_map. We don't ifdef it out here because we will probably make a
+    // set<DownloadId> at some point, when GCC will require it.
+    return ((that.local_id_ < local_id_) &&
+            (that.manager_ < manager_));
+  }
+
+  size_t hash() const {
+    // The top half of manager is unlikely to be distinct, and the user is
+    // unlikely to have >64K downloads. If these assumptions are incorrect, then
+    // DownloadFileManager's hash_map might have a few collisions, but it will
+    // use operator== to safely disambiguate.
+    return reinterpret_cast<size_t>(manager_) +
+           (static_cast<size_t>(local_id_) << (4 * sizeof(size_t)));
+  }
+
+ private:
+  // DownloadId is used mostly off the UI thread, so manager's methods can't be
+  // called, but the pointer can be compared.
+  const DownloadManager* manager_;
+
+  int32 local_id_;
+
+  friend std::ostream& operator<<(std::ostream& out,
+                                  const DownloadId& global_id);
+
+  // Allow copy and assign.
+};
+
+// Allow logging DownloadIds. Looks like "0x01234567:42".
+std::ostream& operator<<(std::ostream& out, const DownloadId& global_id);
+
+// Allow using DownloadIds as keys in hash_maps.
+namespace BASE_HASH_NAMESPACE {
+#if defined(COMPILER_GCC)
+template<> struct hash<DownloadId> {
+  std::size_t operator()(const DownloadId& id) const {
+    return id.hash();
+  }
+};
+#elif defined(COMPILER_MSVC)
+inline size_t hash_value(const DownloadId& id) {
+  return id.hash();
+}
+#endif  // COMPILER
+}
+#endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_
diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc
index 0809872..7f1b5a5 100644
--- a/content/browser/download/download_item.cc
+++ b/content/browser/download/download_item.cc
@@ -19,6 +19,7 @@
 #include "content/browser/download/download_file.h"
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file_manager.h"
+#include "content/browser/download/download_id.h"
 #include "content/browser/download/download_manager.h"
 #include "content/browser/download/download_manager_delegate.h"
 #include "content/browser/download/download_persistent_store_info.h"
@@ -191,8 +192,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const FilePath& path,
                            const GURL& url,
                            bool is_otr,
-                           int download_id)
-    : download_id_(download_id),
+                           DownloadId download_id)
+    : download_id_(download_id.local()),
       full_path_(path),
       url_chain_(1, url),
       referrer_url_(GURL()),
@@ -225,6 +226,10 @@ DownloadItem::~DownloadItem() {
   download_manager_->AssertQueueStateConsistent(this);
 }
 
+DownloadId DownloadItem::global_id() const {
+  return DownloadId(download_manager_, id());
+}
+
 void DownloadItem::AddObserver(Observer* observer) {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -577,16 +582,15 @@ void DownloadItem::OnDownloadCompleting(DownloadFileManager* file_manager) {
   if (NeedsRename()) {
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
         NewRunnableMethod(file_manager,
-            &DownloadFileManager::RenameCompletingDownloadFile, id(),
+            &DownloadFileManager::RenameCompletingDownloadFile, global_id(),
             GetTargetFilePath(), safety_state() == SAFE));
     return;
   }
 
   Completed();
 
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(file_manager, &DownloadFileManager::CompleteDownload,
-                        id()));
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, NewRunnableMethod(
+        file_manager, &DownloadFileManager::CompleteDownload, global_id()));
 }
 
 void DownloadItem::OnDownloadRenamedToFinalName(const FilePath& full_path) {
@@ -712,7 +716,7 @@ void DownloadItem::OffThreadCancel(DownloadFileManager* file_manager) {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
-          file_manager, &DownloadFileManager::CancelDownload, download_id_));
+          file_manager, &DownloadFileManager::CancelDownload, global_id()));
 }
 
 void DownloadItem::Init(bool active) {
diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h
index 6e32d08..6256e2d 100644
--- a/content/browser/download/download_item.h
+++ b/content/browser/download/download_item.h
@@ -31,6 +31,7 @@
 #include "net/base/net_errors.h"
 
 class DownloadFileManager;
+class DownloadId;
 class DownloadManager;
 struct DownloadCreateInfo;
 struct DownloadPersistentStoreInfo;
@@ -123,7 +124,7 @@ class DownloadItem {
                const FilePath& path,
                const GURL& url,
                bool is_otr,
-               int download_id);
+               DownloadId download_id);
 
   virtual ~DownloadItem();
 
@@ -274,6 +275,7 @@ class DownloadItem {
   }
   int64 received_bytes() const { return received_bytes_; }
   int32 id() const { return download_id_; }
+  DownloadId global_id() const;
   base::Time start_time() const { return start_time_; }
   void set_db_handle(int64 handle) { db_handle_ = handle; }
   int64 db_handle() const { return db_handle_; }
diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc
index 7657502..97fd787 100644
--- a/content/browser/download/download_manager.cc
+++ b/content/browser/download/download_manager.cc
@@ -6,11 +6,15 @@
 
 #include <iterator>
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "base/stringprintf.h"
+#include "base/synchronization/lock.h"
+#include "base/sys_string_conversions.h"
 #include "base/task.h"
 #include "build/build_config.h"
 #include "content/browser/browser_context.h"
@@ -50,17 +54,22 @@ DownloadManager::DownloadManager(DownloadManagerDelegate* delegate,
                                  DownloadStatusUpdater* status_updater)
     : shutdown_needed_(false),
       browser_context_(NULL),
+      next_id_(0),
       file_manager_(NULL),
-      status_updater_(status_updater->AsWeakPtr()),
+      status_updater_((status_updater != NULL)
+                      ? status_updater->AsWeakPtr()
+                      : base::WeakPtr<DownloadStatusUpdater>()),
       delegate_(delegate),
       largest_db_handle_in_history_(DownloadItem::kUninitializedHandle) {
-  if (status_updater_)
+  // NOTE(benjhayden): status_updater may be NULL when using
+  // TestingBrowserProcess.
+  if (status_updater_.get() != NULL)
     status_updater_->AddDelegate(this);
 }
 
 DownloadManager::~DownloadManager() {
   DCHECK(!shutdown_needed_);
-  if (status_updater_)
+  if (status_updater_.get() != NULL)
     status_updater_->RemoveDelegate(this);
 }
 
@@ -177,6 +186,30 @@ void DownloadManager::SearchDownloads(const string16& query,
   }
 }
 
+void DownloadManager::OnPersistentStoreGetNextId(int next_id) {
+  DVLOG(1) << __FUNCTION__ << " " << next_id;
+  base::AutoLock lock(next_id_lock_);
+  // TODO(benjhayden) Delay Profile initialization until here, and set next_id_
+  // = next_id. The '+=' works for now because these ids are not yet persisted
+  // to the database. GetNextId() can allocate zero or more ids starting from 0,
+  // then this callback can increment next_id_, and the items with lower ids
+  // won't clash with any other items even though there may be items loaded from
+  // the history because items from the history don't have valid ids.
+  next_id_ += next_id;
+}
+
+DownloadId DownloadManager::GetNextId() {
+  // May be called on any thread via the GetNextIdThunk.
+  // TODO(benjhayden) If otr, forward to parent DM.
+  base::AutoLock lock(next_id_lock_);
+  return DownloadId(this, next_id_++);
+}
+
+DownloadManager::GetNextIdThunkType DownloadManager::GetNextIdThunk() {
+  // TODO(benjhayden) If otr, forward to parent DM.
+  return base::Bind(&DownloadManager::GetNextId, this);
+}
+
 // Query the history service for information about all persisted downloads.
 bool DownloadManager::Init(content::BrowserContext* browser_context) {
   DCHECK(browser_context);
@@ -328,7 +361,7 @@ void DownloadManager::ContinueDownloadWithPath(DownloadItem* download,
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
           file_manager_, &DownloadFileManager::RenameInProgressDownloadFile,
-          download->id(), download_path));
+          download->global_id(), download_path));
 
   download->Rename(download_path);
 
@@ -483,10 +516,10 @@ void DownloadManager::OnDownloadRenamedToFinalName(int download_id,
     DCHECK_EQ(0, uniquifier) << "We should not uniquify SAFE downloads twice";
   }
 
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::CompleteDownload, download_id));
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, NewRunnableMethod(
+      file_manager_,
+      &DownloadFileManager::CompleteDownload,
+      item->global_id()));
 
   if (uniquifier)
     item->set_path_uniquifier(uniquifier);
diff --git a/content/browser/download/download_manager.h b/content/browser/download/download_manager.h
index 994db82..7d96c53 100644
--- a/content/browser/download/download_manager.h
+++ b/content/browser/download/download_manager.h
@@ -34,6 +34,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/hash_tables.h"
@@ -41,6 +42,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/synchronization/lock.h"
 #include "base/time.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/download/download_item.h"
@@ -107,6 +109,24 @@ class DownloadManager
   // everything.
   void SearchDownloads(const string16& query, DownloadVector* result);
 
+  // Returns the next download id in a DownloadId and increments the counter.
+  // May be called on any thread. The incremented counter is not persisted, but
+  // the base counter for this accessor is initialized from the largest id
+  // actually saved to the download history database.
+  DownloadId GetNextId();
+
+  // Instead of passing a DownloadManager* between threads and hoping users only
+  // call GetNextId(), you can pass this thunk around instead. Pass the thunk
+  // around by const ref and store it by copy per the base::Callback interface.
+  // The thunk may be copied, including between threads.  If you change
+  // GetNextIdThunkType from base::Callback, then you should think about how
+  // you're changing the ref-count of DownloadManager. Use it like:
+  //   const DownloadManager::GetNextIdThunkType& next_id_thunk =
+  //       download_manager->GetNextIdThunk();
+  //   id = next_id_thunk.Run();
+  typedef base::Callback<DownloadId(void)> GetNextIdThunkType;
+  GetNextIdThunkType GetNextIdThunk();
+
   // Returns true if initialized properly.
   bool Init(content::BrowserContext* browser_context);
 
@@ -196,6 +216,11 @@ class DownloadManager
   // Remove a download observer from ourself.
   void RemoveObserver(Observer* observer);
 
+  // Called by the embedder after creating the download manager to inform it of
+  // the next available download id.
+  // TODO(benjhayden): Separate this functionality out into a separate object.
+  void OnPersistentStoreGetNextId(int next_id);
+
   // Called by the embedder, after creating the download manager, to let it know
   // about downloads from previous runs of the browser.
   void OnPersistentStoreQueryComplete(
@@ -384,6 +409,9 @@ class DownloadManager
   // The current active browser context.
   content::BrowserContext* browser_context_;
 
+  base::Lock next_id_lock_;
+  int next_id_;
+
   // Non-owning pointer for handling file writing on the download_thread_.
   DownloadFileManager* file_manager_;
 
diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc
index 9671348..c6c86ec 100644
--- a/content/browser/download/download_resource_handler.cc
+++ b/content/browser/download/download_resource_handler.cc
@@ -31,11 +31,12 @@ DownloadResourceHandler::DownloadResourceHandler(
     int render_view_id,
     int request_id,
     const GURL& url,
+    DownloadId dl_id,
     DownloadFileManager* download_file_manager,
     net::URLRequest* request,
     bool save_as,
     const DownloadSaveInfo& save_info)
-    : download_id_(-1),
+    : download_id_(dl_id),
       global_id_(render_process_host_id, request_id),
       render_view_id_(render_view_id),
       content_length_(0),
@@ -46,6 +47,7 @@ DownloadResourceHandler::DownloadResourceHandler(
       buffer_(new DownloadBuffer),
       rdh_(rdh),
       is_paused_(false) {
+  DCHECK(dl_id.IsValid());
   download_stats::RecordDownloadCount(download_stats::UNTHROTTLED_COUNT);
 }
 
@@ -66,6 +68,7 @@ bool DownloadResourceHandler::OnRequestRedirected(int request_id,
 // Send the download creation information to the download thread.
 bool DownloadResourceHandler::OnResponseStarted(int request_id,
                                                 ResourceResponse* response) {
+  DCHECK(download_id_.IsValid());
   VLOG(20) << __FUNCTION__ << "()" << DebugString()
            << " request_id = " << request_id;
   download_start_time_ = base::TimeTicks::Now();
@@ -78,15 +81,19 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id,
   const ResourceDispatcherHostRequestInfo* request_info =
     ResourceDispatcherHost::InfoForRequest(request_);
 
-  download_id_ = download_file_manager_->GetNextId();
-
   // Deleted in DownloadManager.
   DownloadCreateInfo* info = new DownloadCreateInfo(FilePath(), GURL(),
       base::Time::Now(), 0, content_length_, DownloadItem::IN_PROGRESS,
-      download_id_, request_info->has_user_gesture(),
+      download_id_.local(), request_info->has_user_gesture(),
       request_info->transition_type());
   info->url_chain = request_->url_chain();
   info->referrer_url = GURL(request_->referrer());
+  info->start_time = base::Time::Now();
+  info->received_bytes = 0;
+  info->total_bytes = content_length_;
+  info->state = DownloadItem::IN_PROGRESS;
+  info->download_id = download_id_.local();
+  info->has_user_gesture = request_info->has_user_gesture();
   info->request_handle = DownloadRequestHandle(rdh_,
                                                global_id_.child_id,
                                                render_view_id_,
@@ -256,7 +263,7 @@ std::string DownloadResourceHandler::DebugString() const {
                             " save_info_.file_path = \"%" PRFilePath "\""
                             " }",
                             request_->url().spec().c_str(),
-                            download_id_,
+                            download_id_.local(),
                             global_id_.child_id,
                             global_id_.request_id,
                             render_view_id_,
diff --git a/content/browser/download/download_resource_handler.h b/content/browser/download/download_resource_handler.h
index b4cdbd6..e864c0e 100644
--- a/content/browser/download/download_resource_handler.h
+++ b/content/browser/download/download_resource_handler.h
@@ -11,6 +11,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/timer.h"
 #include "content/browser/download/download_file.h"
+#include "content/browser/download/download_id.h"
 #include "content/browser/renderer_host/global_request_id.h"
 #include "content/browser/renderer_host/resource_handler.h"
 
@@ -25,15 +26,17 @@ class URLRequest;
 // Forwards data to the download thread.
 class DownloadResourceHandler : public ResourceHandler {
  public:
-  DownloadResourceHandler(ResourceDispatcherHost* rdh,
-                          int render_process_host_id,
-                          int render_view_id,
-                          int request_id,
-                          const GURL& url,
-                          DownloadFileManager* download_file_manager,
-                          net::URLRequest* request,
-                          bool save_as,
-                          const DownloadSaveInfo& save_info);
+  DownloadResourceHandler(
+      ResourceDispatcherHost* rdh,
+      int render_process_host_id,
+      int render_view_id,
+      int request_id,
+      const GURL& url,
+      DownloadId dl_id,
+      DownloadFileManager* download_file_manager,
+      net::URLRequest* request,
+      bool save_as,
+      const DownloadSaveInfo& save_info);
 
   virtual bool OnUploadProgress(int request_id, uint64 position, uint64 size);
 
@@ -75,7 +78,7 @@ class DownloadResourceHandler : public ResourceHandler {
 
   void StartPauseTimer();
 
-  int download_id_;
+  DownloadId download_id_;
   GlobalRequestID global_id_;
   int render_view_id_;
   scoped_refptr<net::IOBuffer> read_buffer_;
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 5cb56df..db6b423 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -263,15 +263,12 @@ bool SavePackage::Init() {
     return false;
   }
 
-  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(),
-                               rdh->download_file_manager()->GetNextId());
+                               download_manager_->GetNextId());
   download_->AddObserver(this);
 
   // Transfer ownership to the download manager.
diff --git a/content/browser/renderer_host/buffered_resource_handler.cc b/content/browser/renderer_host/buffered_resource_handler.cc
index 026ce66..72e828a 100644
--- a/content/browser/renderer_host/buffered_resource_handler.cc
+++ b/content/browser/renderer_host/buffered_resource_handler.cc
@@ -17,6 +17,7 @@
 #include "content/browser/renderer_host/resource_dispatcher_host_delegate.h"
 #include "content/browser/renderer_host/resource_dispatcher_host_request_info.h"
 #include "content/browser/renderer_host/x509_user_cert_resource_handler.h"
+#include "content/browser/resource_context.h"
 #include "content/common/resource_response.h"
 #include "net/base/io_buffer.h"
 #include "net/base/mime_sniffer.h"
@@ -309,12 +310,15 @@ bool BufferedResourceHandler::CompleteResponseStarted(int request_id,
 
     info->set_is_download(true);
 
+    DownloadId dl_id = info->context()->next_download_id_thunk().Run();
+
     scoped_refptr<ResourceHandler> handler(
       new DownloadResourceHandler(host_,
                                   info->child_id(),
                                   info->route_id(),
                                   info->request_id(),
                                   request_->url(),
+                                  dl_id,
                                   host_->download_file_manager(),
                                   request_,
                                   false,
diff --git a/content/browser/renderer_host/resource_dispatcher_host.cc b/content/browser/renderer_host/resource_dispatcher_host.cc
index d845c8f..dcfff81 100644
--- a/content/browser/renderer_host/resource_dispatcher_host.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host.cc
@@ -32,7 +32,6 @@
 #include "content/browser/download/save_file_resource_handler.h"
 #include "content/browser/in_process_webkit/webkit_thread.h"
 #include "content/browser/plugin_service.h"
-#include "content/browser/resource_context.h"
 #include "content/browser/renderer_host/async_resource_handler.h"
 #include "content/browser/renderer_host/buffered_resource_handler.h"
 #include "content/browser/renderer_host/cross_site_resource_handler.h"
@@ -48,6 +47,7 @@
 #include "content/browser/renderer_host/resource_queue.h"
 #include "content/browser/renderer_host/resource_request_details.h"
 #include "content/browser/renderer_host/sync_resource_handler.h"
+#include "content/browser/resource_context.h"
 #include "content/browser/ssl/ssl_client_auth_handler.h"
 #include "content/browser/ssl/ssl_manager.h"
 #include "content/browser/worker_host/worker_service.h"
@@ -809,12 +809,15 @@ void ResourceDispatcherHost::BeginDownload(
 
   request_id_--;
 
+  DownloadId dl_id = context.next_download_id_thunk().Run();
+
   scoped_refptr<ResourceHandler> handler(
       new DownloadResourceHandler(this,
                                   child_id,
                                   route_id,
                                   request_id_,
                                   url,
+                                  dl_id,
                                   download_file_manager_.get(),
                                   request,
                                   prompt_for_save_location,
@@ -836,7 +839,7 @@ void ResourceDispatcherHost::BeginDownload(
 
   request->set_method("GET");
   request->set_referrer(MaybeStripReferrer(referrer).spec());
-  request->set_context(context.request_context());
+  request->set_context(request_context);
   request->set_load_flags(request->load_flags() |
       net::LOAD_IS_DOWNLOAD);
 
diff --git a/content/browser/renderer_host/resource_dispatcher_host_unittest.cc b/content/browser/renderer_host/resource_dispatcher_host_unittest.cc
index 1d21abc..783a06c 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_unittest.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host_unittest.cc
@@ -6,11 +6,13 @@
 
 #include <vector>
 
+#include "base/bind.h"
 #include "base/file_path.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/child_process_security_policy.h"
+#include "content/browser/download/download_id.h"
 #include "content/browser/mock_resource_context.h"
 #include "content/browser/renderer_host/dummy_resource_handler.h"
 #include "content/browser/renderer_host/global_request_id.h"
@@ -1133,6 +1135,12 @@ TEST_F(ResourceDispatcherHostTest, ForbiddenDownload) {
   EXPECT_EQ(net::ERR_FILE_NOT_FOUND, status.os_error());
 }
 
+namespace {
+DownloadId MockNextDownloadId() {
+  return DownloadId(reinterpret_cast<DownloadManager*>(0xFFFFFFFF), 0);
+}
+}
+
 // Test for http://crbug.com/76202 .  We don't want to destroy a
 // download request prematurely when processing a cancellation from
 // the renderer.
@@ -1153,6 +1161,8 @@ TEST_F(ResourceDispatcherHostTest, IgnoreCancelForDownloads) {
   HandleScheme("http");
 
   MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+  content::MockResourceContext::GetInstance()->set_next_download_id_thunk(
+      base::Bind(&MockNextDownloadId));
   // Return some data so that the request is identified as a download
   // and the proper resource handlers are created.
   EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
@@ -1188,6 +1198,8 @@ TEST_F(ResourceDispatcherHostTest, CancelRequestsForContext) {
   HandleScheme("http");
 
   MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
+  content::MockResourceContext::GetInstance()->set_next_download_id_thunk(
+      base::Bind(&MockNextDownloadId));
   // Return some data so that the request is identified as a download
   // and the proper resource handlers are created.
   EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
diff --git a/content/browser/resource_context.cc b/content/browser/resource_context.cc
index efb73cd..d4b798f 100644
--- a/content/browser/resource_context.cc
+++ b/content/browser/resource_context.cc
@@ -145,6 +145,18 @@ void ResourceContext::set_media_observer(MediaObserver* media_observer) {
   media_observer_ = media_observer;
 }
 
+const DownloadManager::GetNextIdThunkType&
+ResourceContext::next_download_id_thunk() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  EnsureInitialized();
+  return next_download_id_thunk_;
+}
+void ResourceContext::set_next_download_id_thunk(
+    const DownloadManager::GetNextIdThunkType& thunk) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  next_download_id_thunk_ = thunk;
+}
+
 const base::Callback<prerender::PrerenderManager*(void)>&
 ResourceContext::prerender_manager_getter() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
diff --git a/content/browser/resource_context.h b/content/browser/resource_context.h
index f2ac4c5..475b0dc 100644
--- a/content/browser/resource_context.h
+++ b/content/browser/resource_context.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
+#include "content/browser/download/download_manager.h"
 
 class ChromeAppCacheService;
 class ChromeBlobStorageContext;
@@ -75,6 +76,11 @@ class ResourceContext {
   MediaObserver* media_observer() const;
   void set_media_observer(MediaObserver* media_observer);
 
+  // TODO(benjhayden): Promote GetNextIdThunkType to a separate object.
+  const DownloadManager::GetNextIdThunkType& next_download_id_thunk() const;
+  void set_next_download_id_thunk(
+      const DownloadManager::GetNextIdThunkType& thunk);
+
   // =======================================================================
   // TODO(willchan): These don't belong in content/. Remove them eventually.
 
@@ -100,6 +106,7 @@ class ResourceContext {
   quota::QuotaManager* quota_manager_;
   HostZoomMap* host_zoom_map_;
   MediaObserver* media_observer_;
+  DownloadManager::GetNextIdThunkType next_download_id_thunk_;
 
   // Externally-defined data accessible by key.
   typedef std::map<const void*, void*> UserDataMap;
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index 44e8364..8ad694c 100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -119,6 +119,8 @@
         'browser/download/download_file.h',
         'browser/download/download_file_manager.cc',
         'browser/download/download_file_manager.h',
+        'browser/download/download_id.cc',
+        'browser/download/download_id.h',
         'browser/download/download_item.cc',
         'browser/download/download_item.h',
         'browser/download/download_manager.cc',