Make UpgradeDetector listen to VariationsService changes.

chrome/browser/upgrade_detector_impl.cc

Index: chrome/browser/upgrade_detector_impl.cc
===================================================================
--- chrome/browser/upgrade_detector_impl.cc	(revision 286992)
+++ chrome/browser/upgrade_detector_impl.cc	(working copy)
@@ -284,6 +284,11 @@
                                      start_upgrade_check_timer_task,
                                      &is_unstable_channel_));
 #endif
+
+  // Register for experiment notifications. Note that since this class is a
+  // singleton, it does not need to unregister for notifications when destroyed,
+  // since it outlines the VariationsService.

[MAD, 2014/08/01 17:41:30]

outlines -> outlives

[Alexei Svitkine (slow), 2014/08/04 19:43:10]

Done.

+  g_browser_process->variations_service()->AddObserver(this);
 }
 
 UpgradeDetectorImpl::~UpgradeDetectorImpl() {
@@ -346,6 +351,24 @@
       this, &UpgradeDetectorImpl::CheckForUpgrade);
 }
 
+void UpgradeDetectorImpl::StartUpgradeNotificationTimer() {
+  // The timer may already be running (e.g. due to both a software upgrade and
+  // experiment updates being available).
+  if (upgrade_notification_timer_.IsRunning())
+    return;
+
+  upgrade_detected_time_ = base::Time::Now();
+
+  // Start the repeating timer for notifying the user after a certain period.
+  // The called function will eventually figure out that enough time has passed
+  // and stop the timer.
+  const int cycle_time_ms = IsTesting() ?
+      kNotifyCycleTimeForTestingMs : kNotifyCycleTimeMs;
+  upgrade_notification_timer_.Start(FROM_HERE,
+      base::TimeDelta::FromMilliseconds(cycle_time_ms),
+      this, &UpgradeDetectorImpl::NotifyOnUpgrade);
+}
+
 void UpgradeDetectorImpl::CheckForUpgrade() {
   // Interrupt any (unlikely) unfinished execution of DetectUpgradeTask, or at
   // least prevent the callback from being executed, because we will potentially
@@ -414,30 +437,28 @@
   return simulate_outdated;
 }
 
+void UpgradeDetectorImpl::OnExperimentChangesDetected(Severity severity) {
+  best_effort_experiment_updates_available_ = (severity == BEST_EFFORT);
+  critical_experiment_updates_available_ = (severity == CRITICAL);
+  StartUpgradeNotificationTimer();
+}
+
 void UpgradeDetectorImpl::UpgradeDetected(UpgradeAvailable upgrade_available) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   upgrade_available_ = upgrade_available;
 
   // Stop the recurring timer (that is checking for changes).
   detect_upgrade_timer_.Stop();
+  critical_update_acknowledged_ = false;
 
-  NotifyUpgradeDetected();
-
-  // Start the repeating timer for notifying the user after a certain period.
-  // The called function will eventually figure out that enough time has passed
-  // and stop the timer.
-  int cycle_time = IsTesting() ?
-      kNotifyCycleTimeForTestingMs : kNotifyCycleTimeMs;
-  upgrade_notification_timer_.Start(FROM_HERE,
-      base::TimeDelta::FromMilliseconds(cycle_time),
-      this, &UpgradeDetectorImpl::NotifyOnUpgrade);
+  StartUpgradeNotificationTimer();
 }
 
-void UpgradeDetectorImpl::NotifyOnUpgrade() {
-  const base::TimeDelta time_passed =
-      base::Time::Now() - upgrade_detected_time();
-
-  bool is_critical_or_outdated = upgrade_available_ > UPGRADE_AVAILABLE_REGULAR;
+void UpgradeDetectorImpl::NotifyOnUpgradeWithTimePassed(
+    base::TimeDelta time_passed) {
+  const bool is_critical_or_outdated =
+      upgrade_available_ > UPGRADE_AVAILABLE_REGULAR ||
+      critical_experiment_updates_available_;
   if (is_unstable_channel_) {
     // There's only one threat level for unstable channels like dev and
     // canary, and it hits after one hour. During testing, it hits after one
@@ -487,6 +508,12 @@
   NotifyUpgradeRecommended();
 }
 
+void UpgradeDetectorImpl::NotifyOnUpgrade() {
+  const base::TimeDelta time_passed =
+      base::Time::Now() - upgrade_detected_time_;

[sky, 2014/08/04 18:24:07]

Don't you want timeticks?

[Alexei Svitkine (slow), 2014/08/04 19:43:11]

This CL doesn't actually change the logic around this (just refactors it).

I agree that time ticks makes more sense here. But such a change should be in a
separate CL, so I'll do it after this lands. I filed http://crbug.com/400472 for
that fix.

+  NotifyOnUpgradeWithTimePassed(time_passed);
+}
+
 // static
 UpgradeDetectorImpl* UpgradeDetectorImpl::GetInstance() {
   return Singleton<UpgradeDetectorImpl>::get();