Download filename determination refactor (3/3)

content/browser/download/download_item_impl_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 "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/threading/thread.h"
 #include "content/browser/download/byte_stream.h"
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file_manager.h"
@@ skipping 16 matching lines @@
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Property;
 using ::testing::Return;
 
 DownloadId::Domain kValidDownloadItemIdDomain = "valid DownloadId::Domain";
 
 namespace {
 class MockDelegate : public DownloadItemImplDelegate {
  public:
+  MockDelegate(DownloadFileManager* file_manager)
+      : file_manager_(file_manager) {
+  }
   MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const FilePath& path));
   MOCK_METHOD1(ShouldOpenDownload, bool(DownloadItemImpl* download));
   MOCK_METHOD1(CheckForFileRemoval, void(DownloadItemImpl* download));
   MOCK_METHOD1(MaybeCompleteDownload, void(DownloadItemImpl* download));
   MOCK_CONST_METHOD0(GetBrowserContext, content::BrowserContext*());
   MOCK_METHOD1(DownloadStopped, void(DownloadItemImpl* download));
   MOCK_METHOD1(DownloadCompleted, void(DownloadItemImpl* download));
   MOCK_METHOD1(DownloadOpened, void(DownloadItemImpl* download));
   MOCK_METHOD1(DownloadRemoved, void(DownloadItemImpl* download));
   MOCK_METHOD1(DownloadRenamedToIntermediateName,
                void(DownloadItemImpl* download));
   MOCK_METHOD1(DownloadRenamedToFinalName, void(DownloadItemImpl* download));
   MOCK_CONST_METHOD1(AssertStateConsistent, void(DownloadItemImpl* download));
+  virtual DownloadFileManager* GetDownloadFileManager() OVERRIDE {
+    return file_manager_;
+  }
+ private:
+  DownloadFileManager* file_manager_;
 };
 
 class MockRequestHandle : public DownloadRequestHandleInterface {
  public:
   MOCK_CONST_METHOD0(GetWebContents, WebContents*());
   MOCK_CONST_METHOD0(GetDownloadManager, DownloadManager*());
   MOCK_CONST_METHOD0(PauseRequest, void());
   MOCK_CONST_METHOD0(ResumeRequest, void());
   MOCK_CONST_METHOD0(CancelRequest, void());
   MOCK_CONST_METHOD0(DebugString, std::string());
@@ skipping 80 matching lines @@
       return was_updated;
     }
 
    private:
     DownloadItem* item_;
     bool updated_;
   };
 
   DownloadItemTest()
       : ui_thread_(BrowserThread::UI, &loop_),
-        file_thread_(BrowserThread::FILE, &loop_) {
+        file_thread_(BrowserThread::FILE, &loop_),
+        file_manager_(new MockDownloadFileManager),
+        delegate_(file_manager_.get()) {
   }
 
   ~DownloadItemTest() {
   }
 
   virtual void SetUp() {
   }
 
   virtual void TearDown() {
     ui_thread_.DeprecatedGetThreadObject()->message_loop()->RunAllPending();
@@ skipping 34 matching lines @@
   }
 
   void RunAllPendingInMessageLoops() {
     loop_.RunAllPending();
   }
 
   MockDelegate* mock_delegate() {
     return &delegate_;
   }
 
+  MockDownloadFileManager* mock_file_manager() {
+    return file_manager_.get();
+  }
+
  private:
   MessageLoopForUI loop_;
   content::TestBrowserThread ui_thread_;    // UI thread
   content::TestBrowserThread file_thread_;  // FILE thread
+  scoped_refptr<MockDownloadFileManager> file_manager_;
   testing::NiceMock<MockDelegate> delegate_;
   std::set<DownloadItem*> allocated_downloads_;
 };
 
 namespace {
 
 const int kDownloadChunkSize = 1000;
 const int kDownloadSpeed = 1000;
 const int kDummyDBHandle = 10;
 const FilePath::CharType kDummyPath[] = FILE_PATH_LITERAL("/testpath");
 
 } // namespace
 
 // Tests to ensure calls that change a DownloadItem generate an update to
 // observers.
 // State changing functions not tested:
 //  void OpenDownload();
 //  void ShowDownloadInShell();
 //  void CompleteDelayedDownload();
-//  void OnDownloadCompleting(DownloadFileManager* file_manager);
+//  void OnDownloadCompleting();
 //  set_* mutators
 
 TEST_F(DownloadItemTest, NotificationAfterUpdate) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->UpdateProgress(kDownloadChunkSize, kDownloadSpeed, "");
   ASSERT_TRUE(observer.CheckUpdated());
   EXPECT_EQ(kDownloadSpeed, item->CurrentSpeed());
 }
@@ skipping 49 matching lines @@
 }
 
 TEST_F(DownloadItemTest, NotificationAfterRemove) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->Remove();
   ASSERT_TRUE(observer.CheckUpdated());
 }
 
-TEST_F(DownloadItemTest, NotificationAfterOnTargetPathDetermined) {
-  DownloadItemImpl* safe_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
-  MockObserver safe_observer(safe_item);
-
-  // Calling OnTargetPathDetermined triggers notification regardless of danger
-  // type.
-  safe_item->OnTargetPathDetermined(
-      FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-      content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  EXPECT_FALSE(safe_observer.CheckUpdated());
-
-  DownloadItemImpl* dangerous_item =
-      CreateDownloadItem(DownloadItem::IN_PROGRESS);
-  MockObserver dangerous_observer(dangerous_item);
-
-  // Calling OnTargetPathDetermined does trigger notification if danger type
-  // anything other than NOT_DANGEROUS.
-  dangerous_item->OnTargetPathDetermined(
-      FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-      content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
-  EXPECT_FALSE(dangerous_observer.CheckUpdated());
-}
-
-TEST_F(DownloadItemTest, NotificationAfterOnTargetPathSelected) {
-  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
-  MockObserver observer(item);
-
-  item->OnTargetPathDetermined(
-      FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_PROMPT,
-      content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  item->OnTargetPathSelected(FilePath(kDummyPath));
-  EXPECT_FALSE(observer.CheckUpdated());
-}
-
 TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   // Setting to NOT_DANGEROUS does not trigger a notification.
   DownloadItemImpl* safe_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver safe_observer(safe_item);
 
-  safe_item->OnTargetPathDetermined(
-      FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-      content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  EXPECT_FALSE(safe_observer.CheckUpdated());
   safe_item->OnAllDataSaved(1, "");
   EXPECT_TRUE(safe_observer.CheckUpdated());
   safe_item->OnContentCheckCompleted(
       content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   EXPECT_TRUE(safe_observer.CheckUpdated());
 
   // Setting to unsafe url or unsafe file should trigger a notification.
   DownloadItemImpl* unsafeurl_item =
       CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver unsafeurl_observer(unsafeurl_item);
 
-  unsafeurl_item->OnTargetPathDetermined(
-      FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-      content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  EXPECT_FALSE(unsafeurl_observer.CheckUpdated());
   unsafeurl_item->OnAllDataSaved(1, "");
   EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
   unsafeurl_item->OnContentCheckCompleted(
       content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL);
   EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
 
   unsafeurl_item->DangerousDownloadValidated();
   EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
 
   DownloadItemImpl* unsafefile_item =
       CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver unsafefile_observer(unsafefile_item);
 
-  unsafefile_item->OnTargetPathDetermined(
-      FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-      content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  EXPECT_FALSE(unsafefile_observer.CheckUpdated());
   unsafefile_item->OnAllDataSaved(1, "");
   EXPECT_TRUE(unsafefile_observer.CheckUpdated());
   unsafefile_item->OnContentCheckCompleted(
       content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
   EXPECT_TRUE(unsafefile_observer.CheckUpdated());
 
   unsafefile_item->DangerousDownloadValidated();
   EXPECT_TRUE(unsafefile_observer.CheckUpdated());
 }
 
-// DownloadItemImpl::OnIntermediatePathDetermined will schedule a task to run
+// DownloadItemImpl::OnDownloadTargetDetermined will schedule a task to run
 // DownloadFileManager::RenameDownloadFile(). Once the rename
 // completes, DownloadItemImpl receives a notification with the new file
 // name. Check that observers are updated when the new filename is available and
 // not before.
-TEST_F(DownloadItemTest, NotificationAfterOnIntermediatePathDetermined) {
+TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
-  FilePath intermediate_path(kDummyPath);
-  FilePath new_intermediate_path(intermediate_path.AppendASCII("foo"));
-  scoped_refptr<MockDownloadFileManager> file_manager(
-      new MockDownloadFileManager);
-  EXPECT_CALL(*file_manager.get(),
+  FilePath target_path(kDummyPath);
+  FilePath intermediate_path(target_path.InsertBeforeExtensionASCII("x"));
+  FilePath new_intermediate_path(target_path.InsertBeforeExtensionASCII("y"));
+  EXPECT_CALL(*mock_file_manager(),
               RenameDownloadFile(_,intermediate_path,false,_))
       .WillOnce(ScheduleRenameCallback(new_intermediate_path));
 
-  item->OnIntermediatePathDetermined(file_manager.get(), intermediate_path);
+  // Currently, a notification would be generated if the danger type is anything
+  // other than NOT_DANGEROUS.
+  item->OnDownloadTargetDetermined(target_path,
+                                   DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                                   content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                                   intermediate_path);
   EXPECT_FALSE(observer.CheckUpdated());
   RunAllPendingInMessageLoops();
   EXPECT_TRUE(observer.CheckUpdated());
   EXPECT_EQ(new_intermediate_path, item->GetFullPath());
 }
 
 TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->TogglePause();
   ASSERT_TRUE(observer.CheckUpdated());
 
   item->TogglePause();
   ASSERT_TRUE(observer.CheckUpdated());
 }
 
 TEST_F(DownloadItemTest, DisplayName) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
-  item->OnTargetPathDetermined(FilePath(kDummyPath).AppendASCII("foo.bar"),
-                               DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                               content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
+  FilePath target_path(FilePath(kDummyPath).AppendASCII("foo.bar"));
+  FilePath intermediate_path(target_path.InsertBeforeExtensionASCII("x"));
+  EXPECT_EQ(FILE_PATH_LITERAL(""),
+            item->GetFileNameToReportUser().value());
+  EXPECT_CALL(*mock_file_manager(),
+              RenameDownloadFile(_,_,false,_))
+      .WillOnce(ScheduleRenameCallback(intermediate_path));
+  item->OnDownloadTargetDetermined(target_path,
+                                   DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                                   content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                                   intermediate_path);
+  RunAllPendingInMessageLoops();
   EXPECT_EQ(FILE_PATH_LITERAL("foo.bar"),
             item->GetFileNameToReportUser().value());
   item->SetDisplayName(FilePath(FILE_PATH_LITERAL("new.name")));
   EXPECT_EQ(FILE_PATH_LITERAL("new.name"),
             item->GetFileNameToReportUser().value());
 }
 
 static char external_data_test_string[] = "External data test";
 static int destructor_called = 0;
 
@@ skipping 61 matching lines @@
   EXPECT_EQ(3, destructor_called);
 }
 
 // Test that the delegate is invoked after the download file is renamed.
 // Delegate::DownloadRenamedToIntermediateName() should be invoked when the
 // download is renamed to the intermediate name.
 // Delegate::DownloadRenamedToFinalName() should be invoked after the final
 // rename.
 TEST_F(DownloadItemTest, CallbackAfterRename) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
-  FilePath intermediate_path(kDummyPath);
-  FilePath new_intermediate_path(intermediate_path.AppendASCII("foo"));
-  FilePath final_path(intermediate_path.AppendASCII("bar"));
-  scoped_refptr<MockDownloadFileManager> file_manager(
-      new MockDownloadFileManager);
-  EXPECT_CALL(*file_manager.get(),
+  FilePath final_path(FilePath(kDummyPath).AppendASCII("foo.bar"));
+  FilePath intermediate_path(final_path.InsertBeforeExtensionASCII("x"));
+  FilePath new_intermediate_path(final_path.InsertBeforeExtensionASCII("y"));
+  EXPECT_CALL(*mock_file_manager(),
               RenameDownloadFile(item->GetGlobalId(),
                                  intermediate_path, false, _))
       .WillOnce(ScheduleRenameCallback(new_intermediate_path));
   // DownloadItemImpl should invoke this callback on the delegate once the
   // download is renamed to the intermediate name. Also check that GetFullPath()
   // returns the intermediate path at the time of the call.
   EXPECT_CALL(*mock_delegate(),
               DownloadRenamedToIntermediateName(
                   AllOf(item,
                         Property(&DownloadItem::GetFullPath,
                                  new_intermediate_path))));
-  item->OnTargetPathDetermined(final_path,
-                               DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                               content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  item->OnIntermediatePathDetermined(file_manager.get(), intermediate_path);
+  item->OnDownloadTargetDetermined(final_path,
+                                   DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                                   content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                                   intermediate_path);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
-  ::testing::Mock::VerifyAndClearExpectations(file_manager.get());
+  ::testing::Mock::VerifyAndClearExpectations(mock_file_manager());
   ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
 
   item->OnAllDataSaved(10, "");
-  EXPECT_CALL(*file_manager.get(),
+  EXPECT_CALL(*mock_file_manager(),
               RenameDownloadFile(item->GetGlobalId(),
-                                           final_path, true, _))
+                                 final_path, true, _))
       .WillOnce(ScheduleRenameCallback(final_path));
-  EXPECT_CALL(*file_manager.get(),
+  EXPECT_CALL(*mock_file_manager(),
               CompleteDownload(item->GetGlobalId(), _))
       .WillOnce(ScheduleCompleteCallback());
   // DownloadItemImpl should invoke this callback on the delegate after the
   // final rename has completed. Also check that GetFullPath() and
   // GetTargetFilePath() return the final path at the time of the call.
   EXPECT_CALL(*mock_delegate(),
               DownloadRenamedToFinalName(
                   AllOf(item,
                         Property(&DownloadItem::GetFullPath, final_path),
                         Property(&DownloadItem::GetTargetFilePath,
                                  final_path))));
   EXPECT_CALL(*mock_delegate(), DownloadCompleted(item));
   EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item))
       .WillOnce(Return(true));
-  item->OnDownloadCompleting(file_manager.get());
+  item->OnDownloadCompleting();
   RunAllPendingInMessageLoops();
-  ::testing::Mock::VerifyAndClearExpectations(file_manager.get());
+  ::testing::Mock::VerifyAndClearExpectations(mock_file_manager());
   ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
 }
 
 TEST_F(DownloadItemTest, Interrupted) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
 
   const content::DownloadInterruptReason reason(
       content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
 
   // Confirm interrupt sets state properly.
@@ skipping 21 matching lines @@
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
 
   EXPECT_FALSE(item->GetFileExternallyRemoved());
   item->OnDownloadedFileRemoved();
   EXPECT_TRUE(item->GetFileExternallyRemoved());
 }
 
 TEST(MockDownloadItem, Compiles) {
   MockDownloadItem mock_item;
 }