Get rid of DownloadIdFactory and instead get the next id from DownloadManagerDelegate. I've also moved DownloadId to the public directory and into the content namespace.

BUG=98716
Review URL: https://chromiumcodereview.appspot.com/9169036

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

12 files changed, 35 insertions, 56 deletions

diff --git a/chrome/browser/DEPS b/chrome/browser/DEPS
index 24c0c38..782262b 100644
--- a/chrome/browser/DEPS
+++ b/chrome/browser/DEPS
@@ -44,8 +44,6 @@ include_rules = [
   "+content/browser/download/download_create_info.h",
   "+content/browser/download/download_file_impl.h",
   "+content/browser/download/download_file_manager.h",
-  "+content/browser/download/download_id.h",
-  "+content/browser/download/download_id_factory.h",
   "+content/browser/download/download_item_impl.h",
   "+content/browser/download/download_persistent_store_info.h",
   "+content/browser/download/download_request_handle.h",
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 4c3cd71..73856ba 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -43,6 +43,7 @@
 
 using content::BrowserThread;
 using content::DownloadFile;
+using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadManager;
 using content::WebContents;
@@ -66,6 +67,7 @@ struct SafeBrowsingState : public DownloadItem::ExternalData {
 
 ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
     : profile_(profile),
+      next_download_id_(0),
       download_prefs_(new DownloadPrefs(profile->GetPrefs())) {
 }
 
@@ -94,6 +96,14 @@ void ChromeDownloadManagerDelegate::Shutdown() {
   download_prefs_.reset();
 }
 
+DownloadId ChromeDownloadManagerDelegate::GetNextId() {
+  if (!profile_->IsOffTheRecord())
+    return DownloadId(this, next_download_id_++);
+
+  return profile_->GetOriginalProfile()->GetDownloadManager()->delegate()->
+      GetNextId();
+}
+
 bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
   // We create a download item and store it in our download map, and inform the
   // history system of a new download. Since this method can be called while the
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index b04282e..d0464763 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -54,6 +54,7 @@ class ChromeDownloadManagerDelegate
   static bool IsExtensionDownload(const content::DownloadItem* item);
 
   virtual void Shutdown() OVERRIDE;
+  virtual content::DownloadId GetNextId() OVERRIDE;
   virtual bool ShouldStartDownload(int32 download_id) OVERRIDE;
   virtual void ChooseDownloadPath(content::WebContents* web_contents,
                                   const FilePath& suggested_path,
@@ -151,6 +152,7 @@ class ChromeDownloadManagerDelegate
   void OnItemAddedToPersistentStore(int32 download_id, int64 db_handle);
 
   Profile* profile_;
+  int next_download_id_;
   scoped_ptr<DownloadPrefs> download_prefs_;
   scoped_ptr<DownloadHistory> download_history_;
 
diff --git a/chrome/browser/download/download_extension_api.cc b/chrome/browser/download/download_extension_api.cc
index ea638ae..eef4509 100644
--- a/chrome/browser/download/download_extension_api.cc
+++ b/chrome/browser/download/download_extension_api.cc
@@ -36,7 +36,6 @@
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/webui/web_ui_util.h"
-#include "content/browser/download/download_id.h"
 #include "content/browser/download/download_state_info.h"
 #include "content/browser/download/download_types.h"
 #include "content/browser/download/interrupt_reasons.h"
@@ -49,6 +48,7 @@
 #include "net/url_request/url_request.h"
 
 using content::BrowserThread;
+using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadManager;
 using content::DownloadQuery;
@@ -459,7 +459,7 @@ void DownloadsDownloadFunction::BeginDownloadOnIOThread() {
     request->AppendBytesToUpload(iodata_->post_body.data(),
                                  iodata_->post_body.size());
   }
-  iodata_->rdh->BeginDownload(
+  net::Error error = iodata_->rdh->BeginDownload(
       request.Pass(),
       save_info,
       iodata_->save_as,
@@ -468,21 +468,20 @@ void DownloadsDownloadFunction::BeginDownloadOnIOThread() {
       iodata_->render_view_host_routing_id,
       *(iodata_->resource_context));
   iodata_.reset();
-}
 
-void DownloadsDownloadFunction::OnStarted(DownloadId dl_id, net::Error error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  VLOG(1) << __FUNCTION__ << " " << dl_id << " " << error;
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
-      &DownloadsDownloadFunction::RespondOnUIThread, this,
-      dl_id.local(), error));
+  if (error != net::OK) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&DownloadsDownloadFunction::OnStarted, this,
+                   DownloadId::Invalid(), error));
+  }
 }
 
-void DownloadsDownloadFunction::RespondOnUIThread(int dl_id, net::Error error) {
+void DownloadsDownloadFunction::OnStarted(DownloadId dl_id, net::Error error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  VLOG(1) << __FUNCTION__;
-  if (dl_id >= 0) {
-    result_.reset(base::Value::CreateIntegerValue(dl_id));
+  VLOG(1) << __FUNCTION__ << " " << dl_id << " " << error;
+  if (dl_id.local() >= 0) {
+    result_.reset(base::Value::CreateIntegerValue(dl_id.local()));
   } else {
     error_ = net::ErrorToString(error);
   }
diff --git a/chrome/browser/download/download_extension_api.h b/chrome/browser/download/download_extension_api.h
index 7e6b8c8..c629be2 100644
--- a/chrome/browser/download/download_extension_api.h
+++ b/chrome/browser/download/download_extension_api.h
@@ -15,6 +15,7 @@
 #include "base/string16.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_function.h"
+#include "content/public/browser/download_id.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 
@@ -139,8 +140,7 @@ class DownloadsDownloadFunction : public AsyncDownloadsFunction {
     int render_view_host_routing_id;
   };
   void BeginDownloadOnIOThread();
-  void OnStarted(DownloadId dl_id, net::Error error);
-  void RespondOnUIThread(int dl_id, net::Error error);
+  void OnStarted(content::DownloadId dl_id, net::Error error);
 
   scoped_ptr<IOData> iodata_;
 
diff --git a/chrome/browser/download/download_item_unittest.cc b/chrome/browser/download/download_item_unittest.cc
index 41c6cf6..dcafc04 100644
--- a/chrome/browser/download/download_item_unittest.cc
+++ b/chrome/browser/download/download_item_unittest.cc
@@ -7,18 +7,18 @@
 #include "base/threading/thread.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/browser/download/download_create_info.h"
-#include "content/browser/download/download_id.h"
-#include "content/browser/download/download_id_factory.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_status_updater.h"
 #include "content/browser/download/interrupt_reasons.h"
 #include "content/browser/download/mock_download_item.h"
+#include "content/public/browser/download_id.h"
 #include "content/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
+using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadManager;
 
@@ -78,8 +78,7 @@ class DownloadItemTest : public testing::Test {
   };
 
   DownloadItemTest()
-      : id_factory_(new DownloadIdFactory(kValidDownloadItemIdDomain)),
-        ui_thread_(BrowserThread::UI, &loop_) {
+      : ui_thread_(BrowserThread::UI, &loop_) {
   }
 
   ~DownloadItemTest() {
@@ -104,7 +103,9 @@ class DownloadItemTest : public testing::Test {
     scoped_ptr<DownloadCreateInfo> info_;
 
     info_.reset(new DownloadCreateInfo());
-    info_->download_id = id_factory_->GetNextId();
+    static int next_id;
+    info_->download_id =
+        content::DownloadId(kValidDownloadItemIdDomain, ++next_id);
     info_->prompt_user_for_save_location = false;
     info_->url_chain.push_back(GURL());
     info_->state = state;
@@ -128,7 +129,6 @@ class DownloadItemTest : public testing::Test {
   DownloadStatusUpdater download_status_updater_;
 
  private:
-  scoped_refptr<DownloadIdFactory> id_factory_;
   MessageLoopForUI loop_;
   // UI thread.
   content::TestBrowserThread ui_thread_;
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index 2b303e8..59723fe 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -28,7 +28,6 @@
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file_impl.h"
 #include "content/browser/download/download_file_manager.h"
-#include "content/browser/download/download_id_factory.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_status_updater.h"
 #include "content/browser/download/interrupt_reasons.h"
@@ -66,6 +65,7 @@
 
 using content::BrowserThread;
 using content::DownloadFile;
+using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadManager;
 using ::testing::ReturnRef;
@@ -176,11 +176,8 @@ class DownloadManagerTest : public testing::Test {
       : profile_(new TestingProfile()),
         download_manager_delegate_(new TestDownloadManagerDelegate(
             profile_.get())),
-        id_factory_(new DownloadIdFactory(kValidIdDomain)),
         download_manager_(DownloadManager::Create(
-            download_manager_delegate_,
-            id_factory_,
-            &download_status_updater_)),
+            download_manager_delegate_, &download_status_updater_)),
         ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE, &message_loop_),
         download_buffer_(new content::DownloadBuffer) {
@@ -255,7 +252,6 @@ class DownloadManagerTest : public testing::Test {
   DownloadStatusUpdater download_status_updater_;
   scoped_ptr<TestingProfile> profile_;
   scoped_refptr<TestDownloadManagerDelegate> download_manager_delegate_;
-  scoped_refptr<DownloadIdFactory> id_factory_;
   scoped_refptr<DownloadManager> download_manager_;
   scoped_refptr<DownloadFileManager> file_manager_;
   MessageLoopForUI message_loop_;
diff --git a/chrome/browser/download/download_service.cc b/chrome/browser/download/download_service.cc
index 98031ec..65e1124 100644
--- a/chrome/browser/download/download_service.cc
+++ b/chrome/browser/download/download_service.cc
@@ -10,7 +10,6 @@
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "content/browser/download/download_id_factory.h"
 #include "content/public/browser/download_manager.h"
 
 using content::DownloadManager;
@@ -18,20 +17,10 @@ using content::DownloadManager;
 DownloadService::DownloadService(Profile* profile)
     : download_manager_created_(false),
       profile_(profile) {
-  if (profile_->IsOffTheRecord()) {
-    id_factory_ = DownloadServiceFactory::GetForProfile(
-        profile_->GetOriginalProfile())->GetDownloadIdFactory();
-  } else {
-    id_factory_ = new DownloadIdFactory(this);
-  }
 }
 
 DownloadService::~DownloadService() {}
 
-DownloadIdFactory* DownloadService::GetDownloadIdFactory() const {
-  return id_factory_.get();
-}
-
 void DownloadService::OnManagerCreated(
     const DownloadService::OnManagerCreatedCallback& cb) {
   if (download_manager_created_) {
@@ -48,9 +37,7 @@ DownloadManager* DownloadService::GetDownloadManager() {
     if (!manager_delegate_.get())
       manager_delegate_ = new ChromeDownloadManagerDelegate(profile_);
     manager_ = DownloadManager::Create(
-        manager_delegate_.get(),
-        id_factory_.get(),
-        g_browser_process->download_status_updater());
+        manager_delegate_.get(), g_browser_process->download_status_updater());
     manager_->Init(profile_);
     manager_delegate_->SetDownloadManager(manager_);
     download_manager_created_ = true;
diff --git a/chrome/browser/download/download_service.h b/chrome/browser/download/download_service.h
index fb9f24a7..b513199 100644
--- a/chrome/browser/download/download_service.h
+++ b/chrome/browser/download/download_service.h
@@ -16,7 +16,6 @@
 
 class ChromeDownloadManagerDelegate;
 class Profile;
-class DownloadIdFactory;
 
 namespace content {
 class DownloadManager;
@@ -29,8 +28,6 @@ class DownloadService : public ProfileKeyedService {
   explicit DownloadService(Profile* profile);
   virtual ~DownloadService();
 
-  DownloadIdFactory* GetDownloadIdFactory() const;
-
   // Register a callback to be called whenever the DownloadManager is created.
   typedef base::Callback<void(content::DownloadManager*)>
       OnManagerCreatedCallback;
@@ -60,8 +57,6 @@ class DownloadService : public ProfileKeyedService {
   virtual void Shutdown() OVERRIDE;
 
  private:
-  scoped_refptr<DownloadIdFactory> id_factory_;
-
   bool download_manager_created_;
   Profile* profile_;
 
diff --git a/chrome/browser/download/download_throttling_resource_handler.cc b/chrome/browser/download/download_throttling_resource_handler.cc
index 3d98eee..85bc6e1 100644
--- a/chrome/browser/download/download_throttling_resource_handler.cc
+++ b/chrome/browser/download/download_throttling_resource_handler.cc
@@ -7,7 +7,6 @@
 #include "base/logging.h"
 #include "chrome/browser/download/download_request_limiter.h"
 #include "chrome/browser/download/download_util.h"
-#include "content/browser/download/download_id.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
 #include "content/public/common/resource_response.h"
 #include "net/base/io_buffer.h"
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index 514f42d..f58e43a 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -41,7 +41,6 @@
 #include "chrome/common/url_constants.h"
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/chrome_blob_storage_context.h"
-#include "content/browser/download/download_id_factory.h"
 #include "content/browser/host_zoom_map.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
@@ -188,9 +187,6 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   PrefService* pref_service = profile->GetPrefs();
 
-  download_id_factory_ = DownloadServiceFactory::GetForProfile(profile)->
-    GetDownloadIdFactory();
-
   scoped_ptr<ProfileParams> params(new ProfileParams);
   params->path = profile->GetPath();
   params->is_incognito = profile->IsOffTheRecord();
@@ -515,7 +511,6 @@ 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_download_id_factory(download_id_factory_);
   resource_context_.set_media_stream_manager(media_stream_manager_.get());
   resource_context_.set_audio_manager(profile_params_->audio_manager);
 
diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h
index 1eac7a7..349d747 100644
--- a/chrome/browser/profiles/profile_io_data.h
+++ b/chrome/browser/profiles/profile_io_data.h
@@ -25,7 +25,6 @@ class ChromeAppCacheService;
 class ChromeBlobStorageContext;
 class CookieSettings;
 class DesktopNotificationService;
-class DownloadIdFactory;
 class ExtensionInfoMap;
 class HostContentSettingsMap;
 class HostZoomMap;
@@ -290,7 +289,6 @@ 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 scoped_refptr<DownloadIdFactory> download_id_factory_;
   mutable scoped_ptr<media_stream::MediaStreamManager> media_stream_manager_;
 
   // TODO(willchan): Remove from ResourceContext.