diff options
Diffstat (limited to 'chrome/installer')
| -rw-r--r-- | chrome/installer/setup/install_worker.cc | 16 | ||||
| -rw-r--r-- | chrome/installer/setup/setup_main.cc | 6 | ||||
| -rw-r--r-- | chrome/installer/util/duplicate_tree_detector.cc | 50 | ||||
| -rw-r--r-- | chrome/installer/util/duplicate_tree_detector.h | 28 | ||||
| -rw-r--r-- | chrome/installer/util/duplicate_tree_detector_unittest.cc | 170 | ||||
| -rw-r--r-- | chrome/installer/util/move_tree_work_item.cc | 67 | ||||
| -rw-r--r-- | chrome/installer/util/move_tree_work_item.h | 29 | ||||
| -rw-r--r-- | chrome/installer/util/move_tree_work_item_unittest.cc | 302 | ||||
| -rw-r--r-- | chrome/installer/util/work_item.cc | 8 | ||||
| -rw-r--r-- | chrome/installer/util/work_item.h | 11 | ||||
| -rw-r--r-- | chrome/installer/util/work_item_list.cc | 6 | ||||
| -rw-r--r-- | chrome/installer/util/work_item_list.h | 3 |
12 files changed, 603 insertions, 93 deletions
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc index 8b45818..07c80bc 100644 --- a/chrome/installer/setup/install_worker.cc +++ b/chrome/installer/setup/install_worker.cc @@ -102,9 +102,9 @@ void AddInstallerCopyTasks(const InstallerState& installer_state, // is in %ProgramFiles% for system level installs (and in %LOCALAPPDATA% // otherwise), there is no need to do this. install_list->AddMoveTreeWorkItem(setup_path.value(), exe_dst.value(), - temp_path.value()); + temp_path.value(), WorkItem::ALWAYS_MOVE); install_list->AddMoveTreeWorkItem(archive_path.value(), archive_dst.value(), - temp_path.value()); + temp_path.value(), WorkItem::ALWAYS_MOVE); } // This method adds work items to create (or update) Chrome uninstall entry in @@ -564,17 +564,25 @@ void AddInstallWorkItems(const InstallationState& original_state, install_list->AddMoveTreeWorkItem( src_path.Append(installer::kWowHelperExe).value(), target_path.Append(installer::kWowHelperExe).value(), - temp_path.value()); + temp_path.value(), + WorkItem::ALWAYS_MOVE); } // In the past, we copied rather than moved for system level installs so that // the permissions of %ProgramFiles% would be picked up. Now that |temp_path| // is in %ProgramFiles% for system level installs (and in %LOCALAPPDATA% // otherwise), there is no need to do this. + // Note that we pass true for check_duplicates to avoid failing on in-use + // repair runs if the current_version is the same as the new_version. + bool check_for_duplicates = + (current_version != NULL && current_version->get() != NULL && + current_version->get()->Equals(new_version)); install_list->AddMoveTreeWorkItem( src_path.AppendASCII(new_version.GetString()).value(), target_path.AppendASCII(new_version.GetString()).value(), - temp_path.value()); + temp_path.value(), + check_for_duplicates ? WorkItem::CHECK_DUPLICATES : + WorkItem::ALWAYS_MOVE); // Copy the default Dictionaries only if the folder doesn't exist already. // TODO(grt): Use AddMoveTreeWorkItem in a conditional WorkItemList, which diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc index 60a570d..8b15a38 100644 --- a/chrome/installer/setup/setup_main.cc +++ b/chrome/installer/setup/setup_main.cc @@ -201,10 +201,12 @@ installer::InstallStatus RenameChromeExecutables( // Move chrome.exe to old_chrome.exe, then move new_chrome.exe to chrome.exe. install_list->AddMoveTreeWorkItem(chrome_exe.value(), chrome_old_exe.value(), - temp_path.path().value()); + temp_path.path().value(), + WorkItem::ALWAYS_MOVE); install_list->AddMoveTreeWorkItem(chrome_new_exe.value(), chrome_exe.value(), - temp_path.path().value()); + temp_path.path().value(), + WorkItem::ALWAYS_MOVE); install_list->AddDeleteTreeWorkItem(chrome_new_exe, temp_path.path()); // old_chrome.exe is still in use in most cases, so ignore failures here. install_list->AddDeleteTreeWorkItem(chrome_old_exe, temp_path.path()) diff --git a/chrome/installer/util/duplicate_tree_detector.cc b/chrome/installer/util/duplicate_tree_detector.cc new file mode 100644 index 0000000..0489f9c --- /dev/null +++ b/chrome/installer/util/duplicate_tree_detector.cc @@ -0,0 +1,50 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// + +#include "chrome/installer/util/duplicate_tree_detector.h" + +#include "base/file_util.h" +#include "base/logging.h" + +namespace installer { + +bool IsIdenticalFileHierarchy(const FilePath& src_path, + const FilePath& dest_path) { + using file_util::FileEnumerator; + base::PlatformFileInfo src_info; + base::PlatformFileInfo dest_info; + + bool is_identical = false; + if (file_util::GetFileInfo(src_path, &src_info) && + file_util::GetFileInfo(dest_path, &dest_info)) { + // Both paths exist, check the types: + if (!src_info.is_directory && !dest_info.is_directory) { + // Two files are "identical" if the file sizes are equivalent. + is_identical = src_info.size == dest_info.size; + } else if (src_info.is_directory && dest_info.is_directory) { + // Two directories are "identical" if dest_path contains entries that are + // "identical" to all the entries in src_path. + is_identical = true; + + FileEnumerator path_enum( + src_path, + false, // Not recursive + static_cast<FileEnumerator::FILE_TYPE>( + FileEnumerator::FILES | FileEnumerator::DIRECTORIES)); + for (FilePath path = path_enum.Next(); is_identical && !path.empty(); + path = path_enum.Next()) { + is_identical = + IsIdenticalFileHierarchy(path, dest_path.Append(path.BaseName())); + } + } else { + // The two paths are of different types, so they cannot be identical. + DCHECK(!is_identical); + } + } + + return is_identical; +} + +} // namespace installer diff --git a/chrome/installer/util/duplicate_tree_detector.h b/chrome/installer/util/duplicate_tree_detector.h new file mode 100644 index 0000000..910e9d2 --- /dev/null +++ b/chrome/installer/util/duplicate_tree_detector.h @@ -0,0 +1,28 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// +// This file declares a helper function that will check to see if a given folder +// is "identical" to another (for some value of identical, see below). +// + +#ifndef CHROME_INSTALLER_UTIL_DUPLICATE_TREE_DETECTOR_H_ +#define CHROME_INSTALLER_UTIL_DUPLICATE_TREE_DETECTOR_H_ +#pragma once + +class FilePath; + +namespace installer { + +// Returns true if |dest_path| contains all the files from |src_path| in the +// same directory structure and each of those files is of the same length. +// src_path_ and |dest_path| must either both be files or both be directories. +// Note that THIS IS A WEAK DEFINITION OF IDENTICAL and is intended only to +// catch cases of missing files or obvious modifications. +// It notably DOES NOT CHECKSUM the files. +bool IsIdenticalFileHierarchy(const FilePath& src_path, + const FilePath& dest_path); + +} // namespace installer + +#endif // CHROME_INSTALLER_UTIL_DUPLICATE_TREE_DETECTOR_H_ diff --git a/chrome/installer/util/duplicate_tree_detector_unittest.cc b/chrome/installer/util/duplicate_tree_detector_unittest.cc new file mode 100644 index 0000000..6a168e6 --- /dev/null +++ b/chrome/installer/util/duplicate_tree_detector_unittest.cc @@ -0,0 +1,170 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include <windows.h> + +#include <fstream> + +#include "base/file_util.h" +#include "base/memory/scoped_temp_dir.h" +#include "base/string_util.h" +#include "chrome/installer/util/duplicate_tree_detector.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class DuplicateTreeDetectorTest : public testing::Test { + protected: + virtual void SetUp() { + ASSERT_TRUE(temp_source_dir_.CreateUniqueTempDir()); + ASSERT_TRUE(temp_dest_dir_.CreateUniqueTempDir()); + } + + // Simple function to dump some text into a new file. + void CreateTextFile(const std::string& filename, + const std::wstring& contents) { + std::wofstream file; + file.open(filename.c_str()); + ASSERT_TRUE(file.is_open()); + file << contents; + file.close(); + } + + // Simple function to read text from a file. + std::wstring ReadTextFile(const FilePath& path) { + WCHAR contents[64]; + std::wifstream file; + file.open(WideToASCII(path.value()).c_str()); + EXPECT_TRUE(file.is_open()); + file.getline(contents, arraysize(contents)); + file.close(); + return std::wstring(contents); + } + + // Creates a two level deep source dir with a file in each. + void CreateFileHierarchy(const FilePath& root) { + FilePath d1(root); + d1 = d1.AppendASCII("D1"); + file_util::CreateDirectory(d1); + ASSERT_TRUE(file_util::PathExists(d1)); + + FilePath f1(d1); + f1 = f1.AppendASCII("F1"); + CreateTextFile(f1.MaybeAsASCII(), text_content_1_); + ASSERT_TRUE(file_util::PathExists(f1)); + + FilePath d2(d1); + d2 = d2.AppendASCII("D2"); + file_util::CreateDirectory(d2); + ASSERT_TRUE(file_util::PathExists(d2)); + + FilePath f2(d2); + f2 = f2.AppendASCII("F2"); + CreateTextFile(f2.MaybeAsASCII(), text_content_2_); + ASSERT_TRUE(file_util::PathExists(f2)); + } + + ScopedTempDir temp_source_dir_; + ScopedTempDir temp_dest_dir_; + + static const wchar_t text_content_1_[]; + static const wchar_t text_content_2_[]; + static const wchar_t text_content_3_[]; +}; + +const wchar_t DuplicateTreeDetectorTest::text_content_1_[] = + L"Gooooooooooooooooooooogle"; +const wchar_t DuplicateTreeDetectorTest::text_content_2_[] = + L"Overwrite Me"; +const wchar_t DuplicateTreeDetectorTest::text_content_3_[] = + L"I'd rather see your watermelon and raise you ham and a half."; + +}; // namespace + +// Test the DuplicateTreeChecker's definition of identity on two identical +// directory structures. +TEST_F(DuplicateTreeDetectorTest, TestIdenticalDirs) { + // Create two sets of identical file hierarchys: + CreateFileHierarchy(temp_source_dir_.path()); + CreateFileHierarchy(temp_dest_dir_.path()); + + EXPECT_TRUE(installer::IsIdenticalFileHierarchy(temp_source_dir_.path(), + temp_dest_dir_.path())); +} + +// Test when source entirely contains dest but contains other files as well. +// IsIdenticalTo should return false in this case. +TEST_F(DuplicateTreeDetectorTest, TestSourceContainsDest) { + // Create two sets of identical file hierarchys: + CreateFileHierarchy(temp_source_dir_.path()); + CreateFileHierarchy(temp_dest_dir_.path()); + + FilePath new_file(temp_source_dir_.path()); + new_file = new_file.AppendASCII("FNew"); + CreateTextFile(new_file.MaybeAsASCII(), text_content_1_); + ASSERT_TRUE(file_util::PathExists(new_file)); + + EXPECT_FALSE(installer::IsIdenticalFileHierarchy(temp_source_dir_.path(), + temp_dest_dir_.path())); +} + +// Test when dest entirely contains source but contains other files as well. +// IsIdenticalTo should return true in this case. +TEST_F(DuplicateTreeDetectorTest, TestDestContainsSource) { + // Create two sets of identical file hierarchys: + CreateFileHierarchy(temp_source_dir_.path()); + CreateFileHierarchy(temp_dest_dir_.path()); + + FilePath new_file(temp_dest_dir_.path()); + new_file = new_file.AppendASCII("FNew"); + CreateTextFile(new_file.MaybeAsASCII(), text_content_1_); + ASSERT_TRUE(file_util::PathExists(new_file)); + + EXPECT_TRUE(installer::IsIdenticalFileHierarchy(temp_source_dir_.path(), + temp_dest_dir_.path())); +} + +// Test when the file hierarchies are the same but one of the files is changed. +TEST_F(DuplicateTreeDetectorTest, TestIdenticalDirsDifferentFiles) { + // Create two sets of identical file hierarchys: + CreateFileHierarchy(temp_source_dir_.path()); + CreateFileHierarchy(temp_dest_dir_.path()); + + FilePath existing_file(temp_dest_dir_.path()); + existing_file = existing_file.AppendASCII("D1") + .AppendASCII("D2") + .AppendASCII("F2"); + CreateTextFile(existing_file.MaybeAsASCII(), text_content_3_); + + EXPECT_FALSE(installer::IsIdenticalFileHierarchy(temp_source_dir_.path(), + temp_dest_dir_.path())); +} + +// Test when both file hierarchies are empty. +TEST_F(DuplicateTreeDetectorTest, TestEmptyDirs) { + EXPECT_TRUE(installer::IsIdenticalFileHierarchy(temp_source_dir_.path(), + temp_dest_dir_.path())); +} + +// Test on single files. +TEST_F(DuplicateTreeDetectorTest, TestSingleFiles) { + // Create a source file. + FilePath source_file(temp_source_dir_.path()); + source_file = source_file.AppendASCII("F1"); + CreateTextFile(source_file.MaybeAsASCII(), text_content_1_); + + // This file should be the same. + FilePath dest_file(temp_dest_dir_.path()); + dest_file = dest_file.AppendASCII("F1"); + CreateTextFile(dest_file.MaybeAsASCII(), text_content_1_); + + // This file should be different. + FilePath other_file(temp_dest_dir_.path()); + other_file = other_file.AppendASCII("F2"); + CreateTextFile(other_file.MaybeAsASCII(), text_content_2_); + + EXPECT_TRUE(installer::IsIdenticalFileHierarchy(source_file, dest_file)); + EXPECT_FALSE(installer::IsIdenticalFileHierarchy(source_file, other_file)); +} + diff --git a/chrome/installer/util/move_tree_work_item.cc b/chrome/installer/util/move_tree_work_item.cc index b4a7cc6..21e0d7b 100644 --- a/chrome/installer/util/move_tree_work_item.cc +++ b/chrome/installer/util/move_tree_work_item.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -8,6 +8,7 @@ #include "base/file_util.h" #include "base/logging.h" +#include "chrome/installer/util/duplicate_tree_detector.h" #include "chrome/installer/util/logging_installer.h" MoveTreeWorkItem::~MoveTreeWorkItem() { @@ -15,12 +16,15 @@ MoveTreeWorkItem::~MoveTreeWorkItem() { MoveTreeWorkItem::MoveTreeWorkItem(const FilePath& source_path, const FilePath& dest_path, - const FilePath& temp_dir) + const FilePath& temp_dir, + MoveTreeOption duplicate_option) : source_path_(source_path), dest_path_(dest_path), temp_dir_(temp_dir), + duplicate_option_(duplicate_option), moved_to_dest_path_(false), - moved_to_backup_(false) { + moved_to_backup_(false), + source_moved_to_backup_(false) { } bool MoveTreeWorkItem::Do() { @@ -29,7 +33,14 @@ bool MoveTreeWorkItem::Do() { return false; } - // If dest_path_ exists, move destination to a backup path. + // If dest_path_ exists, we can do one of two things: + // 1) If the contents of src_path_are already fully contained in dest_path_ + // then do nothing and return success. Fully contained means the full + // file structure with identical files is contained in dest_path_. For + // Chrome, if dest_path_ exists, this is expected to be the common case. + // 2) If the contents of src_path_ are NOT fully contained in dest_path_, we + // attempt to backup dest_path_ and replace it with src_path_. This will + // fail if files in dest_path_ are in use. if (file_util::PathExists(dest_path_)) { // Generate a backup path that can keep the original files under dest_path_. if (!backup_path_.CreateUniqueTempDirUnderPath(temp_dir_)) { @@ -37,16 +48,39 @@ bool MoveTreeWorkItem::Do() { << temp_dir_.value(); return false; } - FilePath backup = backup_path_.path().Append(dest_path_.BaseName()); + if (duplicate_option_ == CHECK_DUPLICATES) { + if (installer::IsIdenticalFileHierarchy(source_path_, dest_path_)) { + // The files we are moving are already present in the destination path. + // We most likely don't need to do anything. As such, just move the + // source files to the temp folder as backup. + if (file_util::Move(source_path_, backup)) { + source_moved_to_backup_ = true; + VLOG(1) << "Moved source " << source_path_.value() + << " to backup path " << backup.value(); + return true; + } else { + // We failed to move the source tree to the backup path. This is odd + // but just fall through and attempt the regular behaviour as well. + LOG(ERROR) << "Failed to backup source " << source_path_.value() + << " to backup path " << backup.value() + << " for duplicate trees. Trying regular Move instead."; + } + } else { + VLOG(1) << "Source path " << source_path_.value() + << " differs from " << dest_path_.value() + << ", updating now."; + } + } + if (file_util::Move(dest_path_, backup)) { moved_to_backup_ = true; VLOG(1) << "Moved destination " << dest_path_.value() << " to backup path " << backup.value(); } else { - LOG(ERROR) << "failed moving " << dest_path_.value() - << " to " << backup.value(); + PLOG(ERROR) << "failed moving " << dest_path_.value() + << " to " << backup.value(); return false; } } @@ -57,8 +91,8 @@ bool MoveTreeWorkItem::Do() { VLOG(1) << "Moved source " << source_path_.value() << " to destination " << dest_path_.value(); } else { - LOG(ERROR) << "failed move " << source_path_.value() - << " to " << dest_path_.value(); + PLOG(ERROR) << "failed move " << source_path_.value() + << " to " << dest_path_.value(); return false; } @@ -70,11 +104,14 @@ void MoveTreeWorkItem::Rollback() { LOG(ERROR) << "Can not move " << dest_path_.value() << " to " << source_path_.value(); - if (moved_to_backup_) { - FilePath backup = backup_path_.path().Append(dest_path_.BaseName()); - if (!file_util::Move(backup, dest_path_)) { - LOG(ERROR) << "failed move " << backup.value() - << " to " << dest_path_.value(); - } + FilePath backup = backup_path_.path().Append(dest_path_.BaseName()); + if (moved_to_backup_ && !file_util::Move(backup, dest_path_)) { + LOG(ERROR) << "failed move " << backup.value() + << " to " << dest_path_.value(); + } + + if (source_moved_to_backup_ && !file_util::Move(backup, source_path_)) { + LOG(ERROR) << "Can not restore " << backup.value() + << " to " << source_path_.value(); } } diff --git a/chrome/installer/util/move_tree_work_item.h b/chrome/installer/util/move_tree_work_item.h index 3926b9e..16fec6e 100644 --- a/chrome/installer/util/move_tree_work_item.h +++ b/chrome/installer/util/move_tree_work_item.h @@ -7,6 +7,7 @@ #pragma once #include "base/file_path.h" +#include "base/gtest_prod_util.h" #include "base/memory/scoped_temp_dir.h" #include "chrome/installer/util/work_item.h" @@ -28,14 +29,24 @@ class MoveTreeWorkItem : public WorkItem { private: friend class WorkItem; + FRIEND_TEST_ALL_PREFIXES(MoveTreeWorkItemTest, + MoveDirectoryDestExistsCheckForDuplicatesFull); + FRIEND_TEST_ALL_PREFIXES(MoveTreeWorkItemTest, + MoveDirectoryDestExistsCheckForDuplicatesPartial); - // source_path specifies file or directory that will be moved to location - // specified by dest_path. To facilitate rollback, the caller needs to supply - // a temporary directory (temp_dir) to save the original files if they exist - // under dest_path. + // |source_path| specifies file or directory that will be moved to location + // specified by |dest_path|. To facilitate rollback, the caller needs to + // supply a temporary directory, |temp_dir| to save the original files if + // they exist under dest_path. + // If |check_duplicates| is CHECK_DUPLICATES, then Do() will first check + // whether the directory tree in source_path is entirely contained in + // dest_path and all files in source_path are present and of the same length + // in dest_path. If so, it will do nothing and return true, otherwise it will + // attempt to move source_path to dest_path as stated above. MoveTreeWorkItem(const FilePath& source_path, const FilePath& dest_path, - const FilePath& temp_dir); + const FilePath& temp_dir, + MoveTreeOption duplicate_option); // Source path to move files from. FilePath source_path_; @@ -55,6 +66,14 @@ class MoveTreeWorkItem : public WorkItem { // Whether the original files have been moved to backup path under // temporary directory. If true, moving back is needed during rollback. bool moved_to_backup_; + + // Whether we moved the source files to the backup path instead just to + // preserve the behaviour of a Move. This can only become true if + // duplicate_option_ is CHECK_DUPLICATES. + bool source_moved_to_backup_; + + // Whether to check for duplicates before moving. + MoveTreeOption duplicate_option_; }; #endif // CHROME_INSTALLER_UTIL_MOVE_TREE_WORK_ITEM_H_ diff --git a/chrome/installer/util/move_tree_work_item_unittest.cc b/chrome/installer/util/move_tree_work_item_unittest.cc index c646fd4..e310511 100644 --- a/chrome/installer/util/move_tree_work_item_unittest.cc +++ b/chrome/installer/util/move_tree_work_item_unittest.cc @@ -17,42 +17,42 @@ #include "testing/gtest/include/gtest/gtest.h" namespace { - class MoveTreeWorkItemTest : public testing::Test { - protected: - virtual void SetUp() { - ASSERT_TRUE(temp_from_dir_.CreateUniqueTempDir()); - ASSERT_TRUE(temp_to_dir_.CreateUniqueTempDir()); - } - - ScopedTempDir temp_from_dir_; - ScopedTempDir temp_to_dir_; - }; - - // Simple function to dump some text into a new file. - void CreateTextFile(const std::wstring& filename, - const std::wstring& contents) { - std::ofstream file; - file.open(filename.c_str()); - ASSERT_TRUE(file.is_open()); - file << contents; - file.close(); +class MoveTreeWorkItemTest : public testing::Test { + protected: + virtual void SetUp() { + ASSERT_TRUE(temp_from_dir_.CreateUniqueTempDir()); + ASSERT_TRUE(temp_to_dir_.CreateUniqueTempDir()); } - // Simple function to read text from a file. - std::wstring ReadTextFile(const FilePath& path) { - WCHAR contents[64]; - std::wifstream file; - file.open(path.value().c_str()); - EXPECT_TRUE(file.is_open()); - file.getline(contents, 64); - file.close(); - return std::wstring(contents); - } - - wchar_t text_content_1[] = L"Gooooooooooooooooooooogle"; - wchar_t text_content_2[] = L"Overwrite Me"; + ScopedTempDir temp_from_dir_; + ScopedTempDir temp_to_dir_; }; +// Simple function to dump some text into a new file. +void CreateTextFile(const std::wstring& filename, + const std::wstring& contents) { + std::wofstream file; + file.open(WideToASCII(filename).c_str()); + ASSERT_TRUE(file.is_open()); + file << contents; + file.close(); +} + +// Simple function to read text from a file. +std::wstring ReadTextFile(const FilePath& path) { + WCHAR contents[64]; + std::wifstream file; + file.open(WideToASCII(path.value()).c_str()); + EXPECT_TRUE(file.is_open()); + file.getline(contents, arraysize(contents)); + file.close(); + return std::wstring(contents); +} + +const wchar_t kTextContent1[] = L"Gooooooooooooooooooooogle"; +const wchar_t kTextContent2[] = L"Overwrite Me"; +}; // namespace + // Move one directory from source to destination when destination does not // exist. TEST_F(MoveTreeWorkItemTest, MoveDirectory) { @@ -69,7 +69,7 @@ TEST_F(MoveTreeWorkItemTest, MoveDirectory) { FilePath from_file(from_dir2); from_file = from_file.AppendASCII("From_File"); - CreateTextFile(from_file.value(), text_content_1); + CreateTextFile(from_file.value(), kTextContent1); ASSERT_TRUE(file_util::PathExists(from_file)); // Generate destination path @@ -84,7 +84,10 @@ TEST_F(MoveTreeWorkItemTest, MoveDirectory) { // test Do() scoped_ptr<MoveTreeWorkItem> work_item( - WorkItem::CreateMoveTreeWorkItem(from_dir1, to_dir, temp_to_dir_.path())); + WorkItem::CreateMoveTreeWorkItem(from_dir1, + to_dir, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); EXPECT_TRUE(work_item->Do()); EXPECT_FALSE(file_util::PathExists(from_dir1)); @@ -115,7 +118,7 @@ TEST_F(MoveTreeWorkItemTest, MoveDirectoryDestExists) { FilePath from_file(from_dir2); from_file = from_file.AppendASCII("From_File"); - CreateTextFile(from_file.value(), text_content_1); + CreateTextFile(from_file.value(), kTextContent1); ASSERT_TRUE(file_util::PathExists(from_file)); // Create destination path @@ -126,7 +129,7 @@ TEST_F(MoveTreeWorkItemTest, MoveDirectoryDestExists) { FilePath orig_to_file(to_dir); orig_to_file = orig_to_file.AppendASCII("To_File"); - CreateTextFile(orig_to_file.value(), text_content_2); + CreateTextFile(orig_to_file.value(), kTextContent2); ASSERT_TRUE(file_util::PathExists(orig_to_file)); FilePath new_to_file(to_dir); @@ -134,9 +137,12 @@ TEST_F(MoveTreeWorkItemTest, MoveDirectoryDestExists) { new_to_file = new_to_file.AppendASCII("From_File"); ASSERT_FALSE(file_util::PathExists(new_to_file)); - // test Do() + // test Do(), don't check for duplicates. scoped_ptr<MoveTreeWorkItem> work_item( - WorkItem::CreateMoveTreeWorkItem(from_dir1, to_dir, temp_to_dir_.path())); + WorkItem::CreateMoveTreeWorkItem(from_dir1, + to_dir, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); EXPECT_TRUE(work_item->Do()); EXPECT_FALSE(file_util::PathExists(from_dir1)); @@ -151,8 +157,8 @@ TEST_F(MoveTreeWorkItemTest, MoveDirectoryDestExists) { EXPECT_TRUE(file_util::PathExists(to_dir)); EXPECT_FALSE(file_util::PathExists(new_to_file)); EXPECT_TRUE(file_util::PathExists(orig_to_file)); - EXPECT_EQ(0, ReadTextFile(orig_to_file).compare(text_content_2)); - EXPECT_EQ(0, ReadTextFile(from_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(orig_to_file).compare(kTextContent2)); + EXPECT_EQ(0, ReadTextFile(from_file).compare(kTextContent1)); } // Move one file from source to destination when destination does not @@ -166,7 +172,7 @@ TEST_F(MoveTreeWorkItemTest, MoveAFile) { FilePath from_file(from_dir); from_file = from_file.AppendASCII("From_File"); - CreateTextFile(from_file.value(), text_content_1); + CreateTextFile(from_file.value(), kTextContent1); ASSERT_TRUE(file_util::PathExists(from_file)); // Generate destination file name @@ -176,14 +182,16 @@ TEST_F(MoveTreeWorkItemTest, MoveAFile) { // test Do() scoped_ptr<MoveTreeWorkItem> work_item( - WorkItem::CreateMoveTreeWorkItem( - from_file, to_file, temp_to_dir_.path())); + WorkItem::CreateMoveTreeWorkItem(from_file, + to_file, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); EXPECT_TRUE(work_item->Do()); EXPECT_TRUE(file_util::PathExists(from_dir)); EXPECT_FALSE(file_util::PathExists(from_file)); EXPECT_TRUE(file_util::PathExists(to_file)); - EXPECT_EQ(0, ReadTextFile(to_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(to_file).compare(kTextContent1)); // test rollback() work_item->Rollback(); @@ -191,7 +199,7 @@ TEST_F(MoveTreeWorkItemTest, MoveAFile) { EXPECT_TRUE(file_util::PathExists(from_dir)); EXPECT_TRUE(file_util::PathExists(from_file)); EXPECT_FALSE(file_util::PathExists(to_file)); - EXPECT_EQ(0, ReadTextFile(from_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(from_file).compare(kTextContent1)); } // Move one file from source to destination when destination already @@ -205,7 +213,7 @@ TEST_F(MoveTreeWorkItemTest, MoveFileDestExists) { FilePath from_file(from_dir); from_file = from_file.AppendASCII("From_File"); - CreateTextFile(from_file.value(), text_content_1); + CreateTextFile(from_file.value(), kTextContent1); ASSERT_TRUE(file_util::PathExists(from_file)); // Create destination path @@ -216,27 +224,30 @@ TEST_F(MoveTreeWorkItemTest, MoveFileDestExists) { FilePath to_file(to_dir); to_file = to_file.AppendASCII("To_File"); - CreateTextFile(to_file.value(), text_content_2); + CreateTextFile(to_file.value(), kTextContent2); ASSERT_TRUE(file_util::PathExists(to_file)); // test Do() scoped_ptr<MoveTreeWorkItem> work_item( - WorkItem::CreateMoveTreeWorkItem(from_file, to_dir, temp_to_dir_.path())); + WorkItem::CreateMoveTreeWorkItem(from_file, + to_dir, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); EXPECT_TRUE(work_item->Do()); EXPECT_TRUE(file_util::PathExists(from_dir)); EXPECT_FALSE(file_util::PathExists(from_file)); EXPECT_TRUE(file_util::PathExists(to_dir)); EXPECT_FALSE(file_util::PathExists(to_file)); - EXPECT_EQ(0, ReadTextFile(to_dir).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(to_dir).compare(kTextContent1)); // test rollback() work_item->Rollback(); EXPECT_TRUE(file_util::PathExists(from_dir)); - EXPECT_EQ(0, ReadTextFile(from_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(from_file).compare(kTextContent1)); EXPECT_TRUE(file_util::PathExists(to_dir)); - EXPECT_EQ(0, ReadTextFile(to_file).compare(text_content_2)); + EXPECT_EQ(0, ReadTextFile(to_file).compare(kTextContent2)); } // Move one file from source to destination when destination already @@ -250,7 +261,7 @@ TEST_F(MoveTreeWorkItemTest, MoveFileDestInUse) { FilePath from_file(from_dir); from_file = from_file.AppendASCII("From_File"); - CreateTextFile(from_file.value(), text_content_1); + CreateTextFile(from_file.value(), kTextContent1); ASSERT_TRUE(file_util::PathExists(from_file)); // Create an executable in destination path by copying ourself to it. @@ -278,20 +289,22 @@ TEST_F(MoveTreeWorkItemTest, MoveFileDestInUse) { // test Do() scoped_ptr<MoveTreeWorkItem> work_item( - WorkItem::CreateMoveTreeWorkItem( - from_file, to_file, temp_to_dir_.path())); + WorkItem::CreateMoveTreeWorkItem(from_file, + to_file, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); EXPECT_TRUE(work_item->Do()); EXPECT_TRUE(file_util::PathExists(from_dir)); EXPECT_FALSE(file_util::PathExists(from_file)); EXPECT_TRUE(file_util::PathExists(to_dir)); - EXPECT_EQ(0, ReadTextFile(to_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(to_file).compare(kTextContent1)); // test rollback() work_item->Rollback(); EXPECT_TRUE(file_util::PathExists(from_dir)); - EXPECT_EQ(0, ReadTextFile(from_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(from_file).compare(kTextContent1)); EXPECT_TRUE(file_util::PathExists(to_dir)); EXPECT_TRUE(file_util::ContentsEqual(exe_full_path, to_file)); @@ -325,7 +338,7 @@ TEST_F(MoveTreeWorkItemTest, MoveFileInUse) { FilePath to_file(to_dir); to_file = to_file.AppendASCII("To_File"); - CreateTextFile(to_file.value(), text_content_1); + CreateTextFile(to_file.value(), kTextContent1); ASSERT_TRUE(file_util::PathExists(to_file)); // Run the executable in source path @@ -339,8 +352,10 @@ TEST_F(MoveTreeWorkItemTest, MoveFileInUse) { // test Do() scoped_ptr<MoveTreeWorkItem> work_item( - WorkItem::CreateMoveTreeWorkItem( - from_file, to_file, temp_to_dir_.path())); + WorkItem::CreateMoveTreeWorkItem(from_file, + to_file, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); EXPECT_TRUE(work_item->Do()); EXPECT_TRUE(file_util::PathExists(from_dir)); @@ -366,5 +381,172 @@ TEST_F(MoveTreeWorkItemTest, MoveFileInUse) { EXPECT_TRUE(file_util::PathExists(from_dir)); EXPECT_TRUE(file_util::ContentsEqual(exe_full_path, from_file)); EXPECT_TRUE(file_util::PathExists(to_dir)); - EXPECT_EQ(0, ReadTextFile(to_file).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(to_file).compare(kTextContent1)); +} + +// Move one directory from source to destination when destination already +// exists. +TEST_F(MoveTreeWorkItemTest, MoveDirectoryDestExistsCheckForDuplicatesFull) { + // Create two level deep source dir + FilePath from_dir1(temp_from_dir_.path()); + from_dir1 = from_dir1.AppendASCII("From_Dir1"); + file_util::CreateDirectory(from_dir1); + ASSERT_TRUE(file_util::PathExists(from_dir1)); + + FilePath from_dir2(from_dir1); + from_dir2 = from_dir2.AppendASCII("From_Dir2"); + file_util::CreateDirectory(from_dir2); + ASSERT_TRUE(file_util::PathExists(from_dir2)); + + FilePath from_file(from_dir2); + from_file = from_file.AppendASCII("From_File"); + CreateTextFile(from_file.value(), kTextContent1); + ASSERT_TRUE(file_util::PathExists(from_file)); + + // Create destination path + FilePath to_dir(temp_from_dir_.path()); + to_dir = to_dir.AppendASCII("To_Dir"); + file_util::CreateDirectory(to_dir); + ASSERT_TRUE(file_util::PathExists(to_dir)); + + // Create a sub-directory of the same name as in the source directory. + FilePath to_dir2(to_dir); + to_dir2 = to_dir2.AppendASCII("From_Dir2"); + file_util::CreateDirectory(to_dir2); + ASSERT_TRUE(file_util::PathExists(to_dir2)); + + // Create an identical file in the to sub-directory. + FilePath orig_to_file(to_dir2); + orig_to_file = orig_to_file.AppendASCII("From_File"); + CreateTextFile(orig_to_file.value(), kTextContent1); + ASSERT_TRUE(file_util::PathExists(orig_to_file)); + + // Lock one of the files in the to sub-directory to prevent moves. + file_util::MemoryMappedFile mapped_file; + EXPECT_TRUE(mapped_file.Initialize(orig_to_file)); + + // First check that we can't do the regular Move(). + scoped_ptr<MoveTreeWorkItem> work_item( + WorkItem::CreateMoveTreeWorkItem(from_dir1, + to_dir, + temp_to_dir_.path(), + WorkItem::ALWAYS_MOVE)); + EXPECT_FALSE(work_item->Do()); + work_item->Rollback(); + + // Now test Do() with the check for duplicates. This should pass. + work_item.reset( + WorkItem::CreateMoveTreeWorkItem(from_dir1, + to_dir, + temp_to_dir_.path(), + WorkItem::CHECK_DUPLICATES)); + EXPECT_TRUE(work_item->Do()); + + // Make sure that we "moved" the files, i.e. that the source directory isn't + // there anymore, + EXPECT_FALSE(file_util::PathExists(from_dir1)); + // Make sure that the original directory structure and file are still present. + EXPECT_TRUE(file_util::PathExists(to_dir)); + EXPECT_TRUE(file_util::PathExists(orig_to_file)); + // Make sure that the backup path is not empty. + EXPECT_FALSE(file_util::IsDirectoryEmpty(temp_to_dir_.path())); + + // Check that the work item believes the source to have been moved. + EXPECT_TRUE(work_item->source_moved_to_backup_); + EXPECT_FALSE(work_item->moved_to_dest_path_); + EXPECT_FALSE(work_item->moved_to_backup_); + + // test rollback() + work_item->Rollback(); + + // Once we rollback all the original files should still be there, as should + // the source files. + EXPECT_TRUE(file_util::PathExists(from_dir1)); + EXPECT_TRUE(file_util::PathExists(to_dir)); + EXPECT_TRUE(file_util::PathExists(orig_to_file)); + EXPECT_EQ(0, ReadTextFile(orig_to_file).compare(kTextContent1)); + EXPECT_EQ(0, ReadTextFile(from_file).compare(kTextContent1)); +} + +// Move one directory from source to destination when destination already +// exists but contains only a subset of the files in source. +TEST_F(MoveTreeWorkItemTest, MoveDirectoryDestExistsCheckForDuplicatesPartial) { + // Create two level deep source dir + FilePath from_dir1(temp_from_dir_.path()); + from_dir1 = from_dir1.AppendASCII("From_Dir1"); + file_util::CreateDirectory(from_dir1); + ASSERT_TRUE(file_util::PathExists(from_dir1)); + + FilePath from_dir2(from_dir1); + from_dir2 = from_dir2.AppendASCII("From_Dir2"); + file_util::CreateDirectory(from_dir2); + ASSERT_TRUE(file_util::PathExists(from_dir2)); + + FilePath from_file(from_dir2); + from_file = from_file.AppendASCII("From_File"); + CreateTextFile(from_file.value(), kTextContent1); + ASSERT_TRUE(file_util::PathExists(from_file)); + + FilePath from_file2(from_dir2); + from_file2 = from_file2.AppendASCII("From_File2"); + CreateTextFile(from_file2.value(), kTextContent2); + ASSERT_TRUE(file_util::PathExists(from_file2)); + + // Create destination path + FilePath to_dir(temp_from_dir_.path()); + to_dir = to_dir.AppendASCII("To_Dir"); + file_util::CreateDirectory(to_dir); + ASSERT_TRUE(file_util::PathExists(to_dir)); + + // Create a sub-directory of the same name as in the source directory. + FilePath to_dir2(to_dir); + to_dir2 = to_dir2.AppendASCII("From_Dir2"); + file_util::CreateDirectory(to_dir2); + ASSERT_TRUE(file_util::PathExists(to_dir2)); + + // Create one of the files in the to sub-directory, but not the other. + FilePath orig_to_file(to_dir2); + orig_to_file = orig_to_file.AppendASCII("From_File"); + CreateTextFile(orig_to_file.value(), kTextContent1); + ASSERT_TRUE(file_util::PathExists(orig_to_file)); + + // test Do(), check for duplicates. + scoped_ptr<MoveTreeWorkItem> work_item( + WorkItem::CreateMoveTreeWorkItem(from_dir1, + to_dir, + temp_to_dir_.path(), + WorkItem::CHECK_DUPLICATES)); + EXPECT_TRUE(work_item->Do()); + + // Make sure that we "moved" the files, i.e. that the source directory isn't + // there anymore, + EXPECT_FALSE(file_util::PathExists(from_dir1)); + // Make sure that the original directory structure and file are still present. + EXPECT_TRUE(file_util::PathExists(to_dir)); + EXPECT_TRUE(file_util::PathExists(orig_to_file)); + // Make sure that the backup path is not empty. + EXPECT_FALSE(file_util::IsDirectoryEmpty(temp_to_dir_.path())); + // Make sure that the "new" file is also present. + FilePath new_to_file2(to_dir2); + new_to_file2 = new_to_file2.AppendASCII("From_File2"); + EXPECT_TRUE(file_util::PathExists(new_to_file2)); + + // Check that the work item believes that this was a regular move. + EXPECT_FALSE(work_item->source_moved_to_backup_); + EXPECT_TRUE(work_item->moved_to_dest_path_); + EXPECT_TRUE(work_item->moved_to_backup_); + + // test rollback() + work_item->Rollback(); + + // Once we rollback all the original files should still be there, as should + // the source files. + EXPECT_TRUE(file_util::PathExists(from_dir1)); + EXPECT_TRUE(file_util::PathExists(to_dir)); + EXPECT_TRUE(file_util::PathExists(orig_to_file)); + EXPECT_EQ(0, ReadTextFile(orig_to_file).compare(kTextContent1)); + EXPECT_EQ(0, ReadTextFile(from_file).compare(kTextContent1)); + + // Also, after rollback the new "to" file should be gone. + EXPECT_FALSE(file_util::PathExists(new_to_file2)); } diff --git a/chrome/installer/util/work_item.cc b/chrome/installer/util/work_item.cc index 59583f9..8a047b2 100644 --- a/chrome/installer/util/work_item.cc +++ b/chrome/installer/util/work_item.cc @@ -63,8 +63,12 @@ DeleteTreeWorkItem* WorkItem::CreateDeleteTreeWorkItem( MoveTreeWorkItem* WorkItem::CreateMoveTreeWorkItem( const FilePath& source_path, const FilePath& dest_path, - const FilePath& temp_dir) { - return new MoveTreeWorkItem(source_path, dest_path, temp_dir); + const FilePath& temp_dir, + MoveTreeOption duplicate_option) { + return new MoveTreeWorkItem(source_path, + dest_path, + temp_dir, + duplicate_option); } SetRegValueWorkItem* WorkItem::CreateSetRegValueWorkItem( diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h index 66925ed..a4c8f6e 100644 --- a/chrome/installer/util/work_item.h +++ b/chrome/installer/util/work_item.h @@ -34,7 +34,7 @@ class WorkItemList; class WorkItem { public: // Possible states - typedef enum CopyOverWriteOption { + enum CopyOverWriteOption { ALWAYS, // Always overwrite regardless of what existed before. NEVER, // Not used currently. IF_DIFFERENT, // Overwrite if different. Currently only applies to file. @@ -42,6 +42,12 @@ class WorkItem { NEW_NAME_IF_IN_USE // Copy to a new path if dest is in use(only files). }; + // Options for the MoveTree work item. + enum MoveTreeOption { + ALWAYS_MOVE, // Always attempt to do a move operation. + CHECK_DUPLICATES // Only move if the move target is different. + }; + // Abstract base class for the conditions used by ConditionWorkItemList. // TODO(robertshield): Move this out of WorkItem. class Condition { @@ -97,7 +103,8 @@ class WorkItem { static MoveTreeWorkItem* CreateMoveTreeWorkItem( const FilePath& source_path, const FilePath& dest_path, - const FilePath& temp_dir); + const FilePath& temp_dir, + MoveTreeOption duplicate_option); // Create a SetRegValueWorkItem that sets a registry value with REG_SZ type // at the key with specified path. diff --git a/chrome/installer/util/work_item_list.cc b/chrome/installer/util/work_item_list.cc index e3ccc32..d32f068 100644 --- a/chrome/installer/util/work_item_list.cc +++ b/chrome/installer/util/work_item_list.cc @@ -135,10 +135,12 @@ WorkItem* WorkItemList::AddDeleteTreeWorkItem(const FilePath& root_path, WorkItem* WorkItemList::AddMoveTreeWorkItem(const std::wstring& source_path, const std::wstring& dest_path, - const std::wstring& temp_dir) { + const std::wstring& temp_dir, + MoveTreeOption duplicate_option) { WorkItem* item = WorkItem::CreateMoveTreeWorkItem(FilePath(source_path), FilePath(dest_path), - FilePath(temp_dir)); + FilePath(temp_dir), + duplicate_option); AddWorkItem(item); return item; } diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h index bc74e2d..f1d7c8f 100644 --- a/chrome/installer/util/work_item_list.h +++ b/chrome/installer/util/work_item_list.h @@ -81,7 +81,8 @@ class WorkItemList : public WorkItem { // Add a MoveTreeWorkItem to the list of work items. virtual WorkItem* AddMoveTreeWorkItem(const std::wstring& source_path, const std::wstring& dest_path, - const std::wstring& temp_dir); + const std::wstring& temp_dir, + MoveTreeOption duplicate_option); // Add a SetRegValueWorkItem that sets a registry value with REG_SZ type // at the key with specified path. |