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: [
+<if expr="not chromeos">
+    'updateShowUpdateStatus_(' +
+        'obsoleteSystemInfo_, currentUpdateStatusEvent_)',
+</if>
+<if expr="chromeos">
+    'updateShowUpdateStatus_(' +
+        'obsoleteSystemInfo_, currentUpdateStatusEvent_,' +
+        'hasCheckedForUpdates_)',
+</if>
+
+<if expr="not chromeos">
+    'updateShowRelaunch_(currentUpdateStatusEvent_)',
+</if>
+<if expr="chromeos">
+    'updateShowRelaunch_(currentUpdateStatusEvent_, targetChannel_)',
+</if>
+
+<if expr="not chromeos">
+    'updateShowButtonContainer_(showRelaunch_)',
+</if>
+<if expr="chromeos">
+    'updateShowButtonContainer_(' +
+        'showRelaunch_, showRelaunchAndPowerwash_,showCheckUpdates_)',
+</if>

[Dan Beam, 2016/12/06 02:03:18]

can put each observer into a single if here?  i.e.

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

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

[dpapad, 2016/12/06 17:00:40]

Done.

+  ],
+
+
   /** @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() {
+    this.showButtonContainer_ = this.showRelaunch_;
+<if expr="chromeos">
+    this.showButtonContainer_ = this.showButtonContainer_ ||
+        this.showRelaunchAndPowerwash_ || this.showCheckUpdates_;

[Dan Beam, 2016/12/06 02:03:18]

can you avoid setting this twice for the sake of observers?

var showButtonContainer = this.showRelaunch_;
<if expr="chromeos">
if (this.showRelaunchAndPowerwash_ || this.showCheckUpdates_)
  showButtonContainer = true;
</if>
this.showButtonContainer_ = showButtonContainer;

[dpapad, 2016/12/06 17:00:40]

Done (avoided setting it twice) , but in a simpler way I think.

+</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 02:03:18]

var showRelaunch = this.checkStatus_(UpdateStatus.NEARLY_UPDATED);
<if expr="chromeos">
if (this.isTargetChannelMoreStable_()) showRelaunch = false;
</if>
this.showRelaunch_ = showRelaunch;

[dpapad, 2016/12/06 17:00:40]

Acknowledged. I think that completely separating the code for the two platforms
in this case leads to shorter and more readable code.

   },
 
   /**
-   * @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>
 });