Show proper message in about Chrome OS page

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

 // Copyright 2016 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.
 
 /**
  * @fileoverview 'settings-about-page' contains version and OS related
  * information.
  */
 
 Polymer({
@@ skipping 54 matching lines @@
     /** @private */
     showRelaunchAndPowerwash_: {
       type: Boolean,
       computed: 'computeShowRelaunchAndPowerwash_(' +
           'currentUpdateStatusEvent_, targetChannel_)',
     },
 
     /** @private */
     showCheckUpdates_: {
       type: Boolean,
-      computed: 'computeShowCheckUpdates_(currentUpdateStatusEvent_)',
+      computed: 'computeShowCheckUpdates_(' +
+          'currentUpdateStatusEvent_, hasCheckedForUpdates_)',
     },
 
     /** @private {!Map<string, string>} */
     focusConfig_: {
       type: Object,
       value: function() {
         var map = new Map();
         map.set(
             settings.Route.DETAILED_BUILD_INFO.path,
             '#detailed-build-info-trigger');
@@ skipping 78 matching lines @@
 // </if>
     this.aboutBrowserProxy_.refreshUpdateStatus();
   },
 
   /**
    * @param {!UpdateStatusChangedEvent} event
    * @private
    */
   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};
     }
 // </if>
     this.currentUpdateStatusEvent_ = event;
   },
 
 // <if expr="_google_chrome and is_macosx">
   /**
    * @param {!PromoteUpdaterStatus} status
@@ skipping 33 matching lines @@
 
   /** @private */
   onRelaunchTap_: function() {
     this.lifetimeBrowserProxy_.relaunch();
   },
 
   /** @private */
   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;
     }
 // </if>
     this.showUpdateStatus_ =
         this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED &&
         !this.obsoleteSystemInfo_.endOfLine;
   },
 
   /**
@@ skipping 22 matching lines @@
 // </if>
   },
 
   /**
    * @return {string}
    * @private
    */
   getUpdateStatusMessage_: function() {
     switch (this.currentUpdateStatusEvent_.status) {
       case UpdateStatus.CHECKING:
+      case UpdateStatus.NEED_PERMISSION_TO_UPDATE:
         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 '';
       case UpdateStatus.NEARLY_UPDATED:
 // <if expr="chromeos">
         if (this.currentChannel_ != this.targetChannel_)
           return this.i18n('aboutUpgradeSuccessChannelSwitch');
 // </if>
         return this.i18n('aboutUpgradeRelaunch');
       case UpdateStatus.UPDATED:
         return this.i18n('aboutUpgradeUpToDate');
       case UpdateStatus.UPDATING:
         assert(typeof this.currentUpdateStatusEvent_.progress == 'number');
@@ skipping 121 matching lines @@
   /**
    * @return {boolean}
    * @private
    */
   computeShowCheckUpdates_: function() {
     // Enable the update button if we are in a stale 'updated' status or
     // update has failed. Disable it otherwise.
     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);
   },
 
   /**
    * @return {boolean}
    * @private
    */
   shouldShowRegulatoryInfo_: function() {
     return this.regulatoryInfo_ !== null;
   },
 
   /** @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>
 
   /** @private */
   onProductLogoTap_: function() {
     this.$['product-logo'].animate({
       transform: ['none', 'rotate(-10turn)'],
     }, {
       duration: 500,
       easing: 'cubic-bezier(1, 0, 0, 1)',
     });
   },
 
 // <if expr="_google_chrome">
   /** @private */
   onReportIssueTap_: function() {
     this.aboutBrowserProxy_.openFeedbackDialog();
   },
 // </if>
 });