content/browser/download/download_file_manager_unittest.cc - Issue 10392111: Use ByteStream in downloads system to decouple source and sink.

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: Incorporated comments and fixed tests. Created 8 years, 7 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

« no previous file with comments | « content/browser/download/download_file_manager.cc ('k') | content/browser/download/download_file_unittest.cc » ('j') | content/browser/download/download_manager_impl_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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 15eab3afcf963f84e82c567ef59d35f65e24ecaa..ad32845d8c4b173fad4e40501737dee94bda4a40 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"

@@ -44,6 +44,7 @@ class MockDownloadFileFactory :

virtual content::DownloadFile* CreateFile(

DownloadCreateInfo* info,

+ scoped_ptr<content::ByteStreamOutput> pipe,

const DownloadRequestHandle& request_handle,

content::DownloadManager* download_manager,

bool calculate_hash,

@@ -57,6 +58,7 @@ class MockDownloadFileFactory :

content::DownloadFile* MockDownloadFileFactory::CreateFile(

DownloadCreateInfo* info,

+ scoped_ptr<content::ByteStreamOutput> pipe,

const DownloadRequestHandle& request_handle,

content::DownloadManager* download_manager,

bool calculate_hash,

@@ -127,8 +129,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_) {

}

@@ -166,7 +167,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

@@ -182,7 +184,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())

@@ -190,94 +192,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::ByteStreamOutput>(), *request_handle_);

ProcessAllPendingMessages();

ClearExpectations(id);

}

- // 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);

- 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.

@@ -395,8 +316,6 @@ class DownloadFileManagerTest : public testing::Test {

.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], ""))

@@ -442,7 +361,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_;

@@ -472,50 +390,39 @@ 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;

+ 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);

+ StartDownload(info.Pass(), dummy_id);

CleanUp(dummy_id);

}

TEST_F(DownloadFileManagerTest, DownloadWithError) {

// 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);

+ StartDownload(info.Pass(), 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);

+ StartDownload(info.Pass(), dummy_id);

OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");

@@ -524,14 +431,10 @@ TEST_F(DownloadFileManagerTest, CompleteDownload) {

TEST_F(DownloadFileManagerTest, CompleteDownloadWithError) {

// 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);

+ StartDownload(info.Pass(), dummy_id);

OnResponseCompleted(dummy_id,

content::DOWNLOAD_INTERRUPT_REASON_FILE_VIRUS_INFECTED,

@@ -542,18 +445,15 @@ TEST_F(DownloadFileManagerTest, CompleteDownloadWithError) {

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, "");

@@ -562,15 +462,13 @@ TEST_F(DownloadFileManagerTest, RenameInProgress) {

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);

@@ -580,16 +478,12 @@ TEST_F(DownloadFileManagerTest, RenameInProgressWithError) {

TEST_F(DownloadFileManagerTest, RenameCompleting) {

// 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);

+ StartDownload(info.Pass(), dummy_id);

OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");

@@ -601,16 +495,12 @@ TEST_F(DownloadFileManagerTest, RenameCompleting) {

TEST_F(DownloadFileManagerTest, RenameCompletingWithUniquification) {

// 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);

+ StartDownload(info.Pass(), dummy_id);

OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");

@@ -627,16 +517,12 @@ TEST_F(DownloadFileManagerTest, RenameCompletingWithUniquification) {

TEST_F(DownloadFileManagerTest, RenameCompletingWithError) {

// 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);

+ StartDownload(info.Pass(), dummy_id);

OnResponseCompleted(dummy_id, content::DOWNLOAD_INTERRUPT_REASON_NONE, "");

@@ -649,19 +535,16 @@ TEST_F(DownloadFileManagerTest, RenameCompletingWithError) {

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, "");

@@ -673,22 +556,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);

@@ -697,11 +575,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, "");