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

5 files changed, 38 insertions, 10 deletions

diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc
index ac8457a..c555e39 100644
--- a/chrome/app/breakpad_win.cc
+++ b/chrome/app/breakpad_win.cc
@@ -399,7 +399,8 @@ static DWORD __stdcall InitCrashReporterThread(void* param) {
   } else {
     // Capture more detail in crash dumps for beta and dev channel builds.
     string16 channel_string;
-    GoogleUpdateSettings::GetChromeChannel(&channel_string);
+    GoogleUpdateSettings::GetChromeChannel(!is_per_user_install,
+        &channel_string);
     if (channel_string == L"dev" || channel_string == L"beta")
       dump_type = kLargerDumpType;
   }
diff --git a/chrome/common/platform_util_win.cc b/chrome/common/platform_util_win.cc
index 73311f8..79d1034 100644
--- a/chrome/common/platform_util_win.cc
+++ b/chrome/common/platform_util_win.cc
@@ -12,12 +12,14 @@
 #include "app/win_util.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
+#include "base/path_service.h"
 #include "base/logging.h"
 #include "base/registry.h"
 #include "base/scoped_comptr_win.h"
 #include "base/string_util.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/google_update_constants.h"
+#include "chrome/installer/util/install_util.h"
 #include "gfx/native_widget_types.h"
 #include "googleurl/src/gurl.h"
 
@@ -154,8 +156,14 @@ void SimpleErrorBox(gfx::NativeWindow parent,
 
 string16 GetVersionStringModifier() {
 #if defined(GOOGLE_CHROME_BUILD)
+  FilePath module;
   string16 channel;
-  GoogleUpdateSettings::GetChromeChannel(&channel);
+  if (PathService::Get(base::FILE_MODULE, &module)) {
+    bool is_system_install =
+        !InstallUtil::IsPerUserInstall(module.value().c_str());
+
+    GoogleUpdateSettings::GetChromeChannel(is_system_install, &channel);
+  }
   return channel;
 #else
   return string16();
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());
 }