Read the stats consent setting properly for system-level installs.

I also added comprehensive unit tests, for which I needed to add new Windows-specific helper funcs for the getter and setter.

BUG=108087
TEST=install Chrome with --system-level --multi-install --chrome then try changing the "Automatically send usage statistics and crash reports to Google" setting.


Review URL: http://codereview.chromium.org/9114039

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@117117 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 309 insertions, 27 deletions

diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 2579be6..9bf0e12 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/chrome/installer/util/google_update_settings.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -172,15 +172,8 @@ bool GetUpdatePolicyFromDword(
   return false;
 }
 
-}  // namespace
-
-// Older versions of Chrome unconditionally read from HKCU\...\ClientState\...
-// and then HKLM\...\ClientState\....  This means that system-level Chrome
-// never checked ClientStateMedium (which has priority according to Google
-// Update) and gave preference to a value in HKCU (which was never checked by
-// Google Update).  From now on, Chrome follows Google Update's policy.
-bool GoogleUpdateSettings::GetCollectStatsConsent() {
-  // Determine whether this is a system-level or a user-level install.
+// Determine whether this is a system-level or a user-level install.
+bool IsSystemInstall() {
   bool system_install = false;
   FilePath module_dir;
   if (!PathService::Get(base::DIR_MODULE, &module_dir)) {
@@ -189,6 +182,21 @@ bool GoogleUpdateSettings::GetCollectStatsConsent() {
   } else {
     system_install = !InstallUtil::IsPerUserInstall(module_dir.value().c_str());
   }
+  return system_install;
+}
+
+}  // namespace
+
+bool GoogleUpdateSettings::GetCollectStatsConsent() {
+  return GetCollectStatsConsentAtLevel(IsSystemInstall());
+}
+
+// Older versions of Chrome unconditionally read from HKCU\...\ClientState\...
+// and then HKLM\...\ClientState\....  This means that system-level Chrome
+// never checked ClientStateMedium (which has priority according to Google
+// Update) and gave preference to a value in HKCU (which was never checked by
+// Google Update).  From now on, Chrome follows Google Update's policy.
+bool GoogleUpdateSettings::GetCollectStatsConsentAtLevel(bool system_install) {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
 
   // Consent applies to all products in a multi-install package.
@@ -210,30 +218,28 @@ bool GoogleUpdateSettings::GetCollectStatsConsent() {
                       &value) == ERROR_SUCCESS;
 
   // Otherwise, try ClientState.
-  have_value =
-      !have_value &&
-      key.Open(system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
-               dist->GetStateKey().c_str(), KEY_QUERY_VALUE) == ERROR_SUCCESS &&
-      key.ReadValueDW(google_update::kRegUsageStatsField,
-                      &value) == ERROR_SUCCESS;
+  if (!have_value) {
+    have_value =
+        key.Open(system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
+                 dist->GetStateKey().c_str(),
+                 KEY_QUERY_VALUE) == ERROR_SUCCESS &&
+        key.ReadValueDW(google_update::kRegUsageStatsField,
+                        &value) == ERROR_SUCCESS;
+  }
 
   // Google Update specifically checks that the value is 1, so we do the same.
   return have_value && value == 1;
 }
 
 bool GoogleUpdateSettings::SetCollectStatsConsent(bool consented) {
+  return SetCollectStatsConsentAtLevel(IsSystemInstall(), consented);
+}
+
+bool GoogleUpdateSettings::SetCollectStatsConsentAtLevel(bool system_install,
+                                                         bool consented) {
   // Google Update writes and expects 1 for true, 0 for false.
   DWORD value = consented ? 1 : 0;
 
-  // Determine whether this is a system-level or a user-level install.
-  bool system_install = false;
-  FilePath module_dir;
-  if (!PathService::Get(base::DIR_MODULE, &module_dir)) {
-    LOG(WARNING)
-        << "Failed to get directory of module; assuming per-user install.";
-  } else {
-    system_install = !InstallUtil::IsPerUserInstall(module_dir.value().c_str());
-  }
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
 
   // Consent applies to all products in a multi-install package.
diff --git a/chrome/installer/util/google_update_settings.h b/chrome/installer/util/google_update_settings.h
index ab3ccd8..33c1e04 100644
--- a/chrome/installer/util/google_update_settings.h
+++ b/chrome/installer/util/google_update_settings.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -40,6 +40,18 @@ class GoogleUpdateSettings {
   // false if the setting could not be recorded.
   static bool SetCollectStatsConsent(bool consented);
 
+#if defined(OS_WIN)
+  // Returns whether the user has given consent to collect UMA data and send
+  // crash dumps to Google. This information is collected by the web server
+  // used to download the chrome installer.
+  static bool GetCollectStatsConsentAtLevel(bool system_install);
+
+  // Sets the user consent to send UMA and crash dumps to Google. Returns
+  // false if the setting could not be recorded.
+  static bool SetCollectStatsConsentAtLevel(bool system_install,
+                                            bool consented);
+#endif
+
   // Returns the metrics id set in the registry (that can be used in crash
   // reports). If none found, returns empty string.
   static bool GetMetricsId(std::wstring* metrics_id);
diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc
index 69348b6..a483c6a 100644
--- a/chrome/installer/util/google_update_settings_unittest.cc
+++ b/chrome/installer/util/google_update_settings_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -6,6 +6,7 @@
 #include <shlwapi.h>  // For SHDeleteKey.
 
 #include "base/memory/scoped_ptr.h"
+#include "base/test/test_reg_util_win.h"
 #include "base/win/registry.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/installer/util/browser_distribution.h"
@@ -587,3 +588,266 @@ TEST_F(GoogleUpdateSettingsTest, GetAppUpdatePolicyAppOverride) {
 }
 
 #endif  // defined(GOOGLE_CHROME_BUILD)
+
+// Test values for use by the CollectStatsConsent test fixture.
+class StatsState {
+ public:
+  enum InstallType {
+    SINGLE_INSTALL,
+    MULTI_INSTALL,
+  };
+  enum StateSetting {
+    NO_SETTING,
+    FALSE_SETTING,
+    TRUE_SETTING,
+  };
+  struct UserLevelState {};
+  struct SystemLevelState {};
+  static const UserLevelState kUserLevel;
+  static const SystemLevelState kSystemLevel;
+
+  StatsState(const UserLevelState&,
+             InstallType install_type,
+             StateSetting state_value)
+      : system_level_(false),
+        multi_install_(install_type == MULTI_INSTALL),
+        state_value_(state_value),
+        state_medium_value_(NO_SETTING) {
+  }
+  StatsState(const SystemLevelState&,
+             InstallType install_type,
+             StateSetting state_value,
+             StateSetting state_medium_value)
+      : system_level_(true),
+        multi_install_(install_type == MULTI_INSTALL),
+        state_value_(state_value),
+        state_medium_value_(state_medium_value) {
+  }
+  bool system_level() const { return system_level_; }
+  bool multi_install() const { return multi_install_; }
+  HKEY root_key() const {
+    return system_level_ ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+  }
+  StateSetting state_value() const { return state_value_; }
+  StateSetting state_medium_value() const {
+    return state_medium_value_;
+  }
+  bool is_consent_granted() const {
+    return (system_level_ && state_medium_value_ != NO_SETTING) ?
+        (state_medium_value_ == TRUE_SETTING) :
+        (state_value_ == TRUE_SETTING);
+  }
+ private:
+  bool system_level_;
+  bool multi_install_;
+  StateSetting state_value_;
+  StateSetting state_medium_value_;
+};
+
+const StatsState::UserLevelState StatsState::kUserLevel;
+const StatsState::SystemLevelState StatsState::kSystemLevel;
+
+// A value parameterized test for testing the stats collection consent setting.
+class CollectStatsConsent : public ::testing::TestWithParam<StatsState> {
+ public:
+  static void SetUpTestCase();
+  static void TearDownTestCase();
+ protected:
+  virtual void SetUp() OVERRIDE;
+  static void MakeChromeMultiInstall(HKEY root_key);
+  static void ApplySetting(StatsState::StateSetting setting,
+                           HKEY root_key,
+                           const std::wstring& reg_key);
+
+  static std::wstring* chrome_version_key_;
+  static std::wstring* chrome_state_key_;
+  static std::wstring* chrome_state_medium_key_;
+  static std::wstring* binaries_state_key_;
+  static std::wstring* binaries_state_medium_key_;
+  registry_util::RegistryOverrideManager override_manager_;
+};
+
+std::wstring* CollectStatsConsent::chrome_version_key_;
+std::wstring* CollectStatsConsent::chrome_state_key_;
+std::wstring* CollectStatsConsent::chrome_state_medium_key_;
+std::wstring* CollectStatsConsent::binaries_state_key_;
+std::wstring* CollectStatsConsent::binaries_state_medium_key_;
+
+void CollectStatsConsent::SetUpTestCase() {
+  BrowserDistribution* dist =
+      BrowserDistribution::GetSpecificDistribution(
+          BrowserDistribution::CHROME_BROWSER);
+  chrome_version_key_ = new std::wstring(dist->GetVersionKey());
+  chrome_state_key_ = new std::wstring(dist->GetStateKey());
+  chrome_state_medium_key_ = new std::wstring(dist->GetStateMediumKey());
+
+  dist = BrowserDistribution::GetSpecificDistribution(
+      BrowserDistribution::CHROME_BINARIES);
+  binaries_state_key_ = new std::wstring(dist->GetStateKey());
+  binaries_state_medium_key_ = new std::wstring(dist->GetStateMediumKey());
+}
+
+void CollectStatsConsent::TearDownTestCase() {
+  delete chrome_version_key_;
+  delete chrome_state_key_;
+  delete chrome_state_medium_key_;
+  delete binaries_state_key_;
+  delete binaries_state_medium_key_;
+}
+
+// Install the registry override and apply the settings to the registry.
+void CollectStatsConsent::SetUp() {
+  const StatsState& stats_state = GetParam();
+  const HKEY root_key = stats_state.root_key();
+  std::wstring reg_temp_name(stats_state.system_level() ? L"HKLM_" : L"HKCU_");
+  reg_temp_name += L"CollectStatsConsent";
+  override_manager_.OverrideRegistry(root_key, reg_temp_name);
+
+  if (stats_state.multi_install()) {
+    MakeChromeMultiInstall(root_key);
+    ApplySetting(stats_state.state_value(), root_key, *binaries_state_key_);
+    ApplySetting(stats_state.state_medium_value(), root_key,
+                 *binaries_state_medium_key_);
+  } else {
+    ApplySetting(stats_state.state_value(), root_key, *chrome_state_key_);
+    ApplySetting(stats_state.state_medium_value(), root_key,
+                 *chrome_state_medium_key_);
+  }
+}
+
+// Write values into the registry so that Chrome is considered to be installed
+// as multi-install.
+void CollectStatsConsent::MakeChromeMultiInstall(HKEY root_key) {
+  ASSERT_EQ(
+      ERROR_SUCCESS,
+      RegKey(root_key, chrome_version_key_->c_str(),
+             KEY_SET_VALUE).WriteValue(google_update::kRegVersionField,
+                                       L"1.2.3.4"));
+  ASSERT_EQ(
+      ERROR_SUCCESS,
+      RegKey(root_key, chrome_state_key_->c_str(),
+             KEY_SET_VALUE).WriteValue(installer::kUninstallArgumentsField,
+                                       L"--multi-install"));
+}
+
+// Write the correct value to represent |setting| in the registry.
+void CollectStatsConsent::ApplySetting(StatsState::StateSetting setting,
+                                       HKEY root_key,
+                                       const std::wstring& reg_key) {
+  if (setting != StatsState::NO_SETTING) {
+    DWORD value = setting != StatsState::FALSE_SETTING ? 1 : 0;
+    ASSERT_EQ(
+        ERROR_SUCCESS,
+        RegKey(root_key, reg_key.c_str(),
+               KEY_SET_VALUE).WriteValue(google_update::kRegUsageStatsField,
+                                         value));
+  }
+}
+
+// Test that stats consent can be read.
+TEST_P(CollectStatsConsent, GetCollectStatsConsentAtLevel) {
+  if (GetParam().is_consent_granted()) {
+    EXPECT_TRUE(GoogleUpdateSettings::GetCollectStatsConsentAtLevel(
+                    GetParam().system_level()));
+  } else {
+    EXPECT_FALSE(GoogleUpdateSettings::GetCollectStatsConsentAtLevel(
+                     GetParam().system_level()));
+  }
+}
+
+// Test that stats consent can be flipped to the opposite setting, that the new
+// setting takes affect, and that the correct registry location is modified.
+TEST_P(CollectStatsConsent, SetCollectStatsConsentAtLevel) {
+  EXPECT_TRUE(GoogleUpdateSettings::SetCollectStatsConsentAtLevel(
+                  GetParam().system_level(),
+                  !GetParam().is_consent_granted()));
+  const std::wstring* const reg_keys[] = {
+    chrome_state_key_,
+    chrome_state_medium_key_,
+    binaries_state_key_,
+    binaries_state_medium_key_,
+  };
+  int key_index = ((GetParam().system_level() ? 1 : 0) +
+                   (GetParam().multi_install() ? 2 : 0));
+  const std::wstring& reg_key = *reg_keys[key_index];
+  DWORD value = 0;
+  EXPECT_EQ(
+      ERROR_SUCCESS,
+      RegKey(GetParam().root_key(), reg_key.c_str(),
+             KEY_QUERY_VALUE).ReadValueDW(google_update::kRegUsageStatsField,
+                                          &value));
+  if (GetParam().is_consent_granted()) {
+    EXPECT_FALSE(GoogleUpdateSettings::GetCollectStatsConsentAtLevel(
+                     GetParam().system_level()));
+    EXPECT_EQ(0UL, value);
+  } else {
+    EXPECT_TRUE(GoogleUpdateSettings::GetCollectStatsConsentAtLevel(
+                    GetParam().system_level()));
+    EXPECT_EQ(1UL, value);
+  }
+}
+
+INSTANTIATE_TEST_CASE_P(
+    UserLevelSingleInstall,
+    CollectStatsConsent,
+    ::testing::Values(
+        StatsState(StatsState::kUserLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::NO_SETTING),
+        StatsState(StatsState::kUserLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::FALSE_SETTING),
+        StatsState(StatsState::kUserLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::TRUE_SETTING)));
+INSTANTIATE_TEST_CASE_P(
+    UserLevelMultiInstall,
+    CollectStatsConsent,
+    ::testing::Values(
+        StatsState(StatsState::kUserLevel, StatsState::MULTI_INSTALL,
+                   StatsState::NO_SETTING),
+        StatsState(StatsState::kUserLevel, StatsState::MULTI_INSTALL,
+                   StatsState::FALSE_SETTING),
+        StatsState(StatsState::kUserLevel, StatsState::MULTI_INSTALL,
+                   StatsState::TRUE_SETTING)));
+INSTANTIATE_TEST_CASE_P(
+    SystemLevelSingleInstall,
+    CollectStatsConsent,
+    ::testing::Values(
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::NO_SETTING, StatsState::NO_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::NO_SETTING, StatsState::FALSE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::NO_SETTING, StatsState::TRUE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::FALSE_SETTING, StatsState::NO_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::FALSE_SETTING, StatsState::FALSE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::FALSE_SETTING, StatsState::TRUE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::TRUE_SETTING, StatsState::NO_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::TRUE_SETTING, StatsState::FALSE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::SINGLE_INSTALL,
+                   StatsState::TRUE_SETTING, StatsState::TRUE_SETTING)));
+INSTANTIATE_TEST_CASE_P(
+    SystemLevelMultiInstall,
+    CollectStatsConsent,
+    ::testing::Values(
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::NO_SETTING, StatsState::NO_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::NO_SETTING, StatsState::FALSE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::NO_SETTING, StatsState::TRUE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::FALSE_SETTING, StatsState::NO_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::FALSE_SETTING, StatsState::FALSE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::FALSE_SETTING, StatsState::TRUE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::TRUE_SETTING, StatsState::NO_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::TRUE_SETTING, StatsState::FALSE_SETTING),
+        StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
+                   StatsState::TRUE_SETTING, StatsState::TRUE_SETTING)));