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..192b0f8120ec3efdd144e0e10c326d1c1adb4a12 100644
--- a/chrome/browser/resources/settings/about_page/about_page.js
+++ b/chrome/browser/resources/settings/about_page/about_page.js
@@ -72,7 +72,8 @@ Polymer({
     /** @private */
     showCheckUpdates_: {
       type: Boolean,
-      computed: 'computeShowCheckUpdates_(currentUpdateStatusEvent_)',
+      computed: 'computeShowCheckUpdates_(' +
+          'currentUpdateStatusEvent_, hasCheckedForUpdates_)',
     },
 
     /** @private {!Map<string, string>} */
@@ -171,9 +172,9 @@ 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};
     }
@@ -227,8 +228,10 @@ Polymer({
   updateShowUpdateStatus_: function() {
 // <if expr="chromeos">
     // Assume the "updated" status is stale if we haven't checked yet.
+    // Prevents the updated status being shown to user when an update warning
+    // dialog is shown.

[xiyuan, 2017/05/22 22:36:34]

nit: can you combine the new comment with old one?

e.g.

// Do not show the "updated" status if we haven't checked yet
// or the update warning dialog is shown to user.

[weidongg, 2017/05/23 00:17:00]

Done.

     if (this.currentUpdateStatusEvent_.status == UpdateStatus.UPDATED &&
-        !this.hasCheckedForUpdates_) {
+        (!this.hasCheckedForUpdates_ || this.showUpdateWarningDialog_)) {
       this.showUpdateStatus_ = false;
       return;
     }
@@ -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);
   },
 
   /**
@@ -435,6 +431,9 @@ Polymer({
   /** @private */
   onUpdateWarningDialogClose_: function() {
     this.showUpdateWarningDialog_ = false;
+    // 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 22:36:34]

This runs when user clicks "Continue" on the dialog as well, right? Do we need
to worry that user clicks "Check for Update" before our
SetUpdateOverCellularTarget finishes in "Continue" case?

[weidongg, 2017/05/23 00:17:00]

Yes, you are right. This runs whenever the dialog closes. The button disappear
when transitioning to CHECKING state (I tested in samus, I didn't notice it
appears in the first place.). If user manages to click the button before
transitioning to CHECKING state, I think there will be two AttempUpdate dbus
calls and one of them will get ignored in Update engine.

   },
 // </if>