About page should not automatically trigger OS updates

Currently, if the user opens the Help/About page on Chrome OS it will
automatically check for an update and begin downloading/installing it.
Users should be able to decide when their update is applied, e.g., in
case they are connected to a tethered or bandwidth-limited network.

This disables the auto-update check on Chrome OS, replacing it with a
button the user can press to check for updates. (This has no effect on
the scheduled auto-updating service.)

Screenshots: http://imgur.com/a/k2s6y#0

BUG=394226
R=estade@chromium.org, deymo@chromium.org

Review URL: https://codereview.chromium.org/476983002

Cr-Commit-Position: refs/heads/master@{#291580}

chrome/app/chromeos_strings.grdp

@@ -5083,6 +5083,9 @@ Battery full
   </message>
 
   <!-- About Chrome page -->
+  <message name="IDS_UPGRADE_BUTTON" desc="The button label to check for updates and apply (download and install) if found">
+    Check for and apply updates
+  </message>
   <message name="IDS_UPGRADE_OFFLINE" desc="Status label: Currently offline">
     You are currently offline.
   </message>

chrome/browser/resources/help/help_content.html

@@ -49,6 +49,8 @@
       <button id="relaunch-and-powerwash"
           i18n-content="relaunchAndPowerwash" hidden>
       </button>
+      <button id="request-update" i18n-content="updateButton">
+      </button>
 </if>
     </div>
 </if>

chrome/browser/resources/help/help_page.js

@@ -107,6 +107,13 @@ cr.define('help', function() {
       }
 
       if (cr.isChromeOS) {
+        // Add event listener for the check for and apply updates button.
+        this.maybeSetOnClick_($('request-update'), function() {
+          self.setUpdateStatus_('checking');
+          $('request-update').disabled = true;
+          chrome.send('requestUpdate');
+        });
+
         // Add event listener for the close button when shown as an overlay.
         if ($('about-done')) {
           $('about-done').addEventListener('click', function() {
@@ -276,12 +283,33 @@ cr.define('help', function() {
         $('relaunch-and-powerwash').hidden = relaunchAndPowerwashHidden;
       }
 
+      if (cr.isChromeOS) {
+        // Only enable the update button if it hasn't been used yet or the
+        // status isn't 'updated'.
+        if (!$('request-update').disabled || status != 'updated') {
+          // Disable the button if an update is already in progress.
+          $('request-update').disabled =
+            ['checking', 'updating', 'nearly_updated'].indexOf(status) > -1;
+        }
+      }
+
       var container = $('update-status-container');
       if (container) {
         container.hidden = status == 'disabled';
         $('relaunch').hidden =
             (status != 'nearly_updated') || !relaunchAndPowerwashHidden;
 
+        if (cr.isChromeOS) {
+          // Assume the "updated" status is stale if we haven't checked yet.
+          if (status == 'updated' && !$('request-update').disabled)
+            container.hidden = true;
+
+          // Hide the request update button if auto-updating is disabled or
+          // a relaunch button is showing.
+          $('request-update').hidden = status == 'disabled' ||
+            !$('relaunch').hidden || !relaunchAndPowerwashHidden;
+        }
+
         if (!cr.isMac)
           $('update-percentage').hidden = status != 'updating';
       }

chrome/browser/ui/webui/help/help_browsertest.js

@@ -24,17 +24,68 @@ GEN('#if defined(OS_LINUX) || defined(GOOGLE_CHROME_BUILD)');
 
 // Test that repeated calls to setUpdateStatus work.
 TEST_F('HelpPageWebUITest', 'testUpdateState', function() {
+  var relaunch = $('relaunch');
+  var container = $('update-status-container');
+  var update = $('request-update');
+
+  help.HelpPage.setUpdateStatus('updated', '');
+  expectTrue(relaunch.hidden);
+  expectTrue(cr.isChromeOS == container.hidden);
+  expectTrue(!cr.isChromeOS || !update.hidden && !update.disabled);
+
   help.HelpPage.setUpdateStatus('disabled', '');
-  expectTrue($('relaunch').hidden);
-  expectTrue($('update-status-container').hidden);
+  expectTrue(relaunch.hidden);
+  expectTrue(container.hidden);
+  expectTrue(!cr.isChromeOS || update.hidden);
 
   help.HelpPage.setUpdateStatus('nearly_updated', '');
-  expectTrue(!$('relaunch').hidden);
-  expectTrue(!$('update-status-container').hidden);
+  expectTrue(!relaunch.hidden);
+  expectTrue(!container.hidden);
+  expectTrue(!cr.isChromeOS || update.hidden);
 
   help.HelpPage.setUpdateStatus('disabled', '');
   expectTrue($('relaunch').hidden);
   expectTrue($('update-status-container').hidden);
+  expectTrue(!cr.isChromeOS || update.hidden);
+});
+
+GEN('#endif');
+
+GEN('#if defined(OS_CHROMEOS)');
+
+// Test that the request update button is shown and hidden properly.
+TEST_F('HelpPageWebUITest', 'testRequestUpdate', function() {
+  var container = $('update-status-container');
+  var update = $('request-update');
+
+  help.HelpPage.setUpdateStatus('updated', '');
+  expectTrue(container.hidden);
+  expectTrue(!update.hidden && !update.disabled);
+
+  update.click();
+  expectTrue(!update.hidden && update.disabled);
+  expectFalse(container.hidden);
+
+  help.HelpPage.setUpdateStatus('checking', '');
+  expectFalse(container.hidden);
+  expectTrue(!update.hidden && update.disabled);
+
+  help.HelpPage.setUpdateStatus('failed', 'Error');
+  expectFalse(container.hidden);
+  expectTrue(!update.hidden && !update.disabled);
+
+  update.click();
+  help.HelpPage.setUpdateStatus('checking', '');
+  expectFalse(container.hidden);
+  expectTrue(!update.hidden && update.disabled);
+
+  help.HelpPage.setUpdateStatus('nearly_updated', '');
+  expectFalse(container.hidden);
+  expectTrue(update.hidden);
+
+  help.HelpPage.setUpdateStatus('updated', '');
+  expectFalse(container.hidden);
+  expectTrue(!update.hidden && update.disabled);
 });
 
 GEN('#endif');

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

@@ -48,6 +48,7 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/help/help_utils_chromeos.h"
+#include "chrome/browser/ui/webui/help/version_updater_chromeos.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
@@ -143,6 +144,7 @@ void HelpHandler::GetLocalizedValues(base::DictionaryValue* localized_strings) {
     { "upToDate", IDS_UPGRADE_UP_TO_DATE },
     { "updating", IDS_UPGRADE_UPDATING },
 #if defined(OS_CHROMEOS)
+    { "updateButton", IDS_UPGRADE_BUTTON },
     { "updatingChannelSwitch", IDS_UPGRADE_UPDATING_CHANNEL_SWITCH },
 #endif
     { "updateAlmostDone", IDS_UPGRADE_SUCCESSFUL_RELAUNCH },
@@ -283,6 +285,8 @@ void HelpHandler::RegisterMessages() {
       base::Bind(&HelpHandler::SetChannel, base::Unretained(this)));
   web_ui()->RegisterMessageCallback("relaunchAndPowerwash",
       base::Bind(&HelpHandler::RelaunchAndPowerwash, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("requestUpdate",
+      base::Bind(&HelpHandler::RequestUpdate, base::Unretained(this)));
 #endif
 #if defined(OS_MACOSX)
   web_ui()->RegisterMessageCallback("promoteUpdater",
@@ -296,12 +300,7 @@ void HelpHandler::Observe(int type, const content::NotificationSource& source,
     case chrome::NOTIFICATION_UPGRADE_RECOMMENDED: {
       // A version update is installed and ready to go. Refresh the UI so the
       // correct state will be shown.
-      version_updater_->CheckForUpdate(
-          base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this))
-#if defined(OS_MACOSX)
-          , base::Bind(&HelpHandler::SetPromotionState, base::Unretained(this))
-#endif
-          );
+      RequestUpdate(NULL);
       break;
     }
     default:
@@ -354,12 +353,13 @@ void HelpHandler::OnPageLoaded(const base::ListValue* args) {
                                    base::StringValue(build_date));
 #endif  // defined(OS_CHROMEOS)
 
-  version_updater_->CheckForUpdate(
-      base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this))
-#if defined(OS_MACOSX)
-      , base::Bind(&HelpHandler::SetPromotionState, base::Unretained(this))
+  // On Chrome OS, do not check for an update automatically.
+#if defined(OS_CHROMEOS)
+  static_cast<VersionUpdaterCros*>(version_updater_.get())->GetUpdateStatus(
+      base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this)));
+#else
+  RequestUpdate(NULL);
 #endif
-      );
 
 #if defined(OS_MACOSX)
   web_ui()->CallJavascriptFunction(
@@ -455,6 +455,15 @@ void HelpHandler::RelaunchAndPowerwash(const base::ListValue* args) {
 
 #endif  // defined(OS_CHROMEOS)
 
+void HelpHandler::RequestUpdate(const base::ListValue* args) {
+  version_updater_->CheckForUpdate(
+      base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this))
+#if defined(OS_MACOSX)
+      , base::Bind(&HelpHandler::SetPromotionState, base::Unretained(this))
+#endif
+      );
+}
+
 void HelpHandler::SetUpdateStatus(VersionUpdater::Status status,
                                   int progress, const base::string16& message) {
   // Only UPDATING state should have progress set.

chrome/browser/ui/webui/help/help_handler.h

@@ -71,6 +71,9 @@ class HelpHandler : public content::WebUIMessageHandler,
   void RelaunchAndPowerwash(const base::ListValue* args);
 #endif
 
+  // Checks for and applies update.
+  void RequestUpdate(const base::ListValue* args);
+
   // Callback method which forwards status updates to the page.
   void SetUpdateStatus(VersionUpdater::Status status, int progress,
                        const base::string16& fail_message);

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

@@ -72,19 +72,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 =
@@ -92,25 +86,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))
+    update_engine_client->AddObserver(this);
 
   // Make sure that libcros is loaded and OOBE is complete.
   if (!WizardController::default_controller() ||

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

@@ -21,6 +21,9 @@ class VersionUpdaterCros : public VersionUpdater,
   virtual void GetChannel(bool get_current_channel,
                           const ChannelCallback& callback) OVERRIDE;
 
+  // Gets the last update status, without triggering a new check or download.
+  void GetUpdateStatus(const StatusCallback& callback);
+
  protected:
   friend class VersionUpdater;