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_: {
+      type: Boolean,
+      computed: (function() {

[dpapad, 2016/12/05 23:59:47]

This is unfortunately necessary to make the Closure compiler happy. During
compilation we simply remove the "if expr" lines, so the compiler starts
returning bogus errors, that do not apply in the code that actually gets
executed at runtime.

[Dan Beam, 2016/12/06 00:16:51]

why can't you just make different member names, i.e.

showUpdateStatusDesktop_
showUpdateStatusCros_

with different (or the same) handler?

[Dan Beam, 2016/12/06 00:17:26]

we also talked about this:
https://gist.github.com/danbeam/3ed90de431cdbdcdc13207d4b12e72a2

[dpapad, 2016/12/06 00:23:41]

If I understand the suggestion, that would defeat the purpose. Explaining below.

Before those <if expr> statements where in HTML, and that worked fine, since
Closer Compiler does not care about HTML files. In this CL I had to move them in
JS, to make it more convenient to use them from HTML, otherwise I would have to
repeat the same verbose bindings multiple times (specifically, buttonContainer's
hidden re-uses the computed bindings for all of its children). That was not
possible if the <if expr> statements stayed in JS.

I hope that makes sense, but if it does not, I would be happy to explain this in
person.

[Dan Beam, 2016/12/06 00:29:44]

then make them observers instead of computed properties

+        var binding = null;
+<if expr="not chromeos">
+        binding = 'computeShowUpdateStatus_(' +
+            'obsoleteSystemInfo_, currentUpdateStatusEvent_)';
+</if>
+<if expr="chromeos">
+        binding = 'computeShowUpdateStatus_(' +
+            'obsoleteSystemInfo_, currentUpdateStatusEvent_,' +
+            'hasCheckedForUpdates_)';
+</if>
+        return binding;
+      })(),
+    },
+
+    /** @private */
+    showRelaunch_: {
+      type: Boolean,
+      computed: (function() {
+        var binding = null;
+<if expr="not chromeos">
+        binding = 'computeShowRelaunch_(currentUpdateStatusEvent_)';
+</if>
+<if expr="chromeos">
+        binding = 'computeShowRelaunch_(' +
+            'currentUpdateStatusEvent_, targetChannel_)';
+</if>
+        return binding;
+      })(),
+    },
+
+<if expr="chromeos">
+    /** @private */
+    showRelaunchAndPowerwash_: {
+      type: Boolean,
+      computed: 'computeShowRelaunchAndPowerwash_(' +
+          'currentUpdateStatusEvent_, targetChannel_)',
+    },
+
+    /** @private */
+    showCheckUpdates_: {
+      type: Boolean,
+      computed: 'computeShowCheckUpdates_(currentUpdateStatusEvent_)',
+    },
+</if>
+
+    /** @private */
+    showButtonContainer_: {
+      type: Boolean,
+      computed: (function() {
+        var binding = null;
+<if expr="not chromeos">
+        binding = 'computeShowButtonContainer_(showRelaunch_)';
+</if>
+<if expr="chromeos">
+        binding = 'computeShowButtonContainer_(' +
+            'showRelaunch_, showRelaunchAndPowerwash_, showCheckUpdates_)';
+</if>
+        return binding;
+      })(),
+    },
   },
 
   /** @private {?settings.AboutPageBrowserProxy} */
   aboutBrowserProxy_: null,
 
   /** @private {?settings.LifetimeBrowserProxy} */
   lifetimeBrowserProxy_: null,
 
   /** @override */
   attached: function() {
@@ skipping 54 matching lines @@
 
   /** @private */
   onRelaunchTap_: function() {
     this.lifetimeBrowserProxy_.relaunch();
   },
 
   /**
    * @return {boolean}
    * @private
    */
-  shouldShowUpdateStatusMessage_: function() {
+  computeShowUpdateStatus_: function() {
+<if expr="chromeos">
+    // Assume the "updated" status is stale if we haven't checked yet.
+    if (this.currentUpdateStatusEvent_.status == UpdateStatus.UPDATED &&
+        !this.hasCheckedForUpdates_) {
+      return false;
+    }
+</if>
     return this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED &&
         !this.obsoleteSystemInfo_.endOfLine;
   },
 
   /**
+   * Hide the button container if all buttons are hidden, otherwise the
+   * container displayes an unwanted border (see secondary-action class).
+   * @return {boolean}
+   * @private
+   */
+  computeShowButtonContainer_: function() {
+    var shouldShow = this.showRelaunch_;
+<if expr="chromeos">
+    shouldShow = shouldShow || this.showRelaunchAndPowerwash_ ||
+        this.showCheckUpdates_;
+</if>
+    return shouldShow;
+  },
+
+  /**
    * @return {boolean}
    * @private
    */
-  shouldShowUpdateStatusIcon_: function() {
-    return this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED ||
-        this.obsoleteSystemInfo_.endOfLine;
-  },
-
-  /**
-   * @return {boolean}
-   * @private
-   */
-  shouldShowRelaunch_: function() {
+  computeShowRelaunch_: 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;
   },
@@ skipping 110 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>
 });