Make UpgradeDetector detect updates via observing the BuildState.

This change makes UpgradeDetector an observer of the process's
BuildState, removing its own detection. It instead creates an instance
of either an InstalledVersionPoller (for desktop Chrome) or an
InstalledVersionUpdater (for Chrome OS) to do the actual detection work.

BUG=1043624

Change-Id: Ib04b0f588a3d035f2ce0300561ed86d9024c7b3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2017352
Commit-Queue: Greg Thompson <grt@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738381}

chrome/browser/browser_process.h

@@ -24,6 +24,7 @@
 
 class BackgroundModeManager;
 class BrowserProcessPlatformPart;
+class BuildState;
 class DownloadRequestLimiter;
 class DownloadStatusUpdater;
 class GpuModeManager;
@@ -266,6 +267,8 @@ class BrowserProcess {
   virtual resource_coordinator::ResourceCoordinatorParts*
   resource_coordinator_parts() = 0;
 
+  virtual BuildState* GetBuildState() = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserProcess);
 };

chrome/browser/browser_process_impl.cc

@@ -893,6 +893,16 @@ BrowserProcessImpl::resource_coordinator_parts() {
   return resource_coordinator_parts_.get();
 }
 
+BuildState* BrowserProcessImpl::GetBuildState() {
+#if !defined(OS_ANDROID)
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return &build_state_;
+#else
+  NOTIMPLEMENTED();
+  return nullptr;
+#endif
+}
+
 // static
 void BrowserProcessImpl::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(prefs::kDefaultBrowserSettingEnabled,

chrome/browser/browser_process_impl.h

@@ -35,6 +35,10 @@
 #include "services/network/public/cpp/network_quality_tracker.h"
 #include "services/network/public/mojom/network_service.mojom-forward.h"
 
+#if !defined(OS_ANDROID)
+#include "chrome/browser/upgrade_detector/build_state.h"
+#endif
+
 class BatteryMetrics;
 class ChromeFeatureListCreator;
 class ChromeMetricsServicesManagerClient;
@@ -192,6 +196,8 @@ class BrowserProcessImpl : public BrowserProcess,
   resource_coordinator::ResourceCoordinatorParts* resource_coordinator_parts()
       override;
 
+  BuildState* GetBuildState() override;
+
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
  private:
@@ -400,6 +406,8 @@ class BrowserProcessImpl : public BrowserProcess,
 #if !defined(OS_ANDROID)
   // Called to signal the process' main message loop to exit.
   base::OnceClosure quit_closure_;
+
+  BuildState build_state_;
 #endif
 
   SEQUENCE_CHECKER(sequence_checker_);

chrome/browser/chrome_browser_main.cc

@@ -204,6 +204,7 @@
 #include "chrome/browser/resource_coordinator/tab_activity_watcher.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/upgrade_detector/upgrade_detector.h"
 #include "chrome/browser/usb/web_usb_detector.h"
 #include "ui/base/l10n/l10n_util.h"
 #endif  // defined(OS_ANDROID)
@@ -862,6 +863,12 @@ void ChromeBrowserMainParts::PreMainMessageLoopStart() {
 void ChromeBrowserMainParts::PostMainMessageLoopStart() {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::PostMainMessageLoopStart");
 
+#if !defined(OS_ANDROID)
+  // Initialize the upgrade detector here after ChromeBrowserMainPartsChromeos
+  // has had a chance to connect the DBus services.
+  UpgradeDetector::GetInstance()->Init();
+#endif
+
   ThreadProfiler::SetMainThreadTaskRunner(base::ThreadTaskRunnerHandle::Get());
 
   system_monitor_ = performance_monitor::SystemMonitor::Create();
@@ -1861,6 +1868,10 @@ void ChromeBrowserMainParts::PostMainMessageLoopRun() {
   // Android specific MessageLoop
   NOTREACHED();
 #else
+  // Shutdown the UpgradeDetector here before ChromeBrowserMainPartsChromeos
+  // disconnects DBus services in its PostDestroyThreads.
+  UpgradeDetector::GetInstance()->Shutdown();
+
   // Start watching for jank during shutdown. It gets disarmed when
   // |shutdown_watcher_| object is destructed.
   shutdown_watcher_->Arm(base::TimeDelta::FromSeconds(300));

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -129,7 +129,6 @@
 #include "chrome/browser/ui/ash/assistant/assistant_state_client.h"
 #include "chrome/browser/ui/ash/keyboard/chrome_keyboard_controller_client.h"
 #include "chrome/browser/ui/webui/chromeos/login/discover/discover_manager.h"
-#include "chrome/browser/upgrade_detector/upgrade_detector_chromeos.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
@@ -356,10 +355,6 @@ class DBusServices {
 
     NetworkHandler::Initialize();
 
-    // Likewise, initialize the upgrade detector for Chrome OS. The upgrade
-    // detector starts to monitor changes from the update engine.
-    UpgradeDetectorChromeos::GetInstance()->Init();
-
     DeviceSettingsService::Get()->SetSessionManager(
         SessionManagerClient::Get(),
         OwnerSettingsServiceChromeOSFactory::GetInstance()->GetOwnerKeyUtil());
@@ -1023,11 +1018,6 @@ void ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() {
   if (pre_profile_init_called_)
     KioskModeIdleAppNameNotification::Shutdown();
 
-  // Shutdown the upgrade detector for Chrome OS. The upgrade detector
-  // stops monitoring changes from the update engine.
-  if (UpgradeDetectorChromeos::GetInstance())
-    UpgradeDetectorChromeos::GetInstance()->Shutdown();
-
   // Tell DeviceSettingsService to stop talking to session_manager. Do not
   // shutdown DeviceSettingsService yet, it might still be accessed by
   // BrowserPolicyConnector (owned by g_browser_process).

chrome/browser/upgrade_detector/upgrade_detector.cc

@@ -38,17 +38,7 @@ void UpgradeDetector::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(prefs::kAttemptedToEnableAutoupdate, false);
 }
 
-UpgradeDetector::UpgradeDetector(const base::Clock* clock,
-                                 const base::TickClock* tick_clock)
-    : clock_(clock),
-      tick_clock_(tick_clock),
-      upgrade_available_(UPGRADE_AVAILABLE_NONE),
-      best_effort_experiment_updates_available_(false),
-      critical_experiment_updates_available_(false),
-      critical_update_acknowledged_(false),
-      idle_check_timer_(tick_clock_),
-      upgrade_notification_stage_(UPGRADE_ANNOYANCE_NONE),
-      notify_upgrade_(false) {
+void UpgradeDetector::Init() {
   // Not all tests provide a PrefService for local_state().
   PrefService* local_state = g_browser_process->local_state();
   if (local_state) {
@@ -62,7 +52,27 @@ UpgradeDetector::UpgradeDetector(const base::Clock* clock,
   }
 }
 
-UpgradeDetector::~UpgradeDetector() {}
+void UpgradeDetector::Shutdown() {
+  idle_check_timer_.Stop();
+  pref_change_registrar_.RemoveAll();
+}
+
+UpgradeDetector::UpgradeDetector(const base::Clock* clock,
+                                 const base::TickClock* tick_clock)
+    : clock_(clock),
+      tick_clock_(tick_clock),
+      upgrade_available_(UPGRADE_AVAILABLE_NONE),
+      best_effort_experiment_updates_available_(false),
+      critical_experiment_updates_available_(false),
+      critical_update_acknowledged_(false),
+      idle_check_timer_(tick_clock_),
+      upgrade_notification_stage_(UPGRADE_ANNOYANCE_NONE),
+      notify_upgrade_(false) {}
+
+UpgradeDetector::~UpgradeDetector() {
+  // Ensure that Shutdown() was called.
+  DCHECK(pref_change_registrar_.IsEmpty());
+}
 
 void UpgradeDetector::NotifyOutdatedInstall() {
   for (auto& observer : observer_list_)

chrome/browser/upgrade_detector/upgrade_detector.h

@@ -57,6 +57,9 @@ class UpgradeDetector {
 
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
+  virtual void Init();
+  virtual void Shutdown();
+
   // Returns the time at which an available upgrade was detected.
   base::Time upgrade_detected_time() const { return upgrade_detected_time_; }
 

chrome/browser/upgrade_detector/upgrade_detector_chromeos.cc

@@ -7,21 +7,18 @@
 #include <stdint.h>
 
 #include <algorithm>
-#include <utility>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
-#include "base/macros.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/no_destructor.h"
 #include "base/rand_util.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/time/default_tick_clock.h"
-#include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/upgrade_detector/build_state.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/update_engine_client.h"
@@ -49,69 +46,6 @@ constexpr base::TimeDelta kDefaultHighThreshold = base::TimeDelta::FromDays(7);
 constexpr base::TimeDelta kDefaultHeadsUpPeriod =
     base::TimeDelta::FromDays(3);  // 3 days.
 
-// The reason of the rollback used in the UpgradeDetector.RollbackReason
-// histogram.
-enum class RollbackReason {
-  kToMoreStableChannel = 0,
-  kEnterpriseRollback = 1,
-  kMaxValue = kEnterpriseRollback,
-};
-
-class ChannelsRequester {
- public:
-  typedef base::OnceCallback<void(std::string, std::string)>
-      OnChannelsReceivedCallback;
-
-  static void Begin(OnChannelsReceivedCallback callback) {
-    ChannelsRequester* instance = new ChannelsRequester(std::move(callback));
-    UpdateEngineClient* client =
-        DBusThreadManager::Get()->GetUpdateEngineClient();
-    // base::Unretained is safe because this instance keeps itself alive until
-    // both callbacks have run.
-    // TODO: use BindOnce here; see https://crbug.com/825993.
-    client->GetChannel(true /* get_current_channel */,
-                       base::Bind(&ChannelsRequester::SetCurrentChannel,
-                                  base::Unretained(instance)));
-    client->GetChannel(false /* get_current_channel */,
-                       base::Bind(&ChannelsRequester::SetTargetChannel,
-                                  base::Unretained(instance)));
-  }
-
- private:
-  explicit ChannelsRequester(OnChannelsReceivedCallback callback)
-      : callback_(std::move(callback)) {}
-
-  ~ChannelsRequester() = default;
-
-  void SetCurrentChannel(const std::string& current_channel) {
-    DCHECK(!current_channel.empty());
-    current_channel_ = current_channel;
-    TriggerCallbackAndDieIfReady();
-  }
-
-  void SetTargetChannel(const std::string& target_channel) {
-    DCHECK(!target_channel.empty());
-    target_channel_ = target_channel;
-    TriggerCallbackAndDieIfReady();
-  }
-
-  void TriggerCallbackAndDieIfReady() {
-    if (current_channel_.empty() || target_channel_.empty())
-      return;
-    if (!callback_.is_null()) {
-      std::move(callback_).Run(std::move(current_channel_),
-                               std::move(target_channel_));
-    }
-    delete this;
-  }
-
-  OnChannelsReceivedCallback callback_;
-  std::string current_channel_;
-  std::string target_channel_;
-
-  DISALLOW_COPY_AND_ASSIGN(ChannelsRequester);
-};
-
 }  // namespace
 
 UpgradeDetectorChromeos::UpgradeDetectorChromeos(
@@ -145,7 +79,11 @@ void UpgradeDetectorChromeos::RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 void UpgradeDetectorChromeos::Init() {
+  UpgradeDetector::Init();
   DBusThreadManager::Get()->GetUpdateEngineClient()->AddObserver(this);
+  auto* const build_state = g_browser_process->GetBuildState();
+  build_state->AddObserver(this);
+  installed_version_updater_.emplace(build_state);
   initialized_ = true;
 }
 
@@ -153,10 +91,12 @@ void UpgradeDetectorChromeos::Shutdown() {
   // Init() may not be called from tests.
   if (!initialized_)
     return;
+  installed_version_updater_.reset();
+  g_browser_process->GetBuildState()->RemoveObserver(this);
   DBusThreadManager::Get()->GetUpdateEngineClient()->RemoveObserver(this);
-  // Discard an outstanding request to a ChannelsRequester.
   weak_factory_.InvalidateWeakPtrs();
   upgrade_notification_timer_.Stop();
+  UpgradeDetector::Shutdown();
   initialized_ = false;
 }
 
@@ -168,6 +108,19 @@ base::Time UpgradeDetectorChromeos::GetHighAnnoyanceDeadline() {
   return high_deadline_;
 }
 
+void UpgradeDetectorChromeos::OnUpdate(const BuildState* build_state) {
+  if (upgrade_detected_time().is_null()) {
+    set_upgrade_detected_time(clock()->Now());
+    CalculateDeadlines();
+  }
+
+  set_is_rollback(build_state->update_type() ==
+                  BuildState::UpdateType::kEnterpriseRollback);
+  set_is_factory_reset_required(build_state->update_type() ==
+                                BuildState::UpdateType::kChannelSwitchRollback);
+  NotifyOnUpgrade();
+}
+
 // static
 base::TimeDelta UpgradeDetectorChromeos::GetRelaunchHeadsUpPeriod() {
   // Not all tests provide a PrefService for local_state().
@@ -259,26 +212,6 @@ void UpgradeDetectorChromeos::OnRelaunchPrefChanged() {
 void UpgradeDetectorChromeos::UpdateStatusChanged(
     const update_engine::StatusResult& status) {
   if (status.current_operation() ==
-      update_engine::Operation::UPDATED_NEED_REBOOT) {
-    if (upgrade_detected_time().is_null()) {
-      set_upgrade_detected_time(clock()->Now());
-      CalculateDeadlines();
-    }
-
-    if (status.is_enterprise_rollback()) {
-      // Powerwash will be required, determine what kind of notification to show
-      // based on the channel.
-      ChannelsRequester::Begin(
-          base::BindOnce(&UpgradeDetectorChromeos::OnChannelsReceived,
-                         weak_factory_.GetWeakPtr()));
-    } else {
-      // Not going to an earlier version, no powerwash or rollback message is
-      // required.
-      set_is_rollback(false);
-      set_is_factory_reset_required(false);
-      NotifyOnUpgrade();
-    }
-  } else if (status.current_operation() ==
       update_engine::Operation::NEED_PERMISSION_TO_UPDATE) {
     // Update engine broadcasts this state only when update is available but
     // downloading over cellular connection requires user's agreement.
@@ -350,33 +283,6 @@ void UpgradeDetectorChromeos::NotifyOnUpgrade() {
   }
 }
 
-void UpgradeDetectorChromeos::OnChannelsReceived(std::string current_channel,
-                                                 std::string target_channel) {
-  bool to_more_stable_channel = UpdateEngineClient::IsTargetChannelMoreStable(
-      current_channel, target_channel);
-  // As current update engine status is UPDATE_STATUS_UPDATED_NEED_REBOOT,
-  // if target channel is more stable than current channel, powerwash
-  // will be performed after reboot.
-  set_is_factory_reset_required(to_more_stable_channel);
-  // If we are doing a channel switch, we're currently showing the channel
-  // switch message instead of the rollback message (even if the channel switch
-  // was initiated by the admin).
-  // TODO(crbug.com/864672): Fix this by getting is_rollback from update engine.
-  set_is_rollback(!to_more_stable_channel);
-
-  UMA_HISTOGRAM_ENUMERATION("UpgradeDetector.RollbackReason",
-                            to_more_stable_channel
-                                ? RollbackReason::kToMoreStableChannel
-                                : RollbackReason::kEnterpriseRollback);
-  LOG(WARNING) << "Device is rolling back, will require powerwash. Reason: "
-               << to_more_stable_channel
-               << ", current_channel: " << current_channel
-               << ", target_channel: " << target_channel;
-
-  // ChromeOS shows upgrade arrow once the upgrade becomes available.
-  NotifyOnUpgrade();
-}
-
 // static
 UpgradeDetectorChromeos* UpgradeDetectorChromeos::GetInstance() {
   static base::NoDestructor<UpgradeDetectorChromeos> instance(

chrome/browser/upgrade_detector/upgrade_detector_chromeos.h

@@ -5,13 +5,13 @@
 #ifndef CHROME_BROWSER_UPGRADE_DETECTOR_UPGRADE_DETECTOR_CHROMEOS_H_
 #define CHROME_BROWSER_UPGRADE_DETECTOR_UPGRADE_DETECTOR_CHROMEOS_H_
 
-#include <string>
-
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "chrome/browser/upgrade_detector/build_state_observer.h"
+#include "chrome/browser/upgrade_detector/installed_version_updater_chromeos.h"
 #include "chrome/browser/upgrade_detector/upgrade_detector.h"
 #include "chromeos/dbus/update_engine_client.h"
 
@@ -24,6 +24,7 @@ class TickClock;
 }  // namespace base
 
 class UpgradeDetectorChromeos : public UpgradeDetector,
+                                public BuildStateObserver,
                                 public chromeos::UpdateEngineClient::Observer {
  public:
   ~UpgradeDetectorChromeos() override;
@@ -33,18 +34,15 @@ class UpgradeDetectorChromeos : public UpgradeDetector,
 
   static UpgradeDetectorChromeos* GetInstance();
 
-  // Initializes the object. Starts observing changes from the update
-  // engine.
-  void Init();
-
-  // Shuts down the object. Stops observing observe changes from the
-  // update engine.
-  void Shutdown();
-
   // UpgradeDetector:
+  void Init() override;
+  void Shutdown() override;
   base::TimeDelta GetHighAnnoyanceLevelDelta() override;
   base::Time GetHighAnnoyanceDeadline() override;
 
+  // BuildStateObserver:
+  void OnUpdate(const BuildState* build_state) override;
+
  protected:
   UpgradeDetectorChromeos(const base::Clock* clock,
                           const base::TickClock* tick_clock);
@@ -88,8 +86,7 @@ class UpgradeDetectorChromeos : public UpgradeDetector,
   // user that a new version is available.
   void NotifyOnUpgrade();
 
-  void OnChannelsReceived(std::string current_channel,
-                          std::string target_channel);
+  base::Optional<InstalledVersionUpdater> installed_version_updater_;
 
   // The time when elevated annoyance deadline is reached.
   base::Time elevated_deadline_;

chrome/browser/upgrade_detector/upgrade_detector_chromeos_unittest.cc

@@ -173,6 +173,8 @@ TEST_F(UpgradeDetectorChromeosTest, PolicyNotEnabled) {
   EXPECT_CALL(mock_observer, OnUpgradeRecommended());
 
   NotifyUpdateReadyToInstall();
+
+  upgrade_detector.Shutdown();
 }
 
 TEST_F(UpgradeDetectorChromeosTest, TestHighAnnoyanceDeadline) {

chrome/browser/upgrade_detector/upgrade_detector_impl.h

@@ -7,14 +7,13 @@
 
 #include <array>
 
-#include "base/callback.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/timer/timer.h"
 #include "base/version.h"
-#include "build/build_config.h"
+#include "chrome/browser/upgrade_detector/build_state_observer.h"
+#include "chrome/browser/upgrade_detector/installed_version_poller.h"
 #include "chrome/browser/upgrade_detector/upgrade_detector.h"
 #include "components/variations/service/variations_service.h"
 
@@ -22,17 +21,14 @@ namespace base {
 class Clock;
 template <typename T>
 class NoDestructor;
-class SequencedTaskRunner;
-class TaskRunner;
 class TickClock;
 }  // namespace base
 
 // This class contains the non-CrOS desktop implementation of the detector.
 class UpgradeDetectorImpl : public UpgradeDetector,
+                            public BuildStateObserver,
                             public variations::VariationsService::Observer {
  public:
-  ~UpgradeDetectorImpl() override;
-
   // Returns the currently installed Chrome version, which may be newer than the
   // one currently running. Not supported on Android, iOS or ChromeOS. Must be
   // run on a thread where I/O operations are allowed.
@@ -42,12 +38,18 @@ class UpgradeDetectorImpl : public UpgradeDetector,
   static UpgradeDetectorImpl* GetInstance();
 
   // UpgradeDetector:
+  void Init() override;
+  void Shutdown() override;
   base::TimeDelta GetHighAnnoyanceLevelDelta() override;
   base::Time GetHighAnnoyanceDeadline() override;
 
+  // BuildStateObserver:
+  void OnUpdate(const BuildState* build_state) override;
+
  protected:
   UpgradeDetectorImpl(const base::Clock* clock,
                       const base::TickClock* tick_clock);
+  ~UpgradeDetectorImpl() override;
 
   // Sends out a notification and starts a one shot timer to wait until
   // notifying the user.
@@ -74,9 +76,6 @@ class UpgradeDetectorImpl : public UpgradeDetector,
 
   friend class base::NoDestructor<UpgradeDetectorImpl>;
 
-  // A callback that receives the results of |DetectUpgradeTask|.
-  using UpgradeDetectedCallback = base::OnceCallback<void(UpgradeAvailable)>;
-
   // Returns the index of |level| in |stages_|.
   static LevelIndex AnnoyanceLevelToStagesIndex(
       UpgradeNotificationAnnoyanceLevel level);
@@ -88,18 +87,6 @@ class UpgradeDetectorImpl : public UpgradeDetector,
   // UpgradeDetector:
   void OnRelaunchNotificationPeriodPrefChanged() override;
 
-#if defined(OS_WIN)
-  // Receives the results of AreAutoupdatesEnabled and starts the upgrade check
-  // timer.
-  void OnAutoupdatesEnabledResult(bool auto_updates_enabled);
-#endif
-
-  // Start the timer that will call |CheckForUpgrade()|.
-  void StartTimerForUpgradeCheck();
-
-  // Launches a background task to check if we have the latest version.
-  void CheckForUpgrade();
-
   // Starts the upgrade notification timer that will check periodically whether
   // enough time has elapsed to update the severity (which maps to visual
   // badging) of the notification.
@@ -109,27 +96,20 @@ class UpgradeDetectorImpl : public UpgradeDetector,
   void InitializeThresholds();
   void DoInitializeThresholds();
 
-  // Returns true after calling UpgradeDetected if current install is outdated.
-  bool DetectOutdatedInstall();
+  void StartOutdatedBuildDetector();
+  void DetectOutdatedInstall();
 
   // The function that sends out a notification (after a certain time has
   // elapsed) that lets the rest of the UI know we should start notifying the
   // user that a new version is available.
   void NotifyOnUpgrade();
 
-  // Determines whether or not an update is available, posting |callback| with
-  // the result to |callback_task_runner| if so.
-  static void DetectUpgradeTask(
-      scoped_refptr<base::TaskRunner> callback_task_runner,
-      UpgradeDetectedCallback callback);
-
   SEQUENCE_CHECKER(sequence_checker_);
 
-  // A sequenced task runner on which blocking tasks run.
-  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
+  base::Optional<InstalledVersionPoller> installed_version_poller_;
 
-  // We periodically check to see if Chrome has been upgraded.
-  base::RepeatingTimer detect_upgrade_timer_;
+  // A timer used to periodically check if the build has become outdated.
+  base::OneShotTimer outdated_build_timer_;
 
   // A timer used to move through the various upgrade notification stages and
   // schedule calls to NotifyUpgrade.
@@ -152,9 +132,6 @@ class UpgradeDetectorImpl : public UpgradeDetector,
   // The date the binaries were built.
   base::Time build_date_;
 
-  // We use this factory to create callback tasks for UpgradeDetected. We pass
-  // the task to the actual upgrade detection code, which is in
-  // DetectUpgradeTask.
   base::WeakPtrFactory<UpgradeDetectorImpl> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(UpgradeDetectorImpl);

chrome/browser/upgrade_detector/upgrade_detector_impl_unittest.cc

@@ -5,20 +5,22 @@
 #include "chrome/browser/upgrade_detector/upgrade_detector_impl.h"
 
 #include <initializer_list>
+#include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/macros.h"
-#include "base/test/task_environment.h"
 #include "base/time/clock.h"
 #include "base/time/tick_clock.h"
 #include "base/values.h"
 #include "build/build_config.h"
+#include "chrome/browser/upgrade_detector/installed_version_poller.h"
 #include "chrome/browser/upgrade_detector/upgrade_observer.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "components/prefs/testing_pref_service.h"
+#include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -113,12 +115,19 @@ class UpgradeDetectorImplTest : public ::testing::Test {
  protected:
   UpgradeDetectorImplTest()
       : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME),
-        scoped_local_state_(TestingBrowserProcess::GetGlobal()) {
-    // Disable the detector's check to see if autoupdates are inabled.
+        scoped_local_state_(TestingBrowserProcess::GetGlobal()),
+        scoped_poller_disabler_(
+            InstalledVersionPoller::MakeScopedDisableForTesting()) {
+    // Disable the detector's check to see if autoupdates are enabled.
     // Without this, tests put the detector into an invalid state by detecting
     // upgrades before the detection task completes.
     scoped_local_state_.Get()->SetUserPref(prefs::kAttemptedToEnableAutoupdate,
                                            std::make_unique<base::Value>(true));
+    UpgradeDetector::GetInstance()->Init();
+  }
+
+  ~UpgradeDetectorImplTest() override {
+    UpgradeDetector::GetInstance()->Shutdown();
   }
 
   const base::Clock* GetMockClock() { return task_environment_.GetMockClock(); }
@@ -149,8 +158,9 @@ class UpgradeDetectorImplTest : public ::testing::Test {
   }
 
  private:
-  base::test::TaskEnvironment task_environment_;
+  content::BrowserTaskEnvironment task_environment_;
   ScopedTestingLocalState scoped_local_state_;
+  InstalledVersionPoller::ScopedDisableForTesting scoped_poller_disabler_;
 
   DISALLOW_COPY_AND_ASSIGN(UpgradeDetectorImplTest);
 };
@@ -158,6 +168,7 @@ class UpgradeDetectorImplTest : public ::testing::Test {
 TEST_F(UpgradeDetectorImplTest, VariationsChanges) {
   TestUpgradeDetectorImpl detector(GetMockClock(), GetMockTickClock());
   TestUpgradeNotificationListener notifications_listener(&detector);
+  detector.Init();
   EXPECT_FALSE(detector.notify_upgrade());
   EXPECT_EQ(0, notifications_listener.notification_count());
 
@@ -174,11 +185,13 @@ TEST_F(UpgradeDetectorImplTest, VariationsChanges) {
   // Execute tasks posted by |detector| referencing it while it's still in
   // scope.
   RunUntilIdle();
+  detector.Shutdown();
 }
 
 TEST_F(UpgradeDetectorImplTest, VariationsCriticalChanges) {
   TestUpgradeDetectorImpl detector(GetMockClock(), GetMockTickClock());
   TestUpgradeNotificationListener notifications_listener(&detector);
+  detector.Init();
   EXPECT_FALSE(detector.notify_upgrade());
   EXPECT_EQ(0, notifications_listener.notification_count());
 
@@ -195,6 +208,8 @@ TEST_F(UpgradeDetectorImplTest, VariationsCriticalChanges) {
   // Execute tasks posted by |detector| referencing it while it's still in
   // scope.
   RunUntilIdle();
+
+  detector.Shutdown();
 }
 
 // Tests that the proper notifications are sent for the expected stages as the
@@ -207,6 +222,7 @@ TEST_F(UpgradeDetectorImplTest, VariationsCriticalChanges) {
 TEST_F(UpgradeDetectorImplTest, TestPeriodChanges) {
   TestUpgradeDetectorImpl upgrade_detector(GetMockClock(), GetMockTickClock());
   ::testing::StrictMock<MockUpgradeObserver> mock_observer(&upgrade_detector);
+  upgrade_detector.Init();
 
   // Changing the period when no upgrade has been detected updates the
   // thresholds and nothing else.
@@ -362,6 +378,8 @@ TEST_F(UpgradeDetectorImplTest, TestPeriodChanges) {
   ::testing::Mock::VerifyAndClear(&mock_observer);
   EXPECT_EQ(upgrade_detector.upgrade_notification_stage(),
             UpgradeDetector::UPGRADE_ANNOYANCE_VERY_LOW);
+
+  upgrade_detector.Shutdown();
 }
 
 // Appends the time and stage from detector to |notifications|.
@@ -401,6 +419,7 @@ TEST_P(UpgradeDetectorImplTimerTest, TestNotificationTimer) {
 
   TestUpgradeDetectorImpl detector(GetMockClock(), GetMockTickClock());
   ::testing::StrictMock<MockUpgradeObserver> mock_observer(&detector);
+  detector.Init();
 
   // Cache the thresholds for the detector's annoyance levels.
   const base::TimeDelta thresholds[4] = {
@@ -480,4 +499,6 @@ TEST_P(UpgradeDetectorImplTimerTest, TestNotificationTimer) {
   // No new notifications after high annoyance has been reached.
   FastForwardBy(thresholds[3]);
   ::testing::Mock::VerifyAndClear(&mock_observer);
+
+  detector.Shutdown();
 }

chrome/test/base/chrome_test_launcher.cc

@@ -61,6 +61,7 @@
 #if defined(OS_WIN) || defined(OS_MACOSX) || \
     (defined(OS_LINUX) && !defined(OS_CHROMEOS))
 #include "chrome/browser/first_run/scoped_relaunch_chrome_browser_override.h"
+#include "chrome/browser/upgrade_detector/installed_version_poller.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #endif
 
@@ -76,6 +77,11 @@ int ChromeTestSuiteRunner::RunTestSuite(int argc, char** argv) {
 #if defined(OS_ANDROID)
   // Android browser tests run child processes as threads instead.
   content::ContentTestSuiteBase::RegisterInProcessThreads();
+#endif
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+    (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+  InstalledVersionPoller::ScopedDisableForTesting disable_polling(
+      InstalledVersionPoller::MakeScopedDisableForTesting());
 #endif
   return test_suite.Run();
 }

chrome/test/base/testing_browser_process.cc

@@ -422,6 +422,14 @@ TestingBrowserProcess::resource_coordinator_parts() {
   return resource_coordinator_parts_.get();
 }
 
+BuildState* TestingBrowserProcess::GetBuildState() {
+#if !defined(OS_ANDROID)
+  return &build_state_;
+#else
+  return nullptr;
+#endif
+}
+
 resource_coordinator::TabManager* TestingBrowserProcess::GetTabManager() {
   return resource_coordinator_parts()->tab_manager();
 }

chrome/test/base/testing_browser_process.h

@@ -24,6 +24,10 @@
 #include "media/media_buildflags.h"
 #include "printing/buildflags/buildflags.h"
 
+#if !defined(OS_ANDROID)
+#include "chrome/browser/upgrade_detector/build_state.h"
+#endif
+
 class BackgroundModeManager;
 class NotificationPlatformBridge;
 class NotificationUIManager;
@@ -135,6 +139,7 @@ class TestingBrowserProcess : public BrowserProcess {
   resource_coordinator::TabManager* GetTabManager() override;
   resource_coordinator::ResourceCoordinatorParts* resource_coordinator_parts()
       override;
+  BuildState* GetBuildState() override;
 
   // Set the local state for tests. Consumer is responsible for cleaning it up
   // afterwards (using ScopedTestingLocalState, for example).
@@ -218,6 +223,10 @@ class TestingBrowserProcess : public BrowserProcess {
   std::unique_ptr<resource_coordinator::ResourceCoordinatorParts>
       resource_coordinator_parts_;
 
+#if !defined(OS_ANDROID)
+  BuildState build_state_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(TestingBrowserProcess);
 };