Make UpgradeDetector listen to VariationsService changes.

chrome/browser/upgrade_detector.h

Index: chrome/browser/upgrade_detector.h
===================================================================
--- chrome/browser/upgrade_detector.h	(revision 286992)
+++ chrome/browser/upgrade_detector.h	(working copy)
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UPGRADE_DETECTOR_H_
 
 #include "base/timer/timer.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/idle.h"
 #include "ui/gfx/image/image.h"
 
@@ -65,11 +66,6 @@
     return critical_update_acknowledged_;
   }
 
-  // When the last upgrade was detected.
-  const base::Time& upgrade_detected_time() const {
-    return upgrade_detected_time_;
-  }
-
   // Retrieves the right icon ID based on the degree of severity (see
   // UpgradeNotificationAnnoyanceLevel, each level has an an accompanying icon
   // to go with it) to display within the wrench menu.
@@ -82,16 +78,19 @@
  protected:
   UpgradeDetector();
 
-  // Sends out UPGRADE_DETECTED notification and record upgrade_detected_time_.
-  void NotifyUpgradeDetected();
-
   // Sends out UPGRADE_RECOMMENDED notification and set notify_upgrade_.
   void NotifyUpgradeRecommended();
 
+  // Triggers a critical update, which starts a timer that checks the machine
+  // idle state. Protected and virtual so that it could overridden by tests.

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

could "be" overridden.

+  virtual void TriggerCriticalUpdate();

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

How about an observer instead of this? The observer could handle both the
TriggerNotification and TriggerCriticalNotification. You could install a default
observer that converts the observer function into a
content::NotificationService::current()->Notify. At a later day you could remove
the content::NotificationService::current()->Notify and make consumers install
an observer.

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

I'm not sure it makes sense in this case. The TriggerCriticalUpdate()
implementation does the following:

  - Start a repeating timer that checks if the system is idle
  - The timer task checks if the system is idle and if it is, calls
IdleCallback()
  - IdleCallback() then checks idle state and other info (e.g. incognito session
active) to see if it's an appropriate time to trigger the notification to show
the countdown UI ... or if computer is locked, will actually restart Chrome.

So basically, the implementation of this method is not "tell observers that a
critical update is ready" ... but rather "do some things behind the scenes and
then later once we determined its a good time, tell observers that a critical
update is ready". Thus converting this specific method to an observer method
doesn't make sense.

(I understand your motivation for suggesting this - i.e. to make it easy to
eventually remove the use of notifications from this code - but I think such a
change would be orthogonal to me adding this virtual method and thus doesn't
make sense to include in this CL.)

+
   void set_upgrade_notification_stage(UpgradeNotificationAnnoyanceLevel stage) {
     upgrade_notification_stage_ = stage;
   }
 
+  // Whether any software updates are available (experiment updates are tracked
+  // separately via additional member variables below).
   enum UpgradeAvailable {
     // If no update is available and current install is recent enough.
     UPGRADE_AVAILABLE_NONE,
@@ -108,6 +107,12 @@
     UPGRADE_NEEDED_OUTDATED_INSTALL_NO_AU,
   } upgrade_available_;
 
+  // Whether "best effort" experiment updates are available.

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

Style guide says no protected members.

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

Indeed, done. There were existing protected members here as well, so I moved
them too. (Result is introducing some protected methods, but that's the right
thing to do according to the style guide.)

+  bool best_effort_experiment_updates_available_;
+
+  // Whether "critical" experiment updates are available.
+  bool critical_experiment_updates_available_;
+
   // Whether the user has acknowledged the critical update.
   bool critical_update_acknowledged_;
 
@@ -119,8 +124,8 @@
   // input events since the specified time.
   void IdleCallback(IdleState state);
 
-  // When the upgrade was detected.
-  base::Time upgrade_detected_time_;
+  // Triggers a global notification of the specified |type|.
+  void TriggerNotification(chrome::NotificationType type);
 
   // A timer to check to see if we've been idle for long enough to show the
   // critical warning. Should only be set if |upgrade_available_| is