diff options
author | rahulk@google.com <rahulk@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-07-31 18:26:15 +0000 |
---|---|---|
committer | rahulk@google.com <rahulk@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-07-31 18:26:15 +0000 |
commit | d23b9a6c93357c0217d2f393e812232759a8dbc0 (patch) | |
tree | 1e1eefdf97f020c6b556ff2b79642a00f62bcee5 | |
parent | b3358a60841326bc3da7222191994e33135339ad (diff) | |
download | chromium_src-d23b9a6c93357c0217d2f393e812232759a8dbc0.zip chromium_src-d23b9a6c93357c0217d2f393e812232759a8dbc0.tar.gz chromium_src-d23b9a6c93357c0217d2f393e812232759a8dbc0.tar.bz2 |
My earlier change to blindly copy all the files under Application\Chrome breaks updates if any dictionary is currently open. Fix it to copy the default dictionaries of installer only if there aren't any present already.
BUG=1302580
TEST=Run 149.1 version of Chrome and do an operation that requires dictionary to be open (write in a text area). Simultaneously try to upgrade Chrome and it should not fail.
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@188 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/installer/setup/install.cc | 75 | ||||
-rw-r--r-- | chrome/installer/setup/setup_constants.cc | 1 | ||||
-rw-r--r-- | chrome/installer/setup/setup_constants.h | 1 | ||||
-rw-r--r-- | chrome/installer/util/copy_tree_work_item.cc | 6 | ||||
-rw-r--r-- | chrome/installer/util/copy_tree_work_item_unittest.cc | 79 | ||||
-rw-r--r-- | chrome/installer/util/work_item.h | 1 |
6 files changed, 116 insertions, 47 deletions
diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc index e06f936..9cdd58d 100644 --- a/chrome/installer/setup/install.cc +++ b/chrome/installer/setup/install.cc @@ -187,54 +187,35 @@ bool installer::InstallNewVersion(const std::wstring& exe_path, install_list->AddCreateDirWorkItem(temp_dir); install_list->AddCreateDirWorkItem(install_path); - // Browse for all the files in archive, add work items to copy them while - // handling special cases of executables. - LOG(INFO) << "Looking for Chrome installation files under " << src_path; - std::wstring root_path(src_path); - file_util::AppendToPath(&root_path, L"*"); - WIN32_FIND_DATA find_file_data; - HANDLE file_handle = FindFirstFile(root_path.c_str(), &find_file_data); - BOOL ret = TRUE; - while (ret) { - LOG(INFO) << "directory found: " << find_file_data.cFileName; - // We do not want any directories starting with '.' - if ((wcslen(find_file_data.cFileName) <= 0) || - (find_file_data.cFileName[0] == '.')) { - LOG(INFO) << "Ignoring directory found: " << find_file_data.cFileName; - } else if (_wcsicmp(find_file_data.cFileName, - installer_util::kChromeExe) == 0) { - // Special case of chrome.exe. Delete any new_chrome.exe if present - // (we will create a new one if chrome.exe is in use) and then - // copy chrome.exe. - install_list->AddDeleteTreeWorkItem( - AppendPath(install_path, installer::kChromeNewExe), std::wstring()); - install_list->AddCopyTreeWorkItem( - AppendPath(src_path, installer_util::kChromeExe), - AppendPath(install_path, installer_util::kChromeExe), - temp_dir, WorkItem::RENAME_IF_IN_USE, - AppendPath(install_path, installer::kChromeNewExe)); - } else if (_wcsicmp(find_file_data.cFileName, - installer::kWowHelperExe) == 0) { - // Special case of wow_helper.exe which is required only on 64 bit - // systems. This exe runs only for a short time when Chrome starts so - // it should not be locked most of the times and can be overwritten - // directly. - if (Is64bit()) { - install_list->AddCopyTreeWorkItem( - AppendPath(src_path, installer::kWowHelperExe), - AppendPath(install_path, installer::kWowHelperExe), - temp_dir, WorkItem::ALWAYS); - } - } else { - // In all other cases just copy the file/directory to install location. - install_list->AddCopyTreeWorkItem( - AppendPath(src_path, find_file_data.cFileName), - AppendPath(install_path, find_file_data.cFileName), - temp_dir, WorkItem::ALWAYS); // Always overwrite. - } - ret = FindNextFile(file_handle, &find_file_data); + // Copy the version folder + install_list->AddCopyTreeWorkItem( + AppendPath(src_path, new_version.GetString()), + AppendPath(install_path, new_version.GetString()), + temp_dir, WorkItem::ALWAYS); // Always overwrite. + + // Delete any new_chrome.exe if present (we will end up create a new one + // if required) and then copy chrome.exe + install_list->AddDeleteTreeWorkItem( + AppendPath(install_path, installer::kChromeNewExe), std::wstring()); + install_list->AddCopyTreeWorkItem( + AppendPath(src_path, installer_util::kChromeExe), + AppendPath(install_path, installer_util::kChromeExe), + temp_dir, WorkItem::RENAME_IF_IN_USE, + AppendPath(install_path, installer::kChromeNewExe)); + + // Extra executable for 64 bit systems. + if (Is64bit()) { + install_list->AddCopyTreeWorkItem( + AppendPath(src_path, installer::kWowHelperExe), + AppendPath(install_path, installer::kWowHelperExe), + temp_dir, WorkItem::ALWAYS); } - FindClose(file_handle); + + // Copy the default Dictionaries only if the folder doesnt exist already + install_list->AddCopyTreeWorkItem( + AppendPath(src_path, installer::kDictionaries), + AppendPath(install_path, installer::kDictionaries), + temp_dir, WorkItem::IF_NOT_PRESENT); // Copy installer in install directory and // add shortcut in Control Panel->Add/Remove Programs. diff --git a/chrome/installer/setup/setup_constants.cc b/chrome/installer/setup/setup_constants.cc index 3397836..ef58ddb 100644 --- a/chrome/installer/setup/setup_constants.cc +++ b/chrome/installer/setup/setup_constants.cc @@ -34,6 +34,7 @@ namespace installer { const wchar_t kChromeOldExe[] = L"old_chrome.exe"; const wchar_t kChromeNewExe[] = L"new_chrome.exe"; const wchar_t kWowHelperExe[] = L"wow_helper.exe"; +const wchar_t kDictionaries[] = L"Dictionaries"; const wchar_t kChromeArchive[] = L"chrome.7z"; const wchar_t kChromePatchArchive[] = L"patch.7z"; const wchar_t kChromeCompressedArchive[] = L"chrome.packed.7z"; diff --git a/chrome/installer/setup/setup_constants.h b/chrome/installer/setup/setup_constants.h index 7e4bc48..85f8f88 100644 --- a/chrome/installer/setup/setup_constants.h +++ b/chrome/installer/setup/setup_constants.h @@ -37,6 +37,7 @@ namespace installer { extern const wchar_t kChromeOldExe[]; extern const wchar_t kChromeNewExe[]; extern const wchar_t kWowHelperExe[]; +extern const wchar_t kDictionaries[]; extern const wchar_t kChromeArchive[]; extern const wchar_t kChromePatchArchive[]; extern const wchar_t kChromeCompressedArchive[]; diff --git a/chrome/installer/util/copy_tree_work_item.cc b/chrome/installer/util/copy_tree_work_item.cc index c6eaacd..f61d5c7 100644 --- a/chrome/installer/util/copy_tree_work_item.cc +++ b/chrome/installer/util/copy_tree_work_item.cc @@ -93,6 +93,12 @@ bool CopyTreeWorkItem::Do() { } } + // handle overwrite_option_ = IF_NOT_PRESENT case + if ((dest_exist) && + (overwrite_option_ == WorkItem::IF_NOT_PRESENT)) { + return true; + } + // All other cases where we move dest if it exists, and copy the files if (dest_exist) { if (!GetBackupPath()) diff --git a/chrome/installer/util/copy_tree_work_item_unittest.cc b/chrome/installer/util/copy_tree_work_item_unittest.cc index 40e4a95..da75a8c 100644 --- a/chrome/installer/util/copy_tree_work_item_unittest.cc +++ b/chrome/installer/util/copy_tree_work_item_unittest.cc @@ -500,6 +500,85 @@ TEST_F(CopyTreeWorkItemTest, RenameAndCopyTest) { EXPECT_FALSE(file_util::PathExists(alternate_to)); } +// Test overwrite option IF_NOT_PRESENT: +// 1. If destination file/directory exist, the source should not be copied +// 2. If destination file/directory do not exist, the source should be copied +// in the destination folder after Do() and should be rolled back after +// Rollback(). +TEST_F(CopyTreeWorkItemTest, IfNotPresentTest) { + // Create source file + std::wstring file_name_from(test_dir_); + file_util::AppendToPath(&file_name_from, L"File_From"); + CreateTextFile(file_name_from, text_content_1); + ASSERT_TRUE(file_util::PathExists(file_name_from)); + + // Create an executable in destination path by copying ourself to it. + wchar_t exe_full_path_str[MAX_PATH]; + ::GetModuleFileNameW(NULL, exe_full_path_str, MAX_PATH); + std::wstring exe_full_path(exe_full_path_str); + std::wstring dir_name_to(test_dir_); + file_util::AppendToPath(&dir_name_to, L"Copy_To_Subdir"); + CreateDirectory(dir_name_to.c_str(), NULL); + ASSERT_TRUE(file_util::PathExists(dir_name_to)); + std::wstring file_name_to(dir_name_to); + file_util::AppendToPath(&file_name_to, L"File_To"); + file_util::CopyFile(exe_full_path, file_name_to); + ASSERT_TRUE(file_util::PathExists(file_name_to)); + + // Get the path of backup file + std::wstring backup_file(temp_dir_); + file_util::AppendToPath(&backup_file, L"File_To"); + + // test Do(). + scoped_ptr<CopyTreeWorkItem> work_item( + WorkItem::CreateCopyTreeWorkItem(file_name_from, file_name_to, + temp_dir_, WorkItem::IF_NOT_PRESENT, + L"")); + EXPECT_TRUE(work_item->Do()); + + // verify that the source, destination have not changed and backup path + // does not exist + EXPECT_TRUE(file_util::PathExists(file_name_from)); + EXPECT_TRUE(file_util::PathExists(file_name_to)); + EXPECT_EQ(0, ReadTextFile(file_name_from).compare(text_content_1)); + EXPECT_TRUE(file_util::ContentsEqual(exe_full_path, file_name_to)); + EXPECT_FALSE(file_util::PathExists(backup_file)); + + // test rollback() + work_item->Rollback(); + + // verify that the source, destination have not changed and backup path + // does not exist after rollback also + EXPECT_TRUE(file_util::PathExists(file_name_from)); + EXPECT_TRUE(file_util::PathExists(file_name_to)); + EXPECT_EQ(0, ReadTextFile(file_name_from).compare(text_content_1)); + EXPECT_TRUE(file_util::ContentsEqual(exe_full_path, file_name_to)); + EXPECT_FALSE(file_util::PathExists(backup_file)); + + // Now delete the destination and try copying the file again. + file_util::Delete(file_name_to, true); + work_item.reset(WorkItem::CreateCopyTreeWorkItem( + file_name_from, file_name_to, temp_dir_, WorkItem::IF_NOT_PRESENT, L"")); + EXPECT_TRUE(work_item->Do()); + + // verify that the source, destination are the same and backup path + // does not exist + EXPECT_TRUE(file_util::PathExists(file_name_from)); + EXPECT_TRUE(file_util::PathExists(file_name_to)); + EXPECT_EQ(0, ReadTextFile(file_name_from).compare(text_content_1)); + EXPECT_EQ(0, ReadTextFile(file_name_to).compare(text_content_1)); + EXPECT_FALSE(file_util::PathExists(backup_file)); + + // test rollback() + work_item->Rollback(); + + // verify that the destination does not exist anymore + EXPECT_TRUE(file_util::PathExists(file_name_from)); + EXPECT_FALSE(file_util::PathExists(file_name_to)); + EXPECT_EQ(0, ReadTextFile(file_name_from).compare(text_content_1)); + EXPECT_FALSE(file_util::PathExists(backup_file)); +} + // Copy one file without rollback. The existing one in destination is in use. // Verify it is moved to backup location and stays there. TEST_F(CopyTreeWorkItemTest, CopyFileInUseAndCleanup) { diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h index c167cf3..1060857 100644 --- a/chrome/installer/util/work_item.h +++ b/chrome/installer/util/work_item.h @@ -53,6 +53,7 @@ class WorkItem { ALWAYS, // Always overwrite regardless of what existed before. NEVER, // Not used currently. IF_DIFFERENT, // Overwrite if different. Currently only applies to file. + IF_NOT_PRESENT, // Copy only if file/directory do not exist already. RENAME_IF_IN_USE // Copy to a new path instead of overwriting (only files). }; |