Move Rename functionality from DownloadFileManager to DownloadFileImple.

content/browser/download/download_file_manager_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/download_file_manager.h"
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
@@ skipping 13 matching lines @@
 
 using content::BrowserThread;
 using content::BrowserThreadImpl;
 using content::DownloadId;
 using content::MockDownloadManager;
 
 using ::testing::_;
 using ::testing::AtLeast;
 using ::testing::Mock;
 using ::testing::Return;
+using ::testing::SaveArg;
 using ::testing::StrictMock;
 using ::testing::StrEq;
 
 namespace {
 
 // MockDownloadManager with the addition of a mock callback used for testing.
 class TestDownloadManager : public MockDownloadManager {
  public:
-  MOCK_METHOD2(OnDownloadRenamed,
-               void(int download_id, const FilePath& full_path));
+  MOCK_METHOD3(OnDownloadRenamed,
+               void(int download_id,
+                    content::DownloadInterruptReason reason,
+                    const FilePath& full_path));
  private:
   ~TestDownloadManager() {}
 };
 
 class MockDownloadFileFactory :
     public DownloadFileManager::DownloadFileFactory {
 
  public:
   MockDownloadFileFactory() {}
   virtual ~MockDownloadFileFactory() {}
@@ skipping 151 matching lines @@
   //                renamed to. If there is an existing file at
   //                |new_path| and |replace| is false, then |new_path|
   //                will be uniquified.
   // |rename_error| is the error to inject.  For no error,
   //                use content::DOWNLOAD_INTERRUPT_REASON_NONE.
   // |state|        whether we are renaming an in-progress download or a
   //                completed download.
   // |should_overwrite| indicates whether to replace or uniquify the file.
   void RenameFile(const DownloadId& id,
                   const FilePath& new_path,
                   const FilePath& unique_path,

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

|unique_path| should be unused.

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

Yeah--I got lazy here since this file is about to go away.  But temporary
solutions aren't; I've cleaned it up.

Done.

                   content::DownloadInterruptReason rename_error,

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

|rename_error| is unused.

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

Done.

                   RenameFileState state,

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

|state| is unused now.

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

Done.

                   RenameFileOverwrite should_overwrite) {
     MockDownloadFile* file = download_file_factory_->GetExistingFile(id);
     ASSERT_TRUE(file != NULL);
+    content::DownloadFile::RenameCompletionCallback rename_callback;
 
-    EXPECT_CALL(*file, Rename(unique_path))
-        .Times(1)
-        .WillOnce(Return(rename_error));
+    EXPECT_CALL(*file, Rename(unique_path, (should_overwrite == OVERWRITE), _))

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

Should be expecting Rename(new_path,...) since unique_path hasn't be determined
at the time download_file->Rename() is called.

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

Done.

+        .WillOnce(SaveArg<2>(&rename_callback));
 
-    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 == OVERWRITE),
+    content::DownloadFile::RenameCompletionCallback passed_callback(
         base::Bind(&TestDownloadManager::OnDownloadRenamed,
                    download_manager_, id.local()));
 
-    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();
-    }
+    download_file_manager_->RenameDownloadFile(
+        id, new_path, (should_overwrite == OVERWRITE), passed_callback);
+
+    EXPECT_TRUE(rename_callback.Equals(passed_callback));
   }
 
   void Complete(DownloadId id) {
     MockDownloadFile* file = download_file_factory_->GetExistingFile(id);
     ASSERT_TRUE(file != NULL);
 
     EXPECT_CALL(*file, AnnotateWithSourceInformation())
         .WillOnce(Return());
     EXPECT_CALL(*file, Detach())
         .WillOnce(Return());
@@ skipping 71 matching lines @@
 TEST_F(DownloadFileManagerTest, Complete) {
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
   info->download_id = dummy_id;
 
   CreateDownloadFile(info.Pass());
 
   Complete(dummy_id);
 }
 
 TEST_F(DownloadFileManagerTest, RenameInProgress) {

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

The state of the download (whether it's IN_PROGRESS or COMPLETE) is now
irrelevant to DFM during a rename.

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

Done.

   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) {

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

This can't be tested here anymore. See suggestion for adding test in
download_file_unittest.cc.

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

Done.

   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_FILE_NAME_TOO_LONG,
              IN_PROGRESS, OVERWRITE);
 
   CleanUp(dummy_id);
 }
 
-TEST_F(DownloadFileManagerTest, RenameWithUniquification) {
-  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)")));
-  // 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);
-}
-
 TEST_F(DownloadFileManagerTest, RenameTwice) {
   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 crfoo(download_dir.path().Append(
@@ skipping 45 matching lines @@
 
   CleanUp(dummy_id);
 }
 
 // TODO(ahendrickson) -- A test for download manager shutdown.
 // Expected call sequence:
 //  OnDownloadManagerShutdown
 //    DownloadFile::GetDownloadManager
 //    DownloadFile::CancelDownloadRequest
 //    DownloadFile::~DownloadFile