Move Rename functionality from DownloadFileManager to DownloadFileImple.

content/browser/download/download_item_impl.cc

 // Copyright (c) 2012 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_item_impl.h"
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 471 matching lines @@
 
   download_stats::RecordDownloadCount(download_stats::CANCELLED_COUNT);
 
   TransitionTo(CANCELLED);
   if (user_cancel)
     delegate_->DownloadStopped(this);
 }
 
 // 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(
       reason, received_bytes_, total_bytes_);
   delegate_->DownloadStopped(this);
 }
 
 void DownloadItemImpl::MarkAsComplete() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ skipping 228 matching lines @@
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&DownloadFileManager::CompleteDownload,
                    file_manager, 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));
 
   VLOG(20) << __FUNCTION__ << "()"
            << " full_path = \"" << full_path.value() << "\""
            << " needed rename = " << NeedsRename()
            << " " << 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;
   SetFullPath(full_path);
   delegate_->DownloadRenamedToFinalName(this);
 
   // Complete the download and release the DownloadFile.
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFileManager::CompleteDownload,
                  file_manager, GetGlobalId(),
                  base::Bind(&DownloadItemImpl::OnDownloadFileReleased,
                             weak_ptr_factory_.GetWeakPtr())));
 }
 
 void DownloadItemImpl::OnDownloadFileReleased() {
   if (delegate_->ShouldOpenDownload(this))
     Completed();
   else
     delegate_delayed_complete_ = true;
 }
 
 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);
 }
 
 bool DownloadItemImpl::MatchesQuery(const string16& query) const {
   if (query.empty())
     return true;
 
   DCHECK_EQ(query, base::i18n::ToLower(query));
 
   string16 url_raw(UTF8ToUTF16(GetURL().spec()));
@@ skipping 392 matching lines @@
   std::map<const void*, ExternalData*>::iterator it =
       external_data_map_.find(key);
 
   if (it == external_data_map_.end()) {
     external_data_map_[key] = data;
   } else if (it->second != data) {
     delete it->second;
     it->second = data;
   }
 }