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

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;

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::DownloadInterruptReason DownloadFileImpl::Rename(
+    const FilePath& full_path) {
+  return content::ConvertNetErrorToInterruptReason(
+      file_.Rename(full_path),
       content::DOWNLOAD_INTERRUPT_FROM_DISK);
-
-    new_path.clear();
-  }
-
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(callback, reason, new_path));
 }
 
 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());

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_;
 

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 {

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:

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));
+
+    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));
+    }
 
-    content::DownloadFile::RenameCompletionCallback passed_callback(
+    download_file_manager_->RenameDownloadFile(
+        id, new_path, (should_overwrite == OVERWRITE),
         base::Bind(&TestDownloadManager::OnDownloadRenamed,
                    download_manager_, id.local()));
 
-    download_file_manager_->RenameDownloadFile(
-        id, new_path, should_overwrite, passed_callback);
-
-    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);
 }

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);
 }
 

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();

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.

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);
 }
 

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;

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));
 }
 

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&());

content/public/browser/download_manager.h

@@ -152,6 +152,8 @@ class CONTENT_EXPORT DownloadManager
   // |reason| is a download interrupt reason code.
   virtual void OnDownloadInterrupted(
       int32 download_id,
+      int64 size,
+      const std::string& hash_state,
       DownloadInterruptReason reason) = 0;
 
   // Remove downloads after remove_begin (inclusive) and before remove_end

content/public/test/mock_download_manager.h

@@ -49,8 +49,10 @@ class MockDownloadManager : public content::DownloadManager {
                                          int64 size,
                                          const std::string& hash));
   MOCK_METHOD1(CancelDownload, void(int32 download_id));
-  MOCK_METHOD2(OnDownloadInterrupted,
+  MOCK_METHOD4(OnDownloadInterrupted,
                void(int32 download_id,
+                    int64 size,
+                    const std::string& hash_state,
                     content::DownloadInterruptReason reason));
   MOCK_METHOD2(RemoveDownloadsBetween, int(base::Time remove_begin,
                                            base::Time remove_end));

content/test/test_file_error_injector.cc

@@ -55,9 +55,8 @@ class DownloadFileWithErrors: public DownloadFileImpl {
   virtual content::DownloadInterruptReason Initialize() OVERRIDE;
   virtual content::DownloadInterruptReason AppendDataToFile(
       const char* data, size_t data_len) 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;
 
  private:
   // Error generating helper.
@@ -65,13 +64,6 @@ class DownloadFileWithErrors: public DownloadFileImpl {
       content::TestFileErrorInjector::FileOperationCode code,
       content::DownloadInterruptReason original_error);
 
-  // Used in place of original rename callback to intercept with
-  // ShouldReturnError.
-  void RenameErrorCallback(
-    const RenameCompletionCallback& original_callback,
-    content::DownloadInterruptReason original_error,
-    const FilePath& path_result);
-
   // Source URL for the file being downloaded.
   GURL source_url_;
 
@@ -126,16 +118,11 @@ content::DownloadInterruptReason DownloadFileWithErrors::AppendDataToFile(
       DownloadFileImpl::AppendDataToFile(data, data_len));
 }
 
-void DownloadFileWithErrors::Rename(
-    const FilePath& full_path,
-    bool overwrite_existing_file,
-    const RenameCompletionCallback& callback) {
-  DownloadFileImpl::Rename(
-      full_path, overwrite_existing_file,
-      base::Bind(&DownloadFileWithErrors::RenameErrorCallback,
-                 // Unretained since this'll only be called from
-                 // the DownloadFileImpl slice of the same object.
-                 base::Unretained(this), callback));
+content::DownloadInterruptReason DownloadFileWithErrors::Rename(
+    const FilePath& full_path) {
+  return ShouldReturnError(
+      content::TestFileErrorInjector::FILE_OPERATION_RENAME,
+      DownloadFileImpl::Rename(full_path));
 }
 
 content::DownloadInterruptReason DownloadFileWithErrors::ShouldReturnError(
@@ -165,15 +152,6 @@ content::DownloadInterruptReason DownloadFileWithErrors::ShouldReturnError(
   return error_info_.error;
 }
 
-void DownloadFileWithErrors::RenameErrorCallback(
-    const RenameCompletionCallback& original_callback,
-    content::DownloadInterruptReason original_error,
-    const FilePath& path_result) {
-  original_callback.Run(ShouldReturnError(
-      content::TestFileErrorInjector::FILE_OPERATION_RENAME,
-      original_error), path_result);
-}
-
 }  // namespace
 
 namespace content {
@@ -189,7 +167,7 @@ class DownloadFileWithErrorsFactory
   virtual ~DownloadFileWithErrorsFactory();
 
   // DownloadFileFactory interface.
-  virtual DownloadFile* CreateFile(
+  virtual content::DownloadFile* CreateFile(
       DownloadCreateInfo* info,
       scoped_ptr<content::ByteStreamReader> stream,
       content::DownloadManager* download_manager,