Make sure GoogleUpdateSettings::GetChromeChannel does not read

from the wrong section of registry when a per-user install has left 
some registrations behind. 
Fix the odd race and other bugs I encountered along the way. 

BUG=40994
TEST=Unittests in this change. Open about box and look at the channel string. 

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

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

3 files changed, 27 insertions, 8 deletions

diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 2dabe50..dd8851c 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/chrome/installer/util/google_update_settings.cc
@@ -143,9 +143,14 @@ bool GoogleUpdateSettings::ClearReferral() {
   return ClearGoogleUpdateStrKey(google_update::kRegReferralField);
 }
 
-bool GoogleUpdateSettings::GetChromeChannel(std::wstring* channel) {
+bool GoogleUpdateSettings::GetChromeChannel(bool system_install,
+    std::wstring* channel) {
+  HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  std::wstring reg_path = dist->GetStateKey();
+  RegKey key(root_key, reg_path.c_str(), KEY_READ);
   std::wstring update_branch;
-  if (!ReadGoogleUpdateStrKey(google_update::kRegApField, &update_branch)) {
+  if (!key.ReadValue(google_update::kRegApField, &update_branch)) {
     *channel = L"unknown";
     return false;
   }
diff --git a/chrome/installer/util/google_update_settings.h b/chrome/installer/util/google_update_settings.h
index 3edfedf..70fbaf8 100644
--- a/chrome/installer/util/google_update_settings.h
+++ b/chrome/installer/util/google_update_settings.h
@@ -81,7 +81,7 @@ class GoogleUpdateSettings {
   // Return a human readable modifier for the version string, e.g.
   // the channel (dev, beta, stable). Returns true if this operation succeeded,
   // on success, channel contains one of "", "unknown", "dev" or "beta".
-  static bool GetChromeChannel(std::wstring* channel);
+  static bool GetChromeChannel(bool system_install, std::wstring* channel);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(GoogleUpdateSettings);
diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc
index bb2e46c..e58e23b 100644
--- a/chrome/installer/util/google_update_settings_unittest.cc
+++ b/chrome/installer/util/google_update_settings_unittest.cc
@@ -107,7 +107,8 @@ class GoogleUpdateSettingsTest: public testing::Test {
           SetApField(install, ap.c_str());
           std::wstring ret_channel;
 
-          EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannel(&ret_channel));
+          EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannel(is_system,
+              &ret_channel));
           EXPECT_STREQ(channel, ret_channel.c_str())
               << "Expecting channel \"" << channel
               << "\" for ap=\"" << ap << "\"";
@@ -122,10 +123,16 @@ class GoogleUpdateSettingsTest: public testing::Test {
 
 }  // namespace
 
-// Verify that we return failure on no registration.
+// Verify that we return failure on no registration,
+// whether per-system or per-user install.
 TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelAbsent) {
+  // Per-system first.
   std::wstring channel;
-  EXPECT_FALSE(GoogleUpdateSettings::GetChromeChannel(&channel));
+  EXPECT_FALSE(GoogleUpdateSettings::GetChromeChannel(true, &channel));
+  EXPECT_STREQ(L"unknown", channel.c_str());
+
+  // Then per-user.
+  EXPECT_FALSE(GoogleUpdateSettings::GetChromeChannel(false, &channel));
   EXPECT_STREQ(L"unknown", channel.c_str());
 }
 
@@ -133,14 +140,21 @@ TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelAbsent) {
 TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelEmptySystem) {
   SetApField(SYSTEM_INSTALL, L"");
   std::wstring channel;
-  EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannel(&channel));
+  EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannel(true, &channel));
   EXPECT_STREQ(L"", channel.c_str());
+
+  // Per-user lookups should fail.
+  EXPECT_FALSE(GoogleUpdateSettings::GetChromeChannel(false, &channel));
 }
 
 TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelEmptyUser) {
   SetApField(USER_INSTALL, L"");
+  // Per-system lookup should fail.
   std::wstring channel;
-  EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannel(&channel));
+  EXPECT_FALSE(GoogleUpdateSettings::GetChromeChannel(true, &channel));
+
+  // Per-user lookup should succeed.
+  EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannel(false, &channel));
   EXPECT_STREQ(L"", channel.c_str());
 }