Replace the DownloadFileManager with direct ownership

content/browser/download/download_item_impl_unittest.cc

Index: content/browser/download/download_item_impl_unittest.cc
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index cbc583852176f42d7d48e7e7ea50d7732b604998..a5d22a2223cb71a1087857730c71a3a5e8f04ecf 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -7,11 +7,13 @@
 #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"
+#include "content/browser/download/download_file_factory.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/browser/download/download_item_impl_delegate.h"
 #include "content/browser/download/download_request_handle.h"
+#include "content/browser/download/mock_download_file.h"
 #include "content/public/browser/download_id.h"
+#include "content/public/browser/download_destination_observer.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/test/mock_download_item.h"
 #include "content/public/test/test_browser_thread.h"
@@ -28,14 +30,16 @@ using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Property;
 using ::testing::Return;
+using ::testing::StrictMock;
 
 DownloadId::Domain kValidDownloadItemIdDomain = "valid DownloadId::Domain";
 
 namespace {
 class MockDelegate : public DownloadItemImplDelegate {
  public:
-  MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const FilePath& path));
+  MOCK_METHOD1(DelegateStart, void(DownloadItemImpl* download));
   MOCK_METHOD1(ShouldOpenDownload, bool(DownloadItemImpl* download));
+  MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const FilePath& path));
   MOCK_METHOD1(CheckForFileRemoval, void(DownloadItemImpl* download));
   MOCK_METHOD1(MaybeCompleteDownload, void(DownloadItemImpl* download));
   MOCK_CONST_METHOD0(GetBrowserContext, content::BrowserContext*());
@@ -59,53 +63,15 @@ class MockRequestHandle : public DownloadRequestHandleInterface {
   MOCK_CONST_METHOD0(DebugString, std::string());
 };
 
-class MockDownloadFileFactory
-    : public DownloadFileManager::DownloadFileFactory {
- public:
-  content::DownloadFile* CreateFile(
-      DownloadCreateInfo* info,
-      scoped_ptr<content::ByteStreamReader> stream_reader,
-      DownloadManager* mgr,
-      bool calculate_hash,
-      const net::BoundNetLog& bound_net_log) {
-    return MockCreateFile(
-        info, stream_reader.get(), info->request_handle, mgr, calculate_hash,
-        bound_net_log);
-  }
-
-  MOCK_METHOD6(MockCreateFile,
-               content::DownloadFile*(DownloadCreateInfo*,
-                                      content::ByteStreamReader*,
-                                      const DownloadRequestHandle&,
-                                      DownloadManager*,
-                                      bool,
-                                      const net::BoundNetLog&));
-};
-
-class MockDownloadFileManager : public DownloadFileManager {
- public:
-  MockDownloadFileManager();
-  MOCK_METHOD0(Shutdown, void());
-  MOCK_METHOD1(CancelDownload, void(DownloadId));
-  MOCK_METHOD2(CompleteDownload, void(DownloadId, const base::Closure&));
-  MOCK_METHOD1(OnDownloadManagerShutdown, void(DownloadManager*));
-  MOCK_METHOD4(RenameDownloadFile, void(DownloadId, const FilePath&, bool,
-                                        const RenameCompletionCallback&));
-  MOCK_CONST_METHOD0(NumberOfActiveDownloads, int());
- private:
-  ~MockDownloadFileManager() {}
-};
-
 // Schedules a task to invoke the RenameCompletionCallback with |new_path| on
 // the UI thread. Should only be used as the action for
-// MockDownloadFileManager::Rename*DownloadFile as follows:
-//   EXPECT_CALL(mock_download_file_manager,
-//               RenameDownloadFile(_,_,_,_))
+// MockDownloadFile::Rename as follows:
+//   EXPECT_CALL(download_file, Rename(_,_,_))
 //       .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));
+      base::Bind(arg2, content::DOWNLOAD_INTERRUPT_REASON_NONE, new_path));
 }
 
 // Similarly for scheduling a completion callback.
@@ -113,10 +79,6 @@ ACTION(ScheduleCompleteCallback) {
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(arg1));
 }
 
-MockDownloadFileManager::MockDownloadFileManager()
-    : DownloadFileManager(new MockDownloadFileFactory) {
-}
-
 }  // namespace
 
 class DownloadItemTest : public testing::Test {
@@ -188,6 +150,22 @@ class DownloadItemTest : public testing::Test {
     return download;
   }
 
+  // Add DownloadFile to DownloadItem
+  MockDownloadFile* AddDownloadFileToDownloadItem(DownloadItemImpl* item) {
+    MockDownloadFile* mock_download_file(new StrictMock<MockDownloadFile>);
+    scoped_ptr<content::DownloadFile> download_file(mock_download_file);
+    EXPECT_CALL(*mock_download_file, Initialize(_));
+    item->Start(download_file.Pass());
+    loop_.RunAllPending();
+    return mock_download_file;
+  }
+
+  // Cleanup a download item (specifically get rid of the DownloadFile on it).
+  void CleanupItem(DownloadItemImpl* item, MockDownloadFile* download_file) {
+    EXPECT_CALL(*download_file, Cancel());
+    item->OffThreadCancel();
+  }
+
   // Destroy a previously created download item.
   void DestroyDownloadItem(DownloadItem* item) {
     allocated_downloads_.erase(item);
@@ -225,7 +203,7 @@ const FilePath::CharType kDummyPath[] = FILE_PATH_LITERAL("/testpath");
 //  void OpenDownload();
 //  void ShowDownloadInShell();
 //  void CompleteDelayedDownload();
-//  void OnDownloadCompleting(DownloadFileManager* file_manager);
+//  void OnDownloadCompleting();
 //  set_* mutators
 
 TEST_F(DownloadItemTest, NotificationAfterUpdate) {
@@ -256,7 +234,7 @@ TEST_F(DownloadItemTest, NotificationAfterComplete) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
-  item->OnAllDataSaved(kDownloadChunkSize, DownloadItem::kEmptyFileHash);
+  item->OnAllDataSaved(DownloadItem::kEmptyFileHash);
   ASSERT_TRUE(observer.CheckUpdated());
 
   item->MarkAsComplete();
@@ -338,7 +316,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
       FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
       content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   EXPECT_FALSE(safe_observer.CheckUpdated());
-  safe_item->OnAllDataSaved(1, "");
+  safe_item->OnAllDataSaved("");
   EXPECT_TRUE(safe_observer.CheckUpdated());
   safe_item->OnContentCheckCompleted(
       content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
@@ -353,7 +331,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
       FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
       content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   EXPECT_FALSE(unsafeurl_observer.CheckUpdated());
-  unsafeurl_item->OnAllDataSaved(1, "");
+  unsafeurl_item->OnAllDataSaved("");
   EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
   unsafeurl_item->OnContentCheckCompleted(
       content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL);
@@ -370,7 +348,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
       FilePath(kDummyPath), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
       content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   EXPECT_FALSE(unsafefile_observer.CheckUpdated());
-  unsafefile_item->OnAllDataSaved(1, "");
+  unsafefile_item->OnAllDataSaved("");
   EXPECT_TRUE(unsafefile_observer.CheckUpdated());
   unsafefile_item->OnContentCheckCompleted(
       content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
@@ -381,26 +359,27 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
 }
 
 // DownloadItemImpl::OnIntermediatePathDetermined will schedule a task to run
-// DownloadFileManager::RenameDownloadFile(). Once the rename
+// DownloadFile::Rename(). 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) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item);
   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(),
-              RenameDownloadFile(_,intermediate_path,false,_))
+  EXPECT_CALL(*download_file, Rename(intermediate_path, false, _))
       .WillOnce(ScheduleRenameCallback(new_intermediate_path));
 
-  item->OnIntermediatePathDetermined(file_manager.get(), intermediate_path);
+  item->OnIntermediatePathDetermined(intermediate_path);
   EXPECT_FALSE(observer.CheckUpdated());
   RunAllPendingInMessageLoops();
   EXPECT_TRUE(observer.CheckUpdated());
   EXPECT_EQ(new_intermediate_path, item->GetFullPath());
+
+  CleanupItem(item, download_file);
+  RunAllPendingInMessageLoops();
 }
 
 TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
@@ -426,6 +405,17 @@ TEST_F(DownloadItemTest, DisplayName) {
             item->GetFileNameToReportUser().value());
 }
 
+// Test to make sure that Start method calls DF initialize properly.
+TEST_F(DownloadItemTest, Start) {
+  MockDownloadFile* mock_download_file(new MockDownloadFile);
+  scoped_ptr<content::DownloadFile> download_file(mock_download_file);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  EXPECT_CALL(*mock_download_file, Initialize(_));
+  item->Start(download_file.Pass());
+
+  CleanupItem(item, mock_download_file);
+}
+
 static char external_data_test_string[] = "External data test";
 static int destructor_called = 0;
 
@@ -500,15 +490,13 @@ TEST_F(DownloadItemTest, ExternalData) {
 // rename.
 TEST_F(DownloadItemTest, CallbackAfterRename) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item);
   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(),
-              RenameDownloadFile(item->GetGlobalId(),
-                                 intermediate_path, false, _))
+  EXPECT_CALL(*download_file, Rename(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.
@@ -520,20 +508,16 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
   item->OnTargetPathDetermined(final_path,
                                DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                                content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-  item->OnIntermediatePathDetermined(file_manager.get(), intermediate_path);
+  item->OnIntermediatePathDetermined(intermediate_path);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
-  ::testing::Mock::VerifyAndClearExpectations(file_manager.get());
+  ::testing::Mock::VerifyAndClearExpectations(download_file);
   ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
 
-  item->OnAllDataSaved(10, "");
-  EXPECT_CALL(*file_manager.get(),
-              RenameDownloadFile(item->GetGlobalId(),
-                                           final_path, true, _))
+  item->OnAllDataSaved("");
+  EXPECT_CALL(*download_file, Rename(final_path, true, _))
       .WillOnce(ScheduleRenameCallback(final_path));
-  EXPECT_CALL(*file_manager.get(),
-              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.
@@ -546,9 +530,10 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
   EXPECT_CALL(*mock_delegate(), DownloadCompleted(item));
   EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item))
       .WillOnce(Return(true));
-  item->OnDownloadCompleting(file_manager.get());
+  EXPECT_CALL(*download_file, Detach());
+  item->OnDownloadCompleting();
   RunAllPendingInMessageLoops();
-  ::testing::Mock::VerifyAndClearExpectations(file_manager.get());
+  ::testing::Mock::VerifyAndClearExpectations(download_file);
   ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
 }
 
@@ -587,6 +572,86 @@ TEST_F(DownloadItemTest, FileRemoved) {
   EXPECT_TRUE(item->GetFileExternallyRemoved());
 }
 
+TEST_F(DownloadItemTest, DestinationUpdate) {
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  base::WeakPtr<content::DownloadDestinationObserver> as_observer(
+      item->DestinationObserverAsWeakPtr());
+  MockObserver observer(item);
+
+  EXPECT_EQ(0l, item->CurrentSpeed());
+  EXPECT_EQ("", item->GetHashState());
+  EXPECT_EQ(0l, item->GetReceivedBytes());
+  EXPECT_EQ(0l, item->GetTotalBytes());
+  EXPECT_FALSE(observer.CheckUpdated());
+  item->SetTotalBytes(100l);
+  EXPECT_EQ(100l, item->GetTotalBytes());
+
+  as_observer->DestinationUpdate(10, 20, "deadbeef");
+  EXPECT_EQ(20l, item->CurrentSpeed());
+  EXPECT_EQ("deadbeef", item->GetHashState());
+  EXPECT_EQ(10l, item->GetReceivedBytes());
+  EXPECT_EQ(100l, item->GetTotalBytes());
+  EXPECT_TRUE(observer.CheckUpdated());
+
+  as_observer->DestinationUpdate(200, 20, "livebeef");
+  EXPECT_EQ(20l, item->CurrentSpeed());
+  EXPECT_EQ("livebeef", item->GetHashState());
+  EXPECT_EQ(200l, item->GetReceivedBytes());
+  EXPECT_EQ(0l, item->GetTotalBytes());
+  EXPECT_TRUE(observer.CheckUpdated());
+}
+
+TEST_F(DownloadItemTest, DestinationError) {
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  base::WeakPtr<content::DownloadDestinationObserver> as_observer(
+      item->DestinationObserverAsWeakPtr());
+  MockObserver observer(item);
+
+  EXPECT_EQ(content::DownloadItem::IN_PROGRESS, item->GetState());
+  EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE, item->GetLastReason());
+  EXPECT_FALSE(observer.CheckUpdated());
+
+  EXPECT_CALL(*mock_delegate(), DownloadStopped(item));
+  as_observer->DestinationError(
+      content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
+  ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+  EXPECT_TRUE(observer.CheckUpdated());
+  EXPECT_EQ(content::DownloadItem::INTERRUPTED, item->GetState());
+  EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
+            item->GetLastReason());
+}
+
+TEST_F(DownloadItemTest, DestinationCompleted) {
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  base::WeakPtr<content::DownloadDestinationObserver> as_observer(
+      item->DestinationObserverAsWeakPtr());
+  MockObserver observer(item);
+
+  EXPECT_EQ(content::DownloadItem::IN_PROGRESS, item->GetState());
+  EXPECT_EQ("", item->GetHash());
+  EXPECT_EQ("", item->GetHashState());
+  EXPECT_FALSE(item->AllDataSaved());
+  EXPECT_FALSE(observer.CheckUpdated());
+
+  as_observer->DestinationUpdate(10, 20, "deadbeef");
+  EXPECT_TRUE(observer.CheckUpdated());
+  EXPECT_FALSE(observer.CheckUpdated()); // Confirm reset.
+  EXPECT_EQ(content::DownloadItem::IN_PROGRESS, item->GetState());
+  EXPECT_EQ("", item->GetHash());
+  EXPECT_EQ("deadbeef", item->GetHashState());
+  EXPECT_FALSE(item->AllDataSaved());
+
+  EXPECT_CALL(*mock_delegate(), MaybeCompleteDownload(item));
+  as_observer->DestinationCompleted("livebeef");
+  ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+  EXPECT_EQ(content::DownloadItem::IN_PROGRESS, item->GetState());
+  EXPECT_TRUE(observer.CheckUpdated());
+  EXPECT_EQ("livebeef", item->GetHash());
+  EXPECT_EQ("", item->GetHashState());
+  EXPECT_TRUE(item->AllDataSaved());
+}
+
 TEST(MockDownloadItem, Compiles) {
   MockDownloadItem mock_item;
 }
+