Revert 146162 - Move Rename functionality from DownloadFileManager to DownloadFileImple.

BUG=123998
R=asanka@chromium.org


Review URL: https://chromiumcodereview.appspot.com/10689093

TBR=rdsmith@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10702151

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

14 files changed, 177 insertions, 301 deletions

diff --git a/content/browser/download/download_file.h b/content/browser/download/download_file.h
index e707a26..59e8b9b 100644
--- a/content/browser/download/download_file.h
+++ b/content/browser/download/download_file.h
@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/callback_forward.h"
 #include "base/file_path.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/download_id.h"
@@ -24,13 +23,6 @@ class DownloadManager;
 // cancelled, the DownloadFile is destroyed.
 class CONTENT_EXPORT DownloadFile {
  public:
-  // Callback used with Rename().  On a successful rename |reason| will be
-  // DOWNLOAD_INTERRUPT_REASON_NONE and |path| the path the rename
-  // was done to.  On a failed rename, |reason| will contain the
-  // error.
-  typedef base::Callback<void(content::DownloadInterruptReason reason,
-                              const FilePath& path)> RenameCompletionCallback;
-
   virtual ~DownloadFile() {}
 
   // If calculate_hash is true, sha256 hash will be calculated.
@@ -38,14 +30,9 @@ class CONTENT_EXPORT DownloadFile {
   // error code on failure.
   virtual DownloadInterruptReason Initialize() = 0;
 
-  // Rename the download file to |full_path|.  If that file exists and
-  // |overwrite_existing_file| is false, |full_path| will be uniquified by
-  // suffixing " (<number>)" to the file name before the extension.
-  // Upon completion, |callback| will be called on the UI thread
-  // as per the comment above.
-  virtual void Rename(const FilePath& full_path,
-                      bool overwrite_existing_file,
-                      const RenameCompletionCallback& callback) = 0;
+  // Rename the download file.
+  // Returns net::OK on success, or a network error code on failure.
+  virtual DownloadInterruptReason Rename(const FilePath& full_path) = 0;
 
   // Detach the file so it is not deleted on destruction.
   virtual void Detach() = 0;
diff --git a/content/browser/download/download_file_impl.cc b/content/browser/download/download_file_impl.cc
index 8c42515..9d4a6880 100644
--- a/content/browser/download/download_file_impl.cc
+++ b/content/browser/download/download_file_impl.cc
@@ -88,39 +88,11 @@ content::DownloadInterruptReason DownloadFileImpl::AppendDataToFile(
       content::DOWNLOAD_INTERRUPT_FROM_DISK);
 }
 
-void DownloadFileImpl::Rename(const FilePath& full_path,
-                              bool overwrite_existing_file,
-                              const RenameCompletionCallback& callback) {
-  FilePath new_path(full_path);
-  if (!overwrite_existing_file) {
-    // Make the file unique if requested.
-    int uniquifier =
-        file_util::GetUniquePathNumber(new_path, FILE_PATH_LITERAL(""));
-    if (uniquifier > 0) {
-      new_path = new_path.InsertBeforeExtensionASCII(
-          StringPrintf(" (%d)", uniquifier));
-    }
-  }
-
-  net::Error rename_error = file_.Rename(new_path);
-  content::DownloadInterruptReason reason(
-      content::DOWNLOAD_INTERRUPT_REASON_NONE);
-  if (net::OK != rename_error) {
-    // Make sure our information is updated, since we're about to
-    // error out.
-    SendUpdate();
-
-    reason =
-        content::ConvertNetErrorToInterruptReason(
-            rename_error,
-            content::DOWNLOAD_INTERRUPT_FROM_DISK);
-
-    new_path.clear();
-  }
-
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(callback, reason, new_path));
+content::DownloadInterruptReason DownloadFileImpl::Rename(
+    const FilePath& full_path) {
+  return content::ConvertNetErrorToInterruptReason(
+      file_.Rename(full_path),
+      content::DOWNLOAD_INTERRUPT_FROM_DISK);
 }
 
 void DownloadFileImpl::Detach() {
@@ -218,10 +190,6 @@ void DownloadFileImpl::StreamActive() {
           base::TimeTicks write_start(base::TimeTicks::Now());
           reason = AppendDataToFile(
               incoming_data.get()->data(), incoming_data_size);
-          // Note that if we're after a rename failure but before any
-          // cancel that our owner generates based on that rename failure,
-          // we'll get an ERR_UNEXPECTED from the above.  Our consumers
-          // need to handle this situation.
           disk_writes_time_ += (base::TimeTicks::Now() - write_start);
           bytes_seen_ += incoming_data_size;
           total_incoming_data_size += incoming_data_size;
@@ -270,11 +238,11 @@ void DownloadFileImpl::StreamActive() {
     // Our controller will clean us up.
     stream_reader_->RegisterCallback(base::Closure());
     weak_factory_.InvalidateWeakPtrs();
-    SendUpdate();                       // Make info up to date before error.
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&DownloadManager::OnDownloadInterrupted,
-                   download_manager_, id_.local(), reason));
+                   download_manager_, id_.local(),
+                   BytesSoFar(), GetHashState(), reason));
   } else if (state == content::ByteStreamReader::STREAM_COMPLETE) {
     // Signal successful completion and shut down processing.
     stream_reader_->RegisterCallback(base::Closure());
diff --git a/content/browser/download/download_file_impl.h b/content/browser/download/download_file_impl.h
index b3630e5..1083132 100644
--- a/content/browser/download/download_file_impl.h
+++ b/content/browser/download/download_file_impl.h
@@ -40,9 +40,8 @@ class CONTENT_EXPORT DownloadFileImpl : virtual public content::DownloadFile {
 
   // DownloadFile functions.
   virtual content::DownloadInterruptReason Initialize() OVERRIDE;
-  virtual void Rename(const FilePath& full_path,
-                      bool overwrite_existing_file,
-                      const RenameCompletionCallback& callback) OVERRIDE;
+  virtual content::DownloadInterruptReason Rename(
+      const FilePath& full_path) OVERRIDE;
   virtual void Detach() OVERRIDE;
   virtual void Cancel() OVERRIDE;
   virtual void AnnotateWithSourceInformation() OVERRIDE;
@@ -64,13 +63,13 @@ class CONTENT_EXPORT DownloadFileImpl : virtual public content::DownloadFile {
       const char* data, size_t data_len);
 
  private:
-  // Send an update on our progress.
-  void SendUpdate();
-
   // Called when there's some activity on stream_reader_ that needs to be
   // handled.
   void StreamActive();
 
+  // Send updates on our progress.
+  void SendUpdate();
+
   // The base file instance.
   BaseFile file_;
 
diff --git a/content/browser/download/download_file_manager.cc b/content/browser/download/download_file_manager.cc
index 7bebd92..4fc4a86 100644
--- a/content/browser/download/download_file_manager.cc
+++ b/content/browser/download/download_file_manager.cc
@@ -195,14 +195,42 @@ void DownloadFileManager::RenameDownloadFile(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DownloadFile* download_file = GetDownloadFile(global_id);
   if (!download_file) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        base::Bind(callback, content::DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
-                   FilePath()));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(callback, FilePath()));
     return;
   }
 
-  download_file->Rename(full_path, overwrite_existing_file, callback);
+  FilePath new_path(full_path);
+  if (!overwrite_existing_file) {
+    // Make the file unique if requested.
+    int uniquifier =
+        file_util::GetUniquePathNumber(new_path, FILE_PATH_LITERAL(""));
+    if (uniquifier > 0) {
+      new_path = new_path.InsertBeforeExtensionASCII(
+          StringPrintf(" (%d)", uniquifier));
+    }
+  }
+
+  // Do the actual rename
+  content::DownloadInterruptReason rename_error =
+      download_file->Rename(new_path);
+  if (content::DOWNLOAD_INTERRUPT_REASON_NONE != rename_error) {
+    DownloadManager* download_manager = download_file->GetDownloadManager();
+    DCHECK(download_manager);
+
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&DownloadManager::OnDownloadInterrupted,
+                   download_manager,
+                   global_id.local(),
+                   download_file->BytesSoFar(),
+                   download_file->GetHashState(),
+                   rename_error));
+
+    new_path.clear();
+  }
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, new_path));
 }
 
 int DownloadFileManager::NumberOfActiveDownloads() const {
diff --git a/content/browser/download/download_file_manager.h b/content/browser/download/download_file_manager.h
index 919f1c4..bf2a53e 100644
--- a/content/browser/download/download_file_manager.h
+++ b/content/browser/download/download_file_manager.h
@@ -49,7 +49,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/timer.h"
-#include "content/browser/download/download_file.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/download_id.h"
 #include "content/public/browser/download_interrupt_reasons.h"
@@ -62,6 +61,7 @@ class FilePath;
 
 namespace content {
 class ByteStreamReader;
+class DownloadFile;
 class DownloadId;
 class DownloadManager;
 }
@@ -82,8 +82,7 @@ class CONTENT_EXPORT DownloadFileManager
       CreateDownloadFileCallback;
 
   // Callback used with RenameDownloadFile().
-  typedef content::DownloadFile::RenameCompletionCallback
-      RenameCompletionCallback;
+  typedef base::Callback<void(const FilePath&)> RenameCompletionCallback;
 
   class DownloadFileFactory {
    public:
diff --git a/content/browser/download/download_file_manager_unittest.cc b/content/browser/download/download_file_manager_unittest.cc
index 52ce433..04290d4 100644
--- a/content/browser/download/download_file_manager_unittest.cc
+++ b/content/browser/download/download_file_manager_unittest.cc
@@ -31,7 +31,6 @@ using ::testing::_;
 using ::testing::AtLeast;
 using ::testing::Mock;
 using ::testing::Return;
-using ::testing::SaveArg;
 using ::testing::StrictMock;
 using ::testing::StrEq;
 
@@ -40,10 +39,8 @@ namespace {
 // MockDownloadManager with the addition of a mock callback used for testing.
 class TestDownloadManager : public MockDownloadManager {
  public:
-  MOCK_METHOD3(OnDownloadRenamed,
-               void(int download_id,
-                    content::DownloadInterruptReason reason,
-                    const FilePath& full_path));
+  MOCK_METHOD2(OnDownloadRenamed,
+               void(int download_id, const FilePath& full_path));
  private:
   ~TestDownloadManager() {}
 };
@@ -215,22 +212,48 @@ class DownloadFileManagerTest : public testing::Test {
   // |should_overwrite| indicates whether to replace or uniquify the file.
   void RenameFile(const DownloadId& id,
                   const FilePath& new_path,
-                  bool should_overwrite) {
+                  const FilePath& unique_path,
+                  content::DownloadInterruptReason rename_error,
+                  RenameFileState state,
+                  RenameFileOverwrite should_overwrite) {
     MockDownloadFile* file = download_file_factory_->GetExistingFile(id);
     ASSERT_TRUE(file != NULL);
-    content::DownloadFile::RenameCompletionCallback rename_callback;
 
-    EXPECT_CALL(*file, Rename(new_path, should_overwrite, _))
-        .WillOnce(SaveArg<2>(&rename_callback));
+    EXPECT_CALL(*file, Rename(unique_path))
+        .Times(1)
+        .WillOnce(Return(rename_error));
 
-    content::DownloadFile::RenameCompletionCallback passed_callback(
-        base::Bind(&TestDownloadManager::OnDownloadRenamed,
-                   download_manager_, id.local()));
+    if (rename_error != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
+      EXPECT_CALL(*file, BytesSoFar())
+          .Times(AtLeast(1))
+          .WillRepeatedly(Return(byte_count_[id]));
+      EXPECT_CALL(*file, GetHashState())
+          .Times(AtLeast(1));
+      EXPECT_CALL(*file, GetDownloadManager())
+          .Times(AtLeast(1));
+    }
 
     download_file_manager_->RenameDownloadFile(
-        id, new_path, should_overwrite, passed_callback);
+        id, new_path, (should_overwrite == OVERWRITE),
+        base::Bind(&TestDownloadManager::OnDownloadRenamed,
+                   download_manager_, id.local()));
 
-    EXPECT_TRUE(rename_callback.Equals(passed_callback));
+    if (rename_error != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
+      EXPECT_CALL(*download_manager_,
+                  OnDownloadInterrupted(
+                      id.local(),
+                      byte_count_[id],
+                      "",
+                      rename_error));
+      EXPECT_CALL(*download_manager_,
+                  OnDownloadRenamed(id.local(), FilePath()));
+      ProcessAllPendingMessages();
+      ++error_count_[id];
+    } else {
+      EXPECT_CALL(*download_manager_,
+                  OnDownloadRenamed(id.local(), unique_path));
+      ProcessAllPendingMessages();
+    }
   }
 
   void Complete(DownloadId id) {
@@ -322,7 +345,42 @@ TEST_F(DownloadFileManagerTest, Complete) {
   Complete(dummy_id);
 }
 
-TEST_F(DownloadFileManagerTest, Rename) {
+TEST_F(DownloadFileManagerTest, RenameInProgress) {
+  scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
+  DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
+  info->download_id = dummy_id;
+  ScopedTempDir download_dir;
+  ASSERT_TRUE(download_dir.CreateUniqueTempDir());
+
+  CreateDownloadFile(info.Pass());
+
+  FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
+  RenameFile(dummy_id, foo, foo, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             IN_PROGRESS, OVERWRITE);
+
+  CleanUp(dummy_id);
+}
+
+TEST_F(DownloadFileManagerTest, RenameInProgressWithUniquification) {
+  scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
+  DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
+  info->download_id = dummy_id;
+  ScopedTempDir download_dir;
+  ASSERT_TRUE(download_dir.CreateUniqueTempDir());
+
+  CreateDownloadFile(info.Pass());
+
+  FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
+  FilePath unique_foo(foo.InsertBeforeExtension(FILE_PATH_LITERAL(" (1)")));
+  ASSERT_EQ(0, file_util::WriteFile(foo, "", 0));
+  RenameFile(dummy_id, foo, unique_foo,
+             content::DOWNLOAD_INTERRUPT_REASON_NONE, IN_PROGRESS,
+             DONT_OVERWRITE);
+
+  CleanUp(dummy_id);
+}
+
+TEST_F(DownloadFileManagerTest, RenameInProgressWithError) {
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
   info->download_id = dummy_id;
@@ -332,12 +390,14 @@ TEST_F(DownloadFileManagerTest, Rename) {
   CreateDownloadFile(info.Pass());
 
   FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
-  RenameFile(dummy_id, foo, true);
+  RenameFile(dummy_id, foo, foo,
+             content::DOWNLOAD_INTERRUPT_REASON_FILE_NAME_TOO_LONG,
+             IN_PROGRESS, OVERWRITE);
 
   CleanUp(dummy_id);
 }
 
-TEST_F(DownloadFileManagerTest, RenameNoOverwrite) {
+TEST_F(DownloadFileManagerTest, RenameWithUniquification) {
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
   info->download_id = dummy_id;
@@ -347,7 +407,13 @@ TEST_F(DownloadFileManagerTest, RenameNoOverwrite) {
   CreateDownloadFile(info.Pass());
 
   FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
-  RenameFile(dummy_id, foo, false);
+  FilePath unique_foo(foo.InsertBeforeExtension(FILE_PATH_LITERAL(" (1)")));
+  // Create a file at |foo|. Since we are specifying DONT_OVERWRITE,
+  // RenameDownloadFile() should pick "foo (1).txt" instead of
+  // overwriting this file.
+  ASSERT_EQ(0, file_util::WriteFile(foo, "", 0));
+  RenameFile(dummy_id, foo, unique_foo,
+             content::DOWNLOAD_INTERRUPT_REASON_NONE, COMPLETE, DONT_OVERWRITE);
 
   CleanUp(dummy_id);
 }
@@ -363,10 +429,12 @@ TEST_F(DownloadFileManagerTest, RenameTwice) {
 
   FilePath crfoo(download_dir.path().Append(
       FILE_PATH_LITERAL("foo.txt.crdownload")));
-  RenameFile(dummy_id, crfoo, true);
+  RenameFile(dummy_id, crfoo, crfoo, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             IN_PROGRESS, OVERWRITE);
 
   FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
-  RenameFile(dummy_id, foo, true);
+  RenameFile(dummy_id, foo, foo, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             COMPLETE, OVERWRITE);
 
   CleanUp(dummy_id);
 }
@@ -387,20 +455,24 @@ TEST_F(DownloadFileManagerTest, TwoDownloads) {
 
   FilePath crbar(download_dir.path().Append(
       FILE_PATH_LITERAL("bar.txt.crdownload")));
-  RenameFile(dummy_id2, crbar, true);
+  RenameFile(dummy_id2, crbar, crbar, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             IN_PROGRESS, OVERWRITE);
 
   FilePath crfoo(download_dir.path().Append(
       FILE_PATH_LITERAL("foo.txt.crdownload")));
-  RenameFile(dummy_id, crfoo, true);
+  RenameFile(dummy_id, crfoo, crfoo, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             IN_PROGRESS, OVERWRITE);
 
 
   FilePath bar(download_dir.path().Append(FILE_PATH_LITERAL("bar.txt")));
-  RenameFile(dummy_id2, bar, true);
+  RenameFile(dummy_id2, bar, bar, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             COMPLETE, OVERWRITE);
 
   CleanUp(dummy_id2);
 
   FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
-  RenameFile(dummy_id, foo, true);
+  RenameFile(dummy_id, foo, foo, content::DOWNLOAD_INTERRUPT_REASON_NONE,
+             COMPLETE, OVERWRITE);
 
   CleanUp(dummy_id);
 }
diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc
index aa07554..01009a8 100644
--- a/content/browser/download/download_file_unittest.cc
+++ b/content/browser/download/download_file_unittest.cc
@@ -5,7 +5,6 @@
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
-#include "base/test/test_file_util.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/download/byte_stream.h"
 #include "content/browser/download/download_create_info.h"
@@ -16,7 +15,6 @@
 #include "content/public/browser/download_manager.h"
 #include "content/public/test/mock_download_manager.h"
 #include "net/base/file_stream.h"
-#include "net/base/mock_file_stream.h"
 #include "net/base/net_errors.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -29,7 +27,6 @@ using content::DownloadManager;
 using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::DoAll;
-using ::testing::InSequence;
 using ::testing::Return;
 using ::testing::SetArgPointee;
 using ::testing::StrictMock;
@@ -48,14 +45,6 @@ class MockByteStreamReader : public content::ByteStreamReader {
   MOCK_METHOD1(RegisterCallback, void(const base::Closure&));
 };
 
-class LocalMockDownloadManager : public content::MockDownloadManager {
- public:
-  MOCK_METHOD4(CurrentUpdateStatus,
-               void(int32, int64, int64, const std::string&));
- protected:
-  virtual ~LocalMockDownloadManager() {}
-};
-
 }  // namespace
 
 DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";
@@ -76,10 +65,6 @@ class DownloadFileTest : public testing::Test {
   // calling Release() on |download_manager_| won't ever result in its
   // destructor being called and we get a leak.
   DownloadFileTest() :
-      update_download_id_(-1),
-      bytes_(-1),
-      bytes_per_sec_(-1),
-      hash_state_("xyzzy"),
       ui_thread_(BrowserThread::UI, &loop_),
       file_thread_(BrowserThread::FILE, &loop_) {
   }
@@ -89,19 +74,13 @@ class DownloadFileTest : public testing::Test {
 
   void SetUpdateDownloadInfo(int32 id, int64 bytes, int64 bytes_per_sec,
                              const std::string& hash_state) {
-    update_download_id_ = id;
     bytes_ = bytes;
     bytes_per_sec_ = bytes_per_sec;
     hash_state_ = hash_state;
   }
 
-  void ConfirmUpdateDownloadInfo() {
-    download_manager_->CurrentUpdateStatus(
-        update_download_id_, bytes_, bytes_per_sec_, hash_state_);
-  }
-
   virtual void SetUp() {
-    download_manager_ = new StrictMock<LocalMockDownloadManager>;
+    download_manager_ = new StrictMock<content::MockDownloadManager>;
     EXPECT_CALL(*(download_manager_.get()),
                 UpdateDownload(
                     DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
@@ -138,8 +117,8 @@ class DownloadFileTest : public testing::Test {
         .RetiresOnSaturation();
 
     DownloadCreateInfo info;
-    // info.request_handle default constructed to null.
     info.download_id = DownloadId(kValidIdDomain, kDummyDownloadId + offset);
+    // info.request_handle default constructed to null.
     info.save_info.file_stream = file_stream_;
     download_file_.reset(
         new DownloadFileImpl(
@@ -249,28 +228,8 @@ class DownloadFileTest : public testing::Test {
     }
   }
 
-  content::DownloadInterruptReason Rename(
-      const FilePath& full_path, bool overwrite_existing_file,
-      FilePath* result_path_p) {
-    base::WeakPtrFactory<DownloadFileTest> weak_ptr_factory(this);
-    content::DownloadInterruptReason result_reason(
-        content::DOWNLOAD_INTERRUPT_REASON_NONE);
-    bool callback_was_called(false);
-    FilePath result_path;
-
-    download_file_->Rename(full_path, overwrite_existing_file,
-                           base::Bind(&DownloadFileTest::SetRenameResult,
-                                      weak_ptr_factory.GetWeakPtr(),
-                                      &callback_was_called,
-                                      &result_reason, result_path_p));
-    loop_.RunAllPending();
-
-    EXPECT_TRUE(callback_was_called);
-    return result_reason;
-  }
-
  protected:
-  scoped_refptr<StrictMock<LocalMockDownloadManager> > download_manager_;
+  scoped_refptr<StrictMock<content::MockDownloadManager> > download_manager_;
 
   linked_ptr<net::FileStream> file_stream_;
 
@@ -285,7 +244,6 @@ class DownloadFileTest : public testing::Test {
   base::Closure sink_callback_;
 
   // Latest update sent to the download manager.
-  int32 update_download_id_;
   int64 bytes_;
   int64 bytes_per_sec_;
   std::string hash_state_;
@@ -293,19 +251,6 @@ class DownloadFileTest : public testing::Test {
   MessageLoop loop_;
 
  private:
-  void SetRenameResult(bool* called_p,
-                       content::DownloadInterruptReason* reason_p,
-                       FilePath* result_path_p,
-                       content::DownloadInterruptReason reason,
-                       const FilePath& result_path) {
-    if (called_p)
-      *called_p = true;
-    if (reason_p)
-      *reason_p = reason;
-    if (result_path_p)
-      *result_path_p = result_path;
-  }
-
   // UI thread.
   BrowserThreadImpl ui_thread_;
   // File thread to satisfy debug checks in DownloadFile.
@@ -336,15 +281,12 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
   FilePath path_2(initial_path.InsertBeforeExtensionASCII("_2"));
   FilePath path_3(initial_path.InsertBeforeExtensionASCII("_3"));
   FilePath path_4(initial_path.InsertBeforeExtensionASCII("_4"));
-  FilePath path_5(initial_path.InsertBeforeExtensionASCII("_5"));
-  FilePath output_path;
 
   // Rename the file before downloading any data.
   EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(path_1, false, &output_path));
+            download_file_->Rename(path_1));
   FilePath renamed_path = download_file_->FullPath();
   EXPECT_EQ(path_1, renamed_path);
-  EXPECT_EQ(path_1, output_path);
 
   // Check the files.
   EXPECT_FALSE(file_util::PathExists(initial_path));
@@ -356,10 +298,9 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
 
   // Rename the file after downloading some data.
   EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(path_2, false, &output_path));
+            download_file_->Rename(path_2));
   renamed_path = download_file_->FullPath();
   EXPECT_EQ(path_2, renamed_path);
-  EXPECT_EQ(path_2, output_path);
 
   // Check the files.
   EXPECT_FALSE(file_util::PathExists(path_1));
@@ -370,10 +311,9 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
 
   // Rename the file after downloading all the data.
   EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(path_3, false, &output_path));
+            download_file_->Rename(path_3));
   renamed_path = download_file_->FullPath();
   EXPECT_EQ(path_3, renamed_path);
-  EXPECT_EQ(path_3, output_path);
 
   // Check the files.
   EXPECT_FALSE(file_util::PathExists(path_2));
@@ -387,10 +327,9 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
 
   // Rename the file after downloading all the data and closing the file.
   EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(path_4, false, &output_path));
+            download_file_->Rename(path_4));
   renamed_path = download_file_->FullPath();
   EXPECT_EQ(path_4, renamed_path);
-  EXPECT_EQ(path_4, output_path);
 
   // Check the files.
   EXPECT_FALSE(file_util::PathExists(path_3));
@@ -400,81 +339,6 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
   EXPECT_TRUE(download_file_->GetHash(&hash));
   EXPECT_EQ(kDataHash, base::HexEncode(hash.data(), hash.size()));
 
-  // Check that a rename with overwrite to an existing file succeeds.
-  std::string file_contents;
-  ASSERT_FALSE(file_util::PathExists(path_5));
-  static const char file_data[] = "xyzzy";
-  ASSERT_EQ(static_cast<int>(sizeof(file_data) - 1),
-            file_util::WriteFile(path_5, file_data, sizeof(file_data) - 1));
-  ASSERT_TRUE(file_util::PathExists(path_5));
-  EXPECT_TRUE(file_util::ReadFileToString(path_5, &file_contents));
-  EXPECT_EQ(std::string(file_data), file_contents);
-
-  EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(path_5, true, &output_path));
-  EXPECT_EQ(path_5, output_path);
-
-  file_contents = "";
-  EXPECT_TRUE(file_util::ReadFileToString(path_5, &file_contents));
-  EXPECT_NE(std::string(file_data), file_contents);
-
-  DestroyDownloadFile(0);
-}
-
-// Test to make sure the rename uniquifies if we aren't overwriting
-// and there's a file where we're aiming.
-TEST_F(DownloadFileTest, RenameUniquifies) {
-  ASSERT_TRUE(CreateDownloadFile(0, true));
-  FilePath initial_path(download_file_->FullPath());
-  EXPECT_TRUE(file_util::PathExists(initial_path));
-  FilePath path_1(initial_path.InsertBeforeExtensionASCII("_1"));
-  FilePath path_1_suffixed(path_1.InsertBeforeExtensionASCII(" (1)"));
-
-  ASSERT_FALSE(file_util::PathExists(path_1));
-  static const char file_data[] = "xyzzy";
-  ASSERT_EQ(static_cast<int>(sizeof(file_data)),
-            file_util::WriteFile(path_1, file_data, sizeof(file_data)));
-  ASSERT_TRUE(file_util::PathExists(path_1));
-
-  EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(path_1, false, NULL));
-  EXPECT_TRUE(file_util::PathExists(path_1_suffixed));
-
-  FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NONE, true);
-  loop_.RunAllPending();
-  DestroyDownloadFile(0);
-}
-
-// Test to make sure we get the proper error on failure.
-TEST_F(DownloadFileTest, RenameError) {
-  ASSERT_TRUE(CreateDownloadFile(0, true));
-  FilePath initial_path(download_file_->FullPath());
-
-  // Create a subdirectory and rename into it.
-  FilePath tempdir(initial_path.DirName().Append(FILE_PATH_LITERAL("tempdir")));
-  ASSERT_TRUE(file_util::CreateDirectory(tempdir));
-  FilePath new_path(tempdir.Append(initial_path.BaseName()));
-  ASSERT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,
-            Rename(new_path, true, NULL));
-
-  // Targets
-  FilePath path_1(new_path.InsertBeforeExtensionASCII("_1"));
-  FilePath path_1_suffixed(path_1.InsertBeforeExtensionASCII(" (1)"));
-  ASSERT_FALSE(file_util::PathExists(path_1));
-  ASSERT_FALSE(file_util::PathExists(path_1_suffixed));
-
-  // Make the directory unwritable and try to rename within it.
-  {
-    file_util::PermissionRestorer restorer(tempdir);
-    ASSERT_TRUE(file_util::MakeFileUnwritable(tempdir));
-
-    EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
-              Rename(path_1, true, NULL));
-    EXPECT_FALSE(file_util::PathExists(path_1_suffixed));
-  }
-
-  FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NONE, true);
-  loop_.RunAllPending();
   DestroyDownloadFile(0);
 }
 
@@ -516,22 +380,10 @@ TEST_F(DownloadFileTest, StreamEmptyError) {
   EXPECT_CALL(*(download_manager_.get()),
               OnDownloadInterrupted(
                   DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
-                  content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED))
-      .WillOnce(InvokeWithoutArgs(
-          this, &DownloadFileTest::ConfirmUpdateDownloadInfo));
-
-  // If this next EXPECT_CALL fails flakily, it's probably a real failure.
-  // We'll be getting a stream of UpdateDownload calls from the timer, and
-  // the last one may have the correct information even if the failure
-  // doesn't produce an update, as the timer update may have triggered at the
-  // same time.
-  EXPECT_CALL(*(download_manager_.get()),
-              CurrentUpdateStatus(kDummyDownloadId + 0, 0, _, _));
-
+                  0, _,
+                  content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED));
   FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED, false);
 
-  loop_.RunAllPending();
-
   DestroyDownloadFile(0);
 }
 
@@ -564,26 +416,12 @@ TEST_F(DownloadFileTest, StreamNonEmptyError) {
   SetupDataAppend(chunks1, 2, s1);
   SetupFinishStream(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED,
                     s1);
-
   EXPECT_CALL(*(download_manager_.get()),
               OnDownloadInterrupted(
                   DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
-                  content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED))
-      .WillOnce(InvokeWithoutArgs(
-          this, &DownloadFileTest::ConfirmUpdateDownloadInfo));
-
-  // If this next EXPECT_CALL fails flakily, it's probably a real failure.
-  // We'll be getting a stream of UpdateDownload calls from the timer, and
-  // the last one may have the correct information even if the failure
-  // doesn't produce an update, as the timer update may have triggered at the
-  // same time.
-  EXPECT_CALL(*(download_manager_.get()),
-              CurrentUpdateStatus(kDummyDownloadId + 0,
-                                  strlen(kTestData1) + strlen(kTestData2),
-                                  _, _));
-
+                  strlen(kTestData1) + strlen(kTestData2), _,
+                  content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED));
   sink_callback_.Run();
-  loop_.RunAllPending();
   VerifyStreamAndSize();
   DestroyDownloadFile(0);
 }
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index 1912cce..3c9521d 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -489,19 +489,8 @@ void DownloadItemImpl::Cancel(bool user_cancel) {
 
 // An error occurred somewhere.
 void DownloadItemImpl::Interrupt(content::DownloadInterruptReason reason) {
-  // 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;
-
+  // It should not be possible both to have an error and complete.
+  DCHECK(IsInProgress());
   last_reason_ = reason;
   TransitionTo(INTERRUPTED);
   download_stats::RecordDownloadInterrupted(
@@ -750,7 +739,6 @@ void DownloadItemImpl::OnDownloadCompleting(DownloadFileManager* file_manager) {
 
 void DownloadItemImpl::OnDownloadRenamedToFinalName(
     DownloadFileManager* file_manager,
-    content::DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -760,13 +748,12 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
            << " " << DebugString(false);
   DCHECK(NeedsRename());
 
-  if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    Interrupt(reason);
+  if (full_path.empty())
+    // Indicates error; also reported
+    // by DownloadManagerImpl::OnDownloadInterrupted.
     return;
-  }
 
   // full_path is now the current and target file path.
-  DCHECK(!full_path.empty());
   target_path_ = full_path;
   SetFullPath(full_path);
   delegate_->DownloadRenamedToFinalName(this);
@@ -788,16 +775,12 @@ void DownloadItemImpl::OnDownloadFileReleased() {
 }
 
 void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
-    content::DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    Interrupt(reason);
-  } else {
+  if (!full_path.empty()) {
     SetFullPath(full_path);
     UpdateObservers();
   }
-
   delegate_->DownloadRenamedToIntermediateName(this);
 }
 
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index a48154c..af0b505 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -253,13 +253,11 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
 
   // Callback invoked when the download has been renamed to its final name.
   void OnDownloadRenamedToFinalName(DownloadFileManager* file_manager,
-                                    content::DownloadInterruptReason reason,
                                     const FilePath& full_path);
 
   // Callback invoked when the download has been renamed to its intermediate
   // name.
-  void OnDownloadRenamedToIntermediateName(
-      content::DownloadInterruptReason reason, const FilePath& full_path);
+  void OnDownloadRenamedToIntermediateName(const FilePath& full_path);
 
   // Callback from file thread when we release the DownloadFile.
   void OnDownloadFileReleased();
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index 3224a83..c7ac84d 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -101,9 +101,8 @@ class MockDownloadFileManager : public DownloadFileManager {
 //               RenameDownloadFile(_,_,_,_))
 //       .WillOnce(ScheduleRenameCallback(new_path));
 ACTION_P(ScheduleRenameCallback, new_path) {
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(arg3, content::DOWNLOAD_INTERRUPT_REASON_NONE, new_path));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(arg3, new_path));
 }
 
 // Similarly for scheduling a completion callback.
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index b798899..f742c7c 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -428,7 +428,7 @@ content::DownloadId DownloadManagerImpl::StartDownload(
 void DownloadManagerImpl::OnDownloadFileCreated(
     int32 download_id, content::DownloadInterruptReason reason) {
   if (reason != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
-    OnDownloadInterrupted(download_id, reason);
+    OnDownloadInterrupted(download_id, 0, "", reason);
     // TODO(rdsmith): It makes no sense to continue along the
     // regular download path after we've gotten an error.  But it's
     // the way the code has historically worked, and this allows us
@@ -771,12 +771,15 @@ void DownloadManagerImpl::DownloadStopped(DownloadItem* download) {
 
 void DownloadManagerImpl::OnDownloadInterrupted(
     int32 download_id,
+    int64 size,
+    const std::string& hash_state,
     content::DownloadInterruptReason reason) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download = GetActiveDownload(download_id);
   if (!download)
     return;
+  download->UpdateProgress(size, 0, hash_state);
   download->Interrupt(reason);
 }
 
diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h
index 6edebaa..ad79935 100644
--- a/content/browser/download/download_manager_impl.h
+++ b/content/browser/download/download_manager_impl.h
@@ -58,6 +58,8 @@ class CONTENT_EXPORT DownloadManagerImpl
   virtual void CancelDownload(int32 download_id) OVERRIDE;
   virtual void OnDownloadInterrupted(
       int32 download_id,
+      int64 size,
+      const std::string& hash_state,
       content::DownloadInterruptReason reason) OVERRIDE;
   virtual int RemoveDownloadsBetween(base::Time remove_begin,
                                      base::Time remove_end) OVERRIDE;
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index da28c5e..124d4ce 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -513,11 +513,15 @@ TEST_F(DownloadManagerTest, OnDownloadInterrupted) {
   content::MockDownloadItem& item(AddItemToManager());
   int download_id = item.GetId();
 
+  int64 size = 0xdeadbeef;
+  const std::string hash_state("Undead beef");
   content::DownloadInterruptReason reason(
       content::DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
 
+  EXPECT_CALL(item, UpdateProgress(size, 0, hash_state));
   EXPECT_CALL(item, Interrupt(reason));
-  download_manager_->OnDownloadInterrupted(download_id, reason);
+  download_manager_->OnDownloadInterrupted(
+      download_id, size, hash_state, reason);
   EXPECT_EQ(&item, GetActiveDownloadItem(download_id));
 }
 
diff --git a/content/browser/download/mock_download_file.h b/content/browser/download/mock_download_file.h
index 34bcb54..8608136 100644
--- a/content/browser/download/mock_download_file.h
+++ b/content/browser/download/mock_download_file.h
@@ -29,9 +29,6 @@ class MockDownloadFile : virtual public content::DownloadFile {
       const char* data, size_t data_len));
   MOCK_METHOD1(Rename, content::DownloadInterruptReason(
       const FilePath& full_path));
-  MOCK_METHOD3(Rename, void(const FilePath& full_path,
-                            bool overwrite_existing_file,
-                            const RenameCompletionCallback& callback));
   MOCK_METHOD0(Detach, void());
   MOCK_METHOD0(Cancel, void());
   MOCK_METHOD0(Finish, void());
@@ -43,7 +40,6 @@ class MockDownloadFile : virtual public content::DownloadFile {
   MOCK_METHOD1(GetHash, bool(std::string* hash));
   MOCK_METHOD0(GetHashState, std::string());
   MOCK_METHOD0(CancelDownloadRequest, void());
-  MOCK_METHOD0(SendUpdate, void());
   MOCK_CONST_METHOD0(Id, int());
   MOCK_METHOD0(GetDownloadManager, content::DownloadManager*());
   MOCK_CONST_METHOD0(GlobalId, const content::DownloadId&());