Download filename determination refactor (3/3)

content/browser/download/download_item_impl.cc

Index: content/browser/download/download_item_impl.cc
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index a3e72ffa469e50f0efd18f92c63a50c2a99e0e16..1a29fad6dfc7c823a6fe3a3fc968ca9b102202d2 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -700,7 +700,7 @@ void DownloadItemImpl::TogglePause() {
   UpdateObservers();
 }
 
-void DownloadItemImpl::OnDownloadCompleting(DownloadFileManager* file_manager) {
+void DownloadItemImpl::OnDownloadCompleting() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   VLOG(20) << __FUNCTION__ << "()"
@@ -708,31 +708,28 @@ void DownloadItemImpl::OnDownloadCompleting(DownloadFileManager* file_manager) {
            << " " << DebugString(true);
   DCHECK(!GetTargetName().empty());
   DCHECK_NE(DANGEROUS, GetSafetyState());
-  DCHECK(file_manager);
 
   if (NeedsRename()) {
     DownloadFileManager::RenameCompletionCallback callback =
         base::Bind(&DownloadItemImpl::OnDownloadRenamedToFinalName,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   base::Unretained(file_manager));
+                   weak_ptr_factory_.GetWeakPtr());
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&DownloadFileManager::RenameDownloadFile,
-                   file_manager, GetGlobalId(), GetTargetFilePath(),
-                   true, callback));
+                   delegate_->GetDownloadFileManager(), GetGlobalId(),
+                   GetTargetFilePath(), true, callback));
   } else {
     // Complete the download and release the DownloadFile.
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&DownloadFileManager::CompleteDownload,
-                   file_manager, GetGlobalId(),
+                   delegate_->GetDownloadFileManager(), GetGlobalId(),
                    base::Bind(&DownloadItemImpl::OnDownloadFileReleased,
                               weak_ptr_factory_.GetWeakPtr())));
   }
 }
 
 void DownloadItemImpl::OnDownloadRenamedToFinalName(
-    DownloadFileManager* file_manager,
     content::DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -758,7 +755,7 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFileManager::CompleteDownload,
-                 file_manager, GetGlobalId(),
+                 delegate_->GetDownloadFileManager(), GetGlobalId(),
                  base::Bind(&DownloadItemImpl::OnDownloadFileReleased,
                             weak_ptr_factory_.GetWeakPtr())));
 }
@@ -867,21 +864,44 @@ DownloadItem::TargetDisposition DownloadItemImpl::GetTargetDisposition() const {
   return target_disposition_;
 }
 
-void DownloadItemImpl::OnTargetPathDetermined(
+void DownloadItemImpl::OnDownloadTargetDetermined(
     const FilePath& target_path,
     TargetDisposition disposition,
-    content::DownloadDangerType danger_type) {
+    content::DownloadDangerType danger_type,
+    const FilePath& intermediate_path) {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // If the |target_path| is empty, then we consider this download to be
+  // canceled.
+  if (target_path.empty()) {
+    Cancel(true);
+    return;
+  }
+
   target_path_ = target_path;
   target_disposition_ = disposition;
   SetDangerType(danger_type);
-}
-
-void DownloadItemImpl::OnTargetPathSelected(const FilePath& target_path) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(TARGET_DISPOSITION_PROMPT, target_disposition_);
-  target_path_ = target_path;
+  // TODO(asanka): SetDangerType() doesn't need to send a notification here.
+
+  // 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.
+  DCHECK(intermediate_path.DirName() == target_path.DirName());
+
+  // Rename to intermediate name.
+  // TODO(asanka): Skip this rename if AllDataSaved() is true. This avoids a
+  //               spurious rename when we can just rename to the final
+  //               filename. Unnecessary renames may cause bugs like
+  //               http://crbug.com/74187.
+  DownloadFileManager::RenameCompletionCallback callback =
+      base::Bind(&DownloadItemImpl::OnDownloadRenamedToIntermediateName,
+                 weak_ptr_factory_.GetWeakPtr());
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadFileManager::RenameDownloadFile,
+                 delegate_->GetDownloadFileManager(), GetGlobalId(),
+                 intermediate_path, false, callback));
 }
 
 void DownloadItemImpl::OnContentCheckCompleted(
@@ -892,19 +912,6 @@ void DownloadItemImpl::OnContentCheckCompleted(
   SetDangerType(danger_type);
 }
 
-void DownloadItemImpl::OnIntermediatePathDetermined(
-    DownloadFileManager* file_manager,
-    const FilePath& intermediate_path) {
-  DownloadFileManager::RenameCompletionCallback callback =
-      base::Bind(&DownloadItemImpl::OnDownloadRenamedToIntermediateName,
-                 weak_ptr_factory_.GetWeakPtr());
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DownloadFileManager::RenameDownloadFile,
-                 file_manager, GetGlobalId(), intermediate_path,
-                 false, callback));
-}
-
 const FilePath& DownloadItemImpl::GetFullPath() const {
   return current_path_;
 }
@@ -924,14 +931,14 @@ FilePath DownloadItemImpl::GetUserVerifiedFilePath() const {
       GetTargetFilePath() : GetFullPath();
 }
 
-void DownloadItemImpl::OffThreadCancel(DownloadFileManager* file_manager) {
+void DownloadItemImpl::OffThreadCancel() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   request_handle_->CancelRequest();
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFileManager::CancelDownload,
-                 file_manager, download_id_));
+                 delegate_->GetDownloadFileManager(), download_id_));
 }
 
 void DownloadItemImpl::Init(bool active,