diff options
author | kuchhal@chromium.org <kuchhal@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-11-07 01:27:57 +0000 |
---|---|---|
committer | kuchhal@chromium.org <kuchhal@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-11-07 01:27:57 +0000 |
commit | 2414e84fdc99feb8e3a674a1776cddf5d24529fd (patch) | |
tree | a7735239d78bdbc37dcaf5a6428b3076d8a5ff33 /chrome | |
parent | 0c294877146c3812e0a69a5a6ec0cf3e29803954 (diff) | |
download | chromium_src-2414e84fdc99feb8e3a674a1776cddf5d24529fd.zip chromium_src-2414e84fdc99feb8e3a674a1776cddf5d24529fd.tar.gz chromium_src-2414e84fdc99feb8e3a674a1776cddf5d24529fd.tar.bz2 |
Implement the new mechanism we are going to use for handling Chromium updates
while it is in use. This should work for per user as well as system level
installs (after some additional changes once Google Update changes are ready).
The following scenarios should work now:
- If Chromium is using two different profiles at the same time we do not switch
chrome executables until all of them are closed.
- The old version of Chromium can be run after a restart if the renaming of
executables fails.
- We will not use environment variable any more but we need to keep it until all
the users get this change on their machines.
- opv/rename registry keys and new_chrome.exe should always together. If one
exist all three should exist because they are created and deleted as one atomic
operation (as much as possible given laws of physics).
BUG=1463346
Review URL: http://codereview.chromium.org/9436
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@4949 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
23 files changed, 514 insertions, 65 deletions
diff --git a/chrome/app/client_util.cc b/chrome/app/client_util.cc index a8841cd..9908cf6 100644 --- a/chrome/app/client_util.cc +++ b/chrome/app/client_util.cc @@ -5,9 +5,9 @@ #include "chrome/app/client_util.h" #include "chrome/installer/util/install_util.h" -namespace client_util { -const wchar_t kProductVersionKey[] = L"pv"; +#include "chrome/installer/util/google_update_constants.h" +namespace client_util { bool FileExists(const wchar_t* const file_path) { WIN32_FILE_ATTRIBUTE_DATA attrs; return ::GetFileAttributesEx(file_path, GetFileExInfoStandard, &attrs) != 0; @@ -25,10 +25,24 @@ bool GetChromiumVersion(const wchar_t* const exe_path, } DWORD size = 0; bool ret = false; - if (::RegQueryValueEx(reg_key, client_util::kProductVersionKey, NULL, NULL, + if (::RegQueryValueEx(reg_key, google_update::kRegOldVersionField, NULL, NULL, + NULL, &size) == ERROR_SUCCESS) { + *version = new wchar_t[1 + (size / sizeof(wchar_t))]; + if (::RegQueryValueEx(reg_key, google_update::kRegOldVersionField, + NULL, NULL, reinterpret_cast<BYTE*>(*version), + &size) == ERROR_SUCCESS) { + ret = true; + } else { + delete[] *version; + } + ::RegCloseKey(reg_key); + return ret; + } + + if (::RegQueryValueEx(reg_key, google_update::kRegVersionField, NULL, NULL, NULL, &size) == ERROR_SUCCESS) { *version = new wchar_t[1 + (size / sizeof(wchar_t))]; - if (::RegQueryValueEx(reg_key, client_util::kProductVersionKey, + if (::RegQueryValueEx(reg_key, google_update::kRegVersionField, NULL, NULL, reinterpret_cast<BYTE*>(*version), &size) == ERROR_SUCCESS) { ret = true; diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index c28edc8..7cf76df9 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -441,10 +441,10 @@ int BrowserMain(CommandLine &parsed_command_line, int show_command, // Sometimes we end up killing browser process (http://b/1308130) so make // sure we recreate unique event to indicate running browser process. message_window.HuntForZombieChromeProcesses(); - CreateUniqueChromeEvent(); + already_running = (already_running && CreateUniqueChromeEvent()); // Do the tasks if chrome has been upgraded while it was last running. - if (DoUpgradeTasks(parsed_command_line)) { + if (!already_running && DoUpgradeTasks(parsed_command_line)) { return ResultCodes::NORMAL_EXIT; } diff --git a/chrome/browser/first_run.cc b/chrome/browser/first_run.cc index ece99a5..b9c9884 100644 --- a/chrome/browser/first_run.cc +++ b/chrome/browser/first_run.cc @@ -13,7 +13,9 @@ #include "base/logging.h" #include "base/object_watcher.h" #include "base/path_service.h" +#include "base/process.h" #include "base/process_util.h" +#include "base/registry.h" #include "base/string_util.h" #include "chrome/app/result_codes.h" #include "chrome/common/chrome_constants.h" @@ -27,8 +29,12 @@ #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/pref_service.h" +#include "chrome/installer/util/browser_distribution.h" +#include "chrome/installer/util/google_update_constants.h" +#include "chrome/installer/util/install_util.h" #include "chrome/installer/util/master_preferences.h" #include "chrome/installer/util/shell_util.h" +#include "chrome/installer/util/util_constants.h" #include "chrome/views/accelerator_handler.h" #include "chrome/views/window.h" @@ -37,10 +43,6 @@ namespace { // The kSentinelFile file absence will tell us it is a first run. const wchar_t kSentinelFile[] = L"First Run"; -// These two names are used for upgrades in-place of the chrome exe. -const wchar_t kChromeUpgradeExe[] = L"new_chrome.exe"; -const wchar_t kChromeBackupExe[] = L"old_chrome.exe"; - // Gives the full path to the sentinel file. The file might not exist. bool GetFirstRunSentinelFilePath(std::wstring* path) { std::wstring first_run_sentinel; @@ -54,14 +56,14 @@ bool GetFirstRunSentinelFilePath(std::wstring* path) { bool GetNewerChromeFile(std::wstring* path) { if (!PathService::Get(base::DIR_EXE, path)) return false; - file_util::AppendToPath(path, kChromeUpgradeExe); + file_util::AppendToPath(path, installer_util::kChromeNewExe); return true; } bool GetBackupChromeFile(std::wstring* path) { if (!PathService::Get(base::DIR_EXE, path)) return false; - file_util::AppendToPath(path, kChromeBackupExe); + file_util::AppendToPath(path, installer_util::kChromeOldExe); return true; } @@ -197,6 +199,22 @@ bool Upgrade::SwapNewChromeExeIfPresent() { std::wstring old_chrome_exe; if (!PathService::Get(base::FILE_EXE, &old_chrome_exe)) return false; + RegKey key; + HKEY reg_root = InstallUtil::IsPerUserInstall(old_chrome_exe.c_str()) ? + HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE; + BrowserDistribution *dist = BrowserDistribution::GetDistribution(); + std::wstring rename_cmd; + if (key.Open(reg_root, dist->GetVersionKey().c_str(), KEY_READ) && + key.ReadValue(google_update::kRegRenameCmdField, &rename_cmd)) { + ProcessHandle handle; + if (process_util::LaunchApp(rename_cmd, true, true, &handle)) { + DWORD exit_code; + ::GetExitCodeProcess(handle, &exit_code); + ::CloseHandle(handle); + if (exit_code == installer_util::RENAME_SUCCESSFUL) + return true; + } + } std::wstring backup_exe; if (!GetBackupChromeFile(&backup_exe)) return false; diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc index 34429b3..b051f82 100644 --- a/chrome/installer/setup/install.cc +++ b/chrome/installer/setup/install.cc @@ -5,13 +5,16 @@ #include <time.h> #include "base/file_util.h" +#include "base/registry.h" #include "base/scoped_ptr.h" #include "base/string_util.h" #include "chrome/installer/setup/setup.h" #include "chrome/installer/setup/setup_constants.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/google_update_constants.h" +#include "chrome/installer/util/set_reg_value_work_item.h" #include "chrome/installer/util/shell_util.h" +#include "chrome/installer/util/work_item.h" namespace { std::wstring AppendPath(const std::wstring parent_path, @@ -154,13 +157,22 @@ bool installer::InstallNewVersion(const std::wstring& exe_path, // 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()); + std::wstring new_chrome_exe = AppendPath(install_path, + installer_util::kChromeNewExe); + BrowserDistribution* dist = BrowserDistribution::GetDistribution(); + RegKey chrome_key(reg_root, dist->GetVersionKey().c_str(), + KEY_READ | KEY_WRITE); + std::wstring current_version; + if (file_util::PathExists(new_chrome_exe)) + chrome_key.ReadValue(google_update::kRegOldVersionField, ¤t_version); + if (current_version.empty()) + chrome_key.ReadValue(google_update::kRegVersionField, ¤t_version); + + install_list->AddDeleteTreeWorkItem(new_chrome_exe, 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)); + temp_dir, WorkItem::NEW_NAME_IF_IN_USE, new_chrome_exe); // Extra executable for 64 bit systems. if (Is64bit()) { @@ -180,14 +192,13 @@ bool installer::InstallNewVersion(const std::wstring& exe_path, // add shortcut in Control Panel->Add/Remove Programs. AddInstallerCopyTasks(exe_path, archive_path, temp_dir, install_path, new_version.GetString(), install_list.get()); - BrowserDistribution* dist = BrowserDistribution::GetDistribution(); std::wstring product_name = dist->GetApplicationName(); AddUninstallShortcutWorkItems(reg_root, exe_path, install_path, product_name, new_version.GetString(), install_list.get()); // Delete any old_chrome.exe if present. install_list->AddDeleteTreeWorkItem( - AppendPath(install_path, installer::kChromeOldExe), std::wstring()); + AppendPath(install_path, installer_util::kChromeOldExe), std::wstring()); // Create Version key (if not already present) and set the new Chrome // version as last step. @@ -203,13 +214,56 @@ bool installer::InstallNewVersion(const std::wstring& exe_path, true); // overwrite version // Perform install operations. - if (!install_list->Do()) { + bool success = install_list->Do(); + + // If the installation worked, handle the in-use update case: + // * If new_chrome.exe exists, then currently Chrome was in use so save + // current version in old version key. + // * If new_chrome.exe doesnt exist, then delete old version key. + if (success) { + if (file_util::PathExists(new_chrome_exe)) { + if (current_version.empty()) { + LOG(ERROR) << "New chrome.exe exists but current version is empty!"; + success = false; + } else { + scoped_ptr<WorkItemList> inuse_list(WorkItem::CreateWorkItemList()); + inuse_list->AddSetRegValueWorkItem(reg_root, + version_key, + google_update::kRegOldVersionField, + current_version.c_str(), + true); + std::wstring rename_cmd(installer::GetInstallerPathUnderChrome( + install_path, new_version.GetString())); + file_util::AppendToPath(&rename_cmd, + file_util::GetFilenameFromPath(exe_path)); + rename_cmd = L"\"" + rename_cmd + L"\" --" + + installer_util::switches::kRenameChromeExe; + inuse_list->AddSetRegValueWorkItem(reg_root, + version_key, + google_update::kRegRenameCmdField, + rename_cmd.c_str(), + true); + if (!inuse_list->Do()) { + LOG(ERROR) << "Couldn't write old version/rename value to registry."; + success = false; + inuse_list->Rollback(); + } + } + } else { + if (chrome_key.ValueExists(google_update::kRegOldVersionField) && + !chrome_key.DeleteValue(google_update::kRegOldVersionField)) { + LOG(ERROR) << "New chrome.exe doesn't exist but failed to delete " + << "old version value in registry."; + success = false; + } + } + } + + if (!success) { LOG(ERROR) << "Install failed, rolling back... "; install_list->Rollback(); LOG(ERROR) << "Rollback complete. "; - return false; } - - return true; + return success; } diff --git a/chrome/installer/setup/main.cc b/chrome/installer/setup/main.cc index 57730e3..730f4ff 100644..100755 --- a/chrome/installer/setup/main.cc +++ b/chrome/installer/setup/main.cc @@ -161,6 +161,55 @@ installer::Version* GetVersionFromDir(const std::wstring& chrome_path) { return version; } +// This function is called when --rename-chrome-exe option is specified on +// setup.exe command line. This function assumes an in-use update has happened +// for Chrome so there should be a file called new_chrome.exe on the file +// system and a key called 'opv' in the registry. This function will move +// new_chrome.exe to chrome.exe and delete 'opv' key in one atomic operation. +installer_util::InstallStatus RenameChromeExecutables(bool system_install) { + std::wstring chrome_path(installer::GetChromeInstallPath(system_install)); + + std::wstring chrome_exe(chrome_path); + file_util::AppendToPath(&chrome_exe, installer_util::kChromeExe); + std::wstring chrome_old_exe(chrome_path); + file_util::AppendToPath(&chrome_old_exe, installer_util::kChromeOldExe); + std::wstring chrome_new_exe(chrome_path); + file_util::AppendToPath(&chrome_new_exe, installer_util::kChromeNewExe); + + scoped_ptr<WorkItemList> install_list(WorkItem::CreateWorkItemList()); + install_list->AddDeleteTreeWorkItem(chrome_old_exe, std::wstring()); + std::wstring temp_path; + if (!file_util::CreateNewTempDirectory(std::wstring(L"chrome_"), + &temp_path)) { + LOG(ERROR) << "Failed to create Temp directory " << temp_path; + return installer_util::RENAME_FAILED; + } + install_list->AddCopyTreeWorkItem(chrome_new_exe, + chrome_exe, + temp_path, + WorkItem::IF_DIFFERENT, + std::wstring()); + HKEY reg_root = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; + BrowserDistribution *dist = BrowserDistribution::GetDistribution(); + install_list->AddDeleteRegValueWorkItem(reg_root, + dist->GetVersionKey(), + google_update::kRegOldVersionField, + true); + install_list->AddDeleteTreeWorkItem(chrome_new_exe, std::wstring()); + install_list->AddDeleteRegValueWorkItem(reg_root, + dist->GetVersionKey(), + google_update::kRegRenameCmdField, + true); + installer_util::InstallStatus ret = installer_util::RENAME_SUCCESSFUL; + if (!install_list->Do()) { + LOG(ERROR) << "Renaming of executables failed. Rolling back any changes."; + install_list->Rollback(); + ret = installer_util::RENAME_FAILED; + } + file_util::Delete(temp_path, true); + return ret; +} + // Parse command line and read master profile, if present, to get distribution // related install options. int GetInstallOptions(const CommandLine& cmd_line) { @@ -450,6 +499,11 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance, std::wstring chrome_exe(parsed_command_line.GetSwitchValue( installer_util::switches::kRegisterChromeBrowser)); return ShellUtil::AddChromeToSetAccessDefaults(chrome_exe, true); + // If --rename-chrome-exe is specified, we want to rename the executables + // and exit. + } else if (parsed_command_line.HasSwitch( + installer_util::switches::kRenameChromeExe)) { + return RenameChromeExecutables(system_install); } if (system_install && diff --git a/chrome/installer/setup/setup_constants.cc b/chrome/installer/setup/setup_constants.cc index 4240022..a307b44 100644 --- a/chrome/installer/setup/setup_constants.cc +++ b/chrome/installer/setup/setup_constants.cc @@ -6,8 +6,6 @@ namespace installer { // Elements that makes up install target path. -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"; diff --git a/chrome/installer/setup/setup_constants.h b/chrome/installer/setup/setup_constants.h index b8a90e8..b95d774 100644 --- a/chrome/installer/setup/setup_constants.h +++ b/chrome/installer/setup/setup_constants.h @@ -9,8 +9,6 @@ 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[]; diff --git a/chrome/installer/util/copy_tree_work_item.cc b/chrome/installer/util/copy_tree_work_item.cc index 3e49a30..fc1d5e3 100644 --- a/chrome/installer/util/copy_tree_work_item.cc +++ b/chrome/installer/util/copy_tree_work_item.cc @@ -36,7 +36,7 @@ bool CopyTreeWorkItem::Do() { } bool dest_exist = file_util::PathExists(dest_path_); - // handle overwrite_option_ = IF_DIFFERENT case + // handle overwrite_option_ = IF_DIFFERENT case. if ((dest_exist) && (overwrite_option_ == WorkItem::IF_DIFFERENT) && // only for single file (!PathIsDirectory(source_path_.c_str())) && @@ -46,14 +46,12 @@ bool CopyTreeWorkItem::Do() { << " and destination file " << dest_path_ << " are exactly same. Returning true."; return true; - } - - // handle overwrite_option_ = RENAME_IF_IN_USE case - if ((dest_exist) && - (overwrite_option_ == WorkItem::RENAME_IF_IN_USE) && // only for a file - (!PathIsDirectory(source_path_.c_str())) && - (!PathIsDirectory(dest_path_.c_str())) && - (IsFileInUse(dest_path_))) { + } else if ((dest_exist) && + (overwrite_option_ == WorkItem::NEW_NAME_IF_IN_USE) && + (!PathIsDirectory(source_path_.c_str())) && + (!PathIsDirectory(dest_path_.c_str())) && + (IsFileInUse(dest_path_))) { + // handle overwrite_option_ = NEW_NAME_IF_IN_USE case. if (alternative_path_.empty() || file_util::PathExists(alternative_path_) || !file_util::CopyFile(source_path_, alternative_path_)) { @@ -66,15 +64,13 @@ bool CopyTreeWorkItem::Do() { << " to alternative path " << alternative_path_; return true; } - } - - // handle overwrite_option_ = IF_NOT_PRESENT case - if ((dest_exist) && - (overwrite_option_ == WorkItem::IF_NOT_PRESENT)) { + } else if ((dest_exist) && + (overwrite_option_ == WorkItem::IF_NOT_PRESENT)) { + // handle overwrite_option_ = IF_NOT_PRESENT case. return true; - } + } - // All other cases where we move dest if it exists, and copy the files + // In all cases that reach here, move dest to a backup path. if (dest_exist) { if (!GetBackupPath()) return false; @@ -89,6 +85,7 @@ bool CopyTreeWorkItem::Do() { } } + // In all cases that reach here, copy source to destination. if (file_util::CopyDirectory(source_path_, dest_path_, true)) { copied_to_dest_path_ = true; LOG(INFO) << "Copied source " << source_path_ diff --git a/chrome/installer/util/copy_tree_work_item.h b/chrome/installer/util/copy_tree_work_item.h index e463822..c7b5e41 100644 --- a/chrome/installer/util/copy_tree_work_item.h +++ b/chrome/installer/util/copy_tree_work_item.h @@ -56,7 +56,7 @@ class CopyTreeWorkItem : public WorkItem { // Controls the behavior for overwriting. CopyOverWriteOption overwrite_option_; - // If overwrite_option_ = RENAME_IF_IN_USE, this variables stores the path + // If overwrite_option_ = NEW_NAME_IF_IN_USE, this variables stores the path // to be used if the file is in use and hence we want to copy it to a // different path. std::wstring alternative_path_; diff --git a/chrome/installer/util/copy_tree_work_item_unittest.cc b/chrome/installer/util/copy_tree_work_item_unittest.cc index db3e8c5..1e0c04a 100644 --- a/chrome/installer/util/copy_tree_work_item_unittest.cc +++ b/chrome/installer/util/copy_tree_work_item_unittest.cc @@ -307,7 +307,7 @@ TEST_F(CopyTreeWorkItemTest, CopyFileInUse) { // 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); + ::GetModuleFileName(NULL, exe_full_path_str, MAX_PATH); std::wstring exe_full_path(exe_full_path_str); std::wstring dir_name_to(test_dir_); @@ -326,7 +326,7 @@ TEST_F(CopyTreeWorkItemTest, CopyFileInUse) { STARTUPINFOW si = {sizeof(si)}; PROCESS_INFORMATION pi = {0}; ASSERT_TRUE( - ::CreateProcessW(NULL, const_cast<wchar_t*>(file_name_to.c_str()), + ::CreateProcess(NULL, const_cast<wchar_t*>(file_name_to.c_str()), NULL, NULL, FALSE, CREATE_NO_WINDOW | CREATE_SUSPENDED, NULL, NULL, &si, &pi)); @@ -361,16 +361,18 @@ TEST_F(CopyTreeWorkItemTest, CopyFileInUse) { TerminateProcess(pi.hProcess, 0); // make sure the handle is closed. - WaitForSingleObject(pi.hProcess, INFINITE); + EXPECT_TRUE(WaitForSingleObject(pi.hProcess, 10000) == WAIT_OBJECT_0); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); } -// Test overwrite option RENAME_IF_IN_USE: +// Test overwrite option NEW_NAME_IF_IN_USE: // 1. If destination file is in use, the source should be copied with the // new name after Do() and this new name file should be deleted // after rollback. // 2. If destination file is not in use, the source should be copied in the // destination folder after Do() and should be rolled back after Rollback(). -TEST_F(CopyTreeWorkItemTest, RenameAndCopyTest) { +TEST_F(CopyTreeWorkItemTest, NewNameAndCopyTest) { // Create source file std::wstring file_name_from(test_dir_); file_util::AppendToPath(&file_name_from, L"File_From"); @@ -379,7 +381,7 @@ TEST_F(CopyTreeWorkItemTest, RenameAndCopyTest) { // 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); + ::GetModuleFileName(NULL, exe_full_path_str, MAX_PATH); std::wstring exe_full_path(exe_full_path_str); std::wstring dir_name_to(test_dir_); @@ -399,7 +401,7 @@ TEST_F(CopyTreeWorkItemTest, RenameAndCopyTest) { STARTUPINFOW si = {sizeof(si)}; PROCESS_INFORMATION pi = {0}; ASSERT_TRUE( - ::CreateProcessW(NULL, const_cast<wchar_t*>(file_name_to.c_str()), + ::CreateProcess(NULL, const_cast<wchar_t*>(file_name_to.c_str()), NULL, NULL, FALSE, CREATE_NO_WINDOW | CREATE_SUSPENDED, NULL, NULL, &si, &pi)); @@ -410,7 +412,7 @@ TEST_F(CopyTreeWorkItemTest, RenameAndCopyTest) { // test Do(). scoped_ptr<CopyTreeWorkItem> work_item( WorkItem::CreateCopyTreeWorkItem(file_name_from, file_name_to, - temp_dir_, WorkItem::RENAME_IF_IN_USE, + temp_dir_, WorkItem::NEW_NAME_IF_IN_USE, alternate_to)); EXPECT_TRUE(work_item->Do()); @@ -436,11 +438,13 @@ TEST_F(CopyTreeWorkItemTest, RenameAndCopyTest) { TerminateProcess(pi.hProcess, 0); // make sure the handle is closed. - WaitForSingleObject(pi.hProcess, INFINITE); + EXPECT_TRUE(WaitForSingleObject(pi.hProcess, 10000) == WAIT_OBJECT_0); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); // Now the process has terminated, lets try overwriting the file again work_item.reset(WorkItem::CreateCopyTreeWorkItem( - file_name_from, file_name_to, temp_dir_, WorkItem::RENAME_IF_IN_USE, + file_name_from, file_name_to, temp_dir_, WorkItem::NEW_NAME_IF_IN_USE, alternate_to)); EXPECT_TRUE(work_item->Do()); @@ -478,7 +482,7 @@ TEST_F(CopyTreeWorkItemTest, IfNotPresentTest) { // 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); + ::GetModuleFileName(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"); @@ -554,7 +558,7 @@ TEST_F(CopyTreeWorkItemTest, CopyFileInUseAndCleanup) { // 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); + ::GetModuleFileName(NULL, exe_full_path_str, MAX_PATH); std::wstring exe_full_path(exe_full_path_str); std::wstring dir_name_to(test_dir_); @@ -573,7 +577,7 @@ TEST_F(CopyTreeWorkItemTest, CopyFileInUseAndCleanup) { STARTUPINFOW si = {sizeof(si)}; PROCESS_INFORMATION pi = {0}; ASSERT_TRUE( - ::CreateProcessW(NULL, const_cast<wchar_t*>(file_name_to.c_str()), + ::CreateProcess(NULL, const_cast<wchar_t*>(file_name_to.c_str()), NULL, NULL, FALSE, CREATE_NO_WINDOW | CREATE_SUSPENDED, NULL, NULL, &si, &pi)); @@ -604,7 +608,9 @@ TEST_F(CopyTreeWorkItemTest, CopyFileInUseAndCleanup) { TerminateProcess(pi.hProcess, 0); // make sure the handle is closed. - WaitForSingleObject(pi.hProcess, INFINITE); + EXPECT_TRUE(WaitForSingleObject(pi.hProcess, 10000) == WAIT_OBJECT_0); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); } // Copy a tree from source to destination. diff --git a/chrome/installer/util/delete_reg_value_work_item.cc b/chrome/installer/util/delete_reg_value_work_item.cc new file mode 100644 index 0000000..b2f162a --- /dev/null +++ b/chrome/installer/util/delete_reg_value_work_item.cc @@ -0,0 +1,82 @@ +// Copyright (c) 2006-2008 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/delete_reg_value_work_item.h" + +#include "base/registry.h" +#include "chrome/installer/util/logging_installer.h" + +DeleteRegValueWorkItem::DeleteRegValueWorkItem(HKEY predefined_root, + std::wstring key_path, + std::wstring value_name, + bool is_str_type) + : predefined_root_(predefined_root), + key_path_(key_path), + value_name_(value_name), + is_str_type_(is_str_type), + status_(DELETE_VALUE) { +} + +DeleteRegValueWorkItem::~DeleteRegValueWorkItem() { +} + +bool DeleteRegValueWorkItem::Do() { + if (status_ != DELETE_VALUE) { + // we already did something. + LOG(ERROR) << "multiple calls to Do()"; + return false; + } + + RegKey key; + status_ = VALUE_UNCHANGED; + bool result = false; + if (!key.Open(predefined_root_, key_path_.c_str(), KEY_READ | KEY_WRITE)) { + LOG(ERROR) << "can not open " << key_path_; + } else if (!key.ValueExists(value_name_.c_str())) { + status_ = VALUE_NOT_FOUND; + result = true; + // Read previous value for rollback and delete + } else if (((is_str_type_ && key.ReadValue(value_name_.c_str(), + &old_str_)) || + (!is_str_type_ && key.ReadValueDW(value_name_.c_str(), + &old_dw_))) && + (key.DeleteValue(value_name_.c_str()))) { + status_ = VALUE_DELETED; + result = true; + } else { + LOG(ERROR) << "failed to read/delete value " << value_name_; + } + + key.Close(); + return result; +} + +void DeleteRegValueWorkItem::Rollback() { + if (status_ == DELETE_VALUE || status_ == VALUE_ROLLED_BACK) { + return; + } else if (status_ == VALUE_UNCHANGED || status_ == VALUE_NOT_FOUND) { + status_ = VALUE_ROLLED_BACK; + LOG(INFO) << "rollback: setting unchanged, nothing to do"; + return; + } + + // At this point only possible state is VALUE_DELETED. + RegKey key; + if (!key.Open(predefined_root_, key_path_.c_str(), KEY_READ | KEY_WRITE)) { + LOG(ERROR) << "rollback: can not open " << key_path_; + // try to restore the previous value + } else if ((is_str_type_ && key.WriteValue(value_name_.c_str(), + old_str_.c_str())) || + (!is_str_type_ && key.WriteValue(value_name_.c_str(), + old_dw_))) { + status_ = VALUE_ROLLED_BACK; + LOG(INFO) << "rollback: restored " << value_name_; + } else { + LOG(ERROR) << "failed to restore value " << value_name_; + } + + key.Close(); + return; +} + diff --git a/chrome/installer/util/delete_reg_value_work_item.h b/chrome/installer/util/delete_reg_value_work_item.h new file mode 100644 index 0000000..164279a --- /dev/null +++ b/chrome/installer/util/delete_reg_value_work_item.h @@ -0,0 +1,65 @@ +// Copyright (c) 2006-2008 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. + +#ifndef CHROME_INSTALLER_UTIL_DELETE_REG_VALUE_WORK_ITEM_H_ +#define CHROME_INSTALLER_UTIL_DELETE_REG_VALUE_WORK_ITEM_H_ + +#include <string> +#include <windows.h> +#include "chrome/installer/util/work_item.h" + +// A WorkItem subclass that sets a registry value with REG_SZ or REG_DWORD +// type at the specified path. The value is only set if the target key exists. +class DeleteRegValueWorkItem : public WorkItem { + public: + virtual ~DeleteRegValueWorkItem(); + + virtual bool Do(); + + virtual void Rollback(); + + private: + friend class WorkItem; + + enum DeletionStatus { + // The status before Do is called. + DELETE_VALUE, + // One possible outcome after Do(). Value is deleted. + VALUE_DELETED, + // One possible outcome after Do(). Value is not found. + VALUE_NOT_FOUND, + // The status after Do() and Rollback() is called. + VALUE_ROLLED_BACK, + // Another possible outcome after Do() (when there is an error). + VALUE_UNCHANGED + }; + + DeleteRegValueWorkItem(HKEY predefined_root, std::wstring key_path, + std::wstring value_name, bool is_str_type); + + // Root key of the target key under which the value is set. The root key can + // only be one of the predefined keys on Windows. + HKEY predefined_root_; + + // Path of the target key under which the value is set. + std::wstring key_path_; + + // Name of the value to be set. + std::wstring value_name_; + + // boolean that tells whether data value is of type REG_SZ or REG_DWORD. + // Ideally we do not need this information from user of this class and can + // check the registry for the type. But to simpify implementation we are + // going to put the burden on the caller for now to provide us the type. + bool is_str_type_; + + DeletionStatus status_; + + // Data of the previous value. + std::wstring old_str_; // if data is of type REG_SZ + DWORD old_dw_; // if data is of type REG_DWORD +}; + +#endif // CHROME_INSTALLER_UTIL_DELETE_REG_VALUE_WORK_ITEM_H_ + diff --git a/chrome/installer/util/delete_reg_value_work_item_unittest.cc b/chrome/installer/util/delete_reg_value_work_item_unittest.cc new file mode 100644 index 0000000..a5c05b6 --- /dev/null +++ b/chrome/installer/util/delete_reg_value_work_item_unittest.cc @@ -0,0 +1,105 @@ +// Copyright (c) 2006-2008 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 "base/file_util.h" +#include "base/logging.h" +#include "base/registry.h" +#include "base/scoped_ptr.h" +#include "base/string_util.h" +#include "chrome/installer/util/delete_reg_value_work_item.h" +#include "chrome/installer/util/work_item.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + wchar_t test_root[] = L"DeleteRegValueWorkItemTest"; + class DeleteRegValueWorkItemTest : public testing::Test { + protected: + virtual void SetUp() { + // Create a temporary key for testing + RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS); + key.DeleteKey(test_root); + ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, test_root, KEY_READ)); + ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, test_root, KEY_READ)); + } + virtual void TearDown() { + logging::CloseLogFile(); + // Clean up the temporary key + RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS); + ASSERT_TRUE(key.DeleteKey(test_root)); + } + }; +}; + +// Delete a value. The value should get deleted after Do() and should be +// recreated after Rollback(). +TEST_F(DeleteRegValueWorkItemTest, DeleteExistingValue) { + RegKey key; + std::wstring parent_key(test_root); + file_util::AppendToPath(&parent_key, L"WriteNew"); + ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(), + KEY_READ | KEY_WRITE)); + std::wstring name_str(L"name_str"); + std::wstring data_str(L"data_111"); + ASSERT_TRUE(key.WriteValue(name_str.c_str(), data_str.c_str())); + std::wstring name_dword(L"name_dword"); + DWORD data_dword = 100; + ASSERT_TRUE(key.WriteValue(name_dword.c_str(), data_dword)); + + scoped_ptr<DeleteRegValueWorkItem> work_item1( + WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key, + name_str, true)); + scoped_ptr<DeleteRegValueWorkItem> work_item2( + WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key, + name_dword, false)); + + EXPECT_TRUE(work_item1->Do()); + EXPECT_TRUE(work_item2->Do()); + + EXPECT_FALSE(key.ValueExists(name_str.c_str())); + EXPECT_FALSE(key.ValueExists(name_dword.c_str())); + + work_item1->Rollback(); + work_item2->Rollback(); + + std::wstring read_str; + DWORD read_dword; + EXPECT_TRUE(key.ReadValue(name_str.c_str(), &read_str)); + EXPECT_TRUE(key.ReadValueDW(name_dword.c_str(), &read_dword)); + EXPECT_EQ(read_str, data_str); + EXPECT_EQ(read_dword, data_dword); +} + +// Try deleting a value that doesn't exist. +TEST_F(DeleteRegValueWorkItemTest, DeleteNonExistentValue) { + RegKey key; + std::wstring parent_key(test_root); + file_util::AppendToPath(&parent_key, L"WriteNew"); + ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(), + KEY_READ | KEY_WRITE)); + std::wstring name_str(L"name_str"); + std::wstring name_dword(L"name_dword"); + EXPECT_FALSE(key.ValueExists(name_str.c_str())); + EXPECT_FALSE(key.ValueExists(name_dword.c_str())); + + scoped_ptr<DeleteRegValueWorkItem> work_item1( + WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key, + name_str, true)); + scoped_ptr<DeleteRegValueWorkItem> work_item2( + WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key, + name_dword, false)); + + EXPECT_TRUE(work_item1->Do()); + EXPECT_TRUE(work_item2->Do()); + + EXPECT_FALSE(key.ValueExists(name_str.c_str())); + EXPECT_FALSE(key.ValueExists(name_dword.c_str())); + + work_item1->Rollback(); + work_item2->Rollback(); + + EXPECT_FALSE(key.ValueExists(name_str.c_str())); + EXPECT_FALSE(key.ValueExists(name_dword.c_str())); +} diff --git a/chrome/installer/util/google_update_constants.cc b/chrome/installer/util/google_update_constants.cc index ee75f0c..a8ab8a8 100644 --- a/chrome/installer/util/google_update_constants.cc +++ b/chrome/installer/util/google_update_constants.cc @@ -18,6 +18,8 @@ const wchar_t kRegDidRunField[] = L"dr"; const wchar_t kRegLangField[] = L"lang"; const wchar_t kRegLastCheckedField[] = L"LastChecked"; const wchar_t kRegNameField[] = L"name"; +const wchar_t kRegOldVersionField[] = L"opv"; +const wchar_t kRegRenameCmdField[] = L"rename"; const wchar_t kRegRLZBrandField[] = L"brand"; const wchar_t kRegUsageStatsField[] = L"usagestats"; const wchar_t kRegVersionField[] = L"pv"; diff --git a/chrome/installer/util/google_update_constants.h b/chrome/installer/util/google_update_constants.h index 2b24540..bf4f0f7 100644 --- a/chrome/installer/util/google_update_constants.h +++ b/chrome/installer/util/google_update_constants.h @@ -23,6 +23,8 @@ extern const wchar_t kRegDidRunField[]; extern const wchar_t kRegLangField[]; extern const wchar_t kRegLastCheckedField[]; extern const wchar_t kRegNameField[]; +extern const wchar_t kRegOldVersionField[]; +extern const wchar_t kRegRenameCmdField[]; extern const wchar_t kRegRLZBrandField[]; extern const wchar_t kRegUsageStatsField[]; extern const wchar_t kRegVersionField[]; diff --git a/chrome/installer/util/installer_unittests.vcproj b/chrome/installer/util/installer_unittests.vcproj index 4f710ac..5ac125b 100644 --- a/chrome/installer/util/installer_unittests.vcproj +++ b/chrome/installer/util/installer_unittests.vcproj @@ -164,6 +164,10 @@ > </File> <File + RelativePath="delete_reg_value_work_item_unittest.cc" + > + </File> + <File RelativePath="delete_tree_work_item_unittest.cc" > </File> diff --git a/chrome/installer/util/util.vcproj b/chrome/installer/util/util.vcproj index 284eb6a..e99dde8 100644 --- a/chrome/installer/util/util.vcproj +++ b/chrome/installer/util/util.vcproj @@ -81,6 +81,14 @@ > </File> <File + RelativePath="delete_reg_value_work_item.cc" + > + </File> + <File + RelativePath="delete_reg_value_work_item.h" + > + </File> + <File RelativePath="google_chrome_distribution.cc" > </File> diff --git a/chrome/installer/util/util_constants.cc b/chrome/installer/util/util_constants.cc index 886795e..4ee1b02 100755 --- a/chrome/installer/util/util_constants.cc +++ b/chrome/installer/util/util_constants.cc @@ -49,6 +49,10 @@ const wchar_t kMakeChromeDefault[] = L"make-chrome-default"; // options kInstallArchive and kUninstall are ignored. const wchar_t kRegisterChromeBrowser[] = L"register-chrome-browser"; +// Renames chrome.exe to old_chrome.exe and renames new_chrome.exe to chrome.exe +// to support in-use updates. Also deletes opv key. +const wchar_t kRenameChromeExe[] = L"rename-chrome-exe"; + // Install Chrome to system wise location. The default is per user install. const wchar_t kSystemLevel[] = L"system-level"; @@ -62,6 +66,8 @@ const wchar_t kVerboseLogging[] = L"verbose-logging"; const wchar_t kInstallBinaryDir[] = L"Application"; const wchar_t kChromeExe[] = L"chrome.exe"; +const wchar_t kChromeOldExe[] = L"old_chrome.exe"; +const wchar_t kChromeNewExe[] = L"new_chrome.exe"; const wchar_t kChromeDll[] = L"chrome.dll"; const wchar_t kSetupExe[] = L"setup.exe"; diff --git a/chrome/installer/util/util_constants.h b/chrome/installer/util/util_constants.h index 941cfe2..662029f 100755 --- a/chrome/installer/util/util_constants.h +++ b/chrome/installer/util/util_constants.h @@ -31,6 +31,8 @@ enum InstallStatus { UNINSTALL_FAILED, // Chrome uninstallation failed UNINSTALL_CANCELLED, // User cancelled Chrome uninstallation UNKNOWN_STATUS, // Unknown status (this should never happen) + RENAME_SUCCESSFUL, // Rename of new_chrome.exe to chrome.exe worked + RENAME_FAILED // Rename of new_chrome.exe failed }; // These are distibution related install options specified through command @@ -64,6 +66,7 @@ extern const wchar_t kInstallerData[]; extern const wchar_t kLogFile[]; extern const wchar_t kMakeChromeDefault[]; extern const wchar_t kRegisterChromeBrowser[]; +extern const wchar_t kRenameChromeExe[]; extern const wchar_t kSystemLevel[]; extern const wchar_t kUninstall[]; extern const wchar_t kVerboseLogging[]; @@ -71,6 +74,8 @@ extern const wchar_t kVerboseLogging[]; extern const wchar_t kInstallBinaryDir[]; extern const wchar_t kChromeExe[]; +extern const wchar_t kChromeOldExe[]; +extern const wchar_t kChromeNewExe[]; extern const wchar_t kChromeDll[]; extern const wchar_t kSetupExe[]; diff --git a/chrome/installer/util/work_item.cc b/chrome/installer/util/work_item.cc index e02bb5b..7905405 100644 --- a/chrome/installer/util/work_item.cc +++ b/chrome/installer/util/work_item.cc @@ -3,10 +3,12 @@ // found in the LICENSE file. #include "chrome/installer/util/work_item.h" + #include "chrome/installer/util/copy_tree_work_item.h" #include "chrome/installer/util/create_dir_work_item.h" #include "chrome/installer/util/create_reg_key_work_item.h" #include "chrome/installer/util/delete_tree_work_item.h" +#include "chrome/installer/util/delete_reg_value_work_item.h" #include "chrome/installer/util/set_reg_value_work_item.h" #include "chrome/installer/util/work_item_list.h" @@ -32,6 +34,13 @@ CreateRegKeyWorkItem* WorkItem::CreateCreateRegKeyWorkItem( return new CreateRegKeyWorkItem(predefined_root, path); } +DeleteRegValueWorkItem* WorkItem::CreateDeleteRegValueWorkItem( + HKEY predefined_root, std::wstring key_path, + std::wstring value_name, bool is_str_type) { + return new DeleteRegValueWorkItem(predefined_root, key_path, + value_name, is_str_type); +} + DeleteTreeWorkItem* WorkItem::CreateDeleteTreeWorkItem(std::wstring root_path, std::wstring key_path) { return new DeleteTreeWorkItem(root_path, key_path); diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h index 267b61b..08942a1 100644 --- a/chrome/installer/util/work_item.h +++ b/chrome/installer/util/work_item.h @@ -6,8 +6,8 @@ // out during install/update/uninstall. Supports rollback of actions if this // process fails. -#ifndef CHROME_INSTALLER_UTIL_WORK_ITEM_H__ -#define CHROME_INSTALLER_UTIL_WORK_ITEM_H__ +#ifndef CHROME_INSTALLER_UTIL_WORK_ITEM_H_ +#define CHROME_INSTALLER_UTIL_WORK_ITEM_H_ #include <string> #include <windows.h> @@ -16,6 +16,7 @@ class CopyTreeWorkItem; class CreateDirWorkItem; class CreateRegKeyWorkItem; class DeleteTreeWorkItem; +class DeleteRegValueWorkItem; class SetRegValueWorkItem; class WorkItemList; @@ -29,16 +30,17 @@ class WorkItem { 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). + NEW_NAME_IF_IN_USE // Copy to a new path if dest is in use(only files). }; virtual ~WorkItem(); // Create a CopyTreeWorkItem that recursively copies a file system hierarchy - // from source path to destination path. If overwrite_option is ALWAYS, the - // created CopyTreeWorkItem always overwrites files. If overwrite_option is - // RENAME_IF_IN_USE, file is copied with an alternate name specified by - // alternative_path. + // from source path to destination path. + // * If overwrite_option is ALWAYS, the created CopyTreeWorkItem always + // overwrites files. + // * If overwrite_option is NEW_NAME_IF_IN_USE, file is copied with an + // alternate name specified by alternative_path. static CopyTreeWorkItem* CreateCopyTreeWorkItem(std::wstring source_path, std::wstring dest_path, std::wstring temp_dir, CopyOverWriteOption overwrite_option, @@ -52,6 +54,11 @@ class WorkItem { static CreateRegKeyWorkItem* CreateCreateRegKeyWorkItem( HKEY predefined_root, std::wstring path); + // Create a DeleteRegValueWorkItem that deletes a registry value + static DeleteRegValueWorkItem* CreateDeleteRegValueWorkItem( + HKEY predefined_root, std::wstring key_path, + std::wstring value_name, bool is_str_type); + // Create a DeleteTreeWorkItem that recursively deletes a file system // hierarchy at the given root path. A key file can be optionally specified // by key_path. @@ -97,5 +104,5 @@ class WorkItem { WorkItem(); }; -#endif // CHROME_INSTALLER_UTIL_WORK_ITEM_H__ +#endif // CHROME_INSTALLER_UTIL_WORK_ITEM_H_ diff --git a/chrome/installer/util/work_item_list.cc b/chrome/installer/util/work_item_list.cc index 1975048..ef0a9c4 100644 --- a/chrome/installer/util/work_item_list.cc +++ b/chrome/installer/util/work_item_list.cc @@ -87,6 +87,16 @@ bool WorkItemList::AddCreateRegKeyWorkItem(HKEY predefined_root, return AddWorkItem(item); } +bool WorkItemList::AddDeleteRegValueWorkItem(HKEY predefined_root, + std::wstring key_path, + std::wstring value_name, + bool is_str_type) { + WorkItem* item = reinterpret_cast<WorkItem*>( + WorkItem::CreateDeleteRegValueWorkItem(predefined_root, key_path, + value_name, is_str_type)); + return AddWorkItem(item); +} + bool WorkItemList::AddDeleteTreeWorkItem(std::wstring root_path, std::wstring key_path) { WorkItem* item = reinterpret_cast<WorkItem*>( diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h index 5d1b071..227533c 100644 --- a/chrome/installer/util/work_item_list.h +++ b/chrome/installer/util/work_item_list.h @@ -44,6 +44,11 @@ class WorkItemList : public WorkItem { // path. bool AddCreateRegKeyWorkItem(HKEY predefined_root, std::wstring path); + // Add a DeleteRegValueWorkItem that deletes registry value of type REG_SZ + // or REG_DWORD. + bool AddDeleteRegValueWorkItem(HKEY predefined_root, std::wstring key_path, + std::wstring value_name, bool it_str_type); + // Add a DeleteTreeWorkItem that recursively deletes a file system // hierarchy at the given root path. A key file can be optionally specified // by key_path. |