Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(763)

Unified Diff: content/browser/download/download_item_impl_unittest.cc

Issue 10799005: Replace the DownloadFileManager with direct ownership (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fixed try job problems. Created 8 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..05cb86375448713757a633cb6faf99cea1502a66 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_controller.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::DownloadDestinationController> as_controller(
+ item->DestinationControllerAsWeakPtr());
+ 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_controller->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_controller->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::DownloadDestinationController> as_controller(
+ item->DestinationControllerAsWeakPtr());
+ 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_controller->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::DownloadDestinationController> as_controller(
+ item->DestinationControllerAsWeakPtr());
+ 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_controller->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_controller->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;
}
+

Powered by Google App Engine
This is Rietveld 408576698