Replace GetStateMediumKey() with GetClientStateMediumKeyPath.

Also moves some consumes of InstallDetails to the new convenience
functions in install_static.

BUG=879568

Change-Id: I01c5c05f10a57407b3981ab64c6d4da2bb8de088
Reviewed-on: https://chromium-review.googlesource.com/1213145
Commit-Queue: Greg Thompson <grt@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589800}

chrome/install_static/install_util.cc

@@ -372,6 +372,10 @@ std::wstring GetClientStateKeyPath() {
   return GetClientStateKeyPath(GetAppGuid());
 }
 
+std::wstring GetClientStateMediumKeyPath() {
+  return GetClientStateMediumKeyPath(GetAppGuid());
+}
+
 std::wstring GetUninstallRegistryPath() {
   std::wstring result(
       L"Software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\");

chrome/install_static/install_util.h

@@ -66,14 +66,28 @@ std::wstring GetChromeInstallSubDirectory();
 // their browsing data.
 std::wstring GetRegistryPath();
 
+// The next set of registry paths are generally for integration with an Omaha
+// updater (see https://github.com/google/omaha); Google Chrome builds integrate
+// with Google Update. For all accesses, HKLM or HKCU must be used based on
+// IsSystemInstall(). Additionally, KEY_WOW64_32KEY must be used for all
+// accesses, as Omaha updaters exclusively use the 32-bit view of the registry.
+
 // Returns the path "Software\Google\Update\Clients\<guid>" where "<guid>" is
-// the current install mode's appguid.
+// the current install mode's appguid. This key is primarily used for
+// registering the browser as an app managed by the updater.
 std::wstring GetClientsKeyPath();
 
 // Returns the path "Software\Google\Update\ClientState\<guid>" where "<guid>"
-// is the current install mode's appguid.
+// is the current install mode's appguid. This key is primarily (but not
+// exclusively) used for holding install-wide state that is used by both the
+// updater and the browser.
 std::wstring GetClientStateKeyPath();
 
+// Returns the path "Software\Google\Update\ClientStateMedium\<guid>" where
+// "<guid>" is the current install mode's appguid. This is is used exclusively
+// for system-wide installs to hold values written by the browser.
+std::wstring GetClientStateMediumKeyPath();
+
 // Returns the path
 // "Software\Microsoft\Windows\CurrentVersion\Uninstall\[kCompanyPathName ]
 // kProductPathName[install_suffix]. This is the key used for the browser's

chrome/installer/setup/install_worker.cc

@@ -409,16 +409,17 @@ void AddMigrateUsageStatsWorkItems(const InstallerState& installer_state,
   if (!AreBinariesInstalled(installer_state))
     return;
 
-  BrowserDistribution* chrome_dist = installer_state.product().distribution();
-
   // Delete any stale value in Chrome's ClientStateMedium key. A new value, if
   // found, will be written to the ClientState key below.
   if (installer_state.system_install()) {
     install_list->AddDeleteRegValueWorkItem(
-        installer_state.root_key(), chrome_dist->GetStateMediumKey(),
-        KEY_WOW64_64KEY, google_update::kRegUsageStatsField);
+        installer_state.root_key(),
+        install_static::GetClientStateMediumKeyPath(), KEY_WOW64_64KEY,
+        google_update::kRegUsageStatsField);
   }
 
+  BrowserDistribution* chrome_dist = installer_state.product().distribution();
+
   google_update::Tristate consent =
       GoogleUpdateSettings::GetCollectStatsConsentForApp(
           installer_state.system_install(), *MakeBinariesRegistrationData());
@@ -768,8 +769,7 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
   // installs.
   if (install_static::kUseGoogleUpdateIntegration &&
       installer_state.system_install()) {
-    const base::string16 path =
-        install_static::InstallDetails::Get().GetClientStateMediumKeyPath();
+    const base::string16 path = install_static::GetClientStateMediumKeyPath();
     post_install_task_list
         ->AddCreateRegKeyWorkItem(HKEY_LOCAL_MACHINE, path, KEY_WOW64_32KEY)
         ->set_best_effort(true);

chrome/installer/setup/user_experiment_unittest.cc

@@ -31,10 +31,7 @@ class UserExperimentTest : public ::testing::TestWithParam<bool> {
 
     // Create the ClientState key.
     base::win::RegKey key;
-    ASSERT_EQ(key.Create(root_,
-                         install_static::InstallDetails::Get()
-                             .GetClientStateKeyPath()
-                             .c_str(),
+    ASSERT_EQ(key.Create(root_, install_static::GetClientStateKeyPath().c_str(),
                          KEY_WOW64_64KEY | KEY_SET_VALUE),
               ERROR_SUCCESS);
   }

chrome/installer/util/app_registration_data.h

@@ -13,7 +13,6 @@ class AppRegistrationData {
  public:
   virtual ~AppRegistrationData() {}
   virtual base::string16 GetStateKey() const = 0;
-  virtual base::string16 GetStateMediumKey() const = 0;
 };
 
 #endif  // CHROME_INSTALLER_UTIL_APP_REGISTRATION_DATA_H_

chrome/installer/util/browser_distribution.cc

@@ -69,7 +69,3 @@ const AppRegistrationData& BrowserDistribution::GetAppRegistrationData() const {
 base::string16 BrowserDistribution::GetStateKey() const {
   return app_reg_data_->GetStateKey();
 }
-
-base::string16 BrowserDistribution::GetStateMediumKey() const {
-  return app_reg_data_->GetStateMediumKey();
-}

chrome/installer/util/browser_distribution.h

@@ -23,7 +23,6 @@ class BrowserDistribution {
   // Getter and adaptors for the underlying |app_reg_data_|.
   const AppRegistrationData& GetAppRegistrationData() const;
   base::string16 GetStateKey() const;
-  base::string16 GetStateMediumKey() const;
 
  protected:
   explicit BrowserDistribution(

chrome/installer/util/experiment_storage_unittest.cc

@@ -6,7 +6,7 @@
 
 #include "base/strings/string_piece.h"
 #include "base/test/test_reg_util_win.h"
-#include "chrome/install_static/install_details.h"
+#include "chrome/install_static/install_util.h"
 #include "chrome/install_static/test/scoped_install_details.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/experiment.h"
@@ -35,11 +35,10 @@ class ExperimentStorageTest : public ::testing::TestWithParam<bool> {
     // Create an empty participation key since participation registry is assumed
     // to be present for chrome build.
     base::win::RegKey key;
-    ASSERT_EQ(ERROR_SUCCESS, key.Create(root(),
-                                        install_static::InstallDetails::Get()
-                                            .GetClientStateKeyPath()
-                                            .c_str(),
-                                        KEY_WOW64_32KEY | KEY_QUERY_VALUE));
+    ASSERT_EQ(
+        ERROR_SUCCESS,
+        key.Create(root(), install_static::GetClientStateKeyPath().c_str(),
+                   KEY_WOW64_32KEY | KEY_QUERY_VALUE));
   }
 
   bool system_level_install_;
@@ -146,7 +145,7 @@ TEST_P(ExperimentStorageTest, TestLoadStoreExperiment) {
   base::win::RegKey key;
   base::string16 client_state_path(
       system_level_install_
-          ? BrowserDistribution::GetDistribution()->GetStateMediumKey()
+          ? install_static::GetClientStateMediumKeyPath()
           : BrowserDistribution::GetDistribution()->GetStateKey());
   client_state_path.append(L"\\Retention");
   EXPECT_EQ(ERROR_SUCCESS, key.Open(root(), client_state_path.c_str(),

chrome/installer/util/google_update_settings.cc

@@ -23,7 +23,6 @@
 #include "base/win/registry.h"
 #include "base/win/win_util.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/install_static/install_details.h"
 #include "chrome/install_static/install_modes.h"
 #include "chrome/install_static/install_util.h"
 #include "chrome/installer/util/app_registration_data.h"
@@ -33,7 +32,6 @@
 #include "chrome/installer/util/installation_state.h"
 
 using base::win::RegKey;
-using install_static::InstallDetails;
 using installer::InstallationState;
 
 const wchar_t GoogleUpdateSettings::kPoliciesKey[] =
@@ -70,7 +68,7 @@ bool ReadGoogleUpdateStrKeyFromRoot(HKEY root,
                                     const wchar_t* const name,
                                     base::string16* value) {
   RegKey key;
-  return key.Open(root, InstallDetails::Get().GetClientStateKeyPath().c_str(),
+  return key.Open(root, install_static::GetClientStateKeyPath().c_str(),
                   KEY_QUERY_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS &&
          key.ReadValue(name, value) == ERROR_SUCCESS;
 }
@@ -112,7 +110,7 @@ bool WriteGoogleUpdateAggregateNumKeyInternal(const wchar_t* const name,
     return false;
   }
 
-  base::string16 reg_path(InstallDetails::Get().GetClientStateMediumKeyPath());
+  base::string16 reg_path(install_static::GetClientStateMediumKeyPath());
   reg_path.append(L"\\").append(name);
   RegKey key;
   if (key.Create(HKEY_LOCAL_MACHINE, reg_path.c_str(),
@@ -130,7 +128,7 @@ bool WriteUserGoogleUpdateStrKey(const wchar_t* const name,
                                  const base::string16& value) {
   RegKey key;
   return key.Create(HKEY_CURRENT_USER,
-                    InstallDetails::Get().GetClientStateKeyPath().c_str(),
+                    install_static::GetClientStateKeyPath().c_str(),
                     KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS &&
          key.WriteValue(name, value.c_str()) == ERROR_SUCCESS;
 }
@@ -155,7 +153,7 @@ bool ClearGoogleUpdateStrKey(const wchar_t* const name) {
   RegKey key;
   auto result = key.Open(install_static::IsSystemInstall() ? HKEY_LOCAL_MACHINE
                                                            : HKEY_CURRENT_USER,
-                         InstallDetails::Get().GetClientStateKeyPath().c_str(),
+                         install_static::GetClientStateKeyPath().c_str(),
                          KEY_QUERY_VALUE | KEY_SET_VALUE | KEY_WOW64_32KEY);
   if (result == ERROR_PATH_NOT_FOUND || result == ERROR_FILE_NOT_FOUND)
     return true;  // The key doesn't exist; consider the value cleared.
@@ -176,7 +174,7 @@ bool ClearGoogleUpdateStrKey(const wchar_t* const name) {
 bool RemoveUserGoogleUpdateStrKey(const wchar_t* const name) {
   RegKey key;
   auto result = key.Open(HKEY_CURRENT_USER,
-                         InstallDetails::Get().GetClientStateKeyPath().c_str(),
+                         install_static::GetClientStateKeyPath().c_str(),
                          KEY_QUERY_VALUE | KEY_SET_VALUE | KEY_WOW64_32KEY);
   if (result == ERROR_PATH_NOT_FOUND || result == ERROR_FILE_NOT_FOUND)
     return true;  // The key doesn't exist; consider the value cleared.
@@ -258,12 +256,12 @@ google_update::Tristate GoogleUpdateSettings::GetCollectStatsConsentForApp(
   const REGSAM kAccess = KEY_QUERY_VALUE | KEY_WOW64_32KEY;
 
   // For system-level installs, try ClientStateMedium first.
-  have_value =
-      system_install &&
-      key.Open(HKEY_LOCAL_MACHINE, reg_data.GetStateMediumKey().c_str(),
-               kAccess) == ERROR_SUCCESS &&
-      key.ReadValueDW(google_update::kRegUsageStatsField, &value) ==
-          ERROR_SUCCESS;
+  have_value = system_install &&
+               key.Open(HKEY_LOCAL_MACHINE,
+                        install_static::GetClientStateMediumKeyPath().c_str(),
+                        kAccess) == ERROR_SUCCESS &&
+               key.ReadValueDW(google_update::kRegUsageStatsField, &value) ==
+                   ERROR_SUCCESS;
 
   // Otherwise, try ClientState.
   if (!have_value) {
@@ -290,8 +288,9 @@ bool GoogleUpdateSettings::SetCollectStatsConsentAtLevel(bool system_install,
 
   // Write to ClientStateMedium for system-level; ClientState otherwise.
   HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
-  base::string16 reg_path =
-      system_install ? dist->GetStateMediumKey() : dist->GetStateKey();
+  base::string16 reg_path = system_install
+                                ? install_static::GetClientStateMediumKeyPath()
+                                : dist->GetStateKey();
   RegKey key;
   LONG result = key.Create(
       root_key, reg_path.c_str(), KEY_SET_VALUE | KEY_WOW64_32KEY);
@@ -379,7 +378,8 @@ bool GoogleUpdateSettings::SetEULAConsent(
   RegKey key;
 
   // Write the consent value into the product's ClientStateMedium key.
-  return key.Create(HKEY_LOCAL_MACHINE, dist->GetStateMediumKey().c_str(),
+  return key.Create(HKEY_LOCAL_MACHINE,
+                    install_static::GetClientStateMediumKeyPath().c_str(),
                     kAccess) == ERROR_SUCCESS &&
          key.WriteValue(google_update::kRegEULAAceptedField, eula_accepted) ==
              ERROR_SUCCESS;
@@ -862,8 +862,9 @@ bool GoogleUpdateSettings::SetExperimentLabels(
   // client state/app guid key.
   bool success = false;
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-  base::string16 client_state_path(system_install ? dist->GetStateMediumKey()
-                                                  : dist->GetStateKey());
+  base::string16 client_state_path(
+      system_install ? install_static::GetClientStateMediumKeyPath()
+                     : dist->GetStateKey());
   RegKey client_state(reg_root, client_state_path.c_str(),
                       KEY_SET_VALUE | KEY_WOW64_32KEY);
   // It is possible that the registry keys do not yet exist or have not yet
@@ -888,7 +889,8 @@ bool GoogleUpdateSettings::ReadExperimentLabels(
   HKEY reg_root = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   base::string16 client_state_path(
-      system_install ? dist->GetStateMediumKey() : dist->GetStateKey());
+      system_install ? install_static::GetClientStateMediumKeyPath()
+                     : dist->GetStateKey());
 
   RegKey client_state;
   LONG result = client_state.Open(

chrome/installer/util/google_update_settings_unittest.cc

@@ -82,8 +82,10 @@ class GoogleUpdateSettingsTest : public testing::Test {
     HKEY root = install == SYSTEM_INSTALL ?
         HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
     BrowserDistribution* chrome = BrowserDistribution::GetDistribution();
-    base::string16 state_key = install == SYSTEM_INSTALL ?
-        chrome->GetStateMediumKey() : chrome->GetStateKey();
+    base::string16 state_key =
+        install == SYSTEM_INSTALL
+            ? install_static::GetClientStateMediumKeyPath()
+            : chrome->GetStateKey();
 
     EXPECT_EQ(ERROR_SUCCESS,
               key.Open(root, state_key.c_str(), KEY_QUERY_VALUE));
@@ -424,8 +426,9 @@ TEST_F(GoogleUpdateSettingsTest, SetEULAConsent) {
   EXPECT_TRUE(GoogleUpdateSettings::SetEULAConsent(machine_state, chrome,
                                                    true));
   EXPECT_EQ(ERROR_SUCCESS,
-      key.Open(HKEY_LOCAL_MACHINE, chrome->GetStateMediumKey().c_str(),
-               KEY_QUERY_VALUE));
+            key.Open(HKEY_LOCAL_MACHINE,
+                     install_static::GetClientStateMediumKeyPath().c_str(),
+                     KEY_QUERY_VALUE));
   EXPECT_EQ(ERROR_SUCCESS,
       key.ReadValueDW(google_update::kRegEULAAceptedField, &value));
   EXPECT_EQ(1U, value);
@@ -466,8 +469,7 @@ TEST_F(GoogleUpdateSettingsTest, UpdateProfileCountsSystemInstall) {
   install_static::ScopedInstallDetails details(true /* system_level */);
 
   // No profile count keys present yet.
-  const base::string16& state_key = BrowserDistribution::GetDistribution()->
-      GetAppRegistrationData().GetStateMediumKey();
+  base::string16 state_key = install_static::GetClientStateMediumKeyPath();
   base::string16 num_profiles_path(state_key);
   num_profiles_path.append(L"\\");
   num_profiles_path.append(google_update::kRegProfilesActive);
@@ -1070,8 +1072,9 @@ void CollectStatsConsent::SetUp() {
   const HKEY root_key = stats_state.root_key();
   ASSERT_NO_FATAL_FAILURE(
       ApplySetting(stats_state.state_value(), root_key, dist_->GetStateKey()));
-  ASSERT_NO_FATAL_FAILURE(ApplySetting(stats_state.state_medium_value(),
-                                       root_key, dist_->GetStateMediumKey()));
+  ASSERT_NO_FATAL_FAILURE(
+      ApplySetting(stats_state.state_medium_value(), root_key,
+                   install_static::GetClientStateMediumKeyPath()));
 }
 
 // Write the correct value to represent |setting| in the registry.
@@ -1116,9 +1119,9 @@ TEST_P(CollectStatsConsent, SetCollectStatsConsentAtLevel) {
                   GetParam().system_level(),
                   !GetParam().is_consent_granted()));
 
-  const base::string16 reg_key = GetParam().system_level()
-                                     ? dist_->GetStateMediumKey()
-                                     : dist_->GetStateKey();
+  const base::string16 reg_key =
+      GetParam().system_level() ? install_static::GetClientStateMediumKeyPath()
+                                : dist_->GetStateKey();
   DWORD value = 0;
   EXPECT_EQ(
       ERROR_SUCCESS,

chrome/installer/util/installation_state.cc

@@ -125,8 +125,8 @@ bool ProductState::Initialize(bool system_install) {
   // Read from the ClientStateMedium key.  Values here override those in
   // ClientState.
   if (system_install &&
-      key.Open(root_key, distribution->GetStateMediumKey().c_str(), kAccess) ==
-          ERROR_SUCCESS) {
+      key.Open(root_key, install_static::GetClientStateMediumKeyPath().c_str(),
+               kAccess) == ERROR_SUCCESS) {
     DWORD dword_value = 0;
 
     if (key.ReadValueDW(google_update::kRegUsageStatsField,

chrome/installer/util/non_updating_app_registration_data.cc

@@ -12,7 +12,3 @@ NonUpdatingAppRegistrationData::~NonUpdatingAppRegistrationData() {}
 base::string16 NonUpdatingAppRegistrationData::GetStateKey() const {
   return key_path_;
 }
-
-base::string16 NonUpdatingAppRegistrationData::GetStateMediumKey() const {
-  return key_path_;
-}

chrome/installer/util/non_updating_app_registration_data.h

@@ -16,7 +16,6 @@ class NonUpdatingAppRegistrationData : public AppRegistrationData {
   explicit NonUpdatingAppRegistrationData(const base::string16& key_path);
   ~NonUpdatingAppRegistrationData() override;
   base::string16 GetStateKey() const override;
-  base::string16 GetStateMediumKey() const override;
 
  private:
   const base::string16 key_path_;

chrome/installer/util/test_app_registration_data.cc

@@ -13,7 +13,3 @@ TestAppRegistrationData::~TestAppRegistrationData() {
 base::string16 TestAppRegistrationData::GetStateKey() const {
   return L"Software\\Chromium\\ClientState\\test_app_guid";
 }
-
-base::string16 TestAppRegistrationData::GetStateMediumKey() const {
-  return L"Software\\Chromium\\ClientStateMedium\\test_app_guid";
-}

chrome/installer/util/test_app_registration_data.h

@@ -12,7 +12,6 @@ class TestAppRegistrationData : public AppRegistrationData {
   TestAppRegistrationData();
   ~TestAppRegistrationData() override;
   base::string16 GetStateKey() const override;
-  base::string16 GetStateMediumKey() const override;
 };
 
 #endif  // CHROME_INSTALLER_UTIL_TEST_APP_REGISTRATION_DATA_H_

chrome/installer/util/updating_app_registration_data.cc

@@ -16,9 +16,3 @@ base::string16 UpdatingAppRegistrationData::GetStateKey() const {
       .append(1, L'\\')
       .append(app_guid_);
 }
-
-base::string16 UpdatingAppRegistrationData::GetStateMediumKey() const {
-  return base::string16(google_update::kRegPathClientStateMedium)
-      .append(1, L'\\')
-      .append(app_guid_);
-}

chrome/installer/util/updating_app_registration_data.h

@@ -16,7 +16,6 @@ class UpdatingAppRegistrationData : public AppRegistrationData {
   explicit UpdatingAppRegistrationData(const base::string16& app_guid);
   ~UpdatingAppRegistrationData() override;
   base::string16 GetStateKey() const override;
-  base::string16 GetStateMediumKey() const override;
 
  private:
   const base::string16 app_guid_;