Fix users of platform_util::GetVersionStringModifier that really only care

about the channel name to use the new platform_util::GetInstallationChannel.
GetVersionStringModifier should only be used for display purposes. Use
GetInstallationChannel to determine the channel name for comparisons.

Fix the upgrade detector to treat the canary channel the same way that it
treats the dev channel, using a shorter notification interval.

BUG=79814
TEST=none
Review URL: http://codereview.chromium.org/6899034

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

11 files changed, 166 insertions, 77 deletions

diff --git a/chrome/browser/platform_util.h b/chrome/browser/platform_util.h
index 41fa0ed..2cc4c1b 100644
--- a/chrome/browser/platform_util.h
+++ b/chrome/browser/platform_util.h
@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_PLATFORM_UTIL_H_
 #pragma once
 
+#include <string>
+
 #include "base/string16.h"
 #include "ui/gfx/native_widget_types.h"
 
@@ -14,6 +16,15 @@ class GURL;
 
 namespace platform_util {
 
+// The possible channels for an installation, from most fun to most stable.
+enum Channel {
+  CHANNEL_UNKNOWN = 0,  // Probably blue
+  CHANNEL_CANARY,       // Yellow
+  CHANNEL_DEV,          // Technicolor
+  CHANNEL_BETA,         // Rainbow
+  CHANNEL_STABLE        // Full-spectrum
+};
+
 // Show the given file in a file manager. If possible, select the file.
 void ShowItemInFolder(const FilePath& full_path);
 
@@ -55,11 +66,24 @@ bool SimpleYesNoBox(gfx::NativeWindow parent,
                     const string16& title,
                     const string16& message);
 
-// Return a human readable modifier for the version string.  For a
-// branded Chrome (not Chromium), this modifier is the channel (dev,
-// beta, but "" for stable).
+// Returns a human-readable modifier for the version string. For a branded
+// build, this modifier is the channel ("canary", "dev", or "beta", but ""
+// for stable). On Windows, this may be modified with additional information
+// after a hyphen. For multi-user installations, it will return "canary-m",
+// "dev-m", "beta-m", and for a stable channel multi-user installation, "m".
+// In branded builds, when the channel cannot be determined, "unknown" will
+// be returned. In unbranded builds, the modifier is usually an empty string
+// (""), although on Linux, it may vary in certain distributions.
+// GetVersionStringModifier() is intended to be used for display purposes.
+// To simply test the channel, use GetChannel().
 std::string GetVersionStringModifier();
 
+// Returns the channel for the installation. In branded builds, this will be
+// CHANNEL_STABLE, CHANNEL_BETA, CHANNEL_DEV, or CHANNEL_CANARY. In unbranded
+// builds, or in branded builds when the channel cannot be determined, this
+// will be CHANNEL_UNKNOWN.
+Channel GetChannel();
+
 // Returns true if the running browser can be set as the default browser.
 bool CanSetAsDefaultBrowser();
 
diff --git a/chrome/browser/platform_util_common_linux.cc b/chrome/browser/platform_util_common_linux.cc
index 195dc66..3e3734d 100644
--- a/chrome/browser/platform_util_common_linux.cc
+++ b/chrome/browser/platform_util_common_linux.cc
@@ -134,6 +134,24 @@ std::string GetVersionStringModifier() {
   return modifier;
 }
 
+// Warning: this may be either Linux or ChromeOS.
+Channel GetChannel() {
+#if defined(GOOGLE_CHROME_BUILD)
+  std::string channel = GetVersionStringModifier();
+  if (channel.empty()) {
+    return CHANNEL_STABLE;
+  } else if (channel == "beta") {
+    return CHANNEL_BETA;
+  } else if (channel == "dev") {
+    return CHANNEL_DEV;
+  } else if (channel == "canary") {
+    return CHANNEL_CANARY;
+  }
+#endif
+
+  return CHANNEL_UNKNOWN;
+}
+
 bool CanSetAsDefaultBrowser() {
   return true;
 }
diff --git a/chrome/browser/platform_util_mac.mm b/chrome/browser/platform_util_mac.mm
index 9a61775..d579106 100644
--- a/chrome/browser/platform_util_mac.mm
+++ b/chrome/browser/platform_util_mac.mm
@@ -209,8 +209,25 @@ std::string GetVersionStringModifier() {
 #endif
 }
 
+Channel GetChannel() {
+#if defined(GOOGLE_CHROME_BUILD)
+  std::string channel = GetVersionStringModifier();
+  if (channel.empty()) {
+    return CHANNEL_STABLE;
+  } else if (channel == "beta") {
+    return CHANNEL_BETA;
+  } else if (channel == "dev") {
+    return CHANNEL_DEV;
+  } else if (channel == "canary") {
+    return CHANNEL_CANARY;
+  }
+#endif
+
+  return CHANNEL_UNKNOWN;
+}
+
 bool CanSetAsDefaultBrowser() {
-  return GetVersionStringModifier().compare("canary") != 0;
+  return GetChannel() != CHANNEL_CANARY;
 }
 
 }  // namespace platform_util
diff --git a/chrome/browser/platform_util_win.cc b/chrome/browser/platform_util_win.cc
index 2076b64..1949e2b 100644
--- a/chrome/browser/platform_util_win.cc
+++ b/chrome/browser/platform_util_win.cc
@@ -212,6 +212,29 @@ std::string GetVersionStringModifier() {
 #endif
 }
 
+Channel GetChannel() {
+#if defined(GOOGLE_CHROME_BUILD)
+  // Call GoogleUpdateSettings::GetChromeChannel with |false| as the first
+  // argument to avoid having it append "-m" to the channel name, or returning
+  // "m" for the stable channel.
+  string16 channel_16;
+  GoogleUpdateSettings::GetChromeChannel(false, &channel_16);
+  std::string channel = UTF16ToASCII(channel_16);
+
+  if (channel.empty()) {
+    return CHANNEL_STABLE;
+  } else if (channel == "beta") {
+    return CHANNEL_BETA;
+  } else if (channel == "dev") {
+    return CHANNEL_DEV;
+  } else if (channel == "canary") {
+    return CHANNEL_CANARY;
+  }
+#endif
+
+  return CHANNEL_UNKNOWN;
+}
+
 bool CanSetAsDefaultBrowser() {
   return BrowserDistribution::GetDistribution()->CanSetAsDefault();
 }
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 814e06e..3c6a4cf 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -86,22 +86,18 @@ ProfileSyncService::ProfileSyncService(ProfileSyncFactory* factory,
                  NotificationType::SYNC_DATA_TYPES_UPDATED,
                  Source<Profile>(profile));
 
-  // By default, dev & chromium users will go to the development servers.
-  // Dev servers have more features than standard sync servers.
-  // Chrome stable and beta builds will go to the standard sync servers.
-#if defined(GOOGLE_CHROME_BUILD)
-  // GetVersionStringModifier hits the registry. See http://crbug.com/70380.
+  // By default, dev, canary, and unbranded Chromium users will go to the
+  // development servers. Development servers have more features than standard
+  // sync servers. Users with officially-branded Chrome stable and beta builds
+  // will go to the standard sync servers.
+  //
+  // GetChannel hits the registry on Windows. See http://crbug.com/70380.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
-  // For stable, this is "". For dev, this is "dev". For beta, this is "beta".
-  // For daily, this is "canary".
-  // For Linux Chromium builds, this could be anything depending on the
-  // distribution, so always direct those users to dev server urls.
-  // If this is an official build, it will always be one of the above.
-  std::string channel = platform_util::GetVersionStringModifier();
-  if (channel.empty() || channel == "beta") {
+  platform_util::Channel channel = platform_util::GetChannel();
+  if (channel == platform_util::CHANNEL_STABLE ||
+      channel == platform_util::CHANNEL_BETA) {
     sync_service_url_ = GURL(kSyncServerUrl);
   }
-#endif
 
   tried_implicit_gaia_remove_when_bug_62103_fixed_ = false;
 }
diff --git a/chrome/browser/ui/cocoa/first_run_dialog.mm b/chrome/browser/ui/cocoa/first_run_dialog.mm
index 602fbc2..da369ad 100644
--- a/chrome/browser/ui/cocoa/first_run_dialog.mm
+++ b/chrome/browser/ui/cocoa/first_run_dialog.mm
@@ -136,7 +136,7 @@ void ShowFirstRun(Profile* profile) {
 // True when the stats checkbox should be checked by default. This is only
 // the case when the canary is running.
 bool StatsCheckboxDefault() {
-  return platform_util::GetVersionStringModifier().compare("canary") == 0;
+  return platform_util::GetChannel() == platform_util::CHANNEL_CANARY;
 }
 
 }  // namespace
diff --git a/chrome/browser/upgrade_detector.cc b/chrome/browser/upgrade_detector.cc
index 19e9703..00688a8 100644
--- a/chrome/browser/upgrade_detector.cc
+++ b/chrome/browser/upgrade_detector.cc
@@ -68,9 +68,9 @@ int GetCheckForUpgradeEveryMs() {
 class DetectUpgradeTask : public Task {
  public:
   explicit DetectUpgradeTask(Task* upgrade_detected_task,
-                             bool* is_dev_channel)
+                             bool* is_unstable_channel)
       : upgrade_detected_task_(upgrade_detected_task),
-        is_dev_channel_(is_dev_channel) {
+        is_unstable_channel_(is_unstable_channel) {
   }
 
   virtual ~DetectUpgradeTask() {
@@ -115,8 +115,9 @@ class DetectUpgradeTask : public Task {
     installed_version.reset(Version::GetVersionFromString(reply));
 #endif
 
-    const std::string channel = platform_util::GetVersionStringModifier();
-    *is_dev_channel_ = channel == "dev";
+    platform_util::Channel channel = platform_util::GetChannel();
+    *is_unstable_channel_ = channel == platform_util::CHANNEL_DEV ||
+                            channel == platform_util::CHANNEL_CANARY;
 
     // Get the version of the currently *running* instance of Chrome.
     chrome::VersionInfo version_info;
@@ -144,7 +145,7 @@ class DetectUpgradeTask : public Task {
 
  private:
   Task* upgrade_detected_task_;
-  bool* is_dev_channel_;
+  bool* is_unstable_channel_;
 };
 
 }  // namespace
@@ -156,7 +157,7 @@ void UpgradeDetector::RegisterPrefs(PrefService* prefs) {
 
 UpgradeDetector::UpgradeDetector()
     : ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
-      is_dev_channel_(false),
+      is_unstable_channel_(false),
       upgrade_notification_stage_(UPGRADE_ANNOYANCE_NONE),
       notify_upgrade_(false) {
   CommandLine command_line(*CommandLine::ForCurrentProcess());
@@ -195,7 +196,7 @@ void UpgradeDetector::CheckForUpgrade() {
   // on Windows checking for an upgrade requires reading a file.
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                           new DetectUpgradeTask(callback_task,
-                                                &is_dev_channel_));
+                                                &is_unstable_channel_));
 }
 
 void UpgradeDetector::UpgradeDetected() {
@@ -231,10 +232,10 @@ void UpgradeDetector::NotifyOnUpgrade() {
   const int kSevereThreshold = 14 * (interval.empty() ? 24 : 1);
   const int kHighThreshold = 7 * (interval.empty() ? 24 : 1);
   const int kElevatedThreshold = 4  * (interval.empty() ? 24 : 1);
-  // Dev channel is fixed at lowest severity after 1 hour. For other channels
-  // it is after 2 hours. And, as before, if a command line is passed in we
-  // drastically reduce the wait time.
-  const int multiplier = is_dev_channel_ ? 1 : 2;
+  // Unstable channels are fixed at lowest severity after 1 hour. For other
+  // channels it is after 2 hours. And, as before, if a command line is passed
+  // in we drastically reduce the wait time.
+  const int multiplier = is_unstable_channel_ ? 1 : 2;
   const int kLowThreshold = multiplier * (interval.empty() ? 24 : 1);
 
   // These if statements (except for the first one) must be sorted (highest
@@ -250,7 +251,7 @@ void UpgradeDetector::NotifyOnUpgrade() {
   else
     return;  // Not ready to recommend upgrade.
 
-  if (is_dev_channel_ ||
+  if (is_unstable_channel_ ||
       upgrade_notification_stage_ == UPGRADE_ANNOYANCE_SEVERE) {
     // We can't get any higher, baby.
     upgrade_notification_timer_.Stop();
diff --git a/chrome/browser/upgrade_detector.h b/chrome/browser/upgrade_detector.h
index 659ffb8..3188efb 100644
--- a/chrome/browser/upgrade_detector.h
+++ b/chrome/browser/upgrade_detector.h
@@ -77,8 +77,8 @@ class UpgradeDetector {
   // When the upgrade was detected.
   base::Time upgrade_detected_time_;
 
-  // Whether this build is a dev channel build or not.
-  bool is_dev_channel_;
+  // True if this build is a dev or canary channel build.
+  bool is_unstable_channel_;
 
   // The stage at which the annoyance level for upgrade notifications is at.
   UpgradeNotificationAnnoyanceLevel upgrade_notification_stage_;
diff --git a/chrome/browser/web_resource/promo_resource_service.cc b/chrome/browser/web_resource/promo_resource_service.cc
index fb8749e..d9f526f 100644
--- a/chrome/browser/web_resource/promo_resource_service.cc
+++ b/chrome/browser/web_resource/promo_resource_service.cc
@@ -75,20 +75,21 @@ void PromoResourceService::RegisterUserPrefs(PrefService* prefs) {
 }
 
 // static
-bool PromoResourceService::IsBuildTargeted(const std::string& channel,
+bool PromoResourceService::IsBuildTargeted(platform_util::Channel channel,
                                            int builds_allowed) {
   if (builds_allowed == NO_BUILD)
     return false;
-  if (channel == "canary" || channel == "canary-m") {
-    return (CANARY_BUILD & builds_allowed) != 0;
-  } else if (channel == "dev" || channel == "dev-m") {
-    return (DEV_BUILD & builds_allowed) != 0;
-  } else if (channel == "beta" || channel == "beta-m") {
-    return (BETA_BUILD & builds_allowed) != 0;
-  } else if (channel == "" || channel == "m") {
-    return (STABLE_BUILD & builds_allowed) != 0;
-  } else {
-    return false;
+  switch (channel) {
+    case platform_util::CHANNEL_CANARY:
+      return (CANARY_BUILD & builds_allowed) != 0;
+    case platform_util::CHANNEL_DEV:
+      return (DEV_BUILD & builds_allowed) != 0;
+    case platform_util::CHANNEL_BETA:
+      return (BETA_BUILD & builds_allowed) != 0;
+    case platform_util::CHANNEL_STABLE:
+      return (STABLE_BUILD & builds_allowed) != 0;
+    default:
+      return false;
   }
 }
 
@@ -102,7 +103,7 @@ PromoResourceService::PromoResourceService(Profile* profile)
                          kStartResourceFetchDelay,
                          kCacheUpdateDelay),
       web_resource_cache_(NULL),
-      channel_(NULL) {
+      channel_(platform_util::CHANNEL_UNKNOWN) {
   Init();
 }
 
@@ -113,9 +114,10 @@ void PromoResourceService::Init() {
 }
 
 bool PromoResourceService::IsThisBuildTargeted(int builds_targeted) {
-  if (channel_ == NULL) {
+  if (channel_ == platform_util::CHANNEL_UNKNOWN) {
+    // GetChannel hits the registry on Windows. See http://crbug.com/70898.
     base::ThreadRestrictions::ScopedAllowIO allow_io;
-    channel_ = platform_util::GetVersionStringModifier().c_str();
+    channel_ = platform_util::GetChannel();
   }
 
   return IsBuildTargeted(channel_, builds_targeted);
@@ -428,9 +430,9 @@ bool CanShowPromo(Profile* profile) {
 
   bool is_promo_build = false;
   if (prefs->HasPrefPath(prefs::kNTPPromoBuild)) {
-    // GetVersionStringModifier hits the registry. See http://crbug.com/70898.
+    // GetChannel hits the registry on Windows. See http://crbug.com/70898.
     base::ThreadRestrictions::ScopedAllowIO allow_io;
-    const std::string channel = platform_util::GetVersionStringModifier();
+    platform_util::Channel channel = platform_util::GetChannel();
     is_promo_build = PromoResourceService::IsBuildTargeted(
         channel, prefs->GetInteger(prefs::kNTPPromoBuild));
   }
diff --git a/chrome/browser/web_resource/promo_resource_service.h b/chrome/browser/web_resource/promo_resource_service.h
index 2149890a..bc9b791 100644
--- a/chrome/browser/web_resource/promo_resource_service.h
+++ b/chrome/browser/web_resource/promo_resource_service.h
@@ -6,6 +6,9 @@
 #define CHROME_BROWSER_WEB_RESOURCE_PROMO_RESOURCE_SERVICE_H_
 #pragma once
 
+#include <string>
+
+#include "chrome/browser/platform_util.h"
 #include "chrome/browser/web_resource/web_resource_service.h"
 
 namespace PromoResourceServiceUtil {
@@ -29,7 +32,8 @@ class PrefService;
 class PromoResourceService
     : public WebResourceService {
  public:
-  static bool IsBuildTargeted(const std::string& channel, int builds_targeted);
+  static bool IsBuildTargeted(platform_util::Channel channel,
+                              int builds_targeted);
 
   static void RegisterPrefs(PrefService* local_state);
 
@@ -80,7 +84,7 @@ class PromoResourceService
   void ScheduleNotificationOnInit();
 
   // Overrides the current Chrome release channel for testing purposes.
-  void set_channel(const char* channel) { channel_ = channel; }
+  void set_channel(platform_util::Channel channel) { channel_ = channel; }
 
   virtual void Unpack(const DictionaryValue& parsed_json);
 
@@ -187,7 +191,7 @@ class PromoResourceService
   DictionaryValue* web_resource_cache_;
 
   // Overrides the current Chrome release channel for testing purposes.
-  const char* channel_;
+  platform_util::Channel channel_;
 
   DISALLOW_COPY_AND_ASSIGN(PromoResourceService);
 };
diff --git a/chrome/browser/web_resource/promo_resource_service_unittest.cc b/chrome/browser/web_resource/promo_resource_service_unittest.cc
index a9c0e22..d149b36 100644
--- a/chrome/browser/web_resource/promo_resource_service_unittest.cc
+++ b/chrome/browser/web_resource/promo_resource_service_unittest.cc
@@ -167,7 +167,7 @@ TEST_F(PromoResourceServiceTest, UnpackPromoSignal) {
 }
 
 TEST_F(PromoResourceServiceTest, UnpackWebStoreSignal) {
-  web_resource_service_->set_channel("dev");
+  web_resource_service_->set_channel(platform_util::CHANNEL_DEV);
 
   // Set up start and end dates and promo line in a Dictionary as if parsed
   // from the service.
@@ -205,35 +205,39 @@ TEST_F(PromoResourceServiceTest, UnpackWebStoreSignal) {
 
 TEST_F(PromoResourceServiceTest, IsBuildTargeted) {
   // canary
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("canary", 1));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("canary", 3));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("canary", 7));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("canary", 15));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("canary", 8));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("canary", 11));
+  const platform_util::Channel canary = platform_util::CHANNEL_CANARY;
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(canary, 1));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(canary, 3));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(canary, 7));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(canary, 15));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(canary, 8));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(canary, 11));
 
   // dev
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("dev", 1));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("dev", 3));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("dev", 7));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("dev", 15));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("dev", 8));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("dev", 11));
+  const platform_util::Channel dev = platform_util::CHANNEL_DEV;
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(dev, 1));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(dev, 3));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(dev, 7));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(dev, 15));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(dev, 8));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(dev, 11));
 
   // beta
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("beta", 1));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("beta", 3));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("beta", 7));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("beta", 15));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("beta", 8));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("beta", 11));
+  const platform_util::Channel beta = platform_util::CHANNEL_BETA;
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(beta, 1));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(beta, 3));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(beta, 7));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(beta, 15));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(beta, 8));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(beta, 11));
 
   // stable
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("", 1));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("", 3));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("", 7));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("", 15));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("", 8));
-  EXPECT_FALSE(PromoResourceService::IsBuildTargeted("", 11));
-  EXPECT_TRUE(PromoResourceService::IsBuildTargeted("", 12));
+  const platform_util::Channel stable = platform_util::CHANNEL_STABLE;
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(stable, 1));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(stable, 3));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(stable, 7));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(stable, 15));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(stable, 8));
+  EXPECT_FALSE(PromoResourceService::IsBuildTargeted(stable, 11));
+  EXPECT_TRUE(PromoResourceService::IsBuildTargeted(stable, 12));
 }