Rename the download to its final name only after the download is finished

chrome/browser/download/download_manager_unittest.cc

Index: chrome/browser/download/download_manager_unittest.cc
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index 70878632076e70aa2b9d1faeeb6754d80d9a00d2..c221a5d3419cfe5cdf58c47e07d607d8192ee2bc 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -7,10 +7,15 @@
 #include "base/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/download/download_file.h"
+#include "chrome/browser/download/download_file_manager.h"
 #include "chrome/browser/download/download_manager.h"
+#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/testing_profile.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
@@ -34,11 +39,19 @@
 #define TAR_EXT L".tar"
 #endif
 
+class MockDownloadManager : public DownloadManager {
+ public:
+  // Override some functions.
+  virtual void UpdateAppIcon() { }
+  virtual void UpdateHistoryForDownload(DownloadItem*) { }
+  virtual void ContinueDownloadFinished(DownloadItem*) { }
+};
+
 class DownloadManagerTest : public testing::Test {
  public:
   DownloadManagerTest()
       : profile_(new TestingProfile()),
-        download_manager_(new DownloadManager()),
+        download_manager_(new MockDownloadManager()),
         ui_thread_(ChromeThread::UI, &message_loop_) {
     download_manager_->Init(profile_.get());
   }
@@ -64,12 +77,25 @@ class DownloadManagerTest : public testing::Test {
     *generated_name_string = generated_name.ToWStringHack();
   }
 
+  void AddDownloadToFileManager(int id, DownloadFile* download) {
+    file_manager()->downloads_[id] = download;
+  }
+
  protected:
   scoped_ptr<TestingProfile> profile_;
   scoped_refptr<DownloadManager> download_manager_;
+  scoped_refptr<DownloadFileManager> file_manager_;
   MessageLoopForUI message_loop_;
   ChromeThread ui_thread_;
 
+  DownloadFileManager* file_manager() {
+    if (!file_manager_) {
+      file_manager_ = new DownloadFileManager(NULL);
+      download_manager_->file_manager_ = file_manager_;
+    }
+    return file_manager_;
+  }
+
   DISALLOW_COPY_AND_ASSIGN(DownloadManagerTest);
 };
 
@@ -695,3 +721,119 @@ TEST_F(DownloadManagerTest, StartDownload) {
         info.prompt_user_for_save_location);
   }
 }
+
+namespace {
+
+const struct {
+  FilePath::StringType suggested_path;
+  bool is_dangerous;
+  bool finish_before_rename;
+  bool will_delete_crdownload;
+  int expected_rename_count;
+} kDownloadRenameCases[] = {
+  // Safe download, download finishes BEFORE rename.
+  // Needs to be renamed only once.  Crdownload file needs to be deleted.
+  { FILE_PATH_LITERAL("foo.zip"),
+    false,
+    true,
+    true,
+    1, },
+  // Dangerous download, download finishes BEFORE rename.
+  // Needs to be renamed only once.
+  { FILE_PATH_LITERAL("unconfirmed xxx.crdownload"),
+    true,
+    true,
+    false,
+    1, },
+  // Safe download, download finishes AFTER rename.
+  // Needs to be renamed twice.
+  { FILE_PATH_LITERAL("foo.zip"),
+    false,
+    false,
+    false,
+    2, },
+  // Dangerous download, download finishes AFTER rename.
+  // Needs to be renamed only once.
+  { FILE_PATH_LITERAL("unconfirmed xxx.crdownload"),
+    true,
+    false,
+    false,
+    1, },
+};
+
+class MockDownloadFile : public DownloadFile {
+ public:
+  MockDownloadFile(DownloadCreateInfo* info)
+      : DownloadFile(info), renamed_count_(0) { }
+  virtual ~MockDownloadFile() { Destructed(); }
+  MOCK_METHOD2(Rename, bool(const FilePath&, bool));
+  MOCK_METHOD0(DeleteCrDownload, void());
+  MOCK_METHOD0(Destructed, void());
+
+  bool TestMultipleRename(
+      int expected_count, bool expected_final, const FilePath& expected,
+      const FilePath& path, bool is_final_rename) {
+    ++renamed_count_;
+    EXPECT_EQ(expected_count, renamed_count_);
+    EXPECT_EQ(expected_final, is_final_rename);
+    EXPECT_EQ(expected.value(), path.value());
+    return true;
+  }
+
+ private:
+  int renamed_count_;
+};
+
+}  // namespace
+
+TEST_F(DownloadManagerTest, DownloadRenameTest) {
+  using ::testing::_;
+  using ::testing::CreateFunctor;
+  using ::testing::Invoke;
+  using ::testing::Return;
+
+  ChromeThread file_thread(ChromeThread::FILE, &message_loop_);
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kDownloadRenameCases); ++i) {
+    // |info| will be destroyed in download_manager_.
+    DownloadCreateInfo* info(new DownloadCreateInfo);
+    info->download_id = static_cast<int>(i);
+    info->prompt_user_for_save_location = false;
+    info->is_dangerous = kDownloadRenameCases[i].is_dangerous;
+    FilePath new_path(kDownloadRenameCases[i].suggested_path);
+
+    MockDownloadFile* download(new MockDownloadFile(info));
+    AddDownloadToFileManager(info->download_id, download);
+
+    // |download| is owned by DownloadFileManager.
+    ::testing::Mock::AllowLeak(download);
+    EXPECT_CALL(*download, Destructed()).Times(1);
+
+    if (kDownloadRenameCases[i].expected_rename_count == 1) {
+      EXPECT_CALL(*download, Rename(new_path, true)).WillOnce(Return(true));
+    } else {
+      ASSERT_EQ(2, kDownloadRenameCases[i].expected_rename_count);
+      FilePath crdownload(download_util::GetCrDownloadPath(new_path));
+      EXPECT_CALL(*download, Rename(_, _))
+          .WillOnce(testing::WithArgs<0, 1>(Invoke(CreateFunctor(
+              download, &MockDownloadFile::TestMultipleRename,
+              1, false, crdownload))))
+          .WillOnce(testing::WithArgs<0, 1>(Invoke(CreateFunctor(
+              download, &MockDownloadFile::TestMultipleRename,
+              2, true, new_path))));
+    }
+
+    if (kDownloadRenameCases[i].will_delete_crdownload)
+      EXPECT_CALL(*download, DeleteCrDownload()).Times(1);
+
+    if (kDownloadRenameCases[i].finish_before_rename) {
+      download_manager_->DownloadFinished(i, 1024);
+      download_manager_->FileSelected(new_path, i, info);
+    } else {
+      download_manager_->FileSelected(new_path, i, info);
+      download_manager_->DownloadFinished(i, 1024);
+    }
+
+    message_loop_.RunAllPending();
+  }
+}