About page should not automatically trigger OS updates

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

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/webui/help/version_updater_chromeos.h"
 
 #include <cmath>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 55 matching lines @@
   chromeos::CrosSettings* settings = chromeos::CrosSettings::Get();
   if (!settings)
     return update_disabled;
   const base::Value* update_disabled_value =
       settings->GetPref(chromeos::kUpdateDisabled);
   if (update_disabled_value)
     CHECK(update_disabled_value->GetAsBoolean(&update_disabled));
   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 =
       chromeos::NetworkHandler::Get()->network_state_handler();
   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);
+    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();
-  update_engine_client->AddObserver(this);
+  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();
+  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() ||
       chromeos::StartupUtils::IsDeviceRegistered()) {
     update_engine_client->RequestUpdateCheck(
         base::Bind(&VersionUpdaterCros::OnUpdateCheck,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
@@ skipping 83 matching lines @@
   last_operation_ = status.status;
 }
 
 void VersionUpdaterCros::OnUpdateCheck(
     UpdateEngineClient::UpdateCheckResult result) {
   // If version updating is not implemented, this binary is the most up-to-date
   // possible with respect to automatic updating.
   if (result == UpdateEngineClient::UPDATE_RESULT_NOTIMPLEMENTED)
     callback_.Run(UPDATED, 0, base::string16());
 }