Merge from Chromium at DEPS revision r200144

This commit was generated by merge_to_master.py.

Change-Id: I85f3a249ae157fd8253431215fb2dfcd12ee9bf3

12 files changed, 150 insertions, 142 deletions

diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc
index 4be05ed4bb..9d1a3c6024 100644
--- a/content/browser/download/download_browsertest.cc
+++ b/content/browser/download/download_browsertest.cc
@@ -476,11 +476,6 @@ class DownloadCreateObserver : DownloadManager::Observer {
 };
 
 
-// Filter for waiting for intermediate file rename.
-bool IntermediateFileRenameFilter(DownloadItem* download) {
-  return !download->GetFullPath().empty();
-}
-
 // Filter for waiting for a certain number of bytes.
 bool DataReceivedFilter(int number_of_bytes, DownloadItem* download) {
   return download->GetReceivedBytes() >= number_of_bytes;
@@ -651,6 +646,8 @@ class DownloadContentTest : public ContentBrowserTest {
       DownloadItem* download, bool file_exists,
       int received_bytes, int total_bytes,
       const base::FilePath& expected_filename) {
+    // expected_filename is only known if the file exists.
+    ASSERT_EQ(file_exists, !expected_filename.empty());
     EXPECT_EQ(received_bytes, download->GetReceivedBytes());
     EXPECT_EQ(total_bytes, download->GetTotalBytes());
     EXPECT_EQ(expected_filename.value(),
@@ -986,7 +983,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeInterruptedDownload) {
   ::testing::Mock::VerifyAndClearExpectations(&dm_observer);
 
   // Confirm resumption while in progress doesn't do anything.
-  download->ResumeInterruptedDownload();
+  download->Resume();
   ASSERT_EQ(GetSafeBufferChunk(), download->GetReceivedBytes());
   ASSERT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
 
@@ -1002,7 +999,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeInterruptedDownload) {
   int initial_size = 0;
   DownloadUpdatedObserver initial_size_observer(
       download, base::Bind(&InitialSizeFilter, &initial_size));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   initial_size_observer.WaitForEvent();
   EXPECT_EQ(GetSafeBufferChunk(), initial_size);
   ::testing::Mock::VerifyAndClearExpectations(&dm_observer);
@@ -1019,7 +1016,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeInterruptedDownload) {
   // Resume and wait for completion.
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
 
   ConfirmFileStatusForResume(
@@ -1027,7 +1024,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeInterruptedDownload) {
       base::FilePath(FILE_PATH_LITERAL("rangereset")));
 
   // Confirm resumption while complete doesn't do anything.
-  download->ResumeInterruptedDownload();
+  download->Resume();
   ASSERT_EQ(GetSafeBufferChunk() * 3, download->GetReceivedBytes());
   ASSERT_EQ(DownloadItem::COMPLETE, download->GetState());
   RunAllPendingInMessageLoop();
@@ -1062,7 +1059,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeInterruptedDownloadNoRange) {
 
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
 
   ConfirmFileStatusForResume(
@@ -1110,7 +1107,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
 
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
 
   ConfirmFileStatusForResume(
@@ -1156,11 +1153,11 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   ReleaseRSTAndConfirmInterruptForResume(download);
   ConfirmFileStatusForResume(
       download, false, GetSafeBufferChunk(), GetSafeBufferChunk() * 3,
-      base::FilePath(FILE_PATH_LITERAL("rangereset.crdownload")));
+      base::FilePath());
 
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
 
   ConfirmFileStatusForResume(
@@ -1207,7 +1204,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeWithDeletedFile) {
 
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
 
   ConfirmFileStatusForResume(
@@ -1274,7 +1271,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeWithFileInitError) {
   // Resume and watch completion.
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
   EXPECT_EQ(download->GetState(), DownloadItem::COMPLETE);
 }
@@ -1307,10 +1304,10 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
             download->GetLastReason());
   EXPECT_TRUE(download->GetFullPath().empty());
-  // Target path will have been set after file name determination,
-  // and reset when the intermediate rename fails, as that suggests
-  // we should re-do file name determination.
-  EXPECT_TRUE(download->GetTargetFilePath().empty());
+  // Target path will have been set after file name determination. GetFullPath()
+  // being empty is sufficient to signal that filename determination needs to be
+  // redone.
+  EXPECT_FALSE(download->GetTargetFilePath().empty());
 
   // We need to make sure that any cross-thread downloads communication has
   // quiesced before clearing and injecting the new errors, as the
@@ -1326,7 +1323,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   // Resume and watch completion.
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
   EXPECT_EQ(download->GetState(), DownloadItem::COMPLETE);
 }
@@ -1359,10 +1356,8 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeWithFileFinalRenameError) {
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
             download->GetLastReason());
   EXPECT_TRUE(download->GetFullPath().empty());
-  // Target path will have been set after file name determination,
-  // and reset when the rename fails, as that suggests
-  // we should re-do file name determination.
-  EXPECT_TRUE(download->GetTargetFilePath().empty());
+  // Target path should still be intact.
+  EXPECT_FALSE(download->GetTargetFilePath().empty());
 
   // We need to make sure that any cross-thread downloads communication has
   // quiesced before clearing and injecting the new errors, as the
@@ -1378,7 +1373,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ResumeWithFileFinalRenameError) {
   // Resume and watch completion.
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
-  download->ResumeInterruptedDownload();
+  download->Resume();
   completion_observer.WaitForEvent();
   EXPECT_EQ(download->GetState(), DownloadItem::COMPLETE);
 }
diff --git a/content/browser/download/download_file.h b/content/browser/download/download_file.h
index e135efcd69..a8e18ffde7 100644
--- a/content/browser/download/download_file.h
+++ b/content/browser/download/download_file.h
@@ -64,7 +64,6 @@ class CONTENT_EXPORT DownloadFile {
 
   virtual base::FilePath FullPath() const = 0;
   virtual bool InProgress() const = 0;
-  virtual int64 BytesSoFar() const = 0;
   virtual int64 CurrentSpeed() const = 0;
 
   // Set |hash| with sha256 digest for the file.
diff --git a/content/browser/download/download_file_impl.cc b/content/browser/download/download_file_impl.cc
index 9c064d20c4..2de327d0b0 100644
--- a/content/browser/download/download_file_impl.cc
+++ b/content/browser/download/download_file_impl.cc
@@ -186,10 +186,6 @@ bool DownloadFileImpl::InProgress() const {
   return file_.in_progress();
 }
 
-int64 DownloadFileImpl::BytesSoFar() const {
-  return file_.bytes_so_far();
-}
-
 int64 DownloadFileImpl::CurrentSpeed() const {
   return file_.CurrentSpeed();
 }
@@ -306,7 +302,8 @@ void DownloadFileImpl::SendUpdate() {
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&DownloadDestinationObserver::DestinationUpdate,
-                 observer_, BytesSoFar(), CurrentSpeed(), GetHashState()));
+                 observer_, file_.bytes_so_far(), CurrentSpeed(),
+                 GetHashState()));
 }
 
 // static
diff --git a/content/browser/download/download_file_impl.h b/content/browser/download/download_file_impl.h
index f3a3624abd..00a2e06c38 100644
--- a/content/browser/download/download_file_impl.h
+++ b/content/browser/download/download_file_impl.h
@@ -59,7 +59,6 @@ class CONTENT_EXPORT DownloadFileImpl : virtual public DownloadFile {
   virtual void Cancel() OVERRIDE;
   virtual base::FilePath FullPath() const OVERRIDE;
   virtual bool InProgress() const OVERRIDE;
-  virtual int64 BytesSoFar() const OVERRIDE;
   virtual int64 CurrentSpeed() const OVERRIDE;
   virtual bool GetHash(std::string* hash) OVERRIDE;
   virtual std::string GetHashState() OVERRIDE;
diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc
index 982692ef96..caa0dbc1ca 100644
--- a/content/browser/download/download_file_unittest.cc
+++ b/content/browser/download/download_file_unittest.cc
@@ -161,8 +161,6 @@ class DownloadFileTest : public testing::Test {
 
   virtual void DestroyDownloadFile(int offset) {
     EXPECT_FALSE(download_file_->InProgress());
-    EXPECT_EQ(static_cast<int64>(expected_data_.size()),
-              download_file_->BytesSoFar());
 
     // Make sure the data has been properly written to disk.
     std::string disk_data;
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index dfb915e4db..1413393f00 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -311,8 +311,10 @@ void DownloadItemImpl::Resume() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Ignore irrelevant states.
-  if (state_ == COMPLETE_INTERNAL || state_ == COMPLETING_INTERNAL ||
-      !is_paused_)
+  if (state_ == COMPLETE_INTERNAL ||
+      state_ == COMPLETING_INTERNAL ||
+      state_ == CANCELLED_INTERNAL ||
+      (state_ == IN_PROGRESS_INTERNAL && !is_paused_))
     return;
 
   if (state_ == INTERRUPTED_INTERNAL) {
@@ -383,6 +385,7 @@ void DownloadItemImpl::Delete(DeleteReason reason) {
   if (!current_path_.empty() && download_file_.get() == NULL) {
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                             base::Bind(&DeleteDownloadedFile, current_path_));
+    current_path_.clear();
   }
   Remove();
   // We have now been deleted.
@@ -392,6 +395,15 @@ void DownloadItemImpl::Remove() {
   VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Remove the intermediate file if we are removing an interrupted download.
+  // Continuable interruptions leave the intermediate file around. However, the
+  // intermediate file will be unusable if the download item is removed.
+  if (!current_path_.empty() && IsInterrupted() && !download_file_.get()) {
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                            base::Bind(&DeleteDownloadedFile, current_path_));
+    current_path_.clear();
+  }
+
   delegate_->AssertStateConsistent(this);
   Cancel(true);
   delegate_->AssertStateConsistent(this);
@@ -780,9 +792,6 @@ std::string DownloadItemImpl::DebugString(bool verbose) const {
 
 DownloadItemImpl::ResumeMode DownloadItemImpl::GetResumeMode() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!IsInterrupted())
-    return RESUME_MODE_INVALID;
-
   // We can't continue without a handle on the intermediate file.
   // We also can't continue if we don't have some verifier to make sure
   // we're getting the same file.
@@ -851,52 +860,6 @@ DownloadItemImpl::ResumeMode DownloadItemImpl::GetResumeMode() const {
   return mode;
 }
 
-void DownloadItemImpl::ResumeInterruptedDownload() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  // If the flag for downloads resumption isn't enabled, ignore
-  // this request.
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (!command_line.HasSwitch(switches::kEnableDownloadResumption))
-    return;
-
-  // If we're not interrupted, ignore the request; our caller is drunk.
-  if (!IsInterrupted())
-    return;
-
-  // If we can't get a web contents, we can't resume the download.
-  // TODO(rdsmith): Find some alternative web contents to use--this
-  // means we can't restart a download if it's a download imported
-  // from the history.
-  if (!GetWebContents())
-    return;
-
-  // Reset the appropriate state if restarting.
-  ResumeMode mode = GetResumeMode();
-  if (mode == RESUME_MODE_IMMEDIATE_RESTART ||
-      mode == RESUME_MODE_USER_RESTART) {
-    received_bytes_ = 0;
-    hash_state_ = "";
-    last_modified_time_ = "";
-    etag_ = "";
-  }
-
-  scoped_ptr<DownloadUrlParameters> download_params(
-      DownloadUrlParameters::FromWebContents(GetWebContents(),
-                                             GetOriginalUrl()));
-
-  download_params->set_file_path(GetFullPath());
-  download_params->set_offset(GetReceivedBytes());
-  download_params->set_hash_state(GetHashState());
-  download_params->set_last_modified(GetLastModifiedTime());
-  download_params->set_etag(GetETag());
-
-  delegate_->ResumeInterruptedDownload(download_params.Pass(), GetGlobalId());
-
-  // Just in case we were interrupted while paused.
-  is_paused_ = false;
-}
-
 void DownloadItemImpl::NotifyRemoved() {
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadRemoved(this));
 }
@@ -1045,8 +1008,6 @@ void DownloadItemImpl::Init(bool active,
   if (active)
     RecordDownloadCount(START_COUNT);
 
-  if (target_path_.empty())
-    target_path_ = current_path_;
   std::string file_name;
   if (download_type == SRC_HISTORY_IMPORT) {
     // target_path_ works for History and Save As versions.
@@ -1116,8 +1077,10 @@ void DownloadItemImpl::OnDownloadFileInitialized(
     return;
   }
 
-  // If we're resuming an interrupted download, we may already know
-  // the download target so we can skip target name determination.
+  // If we're resuming an interrupted download, we may already know the download
+  // target so we can skip target name determination. GetFullPath() is non-empty
+  // for interrupted downloads where the intermediate file is still present, and
+  // also for downloads with forced paths.
   if (!GetTargetFilePath().empty() && !GetFullPath().empty()) {
     // TODO(rdsmith/asanka): Check to confirm that the target path isn't
     // present on disk; if it is, we should re-do filename determination to
@@ -1126,11 +1089,6 @@ void DownloadItemImpl::OnDownloadFileInitialized(
     return;
   }
 
-  // The target path might be set and the full path empty if we failed
-  // the intermediate rename--re-do file name determination in this case.
-  // TODO(rdsmith,asanka): Clean up this logic.
-  target_path_ = base::FilePath();
-
   delegate_->DetermineDownloadTarget(
       this, base::Bind(&DownloadItemImpl::OnDownloadTargetDetermined,
                        weak_ptr_factory_.GetWeakPtr()));
@@ -1197,24 +1155,18 @@ void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   VLOG(20) << __FUNCTION__ << " download=" << DebugString(true);
 
-  // Process destination error.  If both reason and destination_error_
-  // refer to actual errors, we want to use the destination_error_ as the
-  // argument to the Interrupt() routine, as it happened first.
-  if (destination_error_ != DOWNLOAD_INTERRUPT_REASON_NONE) {
+  if (DOWNLOAD_INTERRUPT_REASON_NONE != destination_error_) {
+    // Process destination error.  If both |reason| and |destination_error_|
+    // refer to actual errors, we want to use the |destination_error_| as the
+    // argument to the Interrupt() routine, as it happened first.
     Interrupt(destination_error_);
     destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
-  }
-
-  // Process the results of the rename.
-  if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    // Will be ignored if we called Interrupt() above.
+  } else if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
     Interrupt(reason);
-
-    // All file errors result in file deletion above; no need to cleanup.
-    // Reset the target path so on resumption we re-do file name determination.
-    // A restart will be forced because we don't set the full path on this
-    // branch.
-    target_path_ = base::FilePath();
+    // All file errors result in file deletion above; no need to cleanup.  The
+    // current_path_ should be empty. Resuming this download will force a
+    // restart and a re-doing of filename determination.
+    DCHECK(current_path_.empty());
   } else {
     SetFullPath(full_path);
     MaybeCompleteDownload();
@@ -1308,11 +1260,9 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
   if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
     Interrupt(reason);
 
-    // All file errors result in file deletion above; no need to cleanup.
-    // Reset the paths so on resumption we re-do file name determination.
-    target_path_ = base::FilePath();
-    current_path_ = base::FilePath();
-    UpdateObservers();
+    // All file errors should have resulted in in file deletion above. On
+    // resumption we will need to re-do filename determination.
+    DCHECK(current_path_.empty());
     return;
   }
 
@@ -1397,15 +1347,12 @@ void DownloadItemImpl::Interrupt(DownloadInterruptReason reason) {
 
   last_reason_ = reason;
 
-  TransitionTo(INTERRUPTED_INTERNAL);
-
   ResumeMode resume_mode = GetResumeMode();
   // Cancel (delete file) if we're going to restart; no point in leaving
   // data around we aren't going to use.  Also cancel if resumption isn't
   // enabled for the same reason.
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  bool resumption_enabled =
-      command_line.HasSwitch(switches::kEnableDownloadResumption);
+  bool resumption_enabled = CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableDownloadResumption);
   ReleaseDownloadFile(resume_mode == RESUME_MODE_IMMEDIATE_RESTART ||
                       resume_mode == RESUME_MODE_USER_RESTART ||
                       !resumption_enabled);
@@ -1425,7 +1372,9 @@ void DownloadItemImpl::Interrupt(DownloadInterruptReason reason) {
   // Cancel the originating URL request.
   request_handle_->CancelRequest();
 
+  TransitionTo(INTERRUPTED_INTERNAL);
   RecordDownloadInterrupted(reason, received_bytes_, total_bytes_);
+
   AutoResumeIfValid();
 }
 
@@ -1437,6 +1386,9 @@ void DownloadItemImpl::ReleaseDownloadFile(bool destroy_file) {
         BrowserThread::FILE, FROM_HERE,
         // Will be deleted at end of task execution.
         base::Bind(&DownloadFileCancel, base::Passed(&download_file_)));
+    // Avoid attempting to reuse the intermediate file by clearing out
+    // current_path_.
+    current_path_.clear();
   } else {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
@@ -1447,17 +1399,6 @@ void DownloadItemImpl::ReleaseDownloadFile(bool destroy_file) {
   // out any previous "all data received".  This also breaks links to
   // other entities we've given out weak pointers to.
   weak_ptr_factory_.InvalidateWeakPtrs();
-
-  // TODO(rdsmith/benjhayden): Remove condition as part of
-  // |SavePackage| integration.
-  // |download_file_| can be NULL if Interrupt() is called after the
-  // download file has been released.
-  if (!is_save_package_download_ && download_file_) {
-    BrowserThread::PostTask(
-        BrowserThread::FILE, FROM_HERE,
-        // Will be deleted at end of task execution.
-        base::Bind(&DownloadFileCancel, base::Passed(&download_file_)));
-  }
 }
 
 bool DownloadItemImpl::IsDownloadReadyForCompletion(
@@ -1604,6 +1545,52 @@ void DownloadItemImpl::AutoResumeIfValid() {
   ResumeInterruptedDownload();
 }
 
+void DownloadItemImpl::ResumeInterruptedDownload() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // If the flag for downloads resumption isn't enabled, ignore
+  // this request.
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (!command_line.HasSwitch(switches::kEnableDownloadResumption))
+    return;
+
+  // If we're not interrupted, ignore the request; our caller is drunk.
+  if (!IsInterrupted())
+    return;
+
+  // If we can't get a web contents, we can't resume the download.
+  // TODO(rdsmith): Find some alternative web contents to use--this
+  // means we can't restart a download if it's a download imported
+  // from the history.
+  if (!GetWebContents())
+    return;
+
+  // Reset the appropriate state if restarting.
+  ResumeMode mode = GetResumeMode();
+  if (mode == RESUME_MODE_IMMEDIATE_RESTART ||
+      mode == RESUME_MODE_USER_RESTART) {
+    received_bytes_ = 0;
+    hash_state_ = "";
+    last_modified_time_ = "";
+    etag_ = "";
+  }
+
+  scoped_ptr<DownloadUrlParameters> download_params(
+      DownloadUrlParameters::FromWebContents(GetWebContents(),
+                                             GetOriginalUrl()));
+
+  download_params->set_file_path(GetFullPath());
+  download_params->set_offset(GetReceivedBytes());
+  download_params->set_hash_state(GetHashState());
+  download_params->set_last_modified(GetLastModifiedTime());
+  download_params->set_etag(GetETag());
+
+  delegate_->ResumeInterruptedDownload(download_params.Pass(), GetGlobalId());
+
+  // Just in case we were interrupted while paused.
+  is_paused_ = false;
+}
+
 // static
 DownloadItem::DownloadState DownloadItemImpl::InternalToExternalState(
     DownloadInternalState internal_state) {
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index cc5e2b610a..083a1591ab 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -93,7 +93,6 @@ class CONTENT_EXPORT DownloadItemImpl
   virtual void DangerousDownloadValidated() OVERRIDE;
   virtual void Pause() OVERRIDE;
   virtual void Resume() OVERRIDE;
-  virtual void ResumeInterruptedDownload() OVERRIDE;
   virtual void Cancel(bool user_cancel) OVERRIDE;
   virtual void Delete(DeleteReason reason) OVERRIDE;
   virtual void Remove() OVERRIDE;
@@ -161,6 +160,9 @@ class CONTENT_EXPORT DownloadItemImpl
   // All remaining public interfaces virtual to allow for DownloadItemImpl
   // mocks.
 
+  // Determines the resume mode for an interrupted download. Requires
+  // last_reason_ to be set, but doesn't require the download to be in
+  // INTERRUPTED state.
   virtual ResumeMode GetResumeMode() const;
 
   // State transition operations on regular downloads --------------------------
@@ -301,8 +303,9 @@ class CONTENT_EXPORT DownloadItemImpl
   void Interrupt(DownloadInterruptReason reason);
 
   // Destroy the DownloadFile object.  If |destroy_file| is true, the file is
-  // destroyed with it.  Otherwise, DownloadFile::Detach() is called
-  // before object destruction to prevent file destruction.
+  // destroyed with it.  Otherwise, DownloadFile::Detach() is called before
+  // object destruction to prevent file destruction. Destroying the file also
+  // resets |current_path_|.
   void ReleaseDownloadFile(bool destroy_file);
 
   // Check if a download is ready for completion.  The callback provided
@@ -320,6 +323,8 @@ class CONTENT_EXPORT DownloadItemImpl
 
   void AutoResumeIfValid();
 
+  void ResumeInterruptedDownload();
+
   static DownloadState InternalToExternalState(
       DownloadInternalState internal_state);
   static DownloadInternalState ExternalToInternalState(
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index 275b4d2c29..7a29f28e71 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -748,6 +748,38 @@ TEST_F(DownloadItemTest, Interrupted) {
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_USER_CANCELED, item->GetLastReason());
 }
 
+// Destination errors that occur before the intermediate rename shouldn't cause
+// the download to be marked as interrupted until after the intermediate rename.
+TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename) {
+  DownloadItemImpl* item = CreateDownloadItem();
+  DownloadItemImplDelegate::DownloadTargetCallback callback;
+  MockDownloadFile* download_file =
+      AddDownloadFileToDownloadItem(item, &callback);
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
+  ASSERT_TRUE(item->IsInProgress());
+
+  base::FilePath final_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+  base::FilePath intermediate_path(final_path.InsertBeforeExtensionASCII("x"));
+  base::FilePath new_intermediate_path(
+      final_path.InsertBeforeExtensionASCII("y"));
+  EXPECT_CALL(*download_file, RenameAndUniquify(intermediate_path, _))
+      .WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
+                                       new_intermediate_path));
+  EXPECT_CALL(*download_file, Cancel())
+      .Times(1);
+
+  callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+  RunAllPendingInMessageLoops();
+  // All the callbacks should have happened by now.
+  ::testing::Mock::VerifyAndClearExpectations(download_file);
+  mock_delegate()->VerifyAndClearExpectations();
+  EXPECT_TRUE(item->IsInterrupted());
+  EXPECT_TRUE(item->GetFullPath().empty());
+  EXPECT_EQ(final_path, item->GetTargetFilePath());
+}
+
 TEST_F(DownloadItemTest, Canceled) {
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index 2abb8f96e8..b1e1c045bd 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -6,7 +6,6 @@
 #include <string>
 
 #include "base/bind.h"
-#include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -64,7 +63,7 @@ MATCHER_P2(DownloadCreateInfoWithDefaultPath, info, download_directory, "") {
 class MockDownloadItemImpl : public DownloadItemImpl {
  public:
   // Use history constructor for minimal base object.
-  MockDownloadItemImpl(DownloadItemImplDelegate* delegate)
+  explicit MockDownloadItemImpl(DownloadItemImplDelegate* delegate)
       : DownloadItemImpl(
           delegate,
           content::DownloadId(),
@@ -426,7 +425,7 @@ class MockDownloadManagerObserver : public DownloadManager::Observer {
         DownloadManager*, int32));
 };
 
-} // namespace
+}  // namespace
 
 class DownloadManagerTest : public testing::Test {
  public:
diff --git a/content/browser/download/drag_download_file.cc b/content/browser/download/drag_download_file.cc
index 5bbc9cb648..3022ebd06a 100644
--- a/content/browser/download/drag_download_file.cc
+++ b/content/browser/download/drag_download_file.cc
@@ -5,7 +5,6 @@
 #include "content/browser/download/drag_download_file.h"
 
 #include "base/bind.h"
-#include "base/file_util.h"
 #include "base/message_loop.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -68,7 +67,7 @@ class DragDownloadFile::DragDownloadFileUI : public DownloadItem::Observer {
     params->set_callback(base::Bind(&DragDownloadFileUI::OnDownloadStarted,
                                     weak_ptr_factory_.GetWeakPtr()));
     params->set_file_path(file_path);
-    params->set_file_stream(file_stream.Pass()); // Nulls file_stream.
+    params->set_file_stream(file_stream.Pass());  // Nulls file_stream.
     download_manager->DownloadUrl(params.Pass());
   }
 
diff --git a/content/browser/download/drag_download_file_browsertest.cc b/content/browser/download/drag_download_file_browsertest.cc
index ebbec5957a..7242c8417b 100644
--- a/content/browser/download/drag_download_file_browsertest.cc
+++ b/content/browser/download/drag_download_file_browsertest.cc
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "content/browser/download/download_file_factory.h"
diff --git a/content/browser/download/save_item.cc b/content/browser/download/save_item.cc
index 450d3839b5..bf4d3de216 100644
--- a/content/browser/download/save_item.cc
+++ b/content/browser/download/save_item.cc
@@ -4,7 +4,6 @@
 
 #include "content/browser/download/save_item.h"
 
-#include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "content/browser/download/save_file.h"
@@ -122,12 +121,12 @@ void SaveItem::Rename(const base::FilePath& full_path) {
 }
 
 void SaveItem::SetSaveId(int32 save_id) {
-  DCHECK(save_id_ == -1);
+  DCHECK_EQ(-1, save_id_);
   save_id_ = save_id;
 }
 
 void SaveItem::SetTotalBytes(int64 total_bytes) {
-  DCHECK(total_bytes_ == 0);
+  DCHECK_EQ(0, total_bytes_);
   total_bytes_ = total_bytes;
 }