Move Rename functionality from DownloadFileManager to DownloadFileImple.

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 3c9521d314180cf38803328d6e5e1da5d7dda4e0..99f996b611e62e411478e8d58d68e535e0695801 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -489,8 +489,19 @@ void DownloadItemImpl::Cancel(bool user_cancel) {
 
 // An error occurred somewhere.
 void DownloadItemImpl::Interrupt(content::DownloadInterruptReason reason) {
-  // It should not be possible both to have an error and complete.
-  DCHECK(IsInProgress());
+  // Somewhat counter-intuitively, it is possible for us to receive an
+  // interrupt after we've already been interrupted.  The generation of
+  // interrupts from the file thread Renames and the generation of
+  // interrupts from disk writes go through two different mechanisms (driven
+  // by rename requests from UI thread and by write requests from IO thread,
+  // respectively), and since we choose not to keep state on the File thread,
+  // this is the place where the races collide.  It's also possible for
+  // interrupts to race with cancels.
+
+  // Whatever happens, the first one to hit the UI thread wins.
+  if (!IsInProgress())
+    return;
+
   last_reason_ = reason;
   TransitionTo(INTERRUPTED);
   download_stats::RecordDownloadInterrupted(
@@ -739,6 +750,7 @@ void DownloadItemImpl::OnDownloadCompleting(DownloadFileManager* file_manager) {
 
 void DownloadItemImpl::OnDownloadRenamedToFinalName(
     DownloadFileManager* file_manager,
+    content::DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -748,10 +760,10 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
            << " " << DebugString(false);
   DCHECK(NeedsRename());
 
-  if (full_path.empty())
-    // Indicates error; also reported
-    // by DownloadManagerImpl::OnDownloadInterrupted.
+  if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
+    Interrupt(reason);
     return;
+  }
 
   // full_path is now the current and target file path.

[asanka, 2012/07/05 18:47:21]

Nit: DCHECK(!full_path.empty())

[Randy Smith (Not in Mondays), 2012/07/09 20:35:51]

Done.

   target_path_ = full_path;
@@ -775,12 +787,16 @@ void DownloadItemImpl::OnDownloadFileReleased() {
 }
 
 void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
+    content::DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!full_path.empty()) {
+  if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
+    Interrupt(reason);
+  } else {
     SetFullPath(full_path);
     UpdateObservers();
   }
+
   delegate_->DownloadRenamedToIntermediateName(this);
 }