content/browser/download/download_file_unittest.cc - Issue 10702151: Revert 146162 - Move Rename functionality from DownloadFileManager to DownloadFileImple.

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

Issue 10702151: Revert 146162 - Move Rename functionality from DownloadFileManager to DownloadFileImple. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/

Patch Set: 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_file_unittest.cc

===================================================================

--- content/browser/download/download_file_unittest.cc (revision 146176)

+++ content/browser/download/download_file_unittest.cc (working copy)

@@ -5,7 +5,6 @@

#include "base/file_util.h"

#include "base/message_loop.h"

#include "base/string_number_conversions.h"

-#include "base/test/test_file_util.h"

#include "content/browser/browser_thread_impl.h"

#include "content/browser/download/byte_stream.h"

#include "content/browser/download/download_create_info.h"

@@ -16,7 +15,6 @@

#include "content/public/browser/download_manager.h"

#include "content/public/test/mock_download_manager.h"

#include "net/base/file_stream.h"

-#include "net/base/mock_file_stream.h"

#include "net/base/net_errors.h"

#include "testing/gmock/include/gmock/gmock.h"

#include "testing/gtest/include/gtest/gtest.h"

@@ -29,7 +27,6 @@

using ::testing::_;

using ::testing::AnyNumber;

using ::testing::DoAll;

-using ::testing::InSequence;

using ::testing::Return;

using ::testing::SetArgPointee;

using ::testing::StrictMock;

@@ -48,14 +45,6 @@

MOCK_METHOD1(RegisterCallback, void(const base::Closure&));

};

-class LocalMockDownloadManager : public content::MockDownloadManager {

- public:

- MOCK_METHOD4(CurrentUpdateStatus,

- void(int32, int64, int64, const std::string&));

- protected:

- virtual ~LocalMockDownloadManager() {}

-};

} // namespace

DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";

@@ -76,10 +65,6 @@

// calling Release() on |download_manager_| won't ever result in its

// destructor being called and we get a leak.

DownloadFileTest() :

- update_download_id_(-1),

- bytes_(-1),

- bytes_per_sec_(-1),

- hash_state_("xyzzy"),

ui_thread_(BrowserThread::UI, &loop_),

file_thread_(BrowserThread::FILE, &loop_) {

}

@@ -89,19 +74,13 @@

void SetUpdateDownloadInfo(int32 id, int64 bytes, int64 bytes_per_sec,

const std::string& hash_state) {

- update_download_id_ = id;

bytes_ = bytes;

bytes_per_sec_ = bytes_per_sec;

hash_state_ = hash_state;

}

- void ConfirmUpdateDownloadInfo() {

- download_manager_->CurrentUpdateStatus(

- update_download_id_, bytes_, bytes_per_sec_, hash_state_);

- }

virtual void SetUp() {

- download_manager_ = new StrictMock<LocalMockDownloadManager>;

+ download_manager_ = new StrictMock<content::MockDownloadManager>;

EXPECT_CALL(*(download_manager_.get()),

UpdateDownload(

DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),

@@ -138,8 +117,8 @@

.RetiresOnSaturation();

DownloadCreateInfo info;

- // info.request_handle default constructed to null.

info.download_id = DownloadId(kValidIdDomain, kDummyDownloadId + offset);

+ // info.request_handle default constructed to null.

info.save_info.file_stream = file_stream_;

download_file_.reset(

new DownloadFileImpl(

@@ -249,28 +228,8 @@

}

- content::DownloadInterruptReason Rename(

- const FilePath& full_path, bool overwrite_existing_file,

- FilePath* result_path_p) {

- base::WeakPtrFactory<DownloadFileTest> weak_ptr_factory(this);

- content::DownloadInterruptReason result_reason(

- content::DOWNLOAD_INTERRUPT_REASON_NONE);

- bool callback_was_called(false);

- FilePath result_path;

- download_file_->Rename(full_path, overwrite_existing_file,

- base::Bind(&DownloadFileTest::SetRenameResult,

- weak_ptr_factory.GetWeakPtr(),

- &callback_was_called,

- &result_reason, result_path_p));

- loop_.RunAllPending();

- EXPECT_TRUE(callback_was_called);

- return result_reason;

- }

protected:

- scoped_refptr<StrictMock<LocalMockDownloadManager> > download_manager_;

+ scoped_refptr<StrictMock<content::MockDownloadManager> > download_manager_;

linked_ptr<net::FileStream> file_stream_;

@@ -285,7 +244,6 @@

base::Closure sink_callback_;

// Latest update sent to the download manager.

- int32 update_download_id_;

int64 bytes_;

int64 bytes_per_sec_;

std::string hash_state_;

@@ -293,19 +251,6 @@

MessageLoop loop_;

private:

- void SetRenameResult(bool* called_p,

- content::DownloadInterruptReason* reason_p,

- FilePath* result_path_p,

- content::DownloadInterruptReason reason,

- const FilePath& result_path) {

- if (called_p)

- *called_p = true;

- if (reason_p)

- *reason_p = reason;

- if (result_path_p)

- *result_path_p = result_path;

- }

// UI thread.

BrowserThreadImpl ui_thread_;

// File thread to satisfy debug checks in DownloadFile.

@@ -336,15 +281,12 @@

FilePath path_2(initial_path.InsertBeforeExtensionASCII("_2"));

FilePath path_3(initial_path.InsertBeforeExtensionASCII("_3"));

FilePath path_4(initial_path.InsertBeforeExtensionASCII("_4"));

- FilePath path_5(initial_path.InsertBeforeExtensionASCII("_5"));

- FilePath output_path;

// Rename the file before downloading any data.

EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(path_1, false, &output_path));

+ download_file_->Rename(path_1));

FilePath renamed_path = download_file_->FullPath();

EXPECT_EQ(path_1, renamed_path);

- EXPECT_EQ(path_1, output_path);

// Check the files.

EXPECT_FALSE(file_util::PathExists(initial_path));

@@ -356,10 +298,9 @@

// Rename the file after downloading some data.

EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(path_2, false, &output_path));

+ download_file_->Rename(path_2));

renamed_path = download_file_->FullPath();

EXPECT_EQ(path_2, renamed_path);

- EXPECT_EQ(path_2, output_path);

// Check the files.

EXPECT_FALSE(file_util::PathExists(path_1));

@@ -370,10 +311,9 @@

// Rename the file after downloading all the data.

EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(path_3, false, &output_path));

+ download_file_->Rename(path_3));

renamed_path = download_file_->FullPath();

EXPECT_EQ(path_3, renamed_path);

- EXPECT_EQ(path_3, output_path);

// Check the files.

EXPECT_FALSE(file_util::PathExists(path_2));

@@ -387,10 +327,9 @@

// Rename the file after downloading all the data and closing the file.

EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(path_4, false, &output_path));

+ download_file_->Rename(path_4));

renamed_path = download_file_->FullPath();

EXPECT_EQ(path_4, renamed_path);

- EXPECT_EQ(path_4, output_path);

// Check the files.

EXPECT_FALSE(file_util::PathExists(path_3));

@@ -400,84 +339,9 @@

EXPECT_TRUE(download_file_->GetHash(&hash));

EXPECT_EQ(kDataHash, base::HexEncode(hash.data(), hash.size()));

- // Check that a rename with overwrite to an existing file succeeds.

- std::string file_contents;

- ASSERT_FALSE(file_util::PathExists(path_5));

- static const char file_data[] = "xyzzy";

- ASSERT_EQ(static_cast<int>(sizeof(file_data) - 1),

- file_util::WriteFile(path_5, file_data, sizeof(file_data) - 1));

- ASSERT_TRUE(file_util::PathExists(path_5));

- EXPECT_TRUE(file_util::ReadFileToString(path_5, &file_contents));

- EXPECT_EQ(std::string(file_data), file_contents);

- EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(path_5, true, &output_path));

- EXPECT_EQ(path_5, output_path);

- file_contents = "";

- EXPECT_TRUE(file_util::ReadFileToString(path_5, &file_contents));

- EXPECT_NE(std::string(file_data), file_contents);

DestroyDownloadFile(0);

}

-// Test to make sure the rename uniquifies if we aren't overwriting

-// and there's a file where we're aiming.

-TEST_F(DownloadFileTest, RenameUniquifies) {

- ASSERT_TRUE(CreateDownloadFile(0, true));

- FilePath initial_path(download_file_->FullPath());

- EXPECT_TRUE(file_util::PathExists(initial_path));

- FilePath path_1(initial_path.InsertBeforeExtensionASCII("_1"));

- FilePath path_1_suffixed(path_1.InsertBeforeExtensionASCII(" (1)"));

- ASSERT_FALSE(file_util::PathExists(path_1));

- static const char file_data[] = "xyzzy";

- ASSERT_EQ(static_cast<int>(sizeof(file_data)),

- file_util::WriteFile(path_1, file_data, sizeof(file_data)));

- ASSERT_TRUE(file_util::PathExists(path_1));

- EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(path_1, false, NULL));

- EXPECT_TRUE(file_util::PathExists(path_1_suffixed));

- FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NONE, true);

- loop_.RunAllPending();

- DestroyDownloadFile(0);

-// Test to make sure we get the proper error on failure.

-TEST_F(DownloadFileTest, RenameError) {

- ASSERT_TRUE(CreateDownloadFile(0, true));

- FilePath initial_path(download_file_->FullPath());

- // Create a subdirectory and rename into it.

- FilePath tempdir(initial_path.DirName().Append(FILE_PATH_LITERAL("tempdir")));

- ASSERT_TRUE(file_util::CreateDirectory(tempdir));

- FilePath new_path(tempdir.Append(initial_path.BaseName()));

- ASSERT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE,

- Rename(new_path, true, NULL));

- // Targets

- FilePath path_1(new_path.InsertBeforeExtensionASCII("_1"));

- FilePath path_1_suffixed(path_1.InsertBeforeExtensionASCII(" (1)"));

- ASSERT_FALSE(file_util::PathExists(path_1));

- ASSERT_FALSE(file_util::PathExists(path_1_suffixed));

- // Make the directory unwritable and try to rename within it.

- {

- file_util::PermissionRestorer restorer(tempdir);

- ASSERT_TRUE(file_util::MakeFileUnwritable(tempdir));

- EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,

- Rename(path_1, true, NULL));

- EXPECT_FALSE(file_util::PathExists(path_1_suffixed));

- }

- FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NONE, true);

- loop_.RunAllPending();

- DestroyDownloadFile(0);

// Various tests of the StreamActive method.

TEST_F(DownloadFileTest, StreamEmptySuccess) {

ASSERT_TRUE(CreateDownloadFile(0, true));

@@ -516,22 +380,10 @@

EXPECT_CALL(*(download_manager_.get()),

OnDownloadInterrupted(

DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),

- content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED))

- .WillOnce(InvokeWithoutArgs(

- this, &DownloadFileTest::ConfirmUpdateDownloadInfo));

- // If this next EXPECT_CALL fails flakily, it's probably a real failure.

- // We'll be getting a stream of UpdateDownload calls from the timer, and

- // the last one may have the correct information even if the failure

- // doesn't produce an update, as the timer update may have triggered at the

- // same time.

- EXPECT_CALL(*(download_manager_.get()),

- CurrentUpdateStatus(kDummyDownloadId + 0, 0, _, _));

+ 0, _,

+ content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED));

FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED, false);

- loop_.RunAllPending();

DestroyDownloadFile(0);

}

@@ -564,26 +416,12 @@

SetupDataAppend(chunks1, 2, s1);

SetupFinishStream(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED,

s1);

EXPECT_CALL(*(download_manager_.get()),

OnDownloadInterrupted(

DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),

- content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED))

- .WillOnce(InvokeWithoutArgs(

- this, &DownloadFileTest::ConfirmUpdateDownloadInfo));

- // If this next EXPECT_CALL fails flakily, it's probably a real failure.

- // We'll be getting a stream of UpdateDownload calls from the timer, and

- // the last one may have the correct information even if the failure

- // doesn't produce an update, as the timer update may have triggered at the

- // same time.

- EXPECT_CALL(*(download_manager_.get()),

- CurrentUpdateStatus(kDummyDownloadId + 0,

- strlen(kTestData1) + strlen(kTestData2),

- _, _));

+ strlen(kTestData1) + strlen(kTestData2), _,

+ content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED));

sink_callback_.Run();

- loop_.RunAllPending();

VerifyStreamAndSize();

DestroyDownloadFile(0);

}

« no previous file with comments | « content/browser/download/download_file_manager_unittest.cc ('k') | content/browser/download/download_item_impl.h » ('j') | no next file with comments »