About page should not automatically trigger OS updates

chrome/browser/ui/webui/help/version_updater_chromeos.cc

Index: chrome/browser/ui/webui/help/version_updater_chromeos.cc
diff --git a/chrome/browser/ui/webui/help/version_updater_chromeos.cc b/chrome/browser/ui/webui/help/version_updater_chromeos.cc
index e6c2f2afe8fa893afd418ce55719ccaf8c843c4f..03822b49e6fb94fa699fb92ae4c0031b9719efc1 100644
--- a/chrome/browser/ui/webui/help/version_updater_chromeos.cc
+++ b/chrome/browser/ui/webui/help/version_updater_chromeos.cc
@@ -73,19 +73,13 @@ bool IsAutoUpdateDisabled() {
   return update_disabled;
 }
 
-}  // namespace
-
-VersionUpdater* VersionUpdater::Create() {
-  return new VersionUpdaterCros;
-}
-
-void VersionUpdaterCros::CheckForUpdate(const StatusCallback& callback) {
-  callback_ = callback;
-
+// Returns whether an update is allowed. If not, it calls the callback with
+// the appropriate status.
+bool EnsureCanUpdate(const VersionUpdater::StatusCallback& callback) {
   if (IsAutoUpdateDisabled()) {
-    callback_.Run(FAILED, 0,
-                  l10n_util::GetStringUTF16(IDS_UPGRADE_DISABLED_BY_POLICY));
-    return;
+    callback.Run(VersionUpdater::FAILED, 0,
+                 l10n_util::GetStringUTF16(IDS_UPGRADE_DISABLED_BY_POLICY));
+    return false;
   }
 
   chromeos::NetworkStateHandler* network_state_handler =
@@ -93,25 +87,55 @@ void VersionUpdaterCros::CheckForUpdate(const StatusCallback& callback) {
   const chromeos::NetworkState* network =
       network_state_handler->DefaultNetwork();
 
-  // Don't proceed to update if we're currently offline or connected
+  // Don't allow an update if we're currently offline or connected
   // to a network for which updates are disallowed.
   NetworkStatus status = GetNetworkStatus(network);
   if (status == NETWORK_STATUS_OFFLINE) {
-    callback_.Run(FAILED_OFFLINE, 0,
+    callback.Run(VersionUpdater::FAILED_OFFLINE, 0,
                   l10n_util::GetStringUTF16(IDS_UPGRADE_OFFLINE));
-    return;
+    return false;
   } else if (status == NETWORK_STATUS_DISALLOWED) {
     base::string16 message =
         l10n_util::GetStringFUTF16(
             IDS_UPGRADE_DISALLOWED,
             help_utils_chromeos::GetConnectionTypeAsUTF16(network->type()));
-    callback_.Run(FAILED_CONNECTION_TYPE_DISALLOWED, 0, message);
-    return;
+    callback.Run(VersionUpdater::FAILED_CONNECTION_TYPE_DISALLOWED, 0, message);
+    return false;
   }
 
+  return true;
+}
+
+}  // namespace
+
+VersionUpdater* VersionUpdater::Create() {
+  return new VersionUpdaterCros;
+}
+
+void VersionUpdaterCros::GetUpdateStatus(const StatusCallback& callback) {
+  callback_ = callback;
+  if (!EnsureCanUpdate(callback))
+    return;
+
+  UpdateEngineClient* update_engine_client =
+      DBusThreadManager::Get()->GetUpdateEngineClient();
+  if (!update_engine_client->HasObserver(this))
+    update_engine_client->AddObserver(this);
+
+  this->UpdateStatusChanged(
+      DBusThreadManager::Get()->GetUpdateEngineClient()->GetLastStatus());
+}
+
+void VersionUpdaterCros::CheckForUpdate(const StatusCallback& callback) {
+  callback_ = callback;
+
+  if (!EnsureCanUpdate(callback))
+    return;
+
   UpdateEngineClient* update_engine_client =
       DBusThreadManager::Get()->GetUpdateEngineClient();
-  update_engine_client->AddObserver(this);
+  if (!update_engine_client->HasObserver(this))

[michaelpg, 2014/08/14 22:52:57]

Note that this is an existing "bug" (a NOT_REACHED should be thrown if you
changed the channel and then this function is triggered, since the observer
would be added twice). It isn't really detectable though because it would occur
on release builds.

But now that this function can be called multiple times, it's definitely
necessary.

+    update_engine_client->AddObserver(this);
 
   // Make sure that libcros is loaded and OOBE is complete.
   if (!WizardController::default_controller() ||