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

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

Issue 10392111: Use ByteStream in downloads system to decouple source and sink. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Sync'd to LKGR. Created 8 years, 6 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_file_manager_unittest.cc
diff --git a/content/browser/download/download_file_manager_unittest.cc b/content/browser/download/download_file_manager_unittest.cc
index c8556ba3fae5e94f8790ca7ce9e950a21741ea12..0845bc68426f65882ea046979da6fdd3398ec600 100644
--- a/content/browser/download/download_file_manager_unittest.cc
+++ b/content/browser/download/download_file_manager_unittest.cc
@@ -10,7 +10,7 @@
#include "base/scoped_temp_dir.h"
#include "base/string_number_conversions.h"
#include "content/browser/browser_thread_impl.h"
-#include "content/browser/download/download_buffer.h"
+#include "content/browser/download/byte_stream.h"
#include "content/browser/download/download_create_info.h"
#include "content/browser/download/download_interrupt_reasons_impl.h"
#include "content/browser/download/download_request_handle.h"
@@ -53,6 +53,7 @@ class MockDownloadFileFactory :
virtual content::DownloadFile* CreateFile(
DownloadCreateInfo* info,
+ scoped_ptr<content::ByteStreamReader> stream,
const DownloadRequestHandle& request_handle,
content::DownloadManager* download_manager,
bool calculate_hash,
@@ -66,6 +67,7 @@ class MockDownloadFileFactory :
content::DownloadFile* MockDownloadFileFactory::CreateFile(
DownloadCreateInfo* info,
+ scoped_ptr<content::ByteStreamReader> stream,
const DownloadRequestHandle& request_handle,
content::DownloadManager* download_manager,
bool calculate_hash,
@@ -136,8 +138,7 @@ class DownloadFileManagerTest : public testing::Test {
// calling Release() on |download_manager_| won't ever result in its
// destructor being called and we get a leak.
DownloadFileManagerTest()
- : download_buffer_(new content::DownloadBuffer()),
- ui_thread_(BrowserThread::UI, &loop_),
+ : ui_thread_(BrowserThread::UI, &loop_),
file_thread_(BrowserThread::FILE, &loop_) {
}
@@ -175,7 +176,8 @@ class DownloadFileManagerTest : public testing::Test {
// Start a download.
// |info| is the information needed to create a new download file.
// |id| is the download ID of the new download file.
- void StartDownload(DownloadCreateInfo* info, const DownloadId& id) {
+ void StartDownload(scoped_ptr<DownloadCreateInfo> info,
+ const DownloadId& id) {
// Expected call sequence:
// StartDownload
// DownloadManager::CreateDownloadItem
@@ -191,7 +193,7 @@ class DownloadFileManagerTest : public testing::Test {
info->download_id = id;
// Set expectations and return values.
- EXPECT_CALL(*download_manager_, CreateDownloadItem(info, _))
+ EXPECT_CALL(*download_manager_, CreateDownloadItem(info.get(), _))
.Times(1)
.WillOnce(Return(net::BoundNetLog()));
EXPECT_CALL(*download_manager_, GenerateFileHash())
@@ -199,94 +201,13 @@ class DownloadFileManagerTest : public testing::Test {
.WillRepeatedly(Return(false));
EXPECT_CALL(*download_manager_, StartDownload(id.local()));
- download_file_manager_->StartDownload(info, *request_handle_);
+ download_file_manager_->StartDownload(
+ info.Pass(), scoped_ptr<content::ByteStreamReader>(), *request_handle_);
ProcessAllPendingMessages();
ClearExpectations(id);
}
- // Creates a new |net::IOBuffer| of size |length| with |data|, and attaches
- // it to the download buffer.
- bool UpdateBuffer(const char* data, size_t length) {
- // We are passing ownership of this buffer to the download file manager.
- // NOTE: we are padding io_buffer with one extra character so that the
- // mock testing framework can treat it as a NULL-delimited string.
- net::IOBuffer* io_buffer = new net::IOBuffer(length + 1);
- // We need |AddRef()| because we do a |Release()| in |UpdateDownload()|.
- io_buffer->AddRef();
- memcpy(io_buffer->data(), data, length);
- io_buffer->data()[length] = 0;
-
- download_buffer_->AddData(io_buffer, length);
-
- return true;
- }
-
- // Updates the download file.
- // |id| is the download ID of the download file.
- // |data| is the buffer containing the data.
- // |length| is the length of the data buffer.
- // |error_to_insert| is the error to simulate. For no error, use net::OK.
- void UpdateDownload(const DownloadId& id,
- const char* data,
- size_t length,
- net::Error error_to_insert) {
- // Expected call sequence:
- // Create DownloadBuffer
- // UpdateDownload
- // GetDownloadFile
- // DownloadFile::AppendDataToFile
- //
- // On error:
- // DownloadFile::GetDownloadManager
- // DownloadFile::BytesSoFar
- // CancelDownload
- // Process one message in the message loop
- // DownloadManager::OnDownloadInterrupted
- EXPECT_TRUE(UpdateBuffer(data, length));
- MockDownloadFile* file = download_file_factory_->GetExistingFile(id);
- ASSERT_TRUE(file != NULL);
- byte_count_[id] += length;
-
- // Make sure our comparison string is NULL-terminated.
- char* expected_data = new char[length + 1];
- memcpy(expected_data, data, length);
- expected_data[length] = 0;
-
- EXPECT_CALL(*file, AppendDataToFile(StrEq(expected_data), length))
- .Times(1)
- .WillOnce(Return(error_to_insert));
-
- if (error_to_insert != net::OK) {
- EXPECT_CALL(*file, GetDownloadManager())
- .Times(AtLeast(1));
- EXPECT_CALL(*file, BytesSoFar())
- .Times(AtLeast(1))
- .WillRepeatedly(Return(byte_count_[id]));
- EXPECT_CALL(*file, GetHashState())
- .Times(AtLeast(1));
- EXPECT_CALL(*file, Cancel());
- }
-
- download_file_manager_->UpdateDownload(id, download_buffer_.get());
-
- if (error_to_insert != net::OK) {
- EXPECT_CALL(*download_manager_,
- OnDownloadInterrupted(
- id.local(),
- byte_count_[id],
- "",
- content::ConvertNetErrorToInterruptReason(
- error_to_insert, content::DOWNLOAD_INTERRUPT_FROM_DISK)));
-
- ProcessAllPendingMessages();
- ++error_count_[id];
- }
-
- ClearExpectations(id);
- delete [] expected_data;
- }
-
// Renames the download file.
// |id| is the download ID of the download file.
// |new_path| is the new file path.
@@ -357,67 +278,6 @@ class DownloadFileManagerTest : public testing::Test {
}
}
- // Finish the download operation.
- // |id| is the download ID of the download file.
- // |reason| is the interrupt reason to inject. For no interrupt, use
- // DOWNLOAD_INTERRUPT_REASON_NONE.
- // |security_string| is the extra security information, if interrupted.
- void OnResponseCompleted(const DownloadId& id,
- content::DownloadInterruptReason reason,
- const std::string& security_string) {
- // OnResponseCompleted
- // GetDownloadFile
- // DownloadFile::Finish
- // DownloadFile::GetDownloadManager
- // DownloadFile::BytesSoFar
- // Process one message in the message loop
- //
- // OK:
- // DownloadFile::GetHash
- // DownloadManager::OnResponseCompleted
- //
- // On Error:
- // DownloadManager::OnDownloadInterrupted
- MockDownloadFile* file = download_file_factory_->GetExistingFile(id);
- ASSERT_TRUE(file != NULL);
-
- EXPECT_CALL(*file, Finish());
- EXPECT_CALL(*file, GetDownloadManager());
- if (reason == content::DOWNLOAD_INTERRUPT_REASON_NONE) {
- EXPECT_CALL(*file, GetHash(_))
- .WillOnce(Return(false));
- } else {
- EXPECT_CALL(*file, GetHashState());
- }
- EXPECT_CALL(*file, BytesSoFar())
- .Times(AtLeast(1))
- .WillRepeatedly(Return(byte_count_[id]));
-
- download_file_manager_->OnResponseCompleted(id, reason, security_string);
-
- if (reason == content::DOWNLOAD_INTERRUPT_REASON_NONE) {
- EXPECT_CALL(*download_manager_,
- OnResponseCompleted(id.local(), byte_count_[id], ""))
- .Times(1);
- } else {
- EXPECT_EQ(0, error_count_[id]);
-
- EXPECT_CALL(*download_manager_,
- OnDownloadInterrupted(id.local(),
- byte_count_[id],
- "",
- reason))
- .Times(1);
- }
-
- ProcessAllPendingMessages();
-
- if (reason != content::DOWNLOAD_INTERRUPT_REASON_NONE)
- ++error_count_[id];
-
- ClearExpectations(id);
- }
-
void CleanUp(DownloadId id) {
// Expected calls:
// DownloadFileManager::CancelDownload
@@ -440,7 +300,6 @@ class DownloadFileManagerTest : public testing::Test {
scoped_ptr<MockDownloadRequestHandle> request_handle_;
MockDownloadFileFactory* download_file_factory_;
scoped_refptr<DownloadFileManager> download_file_manager_;
- scoped_refptr<content::DownloadBuffer> download_buffer_;
// Per-download statistics.
std::map<DownloadId, int64> byte_count_;
@@ -470,127 +329,65 @@ const int DownloadFileManagerTest::kDummyChildId = 3;
const int DownloadFileManagerTest::kDummyRequestId = 67;
TEST_F(DownloadFileManagerTest, CancelAtStart) {
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
- StartDownload(info, dummy_id);
+ StartDownload(info.Pass(), dummy_id);
CleanUp(dummy_id);
}
TEST_F(DownloadFileManagerTest, CancelBeforeFinished) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
- DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
-
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- CleanUp(dummy_id);
-}
-
-TEST_F(DownloadFileManagerTest, DownloadWithError) {
- // Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
- DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
-
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2),
- net::ERR_FILE_NO_SPACE);
-}
-
-TEST_F(DownloadFileManagerTest, CompleteDownload) {
- // Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
-
- CleanUp(dummy_id);
-}
-
-TEST_F(DownloadFileManagerTest, CompleteDownloadWithError) {
- // Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
- DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
-
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id,
- content::DOWNLOAD_INTERRUPT_REASON_FILE_VIRUS_INFECTED,
- "");
+ StartDownload(info.Pass(), dummy_id);
CleanUp(dummy_id);
}
TEST_F(DownloadFileManagerTest, RenameInProgress) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
ScopedTempDir download_dir;
ASSERT_TRUE(download_dir.CreateUniqueTempDir());
- StartDownload(info, dummy_id);
+ StartDownload(info.Pass(), dummy_id);
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
RenameFile(dummy_id, foo, foo, net::OK, IN_PROGRESS, OVERWRITE);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
CleanUp(dummy_id);
}
TEST_F(DownloadFileManagerTest, RenameInProgressWithUniquification) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
ScopedTempDir download_dir;
ASSERT_TRUE(download_dir.CreateUniqueTempDir());
- StartDownload(info, dummy_id);
+ StartDownload(info.Pass(), dummy_id);
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
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, net::OK, IN_PROGRESS, DONT_OVERWRITE);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
CleanUp(dummy_id);
}
TEST_F(DownloadFileManagerTest, RenameInProgressWithError) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
ScopedTempDir download_dir;
ASSERT_TRUE(download_dir.CreateUniqueTempDir());
- StartDownload(info, dummy_id);
+ StartDownload(info.Pass(), dummy_id);
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
RenameFile(dummy_id, foo, foo, net::ERR_FILE_PATH_TOO_LONG,
IN_PROGRESS, OVERWRITE);
@@ -598,41 +395,14 @@ TEST_F(DownloadFileManagerTest, RenameInProgressWithError) {
CleanUp(dummy_id);
}
-TEST_F(DownloadFileManagerTest, RenameCompleting) {
+TEST_F(DownloadFileManagerTest, RenameWithUniquification) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
ScopedTempDir download_dir;
ASSERT_TRUE(download_dir.CreateUniqueTempDir());
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
-
- FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
- RenameFile(dummy_id, foo, foo, net::OK, COMPLETE, OVERWRITE);
-
- CleanUp(dummy_id);
-}
-
-TEST_F(DownloadFileManagerTest, RenameCompletingWithUniquification) {
- // Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
- DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
- ScopedTempDir download_dir;
- ASSERT_TRUE(download_dir.CreateUniqueTempDir());
-
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
+ StartDownload(info.Pass(), dummy_id);
FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
FilePath unique_foo(foo.InsertBeforeExtension(FILE_PATH_LITERAL(" (1)")));
@@ -645,45 +415,18 @@ TEST_F(DownloadFileManagerTest, RenameCompletingWithUniquification) {
CleanUp(dummy_id);
}
-TEST_F(DownloadFileManagerTest, RenameCompletingWithError) {
- // Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
- DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
- ScopedTempDir download_dir;
- ASSERT_TRUE(download_dir.CreateUniqueTempDir());
-
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
-
- FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
- RenameFile(dummy_id, foo, foo, net::ERR_FILE_PATH_TOO_LONG,
- COMPLETE, OVERWRITE);
-
- CleanUp(dummy_id);
-}
-
TEST_F(DownloadFileManagerTest, RenameTwice) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
ScopedTempDir download_dir;
ASSERT_TRUE(download_dir.CreateUniqueTempDir());
- StartDownload(info, dummy_id);
+ StartDownload(info.Pass(), dummy_id);
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
FilePath crfoo(download_dir.path().Append(
FILE_PATH_LITERAL("foo.txt.crdownload")));
RenameFile(dummy_id, crfoo, crfoo, net::OK, IN_PROGRESS, OVERWRITE);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
FilePath foo(download_dir.path().Append(FILE_PATH_LITERAL("foo.txt")));
RenameFile(dummy_id, foo, foo, net::OK, COMPLETE, OVERWRITE);
@@ -693,22 +436,17 @@ TEST_F(DownloadFileManagerTest, RenameTwice) {
TEST_F(DownloadFileManagerTest, TwoDownloads) {
// Same as StartDownload, at first.
- DownloadCreateInfo* info = new DownloadCreateInfo;
- DownloadCreateInfo* info2 = new DownloadCreateInfo;
+ scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
+ scoped_ptr<DownloadCreateInfo> info2(new DownloadCreateInfo);
DownloadId dummy_id(download_manager_.get(), kDummyDownloadId);
DownloadId dummy_id2(download_manager_.get(), kDummyDownloadId2);
ScopedTempDir download_dir;
ASSERT_TRUE(download_dir.CreateUniqueTempDir());
- StartDownload(info, dummy_id);
-
- UpdateDownload(dummy_id, kTestData1, strlen(kTestData1), net::OK);
-
- StartDownload(info2, dummy_id2);
+ StartDownload(info.Pass(), dummy_id);
- UpdateDownload(dummy_id, kTestData2, strlen(kTestData2), net::OK);
+ StartDownload(info2.Pass(), dummy_id2);
- UpdateDownload(dummy_id2, kTestData4, strlen(kTestData4), net::OK);
FilePath crbar(download_dir.path().Append(
FILE_PATH_LITERAL("bar.txt.crdownload")));
RenameFile(dummy_id2, crbar, crbar, net::OK, IN_PROGRESS, OVERWRITE);
@@ -717,14 +455,6 @@ TEST_F(DownloadFileManagerTest, TwoDownloads) {
FILE_PATH_LITERAL("foo.txt.crdownload")));
RenameFile(dummy_id, crfoo, crfoo, net::OK, IN_PROGRESS, OVERWRITE);
- UpdateDownload(dummy_id, kTestData3, strlen(kTestData3), net::OK);
-
- UpdateDownload(dummy_id2, kTestData5, strlen(kTestData5), net::OK);
- UpdateDownload(dummy_id2, kTestData6, strlen(kTestData6), net::OK);
-
- OnResponseCompleted(dummy_id2, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
-
- OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");
FilePath bar(download_dir.path().Append(FILE_PATH_LITERAL("bar.txt")));
RenameFile(dummy_id2, bar, bar, net::OK, COMPLETE, OVERWRITE);
« no previous file with comments | « content/browser/download/download_file_manager.cc ('k') | content/browser/download/download_file_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698