Fix regression preventing accelerated upgrade notifications for unstable Chrome.

BUG=837562

Change-Id: I0f78d94137ad8875e18fe4ce44e6b03ddc9bcb97
Reviewed-on: https://chromium-review.googlesource.com/1033738Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555328}

chrome/browser/upgrade_detector_impl.cc

@@ -102,14 +102,15 @@ base::TimeDelta GetCheckForUpgradeDelay() {
   return kCheckForUpgrade;
 }
 
-#if !defined(OS_WIN) || defined(GOOGLE_CHROME_BUILD)
-// Return true if the current build is one of the unstable channels.
+// Return true if the browser is updating on the dev or canary channels.
 bool IsUnstableChannel() {
-  version_info::Channel channel = chrome::GetChannel();
+  // Unbranded (Chromium) builds are on the UNKNOWN channel, so check explicitly
+  // for the Google Chrome channels that are considered "unstable". This ensures
+  // that Chromium builds get the default behavior.
+  const version_info::Channel channel = chrome::GetChannel();
   return channel == version_info::Channel::DEV ||
          channel == version_info::Channel::CANARY;
 }
-#endif  // !defined(OS_WIN) || defined(GOOGLE_CHROME_BUILD)
 
 // Gets the currently installed version. On Windows, if |critical_update| is not
 // NULL, also retrieves the critical update version info if available.
@@ -158,7 +159,6 @@ UpgradeDetectorImpl::UpgradeDetectorImpl(const base::TickClock* tick_clock)
            base::MayBlock()})),
       detect_upgrade_timer_(this->tick_clock()),
       upgrade_notification_timer_(this->tick_clock()),
-      is_unstable_channel_(false),
       is_auto_update_enabled_(true),
       simulating_outdated_(SimulatingOutdated()),
       is_testing_(simulating_outdated_ || IsTesting()),
@@ -225,11 +225,9 @@ UpgradeDetectorImpl::UpgradeDetectorImpl(const base::TickClock* tick_clock)
     variations_service->AddObserver(this);
 
 #if defined(OS_WIN)
-// Only enable upgrade notifications for Google Chrome builds. Chromium has no
-// upgrade channel.
+// Only enable upgrade notifications for Google Chrome builds. Chromium does not
+// use an auto-updater.
 #if defined(GOOGLE_CHROME_BUILD)
-  // Check whether the build is an unstable channel before starting the timer.
-  is_unstable_channel_ = IsUnstableChannel();
   // There might be a policy/enterprise environment preventing updates, so
   // validate updatability and then call StartTimerForUpgradeCheck
   // appropriately. Skip this step if a past attempt has been made to enable
@@ -258,8 +256,6 @@ UpgradeDetectorImpl::UpgradeDetectorImpl(const base::TickClock* tick_clock)
 #else
   return;
 #endif
-  // Check whether the build is an unstable channel before starting the timer.
-  is_unstable_channel_ = IsUnstableChannel();
   StartTimerForUpgradeCheck();
 #endif  // defined(OS_WIN)
 }
@@ -349,12 +345,12 @@ void UpgradeDetectorImpl::InitializeThresholds() {
   // If elevated_threshold_ and high_threshold_ are present in |stages_|, they
   // must be equal.
   if (stages_.size() != 1) {
-    DCHECK(!is_unstable_channel_);
+    DCHECK(!IsUnstableChannel());
     DCHECK_EQ(stages_.size(), 3U);
     DCHECK_EQ(stages_[1].first, elevated_threshold_);
     DCHECK_EQ(stages_[0].first, high_threshold_);
   } else {
-    DCHECK(is_unstable_channel_);
+    DCHECK(IsUnstableChannel());
   }
 #endif  // DCHECK_IS_ON()
 }
@@ -395,7 +391,7 @@ void UpgradeDetectorImpl::DoInitializeThresholds() {
 
   // Canary and dev channels are extra special, and reach "low" annoyance after
   // one hour (one second in testing) and never advance beyond that.
-  if (is_unstable_channel_) {
+  if (IsUnstableChannel()) {
     low_threshold = is_testing_ ? base::TimeDelta::FromSeconds(1)
                                 : base::TimeDelta::FromHours(1);
     // High and elevated thresholds are not added to |stages_| on unstable

chrome/browser/upgrade_detector_impl.h

@@ -121,9 +121,6 @@ class UpgradeDetectorImpl : public UpgradeDetector,
   // schedule calls to NotifyUpgrade.
   base::OneShotTimer upgrade_notification_timer_;
 
-  // True if this build is a dev or canary channel build.
-  bool is_unstable_channel_;
-
   // True if auto update is turned on.
   bool is_auto_update_enabled_;
 

chrome/browser/upgrade_detector_impl_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/test/scoped_task_environment.h"
 #include "base/time/tick_clock.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "chrome/browser/upgrade_observer.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
@@ -20,6 +21,11 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_WIN)
+#include "chrome/install_static/install_modes.h"
+#include "chrome/install_static/test/scoped_install_details.h"
+#endif  // defined(OS_WIN)
+
 namespace {
 
 class TestUpgradeDetectorImpl : public UpgradeDetectorImpl {
@@ -296,6 +302,29 @@ TEST_F(UpgradeDetectorImplTest, TestPeriodChanges) {
             UpgradeDetector::UPGRADE_ANNOYANCE_NONE);
 }
 
+#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD)
+// Tests that the low threshold for unstable channels is less than that for
+// stable channels.
+TEST_F(UpgradeDetectorImplTest, TestUnstableChannelLowThreshold) {
+  // Grab the low threshold for stable channel.
+  base::TimeDelta default_low_threshold;
+  {
+    TestUpgradeDetectorImpl upgrade_detector(GetMockTickClock());
+    default_low_threshold = upgrade_detector.GetThresholdForLevel(
+        UpgradeDetector::UPGRADE_ANNOYANCE_LOW);
+  }
+
+  // Now make sure that the low threshold for canary is smaller.
+  install_static::ScopedInstallDetails install_details(
+      false, install_static::CANARY_INDEX);
+
+  TestUpgradeDetectorImpl upgrade_detector(GetMockTickClock());
+  EXPECT_LT(upgrade_detector.GetThresholdForLevel(
+                UpgradeDetector::UPGRADE_ANNOYANCE_LOW),
+            default_low_threshold);
+}
+#endif
+
 // Appends the time and stage from detector to |notifications|.
 ACTION_P2(AppendTicksAndStage, detector, notifications) {
   notifications->emplace_back(detector->tick_clock()->NowTicks(),