Make UpgradeDetector listen to VariationsService changes.

chrome/browser/upgrade_detector.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_UPGRADE_DETECTOR_H_
 #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"
 
 class PrefRegistrySimple;
 
 ///////////////////////////////////////////////////////////////////////////////
 // UpgradeDetector
 //
 // This class is a singleton class that monitors when an upgrade happens in the
 // background. We basically ask Omaha what it thinks the latest version is and
@@ skipping 39 matching lines @@
   // so we don't need to complain about it for now.
   void acknowledge_critical_update() {
     critical_update_acknowledged_ = true;
   }
 
   // Whether the user has acknowledged the critical update.
   bool critical_update_acknowledged() const {
     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.
   int GetIconResourceID();
 
   UpgradeNotificationAnnoyanceLevel upgrade_notification_stage() const {
     return upgrade_notification_stage_;
   }
 
  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,
     // If a regular update is available.
     UPGRADE_AVAILABLE_REGULAR,
     // If a critical update to Chrome has been installed, such as a zero-day
     // fix.
     UPGRADE_AVAILABLE_CRITICAL,
     // If no update to Chrome has been installed for more than the recommended
     // time.
     UPGRADE_NEEDED_OUTDATED_INSTALL,
     // If no update to Chrome has been installed for more than the recommended
     // time AND auto-update is turned off.
     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_;
 
  private:
   // Initiates an Idle check. See IdleCallback below.
   void CheckIdle();
 
   // The callback for the IdleCheck. Tells us whether Chrome has received any
   // 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
   // UPGRADE_AVAILABLE_CRITICAL.
   base::RepeatingTimer<UpgradeDetector> idle_check_timer_;
 
   // The stage at which the annoyance level for upgrade notifications is at.
   UpgradeNotificationAnnoyanceLevel upgrade_notification_stage_;
 
   // Whether we have waited long enough after detecting an upgrade (to see
   // is we should start nagging about upgrading).
   bool notify_upgrade_;
 
   DISALLOW_COPY_AND_ASSIGN(UpgradeDetector);
 };
 
 #endif  // CHROME_BROWSER_UPGRADE_DETECTOR_H_