MD Settings: Fix "Check for updates" button regression.

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({
   is: 'settings-about-page',
 
   behaviors: [WebUIListenerBehavior, MainPageBehavior, I18nBehavior],
 
   properties: {
     /** @private {?UpdateStatusChangedEvent} */
     currentUpdateStatusEvent_: {
       type: Object,
       value: {message: '', progress: 0, status: UpdateStatus.DISABLED},
     },
 
 <if expr="chromeos">
     /** @private */
-    hasCheckedForUpdates_: Boolean,
+    hasCheckedForUpdates_: {
+      type: Boolean,
+      value: false,
+    },
 
     /** @private {!BrowserChannel} */
     currentChannel_: String,
 
     /** @private {!BrowserChannel} */
     targetChannel_: String,
 
     /** @private {?RegulatoryInfo} */
     regulatoryInfo_: Object,
 </if>
 
     /** @private {!{obsolete: boolean, endOfLine: boolean}} */
     obsoleteSystemInfo_: {
       type: Object,
       value: function() {
         return {
           obsolete: loadTimeData.getBoolean('aboutObsoleteNowOrSoon'),
           endOfLine: loadTimeData.getBoolean('aboutObsoleteEndOfTheLine'),
         };
       },
     },
+
+    /** @private */
+    showUpdateStatus_: Boolean,
+
+    /** @private */
+    showButtonContainer_: Boolean,
+
+    /** @private */
+    showRelaunch_: Boolean,
+
+<if expr="chromeos">
+    /** @private */
+    showRelaunchAndPowerwash_: {
+      type: Boolean,
+      computed: 'computeShowRelaunchAndPowerwash_(' +
+          'currentUpdateStatusEvent_, targetChannel_)',
+    },
+
+    /** @private */
+    showCheckUpdates_: {
+      type: Boolean,
+      computed: 'computeShowCheckUpdates_(currentUpdateStatusEvent_)',
+    },
+</if>
   },
 
+  observers: [

[dschuyler, 2016/12/06 19:41:21]

Maybe a comment about why this is done this way?

[dpapad, 2016/12/06 21:54:00]

I tried to think of a helpful comment here, but everything I came up with is
pretty much already documented at updateShowButtonContainer_'s docs a few lines
below.

+<if expr="not chromeos">
+    'updateShowUpdateStatus_(' +
+        'obsoleteSystemInfo_, currentUpdateStatusEvent_)',
+    'updateShowRelaunch_(currentUpdateStatusEvent_)',
+    'updateShowButtonContainer_(showRelaunch_)',
+</if>
+
+<if expr="chromeos">
+    'updateShowUpdateStatus_(' +
+        'obsoleteSystemInfo_, currentUpdateStatusEvent_,' +
+        'hasCheckedForUpdates_)',
+    'updateShowRelaunch_(currentUpdateStatusEvent_, targetChannel_)',
+    'updateShowButtonContainer_(' +
+        'showRelaunch_, showRelaunchAndPowerwash_,showCheckUpdates_)',

[dschuyler, 2016/12/06 19:41:21]

nit: space after comma

[dpapad, 2016/12/06 21:54:00]

Done.

+</if>
+  ],
+
+
   /** @private {?settings.AboutPageBrowserProxy} */
   aboutBrowserProxy_: null,
 
   /** @private {?settings.LifetimeBrowserProxy} */
   lifetimeBrowserProxy_: null,
 
   /** @override */
   attached: function() {
     this.aboutBrowserProxy_ = settings.AboutPageBrowserProxyImpl.getInstance();
     this.aboutBrowserProxy_.pageReady();
@@ skipping 48 matching lines @@
   /** @private */
   onHelpTap_: function() {
     this.aboutBrowserProxy_.openHelpPage();
   },
 
   /** @private */
   onRelaunchTap_: function() {
     this.lifetimeBrowserProxy_.relaunch();
   },
 
-  /**
-   * @return {boolean}
-   * @private
-   */
-  shouldShowUpdateStatusMessage_: function() {
-    return this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED &&
+  /** @private */
+  updateShowUpdateStatus_: function() {
+<if expr="chromeos">
+    // Assume the "updated" status is stale if we haven't checked yet.
+    if (this.currentUpdateStatusEvent_.status == UpdateStatus.UPDATED &&
+        !this.hasCheckedForUpdates_) {
+      this.showUpdateStatus_ = false;
+      return;
+    }
+</if>
+    this.showUpdateStatus_ =
+        this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED &&
         !this.obsoleteSystemInfo_.endOfLine;
   },
 
   /**
-   * @return {boolean}
+   * Hide the button container if all buttons are hidden, otherwise the
+   * container displayes an unwanted border (see secondary-action class).
    * @private
    */
-  shouldShowUpdateStatusIcon_: function() {
-    return this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED ||
-        this.obsoleteSystemInfo_.endOfLine;
+  updateShowButtonContainer_: function() {
+<if expr="not chromeos">
+    this.showButtonContainer_ = this.showRelaunch_;
+</if>
+<if expr="chromeos">
+    this.showButtonContainer_ = this.showRelaunch_ ||
+        this.showRelaunchAndPowerwash_ || this.showCheckUpdates_;
+</if>
+  },
+
+  /** @private */
+  updateShowRelaunch_: function() {
+<if expr="not chromeos">
+    this.showRelaunch_ = this.checkStatus_(UpdateStatus.NEARLY_UPDATED);
+</if>
+<if expr="chromeos">
+    this.showRelaunch_ = this.checkStatus_(UpdateStatus.NEARLY_UPDATED) &&
+        !this.isTargetChannelMoreStable_();
+</if>

[Dan Beam, 2016/12/06 19:10:20]

i would argue that this is still more duplicative than my examples, but whatevs

   },
 
   /**
-   * @return {boolean}
-   * @private
-   */
-  shouldShowRelaunch_: function() {
-    var shouldShow = false;
-<if expr="not chromeos">
-    shouldShow = this.checkStatus_(UpdateStatus.NEARLY_UPDATED);
-</if>
-<if expr="chromeos">
-    shouldShow = this.checkStatus_(UpdateStatus.NEARLY_UPDATED) &&
-        !this.isTargetChannelMoreStable_();
-</if>
-    return shouldShow;
-  },
-
-  /**
    * @return {string}
    * @private
    */
   getUpdateStatusMessage_: function() {
     switch (this.currentUpdateStatusEvent_.status) {
       case UpdateStatus.CHECKING:
         return this.i18n('aboutUpgradeCheckStarted');
       case UpdateStatus.NEARLY_UPDATED:
 <if expr="chromeos">
         if (this.currentChannel_ != this.targetChannel_)
@@ skipping 98 matching lines @@
 
   /** @private */
   onRelaunchAndPowerwashTap_: function() {
     this.lifetimeBrowserProxy_.factoryReset();
   },
 
   /**
    * @return {boolean}
    * @private
    */
-  shouldShowRelaunchAndPowerwash_: function() {
+  computeShowRelaunchAndPowerwash_: function() {
     return this.checkStatus_(UpdateStatus.NEARLY_UPDATED) &&
         this.isTargetChannelMoreStable_();
   },
 
   /** @private */
   onCheckUpdatesTap_: function() {
     this.onUpdateStatusChanged_({status: UpdateStatus.CHECKING});
     this.aboutBrowserProxy_.requestUpdate();
   },
 
   /**
    * @return {boolean}
    * @private
    */
-  shouldShowCheckUpdates_: function() {
+  computeShowCheckUpdates_: function() {
     return !this.hasCheckedForUpdates_ ||
         this.checkStatus_(UpdateStatus.FAILED);
   },
 
   /**
    * @return {boolean}
    * @private
    */
   shouldShowRegulatoryInfo_: function() {
     return this.regulatoryInfo_ !== null;
@@ skipping 10 matching lines @@
     });
   },
 
 <if expr="_google_chrome">
   /** @private */
   onReportIssueTap_: function() {
     this.aboutBrowserProxy_.openFeedbackDialog();
   },
 </if>
 });