Commit 26007adf authored by Greg Thompson's avatar Greg Thompson Committed by Commit Bot

Replace GetVersionKey() with GetClientsKeyPath.

BUG=879568

Change-Id: I3edeb811f2c892b6162f21648c7644b387007e25
Reviewed-on: https://chromium-review.googlesource.com/1200868
Commit-Queue: Greg Thompson <grt@chromium.org>
Reviewed-by: default avatarPatrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589110}
parent 19279404
......@@ -124,11 +124,6 @@ void AddInstallerCopyTasks(const InstallerState& installer_state,
}
}
base::string16 GetRegCommandKey(BrowserDistribution* dist,
const wchar_t* name) {
return GetRegistrationDataCommandKey(dist->GetAppRegistrationData(), name);
}
// A callback invoked by |work_item| that adds firewall rules for Chrome. Rules
// are left in-place on rollback unless |remove_on_rollback| is true. This is
// the case for new installs only. Updates and overinstalls leave the rule
......@@ -593,22 +588,17 @@ void AddUninstallShortcutWorkItems(const InstallerState& installer_state,
// Create Version key for a product (if not already present) and sets the new
// product version as the last step.
void AddVersionKeyWorkItems(HKEY root,
const base::string16& version_key,
const base::string16& clients_key,
const base::string16& product_name,
const base::Version& new_version,
bool add_language_identifier,
WorkItemList* list) {
list->AddCreateRegKeyWorkItem(root, version_key, KEY_WOW64_32KEY);
list->AddCreateRegKeyWorkItem(root, clients_key, KEY_WOW64_32KEY);
list->AddSetRegValueWorkItem(root,
version_key,
KEY_WOW64_32KEY,
google_update::kRegNameField,
product_name,
list->AddSetRegValueWorkItem(root, clients_key, KEY_WOW64_32KEY,
google_update::kRegNameField, product_name,
true); // overwrite name also
list->AddSetRegValueWorkItem(root,
version_key,
KEY_WOW64_32KEY,
list->AddSetRegValueWorkItem(root, clients_key, KEY_WOW64_32KEY,
google_update::kRegOopcrashesField,
static_cast<DWORD>(1),
false); // set during first install
......@@ -619,16 +609,11 @@ void AddVersionKeyWorkItems(HKEY root,
base::string16 language(GetCurrentTranslation());
if (base::LowerCaseEqualsASCII(language, "en-us"))
language.resize(2);
list->AddSetRegValueWorkItem(root,
version_key,
KEY_WOW64_32KEY,
google_update::kRegLangField,
language,
list->AddSetRegValueWorkItem(root, clients_key, KEY_WOW64_32KEY,
google_update::kRegLangField, language,
false); // do not overwrite language
}
list->AddSetRegValueWorkItem(root,
version_key,
KEY_WOW64_32KEY,
list->AddSetRegValueWorkItem(root, clients_key, KEY_WOW64_32KEY,
google_update::kRegVersionField,
ASCIIToUTF16(new_version.GetString()),
true); // overwrite version
......@@ -709,23 +694,22 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
installer_state.GetInstallerDirectory(new_version).Append(
setup_path.BaseName()));
BrowserDistribution* dist = installer_state.product().distribution();
const base::string16 version_key(dist->GetVersionKey());
const base::string16 clients_key(install_static::GetClientsKeyPath());
if (current_version) {
in_use_update_work_items->AddSetRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY,
root, clients_key, KEY_WOW64_32KEY,
google_update::kRegOldVersionField,
ASCIIToUTF16(current_version->GetString()), true);
}
if (critical_version.IsValid()) {
in_use_update_work_items->AddSetRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY,
root, clients_key, KEY_WOW64_32KEY,
google_update::kRegCriticalVersionField,
ASCIIToUTF16(critical_version.GetString()), true);
} else {
in_use_update_work_items->AddDeleteRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY,
root, clients_key, KEY_WOW64_32KEY,
google_update::kRegCriticalVersionField);
}
......@@ -738,7 +722,7 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
product_rename_cmd.AppendSwitch(switches::kVerboseLogging);
InstallUtil::AppendModeSwitch(&product_rename_cmd);
in_use_update_work_items->AddSetRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY, google_update::kRegRenameCmdField,
root, clients_key, KEY_WOW64_32KEY, google_update::kRegRenameCmdField,
product_rename_cmd.GetCommandLineString(), true);
post_install_task_list->AddWorkItem(in_use_update_work_items.release());
......@@ -752,15 +736,14 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
regular_update_work_items->set_log_message("RegularUpdateWorkItemList");
// Since this was not an in-use-update, delete 'opv', 'cpv', and 'cmd' keys.
BrowserDistribution* dist = installer_state.product().distribution();
const base::string16 version_key(dist->GetVersionKey());
const base::string16 clients_key(install_static::GetClientsKeyPath());
regular_update_work_items->AddDeleteRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY, google_update::kRegOldVersionField);
root, clients_key, KEY_WOW64_32KEY, google_update::kRegOldVersionField);
regular_update_work_items->AddDeleteRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY,
root, clients_key, KEY_WOW64_32KEY,
google_update::kRegCriticalVersionField);
regular_update_work_items->AddDeleteRegValueWorkItem(
root, version_key, KEY_WOW64_32KEY, google_update::kRegRenameCmdField);
root, clients_key, KEY_WOW64_32KEY, google_update::kRegRenameCmdField);
post_install_task_list->AddWorkItem(regular_update_work_items.release());
}
......@@ -869,8 +852,7 @@ void AddInstallWorkItems(const InstallationState& original_state,
AddUninstallShortcutWorkItems(installer_state, setup_path, new_version,
product, install_list);
BrowserDistribution* dist = product.distribution();
AddVersionKeyWorkItems(root, dist->GetVersionKey(),
AddVersionKeyWorkItems(root, install_static::GetClientsKeyPath(),
InstallUtil::GetDisplayName(), new_version,
add_language_identifier, install_list);
......@@ -1066,8 +1048,7 @@ void AddOsUpgradeWorkItems(const InstallerState& installer_state,
const Product& product,
WorkItemList* install_list) {
const HKEY root_key = installer_state.root_key();
base::string16 cmd_key(
GetRegCommandKey(product.distribution(), kCmdOnOsUpgrade));
const base::string16 cmd_key(GetCommandKey(kCmdOnOsUpgrade));
if (installer_state.operation() == InstallerState::UNINSTALL) {
install_list->AddDeleteRegKeyWorkItem(root_key, cmd_key, KEY_WOW64_32KEY)
......@@ -1102,8 +1083,7 @@ void AddEnterpriseEnrollmentWorkItems(const InstallerState& installer_state,
return;
const HKEY root_key = installer_state.root_key();
const base::string16 cmd_key(
GetRegCommandKey(product.distribution(), kCmdStoreDMToken));
const base::string16 cmd_key(GetCommandKey(kCmdStoreDMToken));
if (installer_state.operation() == InstallerState::UNINSTALL) {
install_list->AddDeleteRegKeyWorkItem(root_key, cmd_key, KEY_WOW64_32KEY)
......
......@@ -90,8 +90,8 @@ TEST_F(InstallerStateTest, WithProduct) {
{
RegistryOverrideManager override_manager;
ASSERT_NO_FATAL_FAILURE(override_manager.OverrideRegistry(root));
BrowserDistribution* dist = BrowserDistribution::GetDistribution();
RegKey chrome_key(root, dist->GetVersionKey().c_str(), KEY_ALL_ACCESS);
RegKey chrome_key(root, install_static::GetClientsKeyPath().c_str(),
KEY_ALL_ACCESS);
EXPECT_TRUE(chrome_key.Valid());
if (chrome_key.Valid()) {
chrome_key.WriteValue(google_update::kRegVersionField,
......
......@@ -445,15 +445,14 @@ installer::InstallStatus RenameChromeExecutables(
// Add work items to delete Chrome's "opv", "cpv", and "cmd" values.
// TODO(grt): Clean this up; https://crbug.com/577816.
HKEY reg_root = installer_state->root_key();
base::string16 version_key;
version_key = installer_state->product().distribution()->GetVersionKey();
install_list->AddDeleteRegValueWorkItem(reg_root, version_key,
const base::string16 clients_key = install_static::GetClientsKeyPath();
install_list->AddDeleteRegValueWorkItem(reg_root, clients_key,
KEY_WOW64_32KEY,
google_update::kRegOldVersionField);
install_list->AddDeleteRegValueWorkItem(
reg_root, version_key, KEY_WOW64_32KEY,
reg_root, clients_key, KEY_WOW64_32KEY,
google_update::kRegCriticalVersionField);
install_list->AddDeleteRegValueWorkItem(reg_root, version_key,
install_list->AddDeleteRegValueWorkItem(reg_root, clients_key,
KEY_WOW64_32KEY,
google_update::kRegRenameCmdField);
// old_chrome.exe is still in use in most cases, so ignore failures here.
......
......@@ -158,18 +158,20 @@ void RemoveMultiChromeFrame(const InstallerState& installer_state) {
// ClientState\UninstallString contains a path including "\Chrome Frame\".
// Multi-install GCF would have had "\Chrome\", and anything else is garbage.
UpdatingAppRegistrationData gcf_data(
L"{8BA986DA-5100-405E-AA35-86F34A02ACBF}");
static constexpr wchar_t kGcfGuid[] =
L"{8BA986DA-5100-405E-AA35-86F34A02ACBF}";
base::string16 clients_key_path = install_static::GetClientsKeyPath(kGcfGuid);
base::win::RegKey clients_key;
base::string16 client_state_key_path =
install_static::GetClientStateKeyPath(kGcfGuid);
base::win::RegKey client_state_key;
const bool has_clients_key =
clients_key.Open(installer_state.root_key(),
gcf_data.GetVersionKey().c_str(),
clients_key.Open(installer_state.root_key(), clients_key_path.c_str(),
KEY_QUERY_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS;
const bool has_client_state_key =
client_state_key.Open(installer_state.root_key(),
gcf_data.GetStateKey().c_str(),
client_state_key_path.c_str(),
KEY_QUERY_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS;
if (!has_clients_key && !has_client_state_key)
return; // Nothing to check or to clean.
......@@ -195,12 +197,11 @@ void RemoveMultiChromeFrame(const InstallerState& installer_state) {
int success_count = 0;
if (InstallUtil::DeleteRegistryKey(installer_state.root_key(),
gcf_data.GetVersionKey(),
KEY_WOW64_32KEY)) {
clients_key_path, KEY_WOW64_32KEY)) {
++success_count;
}
if (InstallUtil::DeleteRegistryKey(installer_state.root_key(),
gcf_data.GetStateKey(), KEY_WOW64_32KEY)) {
client_state_key_path, KEY_WOW64_32KEY)) {
++success_count;
}
if (InstallUtil::DeleteRegistryKey(
......@@ -230,10 +231,10 @@ void RemoveMultiChromeFrame(const InstallerState& installer_state) {
void RemoveAppLauncherVersionKey(const InstallerState& installer_state) {
// The app launcher was only registered for Google Chrome.
#if defined(GOOGLE_CHROME_BUILD)
UpdatingAppRegistrationData reg_data(
L"{FDA71E6F-AC4C-4a00-8B70-9958A68906BF}");
static constexpr wchar_t kLauncherGuid[] =
L"{FDA71E6F-AC4C-4a00-8B70-9958A68906BF}";
base::string16 path(reg_data.GetVersionKey());
base::string16 path = install_static::GetClientsKeyPath(kLauncherGuid);
if (base::win::RegKey(installer_state.root_key(), path.c_str(),
KEY_QUERY_VALUE | KEY_WOW64_32KEY)
.Valid()) {
......@@ -261,9 +262,7 @@ void RemoveAppHostExe(const InstallerState& installer_state) {
void RemoveLegacyChromeAppCommands(const InstallerState& installer_state) {
// These app commands were only registered for Google Chrome.
#if defined(GOOGLE_CHROME_BUILD)
base::string16 path(GetRegistrationDataCommandKey(
installer_state.product().distribution()->GetAppRegistrationData(),
L"install-extension"));
base::string16 path(GetCommandKey(L"install-extension"));
if (base::win::RegKey(installer_state.root_key(), path.c_str(),
KEY_QUERY_VALUE | KEY_WOW64_32KEY)
......@@ -520,10 +519,8 @@ bool IsProcessorSupported() {
return base::CPU().has_sse2();
}
base::string16 GetRegistrationDataCommandKey(
const AppRegistrationData& reg_data,
const wchar_t* name) {
base::string16 cmd_key(reg_data.GetVersionKey());
base::string16 GetCommandKey(const wchar_t* name) {
base::string16 cmd_key = install_static::GetClientsKeyPath();
cmd_key.append(1, base::FilePath::kSeparators[0])
.append(google_update::kRegCommandsKey)
.append(1, base::FilePath::kSeparators[0])
......
......@@ -104,9 +104,7 @@ bool ContainsUnsupportedSwitch(const base::CommandLine& cmd_line);
bool IsProcessorSupported();
// Returns the "...\\Commands\\|name|" registry key for a product's |reg_data|.
base::string16 GetRegistrationDataCommandKey(
const AppRegistrationData& reg_data,
const wchar_t* name);
base::string16 GetCommandKey(const wchar_t* name);
// Deletes all values and subkeys of the key |path| under |root|, preserving
// the keys named in |keys_to_preserve| (each of which must be an ASCII string).
......
......@@ -440,11 +440,8 @@ TEST(SetupUtilTest, ContainsUnsupportedSwitch) {
}
TEST(SetupUtilTest, GetRegistrationDataCommandKey) {
base::string16 app_guid = L"{AAAAAAAA-BBBB-1111-0123-456789ABCDEF}";
UpdatingAppRegistrationData reg_data(app_guid);
base::string16 key =
installer::GetRegistrationDataCommandKey(reg_data, L"test_name");
EXPECT_TRUE(base::EndsWith(key, app_guid + L"\\Commands\\test_name",
const base::string16 key = installer::GetCommandKey(L"test_name");
EXPECT_TRUE(base::EndsWith(key, L"\\Commands\\test_name",
base::CompareCase::SENSITIVE));
}
......
......@@ -879,7 +879,7 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
HKEY reg_root = installer_state.root_key();
// Note that we must retrieve the distribution-specific data before deleting
// product.GetVersionKey().
// the browser's Clients key.
base::string16 distribution_data(GetDistributionData());
// Remove Control Panel uninstall link.
......@@ -890,8 +890,8 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
reg_root, install_static::GetUninstallRegistryPath(), KEY_WOW64_32KEY);
// Remove Omaha product key.
InstallUtil::DeleteRegistryKey(
reg_root, browser_dist->GetVersionKey(), KEY_WOW64_32KEY);
InstallUtil::DeleteRegistryKey(reg_root, install_static::GetClientsKeyPath(),
KEY_WOW64_32KEY);
// Also try to delete the MSI value in the ClientState key (it might not be
// there). This is due to a Google Update behaviour where an uninstall and a
......
......@@ -74,7 +74,7 @@ void AppCommand::AddWorkItems(HKEY predefined_root,
const base::string16& command_path,
WorkItemList* item_list) const {
// Command_path is derived from GetRegCommandKey which always returns
// value from GetVersionKey() which should be 32-bit hive.
// value from GetClientsKeyPath() which should be 32-bit hive.
item_list->AddCreateRegKeyWorkItem(
predefined_root, command_path, KEY_WOW64_32KEY)
->set_log_message("creating AppCommand registry key");
......
......@@ -14,7 +14,6 @@ class AppRegistrationData {
virtual ~AppRegistrationData() {}
virtual base::string16 GetStateKey() const = 0;
virtual base::string16 GetStateMediumKey() const = 0;
virtual base::string16 GetVersionKey() const = 0;
};
#endif // CHROME_INSTALLER_UTIL_APP_REGISTRATION_DATA_H_
......@@ -73,7 +73,3 @@ base::string16 BrowserDistribution::GetStateKey() const {
base::string16 BrowserDistribution::GetStateMediumKey() const {
return app_reg_data_->GetStateMediumKey();
}
base::string16 BrowserDistribution::GetVersionKey() const {
return app_reg_data_->GetVersionKey();
}
......@@ -24,7 +24,6 @@ class BrowserDistribution {
const AppRegistrationData& GetAppRegistrationData() const;
base::string16 GetStateKey() const;
base::string16 GetStateMediumKey() const;
base::string16 GetVersionKey() const;
protected:
explicit BrowserDistribution(
......
......@@ -9,6 +9,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/version.h"
#include "base/win/registry.h"
#include "chrome/install_static/install_util.h"
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/installer/util/google_update_constants.h"
#include "chrome/installer/util/install_util.h"
......@@ -20,13 +21,13 @@ namespace {
// Initializes |commands| from the "Commands" subkey of |version_key|. Returns
// false if there is no "Commands" subkey or on error.
bool InitializeCommands(const base::win::RegKey& version_key,
bool InitializeCommands(const base::win::RegKey& clients_key,
AppCommands* commands) {
static const DWORD kAccess =
KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE | KEY_WOW64_32KEY;
base::win::RegKey commands_key;
if (commands_key.Open(version_key.Handle(), google_update::kRegCommandsKey,
if (commands_key.Open(clients_key.Handle(), google_update::kRegCommandsKey,
kAccess) == ERROR_SUCCESS) {
return commands->Initialize(commands_key, KEY_WOW64_32KEY);
}
......@@ -53,7 +54,7 @@ bool ProductState::Initialize(bool system_install) {
static const DWORD kAccess = KEY_QUERY_VALUE | KEY_WOW64_32KEY;
const BrowserDistribution* distribution =
BrowserDistribution::GetDistribution();
const std::wstring version_key(distribution->GetVersionKey());
const std::wstring clients_key(install_static::GetClientsKeyPath());
const std::wstring state_key(distribution->GetStateKey());
const HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
base::win::RegKey key;
......@@ -62,7 +63,7 @@ bool ProductState::Initialize(bool system_install) {
Clear();
// Read from the Clients key.
if (key.Open(root_key, version_key.c_str(), kAccess) == ERROR_SUCCESS) {
if (key.Open(root_key, clients_key.c_str(), kAccess) == ERROR_SUCCESS) {
base::string16 version_str;
if (key.ReadValue(google_update::kRegVersionField,
&version_str) == ERROR_SUCCESS) {
......
......@@ -16,7 +16,3 @@ base::string16 NonUpdatingAppRegistrationData::GetStateKey() const {
base::string16 NonUpdatingAppRegistrationData::GetStateMediumKey() const {
return key_path_;
}
base::string16 NonUpdatingAppRegistrationData::GetVersionKey() const {
return key_path_;
}
......@@ -17,7 +17,6 @@ class NonUpdatingAppRegistrationData : public AppRegistrationData {
~NonUpdatingAppRegistrationData() override;
base::string16 GetStateKey() const override;
base::string16 GetStateMediumKey() const override;
base::string16 GetVersionKey() const override;
private:
const base::string16 key_path_;
......
......@@ -7,6 +7,7 @@
#include "base/test/test_reg_util_win.h"
#include "base/version.h"
#include "base/win/registry.h"
#include "chrome/install_static/install_util.h"
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/installer/util/google_update_constants.h"
#include "chrome/installer/util/installation_state.h"
......@@ -40,9 +41,10 @@ void ProductStateTest::SetUp() {
registry_override_manager_.OverrideRegistry(overridden_));
BrowserDistribution* dist = BrowserDistribution::GetDistribution();
ASSERT_EQ(ERROR_SUCCESS,
clients_.Create(overridden_, dist->GetVersionKey().c_str(),
KEY_ALL_ACCESS | KEY_WOW64_32KEY));
ASSERT_EQ(
ERROR_SUCCESS,
clients_.Create(overridden_, install_static::GetClientsKeyPath().c_str(),
KEY_ALL_ACCESS | KEY_WOW64_32KEY));
ASSERT_EQ(ERROR_SUCCESS,
client_state_.Create(overridden_, dist->GetStateKey().c_str(),
KEY_ALL_ACCESS | KEY_WOW64_32KEY));
......
......@@ -13,6 +13,7 @@
#include "base/test/test_reg_util_win.h"
#include "base/version.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/install_static/install_util.h"
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/installer/util/google_update_constants.h"
#include "chrome/installer/util/installation_state.h"
......@@ -30,7 +31,6 @@ TEST(ProductTest, ProductInstallBasic) {
std::unique_ptr<Product> product =
std::make_unique<Product>(BrowserDistribution::GetDistribution());
BrowserDistribution* distribution = product->distribution();
base::FilePath user_data_dir;
ASSERT_TRUE(base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
......@@ -53,7 +53,7 @@ TEST(ProductTest, ProductInstallBasic) {
EXPECT_EQ(nullptr, machine_state.GetProductState(system_level));
// Let's pretend chrome is installed.
RegKey version_key(root, distribution->GetVersionKey().c_str(),
RegKey version_key(root, install_static::GetClientsKeyPath().c_str(),
KEY_ALL_ACCESS);
ASSERT_TRUE(version_key.Valid());
......
......@@ -17,7 +17,3 @@ base::string16 TestAppRegistrationData::GetStateKey() const {
base::string16 TestAppRegistrationData::GetStateMediumKey() const {
return L"Software\\Chromium\\ClientStateMedium\\test_app_guid";
}
base::string16 TestAppRegistrationData::GetVersionKey() const {
return L"Software\\Chromium\\Clients\\test_app_guid";
}
......@@ -13,7 +13,6 @@ class TestAppRegistrationData : public AppRegistrationData {
~TestAppRegistrationData() override;
base::string16 GetStateKey() const override;
base::string16 GetStateMediumKey() const override;
base::string16 GetVersionKey() const override;
};
#endif // CHROME_INSTALLER_UTIL_TEST_APP_REGISTRATION_DATA_H_
......@@ -22,9 +22,3 @@ base::string16 UpdatingAppRegistrationData::GetStateMediumKey() const {
.append(1, L'\\')
.append(app_guid_);
}
base::string16 UpdatingAppRegistrationData::GetVersionKey() const {
return base::string16(google_update::kRegPathClients)
.append(1, L'\\')
.append(app_guid_);
}
......@@ -17,7 +17,6 @@ class UpdatingAppRegistrationData : public AppRegistrationData {
~UpdatingAppRegistrationData() override;
base::string16 GetStateKey() const override;
base::string16 GetStateMediumKey() const override;
base::string16 GetVersionKey() const override;
private:
const base::string16 app_guid_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment