Support NULL DownloadManagerDelegates.

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

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

10 files changed, 136 insertions, 112 deletions

diff --git a/content/browser/download/download_file_manager.cc b/content/browser/download/download_file_manager.cc
index f74cb28..f313a98 100644
--- a/content/browser/download/download_file_manager.cc
+++ b/content/browser/download/download_file_manager.cc
@@ -152,7 +152,7 @@ void DownloadFileManager::UpdateInProgressDownloads() {
   }
 }
 
-void DownloadFileManager::StartDownload(
+DownloadId DownloadFileManager::StartDownload(
     scoped_ptr<DownloadCreateInfo> info,
     scoped_ptr<content::ByteStreamReader> stream,
     const DownloadRequestHandle& request_handle) {
@@ -166,6 +166,7 @@ void DownloadFileManager::StartDownload(
   // the download file's events.
   net::BoundNetLog bound_net_log =
       manager->CreateDownloadItem(info.get(), request_handle);
+  DownloadId download_id = info->download_id;
   bool hash_needed = manager->GenerateFileHash();
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
@@ -174,6 +175,7 @@ void DownloadFileManager::StartDownload(
                  request_handle,
                  make_scoped_refptr(manager),
                  hash_needed, bound_net_log));
+  return download_id;
 }
 
 // This method will be sent via a user action, or shutdown on the UI thread, and
diff --git a/content/browser/download/download_file_manager.h b/content/browser/download/download_file_manager.h
index f7737e9..a769946 100644
--- a/content/browser/download/download_file_manager.h
+++ b/content/browser/download/download_file_manager.h
@@ -63,6 +63,7 @@ class FilePath;
 namespace content {
 class ByteStreamReader;
 class DownloadFile;
+class DownloadId;
 class DownloadManager;
 }
 
@@ -101,10 +102,12 @@ class CONTENT_EXPORT DownloadFileManager
   // Called on shutdown on the UI thread.
   virtual void Shutdown();
 
-  // Called on UI thread to make DownloadFileManager start the download.
-  virtual void StartDownload(scoped_ptr<DownloadCreateInfo> info,
-                             scoped_ptr<content::ByteStreamReader> stream,
-                             const DownloadRequestHandle& request_handle);
+  // Called on UI thread to make DownloadFileManager start the download. Creates
+  // and returns the DownloadId of the download.
+  virtual content::DownloadId StartDownload(
+      scoped_ptr<DownloadCreateInfo> info,
+      scoped_ptr<content::ByteStreamReader> stream,
+      const DownloadRequestHandle& request_handle);
 
   // Handlers for notifications sent from the UI thread and run on the
   // FILE thread.  These are both terminal actions with respect to the
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index abe5c57..ed97630 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -87,10 +87,11 @@ class MockDownloadFileManager : public DownloadFileManager {
   MOCK_METHOD3(MockStartDownload,
                void(DownloadCreateInfo*, content::ByteStreamReader*,
                     const DownloadRequestHandle&));
-  virtual void StartDownload(scoped_ptr<DownloadCreateInfo> info,
+  virtual DownloadId StartDownload(scoped_ptr<DownloadCreateInfo> info,
                             scoped_ptr<content::ByteStreamReader> stream,
                             const DownloadRequestHandle& request_handle) {
     MockStartDownload(info.release(), stream.release(), request_handle);
+    return DownloadId();
   }
 
   MOCK_METHOD1(CancelDownload, void(DownloadId));
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index dd9d343b..6b1875d 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -165,14 +165,28 @@ DownloadManagerImpl::~DownloadManagerImpl() {
 }
 
 DownloadId DownloadManagerImpl::GetNextId() {
-  return delegate_->GetNextId();
+  DownloadId id;
+  if (delegate_)
+   id = delegate_->GetNextId();
+  if (!id.IsValid()) {
+    static int next_id;
+    id = DownloadId(browser_context_, ++next_id);
+  }
+
+  return id;
 }
 
 bool DownloadManagerImpl::ShouldOpenDownload(DownloadItem* item) {
+  if (!delegate_)
+    return true;
+
   return delegate_->ShouldOpenDownload(item);
 }
 
 bool DownloadManagerImpl::ShouldOpenFileBasedOnExtension(const FilePath& path) {
+  if (!delegate_)
+    return false;
+
   return delegate_->ShouldOpenFileBasedOnExtension(path);
 }
 
@@ -226,7 +240,8 @@ void DownloadManagerImpl::Shutdown() {
       download->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
     } else if (download->IsPartialDownload()) {
       download->Cancel(false);
-      delegate_->UpdateItemInPersistentStore(download);
+      if (delegate_)
+        delegate_->UpdateItemInPersistentStore(download);
     }
   }
 
@@ -325,7 +340,7 @@ bool DownloadManagerImpl::Init(content::BrowserContext* browser_context) {
 void DownloadManagerImpl::StartDownload(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (delegate_->ShouldStartDownload(download_id))
+  if (!delegate_ || delegate_->ShouldStartDownload(download_id))
     RestartDownload(download_id);
 }
 
@@ -385,10 +400,14 @@ void DownloadManagerImpl::RestartDownload(int32 download_id) {
     // We must ask the user for the place to put the download.
     WebContents* contents = download->GetWebContents();
 
-    delegate_->ChooseDownloadPath(contents, download->GetTargetFilePath(),
-                                  download_id);
-    FOR_EACH_OBSERVER(Observer, observers_,
-                      SelectFileDialogDisplayed(this, download_id));
+    if (delegate_) {
+      delegate_->ChooseDownloadPath(contents, download->GetTargetFilePath(),
+                                    download_id);
+      FOR_EACH_OBSERVER(Observer, observers_,
+                        SelectFileDialogDisplayed(this, download_id));
+    } else {
+      FileSelectionCanceled(download_id);
+    }
   } else {
     // No prompting for download, just continue with the current target path.
     OnTargetPathAvailable(download);
@@ -409,6 +428,7 @@ net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
 
   net::BoundNetLog bound_net_log =
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
+  info->download_id = GetNextId();
   DownloadItem* download = new DownloadItemImpl(
       this, *info, new DownloadRequestHandle(request_handle),
       browser_context_->IsOffTheRecord(), bound_net_log);
@@ -445,7 +465,8 @@ DownloadItem* DownloadManagerImpl::CreateSavePackageDownloadItem(
   save_page_downloads_[download->GetId()] = download;
 
   // Will notify the observer in the callback.
-  delegate_->AddItemToPersistentStore(download);
+  if (delegate_)
+    delegate_->AddItemToPersistentStore(download);
 
   return download;
 }
@@ -470,8 +491,13 @@ void DownloadManagerImpl::OnTargetPathAvailable(DownloadItem* download) {
   //               filename. Unnecessary renames may cause bugs like
   //               http://crbug.com/74187.
   bool ok_to_overwrite = true;
-  FilePath intermediate_path =
-      delegate_->GetIntermediatePath(*download, &ok_to_overwrite);
+  FilePath intermediate_path;
+  if (delegate_) {
+    intermediate_path =
+        delegate_->GetIntermediatePath(*download, &ok_to_overwrite);
+  } else {
+    intermediate_path = download->GetTargetFilePath();
+  }
   // We want the intermediate and target paths to refer to the same directory so
   // that they are both on the same device and subject to same
   // space/permission/availability constraints.
@@ -491,7 +517,8 @@ void DownloadManagerImpl::UpdateDownload(int32 download_id,
     DownloadItem* download = it->second;
     if (download->IsInProgress()) {
       download->UpdateProgress(bytes_so_far, bytes_per_sec, hash_state);
-      delegate_->UpdateItemInPersistentStore(download);
+      if (delegate_)
+        delegate_->UpdateItemInPersistentStore(download);
     }
   }
 }
@@ -607,7 +634,7 @@ void DownloadManagerImpl::MaybeCompleteDownload(DownloadItem* download) {
   // trying to set it to a different cb.  TODO(benjhayden): Change the callback
   // to point directly to the item instead of |this| when DownloadItem supports
   // weak-ptrs.
-  if (!delegate_->ShouldCompleteDownload(download, base::Bind(
+  if (delegate_ && !delegate_->ShouldCompleteDownload(download, base::Bind(
           &DownloadManagerImpl::MaybeCompleteDownloadById,
           this, download->GetId())))
     return;
@@ -615,7 +642,8 @@ void DownloadManagerImpl::MaybeCompleteDownload(DownloadItem* download) {
   VLOG(20) << __FUNCTION__ << "()" << " executing: download = "
            << download->DebugString(false);
 
-  delegate_->UpdateItemInPersistentStore(download);
+  if (delegate_)
+    delegate_->UpdateItemInPersistentStore(download);
   download->OnDownloadCompleting(file_manager_);
 }
 
@@ -628,7 +656,8 @@ void DownloadManagerImpl::MaybeCompleteDownloadById(int download_id) {
 void DownloadManagerImpl::DownloadCompleted(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
-  delegate_->UpdateItemInPersistentStore(download);
+  if (delegate_)
+    delegate_->UpdateItemInPersistentStore(download);
   active_downloads_.erase(download->GetId());
   AssertStateConsistent(download);
 }
@@ -702,12 +731,13 @@ void DownloadManagerImpl::RemoveFromActiveList(DownloadItem* download) {
   // don't have a valid db_handle yet.
   if (download->IsPersisted()) {
     active_downloads_.erase(download->GetId());
-    delegate_->UpdateItemInPersistentStore(download);
+    if (delegate_)
+      delegate_->UpdateItemInPersistentStore(download);
   }
 }
 
 bool DownloadManagerImpl::GenerateFileHash() {
-  return delegate_->GenerateFileHash();
+  return delegate_ && delegate_->GenerateFileHash();
 }
 
 int DownloadManagerImpl::RemoveDownloadItems(
@@ -741,7 +771,8 @@ void DownloadManagerImpl::DownloadRemoved(DownloadItem* download) {
     return;
 
   // Make history update.
-  delegate_->RemoveItemFromPersistentStore(download);
+  if (delegate_)
+    delegate_->RemoveItemFromPersistentStore(download);
 
   // Remove from our tables and delete.
   int downloads_count = RemoveDownloadItems(DownloadVector(1, download));
@@ -750,7 +781,8 @@ void DownloadManagerImpl::DownloadRemoved(DownloadItem* download) {
 
 int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
                                                 base::Time remove_end) {
-  delegate_->RemoveItemsFromPersistentStoreBetween(remove_begin, remove_end);
+  if (delegate_)
+    delegate_->RemoveItemsFromPersistentStoreBetween(remove_begin, remove_end);
 
   // All downloads visible to the user will be in the history,
   // so scan that map.
@@ -930,7 +962,8 @@ void DownloadManagerImpl::OnDownloadItemAddedToPersistentStore(
   } else {
     DCHECK(download->IsCancelled());
     active_downloads_.erase(download_id);
-    delegate_->UpdateItemInPersistentStore(download);
+    if (delegate_)
+      delegate_->UpdateItemInPersistentStore(download);
     download->UpdateObservers();
   }
 }
@@ -942,7 +975,7 @@ void DownloadManagerImpl::ShowDownloadInBrowser(DownloadItem* download) {
 
   // If the contents no longer exists, we ask the embedder to suggest another
   // contents.
-  if (!content)
+  if (!content && delegate_)
     content = delegate_->GetAlternativeWebContentsToNotifyForDownload();
 
   if (content && content->GetDelegate())
@@ -1076,7 +1109,8 @@ void DownloadManagerImpl::OnSavePageItemAddedToPersistentStore(
 
 void DownloadManagerImpl::SavePageDownloadFinished(DownloadItem* download) {
   if (download->IsPersisted()) {
-    delegate_->UpdateItemInPersistentStore(download);
+    if (delegate_)
+      delegate_->UpdateItemInPersistentStore(download);
     DCHECK(ContainsKey(save_page_downloads_, download->GetId()));
     save_page_downloads_.erase(download->GetId());
 
@@ -1089,7 +1123,8 @@ void DownloadManagerImpl::SavePageDownloadFinished(DownloadItem* download) {
 }
 
 void DownloadManagerImpl::DownloadOpened(DownloadItem* download) {
-  delegate_->UpdateItemInPersistentStore(download);
+  if (delegate_)
+    delegate_->UpdateItemInPersistentStore(download);
   int num_unopened = 0;
   for (DownloadMap::iterator it = history_downloads_.begin();
        it != history_downloads_.end(); ++it) {
@@ -1104,7 +1139,8 @@ void DownloadManagerImpl::DownloadRenamedToIntermediateName(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToIntermediateName() call.
-  delegate_->AddItemToPersistentStore(download);
+  if (delegate_)
+    delegate_->AddItemToPersistentStore(download);
 }
 
 void DownloadManagerImpl::DownloadRenamedToFinalName(
@@ -1112,8 +1148,10 @@ void DownloadManagerImpl::DownloadRenamedToFinalName(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToFinalName() call.
-  delegate_->UpdatePathForItemInPersistentStore(download,
-                                                download->GetFullPath());
+  if (delegate_) {
+    delegate_->UpdatePathForItemInPersistentStore(
+        download, download->GetFullPath());
+  }
 }
 
 void DownloadManagerImpl::SetFileManagerForTesting(
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index 325f914..fe6f4a2 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -100,8 +100,6 @@ DownloadFile* MockDownloadFileFactory::CreateFile(
   return NULL;
 }
 
-DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";
-
 class TestDownloadManagerDelegate : public content::DownloadManagerDelegate {
  public:
   explicit TestDownloadManagerDelegate(content::DownloadManager* dm)
@@ -286,24 +284,21 @@ class DownloadManagerTest : public testing::Test {
     return true;
   }
 
-  void AddDownloadToFileManager(int id, DownloadFile* download_file) {
-    file_manager()->downloads_[DownloadId(kValidIdDomain, id)] =
+  void AddDownloadToFileManager(DownloadId id, DownloadFile* download_file) {
+    file_manager()->downloads_[id] =
       download_file;
   }
 
-  void AddMockDownloadToFileManager(int id, MockDownloadFile* download_file) {
+  void AddMockDownloadToFileManager(DownloadId id,
+                                    MockDownloadFile* download_file) {
     AddDownloadToFileManager(id, download_file);
     EXPECT_CALL(*download_file, GetDownloadManager())
         .WillRepeatedly(Return(download_manager_));
   }
 
-  void OnResponseCompleted(int32 download_id, int64 size,
+  void OnResponseCompleted(DownloadId download_id, int64 size,
                            const std::string& hash) {
-    download_manager_->OnResponseCompleted(download_id, size, hash);
-  }
-
-  void FileSelected(const FilePath& path, int32 download_id) {
-    download_manager_->FileSelected(path, download_id);
+    download_manager_->OnResponseCompleted(download_id.local(), size, hash);
   }
 
   void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) {
@@ -313,16 +308,16 @@ class DownloadManagerTest : public testing::Test {
     download_manager_->OnTargetPathAvailable(download);
   }
 
-  void OnDownloadInterrupted(int32 download_id, int64 size,
+  void OnDownloadInterrupted(DownloadId download_id, int64 size,
                              const std::string& hash_state,
                              content::DownloadInterruptReason reason) {
-    download_manager_->OnDownloadInterrupted(download_id, size,
+    download_manager_->OnDownloadInterrupted(download_id.local(), size,
                                              hash_state, reason);
   }
 
   // Get the download item with ID |id|.
-  DownloadItem* GetActiveDownloadItem(int32 id) {
-    return download_manager_->GetActiveDownload(id);
+  DownloadItem* GetActiveDownloadItem(DownloadId id) {
+    return download_manager_->GetActiveDownload(id.local());
   }
 
   FilePath GetPathInDownloadsDir(const FilePath::StringType& fragment) {
@@ -554,18 +549,16 @@ TEST_F(DownloadManagerTest, MAYBE_StartDownload) {
     // responsible for deleting it.  In these unit tests, however, we
     // don't call the function that deletes it, so we do so ourselves.
     scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-    const DownloadId id = DownloadId(kValidIdDomain, static_cast<int>(i));
-    info->download_id = id;
     info->prompt_user_for_save_location = kStartDownloadCases[i].save_as;
     info->url_chain.push_back(GURL(kStartDownloadCases[i].url));
     info->mime_type = kStartDownloadCases[i].mime_type;
     download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
-  scoped_ptr<content::ByteStreamWriter> stream_writer;
-  scoped_ptr<content::ByteStreamReader> stream_reader;
-  content::CreateByteStream(message_loop_.message_loop_proxy(),
-                            message_loop_.message_loop_proxy(),
-                            kTestDataLen, &stream_writer, &stream_reader);
+    scoped_ptr<content::ByteStreamWriter> stream_writer;
+    scoped_ptr<content::ByteStreamReader> stream_reader;
+    content::CreateByteStream(message_loop_.message_loop_proxy(),
+                              message_loop_.message_loop_proxy(),
+                              kTestDataLen, &stream_writer, &stream_reader);
 
     DownloadFile* download_file(
         new DownloadFileImpl(info.get(),
@@ -574,7 +567,7 @@ TEST_F(DownloadManagerTest, MAYBE_StartDownload) {
                              download_manager_, false,
                              scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
                              net::BoundNetLog()));
-    AddDownloadToFileManager(info->download_id.local(), download_file);
+    AddDownloadToFileManager(info->download_id, download_file);
     download_file->Initialize();
     download_manager_->StartDownload(info->download_id.local());
     message_loop_.RunAllPending();
@@ -583,9 +576,9 @@ TEST_F(DownloadManagerTest, MAYBE_StartDownload) {
     // select file dialog.
     // Note that DownloadManager::FileSelectionCanceled() is never called.
     EXPECT_EQ(kStartDownloadCases[i].expected_save_as,
-              observer.ShowedFileDialogForId(i));
+              observer.ShowedFileDialogForId(info->download_id.local()));
 
-    file_manager()->CancelDownload(id);
+    file_manager()->CancelDownload(info->download_id);
   }
 }
 
@@ -817,7 +810,6 @@ TEST_F(DownloadManagerTest, DownloadFilenameTest) {
     scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
 
     SCOPED_TRACE(testing::Message() << "Running test case " << i);
-    info->download_id = DownloadId(kValidIdDomain, i);
     info->url_chain.push_back(GURL());
 
     MockDownloadFile* download_file(
@@ -833,7 +825,6 @@ TEST_F(DownloadManagerTest, DownloadFilenameTest) {
     FilePath expected_final_path =
         GetPathInDownloadsDir(test_case.expected_final_path);
 
-    AddMockDownloadToFileManager(info->download_id.local(), download_file);
     EXPECT_CALL(*download_file, Rename(expected_intermediate_path))
         .WillOnce(Return(net::OK));
     EXPECT_CALL(*download_file, Rename(expected_final_path))
@@ -844,11 +835,12 @@ TEST_F(DownloadManagerTest, DownloadFilenameTest) {
     EXPECT_CALL(*download_file, Detach());
 
     download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-    DownloadItem* download = GetActiveDownloadItem(i);
+    AddMockDownloadToFileManager(info->download_id, download_file);
+    DownloadItem* download = GetActiveDownloadItem(info->download_id);
     ASSERT_TRUE(download != NULL);
 
     if (test_case.completion == COMPLETES_BEFORE_RENAME)
-      OnResponseCompleted(i, 1024, std::string("fake_hash"));
+      OnResponseCompleted(info->download_id, 1024, std::string("fake_hash"));
 
     download->OnTargetPathDetermined(
         target_path, test_case.target_disposition,
@@ -858,11 +850,11 @@ TEST_F(DownloadManagerTest, DownloadFilenameTest) {
     download_manager_delegate_->SetIntermediatePath(
         intermediate_path, test_case.overwrite_intermediate_path);
     download_manager_delegate_->SetShouldCompleteDownload(false);
-    download_manager_->RestartDownload(i);
+    download_manager_->RestartDownload(info->download_id.local());
     message_loop_.RunAllPending();
 
     if (test_case.completion == COMPLETES_AFTER_RENAME) {
-      OnResponseCompleted(i, 1024, std::string("fake_hash"));
+      OnResponseCompleted(info->download_id, 1024, std::string("fake_hash"));
       message_loop_.RunAllPending();
     }
 
@@ -893,7 +885,6 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) {
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-  info->download_id = DownloadId(kValidIdDomain, 0);
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
   info->total_bytes = static_cast<int64>(kTestDataLen);
@@ -902,30 +893,29 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) {
 
   MockDownloadFile* download_file(new NiceMock<MockDownloadFile>());
 
-  // |download_file| is owned by DownloadFileManager.
-  AddMockDownloadToFileManager(info->download_id.local(), download_file);
-
   EXPECT_CALL(*download_file, Rename(cr_path))
       .WillOnce(Return(net::OK));
   EXPECT_CALL(*download_file, Cancel());
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
+  // |download_file| is owned by DownloadFileManager.
+  AddMockDownloadToFileManager(info->download_id, download_file);
 
-  DownloadItem* download = GetActiveDownloadItem(0);
+  DownloadItem* download = GetActiveDownloadItem(info->download_id);
   ASSERT_TRUE(download != NULL);
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
   ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
 
   int64 error_size = 3;
-  OnDownloadInterrupted(0, error_size, "",
+  OnDownloadInterrupted(info->download_id, error_size, "",
                         content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
   message_loop_.RunAllPending();
 
-  EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::COMPLETE));
@@ -974,8 +964,6 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-  static const int32 local_id = 0;
-  info->download_id = DownloadId(kValidIdDomain, local_id);
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
   info->total_bytes = static_cast<int64>(kTestDataLen * 3);
@@ -987,14 +975,14 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {
                             message_loop_.message_loop_proxy(),
                             kTestDataLen, &stream_input, &stream_output);
 
+  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
+
   // Create a download file that we can insert errors into.
   scoped_ptr<DownloadFileWithErrors> download_file(new DownloadFileWithErrors(
       info.get(), stream_output.Pass(), download_manager_, false));
   download_file->Initialize();
 
-  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-
-  DownloadItem* download = GetActiveDownloadItem(0);
+  DownloadItem* download = GetActiveDownloadItem(info->download_id);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
@@ -1006,7 +994,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {
   // Finalize the file name.
   ContinueDownloadWithPath(download, path);
   message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
 
   // Add more data.
   WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
@@ -1017,7 +1005,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {
   message_loop_.RunAllPending();
 
   // Check the state.  The download should have been interrupted.
-  EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::COMPLETE));
@@ -1043,15 +1031,12 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-  DownloadId id = DownloadId(kValidIdDomain, 0);
-  info->download_id = id;
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
   const FilePath new_path(FILE_PATH_LITERAL("foo.zip"));
   const FilePath cr_path(GetTempDownloadPath(new_path));
 
   MockDownloadFile* download_file(new NiceMock<MockDownloadFile>());
-  AddMockDownloadToFileManager(info->download_id.local(), download_file);
 
   // |download_file| is owned by DownloadFileManager.
   EXPECT_CALL(*download_file, Rename(cr_path))
@@ -1059,8 +1044,9 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
   EXPECT_CALL(*download_file, Cancel());
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
+  AddMockDownloadToFileManager(info->download_id, download_file);
 
-  DownloadItem* download = GetActiveDownloadItem(0);
+  DownloadItem* download = GetActiveDownloadItem(info->download_id);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
@@ -1068,12 +1054,12 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
 
   ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
 
   download->Cancel(false);
   message_loop_.RunAllPending();
 
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ -1084,7 +1070,7 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
   EXPECT_FALSE(download->GetFileExternallyRemoved());
   EXPECT_EQ(DownloadItem::CANCELLED, download->GetState());
 
-  file_manager()->CancelDownload(id);
+  file_manager()->CancelDownload(info->download_id);
 
   EXPECT_FALSE(file_util::PathExists(new_path));
   EXPECT_FALSE(file_util::PathExists(cr_path));
@@ -1121,7 +1107,6 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-  info->download_id = DownloadId(kValidIdDomain, 0);
   info->prompt_user_for_save_location = true;
   info->url_chain.push_back(GURL());
   scoped_ptr<content::ByteStreamWriter> stream_input;
@@ -1132,7 +1117,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
-  DownloadItem* download = GetActiveDownloadItem(0);
+  DownloadItem* download = GetActiveDownloadItem(info->download_id);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
@@ -1152,20 +1137,20 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {
   // This creates the .temp version of the file.
   download_file->Initialize();
   // |download_file| is owned by DownloadFileManager.
-  AddDownloadToFileManager(info->download_id.local(), download_file);
+  AddDownloadToFileManager(info->download_id, download_file);
 
   ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
 
   WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
 
   // Finish the download.
-  OnResponseCompleted(0, kTestDataLen, "");
+  OnResponseCompleted(info->download_id, kTestDataLen, "");
   message_loop_.RunAllPending();
 
   // Download is complete.
-  EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ -1201,7 +1186,6 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-  info->download_id = DownloadId(kValidIdDomain, 0);
   info->prompt_user_for_save_location = true;
   info->url_chain.push_back(GURL());
   scoped_ptr<content::ByteStreamWriter> stream_input;
@@ -1212,7 +1196,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
-  DownloadItem* download = GetActiveDownloadItem(0);
+  DownloadItem* download = GetActiveDownloadItem(info->download_id);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
@@ -1232,20 +1216,20 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {
   // This creates the .temp version of the file.
   download_file->Initialize();
   // |download_file| is owned by DownloadFileManager.
-  AddDownloadToFileManager(info->download_id.local(), download_file);
+  AddDownloadToFileManager(info->download_id, download_file);
 
   ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
 
   WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
 
   // Finish the download.
-  OnResponseCompleted(0, kTestDataLen, "");
+  OnResponseCompleted(info->download_id, kTestDataLen, "");
   message_loop_.RunAllPending();
 
   // Download is complete.
-  EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ -1264,7 +1248,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {
   download->OnDownloadedFileRemoved();
   message_loop_.RunAllPending();
 
-  EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc
index 094cf1b..e0feaf5 100644
--- a/content/browser/download/download_resource_handler.cc
+++ b/content/browser/download/download_resource_handler.cc
@@ -71,10 +71,9 @@ static void StartOnUIThread(
       started_cb.Run(DownloadId(), net::ERR_ACCESS_DENIED);
     return;
   }
-  DownloadId download_id = download_manager->GetDelegate()->GetNextId();
-  info->download_id = download_id;
 
-  download_file_manager->StartDownload(info.Pass(), stream.Pass(), handle);
+  DownloadId download_id =
+      download_file_manager->StartDownload(info.Pass(), stream.Pass(), handle);
 
   if (!started_cb.is_null())
     started_cb.Run(download_id, net::OK);
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index ed1ee19..383342f 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -339,9 +339,11 @@ void SavePackage::OnMHTMLGenerated(const FilePath& path, int64 size) {
   // GDataDownloadObserver::ShouldUpload() to return true.
   // ShouldCompleteDownload() may depend on the gdata uploader to finish.
   download_->OnAllDataSaved(size, DownloadItem::kEmptyFileHash);
-  if (download_manager_->GetDelegate()->ShouldCompleteDownload(
-        download_, base::Bind(&SavePackage::Finish, this)))
+  if (!download_manager_->GetDelegate() ||
+      download_manager_->GetDelegate()->ShouldCompleteDownload(
+          download_, base::Bind(&SavePackage::Finish, this))) {
     Finish();
+  }
 }
 
 // On POSIX, the length of |pure_file_name| + |file_name_ext| is further
@@ -1238,8 +1240,10 @@ void SavePackage::GetSaveInfo() {
   // before calling to it.
   FilePath website_save_dir, download_save_dir;
   DCHECK(download_manager_);
-  download_manager_->GetDelegate()->GetSaveDir(
-      web_contents(), &website_save_dir, &download_save_dir);
+  if (download_manager_->GetDelegate()) {
+    download_manager_->GetDelegate()->GetSaveDir(
+        web_contents(), &website_save_dir, &download_save_dir);
+  }
   std::string mime_type = web_contents()->GetContentsMimeType();
   std::string accept_languages =
       content::GetContentClient()->browser()->GetAcceptLangs(
@@ -1301,7 +1305,7 @@ void SavePackage::ContinueGetSaveInfo(const FilePath& suggested_path,
   // The WebContents which owns this SavePackage may have disappeared during
   // the UI->FILE->UI thread hop of
   // GetSaveInfo->CreateDirectoryOnFileThread->ContinueGetSaveInfo.
-  if (!web_contents())
+  if (!web_contents() || !download_manager_->GetDelegate())
     return;
 
   FilePath::StringType default_extension;
diff --git a/content/public/browser/download_manager_delegate.cc b/content/public/browser/download_manager_delegate.cc
index 08b34d2..fb911b0 100644
--- a/content/public/browser/download_manager_delegate.cc
+++ b/content/public/browser/download_manager_delegate.cc
@@ -23,7 +23,6 @@ bool DownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
 FilePath DownloadManagerDelegate::GetIntermediatePath(
     const DownloadItem& item,
     bool* ok_to_overwrite) {
-  *ok_to_overwrite = true;
   return item.GetTargetFilePath();
 }
 
diff --git a/content/shell/shell_download_manager_delegate.cc b/content/shell/shell_download_manager_delegate.cc
index 9a818f42..3c8309a 100644
--- a/content/shell/shell_download_manager_delegate.cc
+++ b/content/shell/shell_download_manager_delegate.cc
@@ -35,11 +35,6 @@ void ShellDownloadManagerDelegate::SetDownloadManager(
   download_manager_ = download_manager;
 }
 
-DownloadId ShellDownloadManagerDelegate::GetNextId() {
-  static int next_id;
-  return DownloadId(this, ++next_id);
-}
-
 bool ShellDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
diff --git a/content/shell/shell_download_manager_delegate.h b/content/shell/shell_download_manager_delegate.h
index a973d26..b9387c8 100644
--- a/content/shell/shell_download_manager_delegate.h
+++ b/content/shell/shell_download_manager_delegate.h
@@ -22,7 +22,6 @@ class ShellDownloadManagerDelegate
 
   void SetDownloadManager(DownloadManager* manager);
 
-  virtual DownloadId GetNextId() OVERRIDE;
   virtual bool ShouldStartDownload(int32 download_id) OVERRIDE;
   virtual void ChooseDownloadPath(WebContents* web_contents,
                                   const FilePath& suggested_path,