Show proper message in about Chrome OS page

chrome/browser/resources/settings/about_page/about_page.js

Index: chrome/browser/resources/settings/about_page/about_page.js
diff --git a/chrome/browser/resources/settings/about_page/about_page.js b/chrome/browser/resources/settings/about_page/about_page.js
index 54ca31e5695f2b294c6a18bbac756df54bc4b0a6..228917dc3779443ae1aed5995fc886029a3d8628 100644
--- a/chrome/browser/resources/settings/about_page/about_page.js
+++ b/chrome/browser/resources/settings/about_page/about_page.js
@@ -171,11 +171,14 @@ Polymer({
    */
   onUpdateStatusChanged_: function(event) {
 // <if expr="chromeos">
-    if (event.status == UpdateStatus.CHECKING)
+    if (event.status == UpdateStatus.CHECKING) {
       this.hasCheckedForUpdates_ = true;
-    else if (event.status == UpdateStatus.NEED_PERMISSION_TO_UPDATE) {
+    } else if (event.status == UpdateStatus.NEED_PERMISSION_TO_UPDATE) {
       this.showUpdateWarningDialog_ = true;
       this.updateInfo_ = {version: event.version, size: event.size};
+      // Shows 'check for updates' button in case that the user cancels the
+      // dialog and then intends to check for update again.
+      this.hasCheckedForUpdates_ = false;

[xiyuan, 2017/05/22 18:14:08]

Should this be moved to onUpdateWarningDialogClose_ ?

[weidongg, 2017/05/22 19:02:54]

There will be a tiny issue if I put it there: although the update
warning dialog is shown above the page, the page shows 'your chromebook is up to
date' in the background (See https://screenshot.googleplex.com/Ts7vdSHHKYO).
Then page turns that message off the dialog closes.
The reason is that both currentUpdateStatusEvent_ and hasCheckedForUpdates_
trigger the message toggling, currentUpdateStatusEvent_ turns the message on
here after receiving UPDATED and hasCheckedForUpdates_ turns the message off
when dialog closes.

[xiyuan, 2017/05/22 19:28:03]

Why would the page gets an UPDATED status while the dialog is open?

[weidongg, 2017/05/22 19:38:03]

The state transition for update engine is
IDLE->CHECKING_FOR_UPDATE->NEED_PERMISSION_TO_UPDATE->REPORTING_ERROR_EVENT->IDLE,
but JS maps IDLE to UPDATED state and version updater[1] blocks the error from
showing to user by map error state to UPDATED. So for JS the state transition
will be UPDATED->CHECKING->NEED_PERMISSION_TO_UPDATE->UPDATED->UPDATED.

[1]:
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/help/version_upd...

[xiyuan, 2017/05/22 20:10:28]

Can we use fix this? Either not sending UPDATED on C++ side (at lest for
need-permission-to-update case). Or don't show the UPDATED message when update
warning dialog is shown.

Setting |this.hasCheckedForUpdates_| feels like a work around and easy to be
broken later.

[weidongg, 2017/05/22 21:13:26]

Sure, I uploaded a new patch set to prevent updated status being shown to user
when the dialog is shown.

     }
 // </if>
     this.currentUpdateStatusEvent_ = event;
@@ -271,14 +274,8 @@ Polymer({
   getUpdateStatusMessage_: function() {
     switch (this.currentUpdateStatusEvent_.status) {
       case UpdateStatus.CHECKING:
-        return this.i18n('aboutUpgradeCheckStarted');
       case UpdateStatus.NEED_PERMISSION_TO_UPDATE:
-        // This status is immediately followed by an reporting error status.
-        // When update engine reports error, UI just shows that your device is
-        // up to date. This is a bug that needs to be fixed in the future.
-        // TODO(weidongg/581071): Show proper message when update engine aborts
-        // due to cellular connection.
-        return '';
+        return this.i18n('aboutUpgradeCheckStarted');
       case UpdateStatus.NEARLY_UPDATED:
 // <if expr="chromeos">
         if (this.currentChannel_ != this.targetChannel_)
@@ -420,8 +417,7 @@ Polymer({
     var staleUpdatedStatus = !this.hasCheckedForUpdates_ &&
         this.checkStatus_(UpdateStatus.UPDATED);
 
-    return staleUpdatedStatus || this.checkStatus_(UpdateStatus.FAILED) ||
-        this.checkStatus_(UpdateStatus.NEED_PERMISSION_TO_UPDATE);
+    return staleUpdatedStatus || this.checkStatus_(UpdateStatus.FAILED);
   },
 
   /**