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

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 966bee8e2b605b9cc5428fc902e22d02d78e5ed7..8ddb7f49dd47ad8e6d5d13a7db84bd7520a885d9 100644
--- a/chrome/browser/resources/settings/about_page/about_page.js
+++ b/chrome/browser/resources/settings/about_page/about_page.js
@@ -20,7 +20,10 @@ Polymer({
 
 <if expr="chromeos">
     /** @private */
-    hasCheckedForUpdates_: Boolean,
+    hasCheckedForUpdates_: {
+      type: Boolean,
+      value: false,
+    },
 
     /** @private {!BrowserChannel} */
     currentChannel_: String,
@@ -42,6 +45,71 @@ Polymer({
         };
       },
     },
+
+    /** @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} */
@@ -116,25 +184,38 @@ Polymer({
    * @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
    */
-  shouldShowUpdateStatusIcon_: function() {
-    return this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED ||
-        this.obsoleteSystemInfo_.endOfLine;
+  computeShowButtonContainer_: function() {
+    var shouldShow = this.showRelaunch_;
+<if expr="chromeos">
+    shouldShow = shouldShow || this.showRelaunchAndPowerwash_ ||
+        this.showCheckUpdates_;
+</if>
+    return shouldShow;
   },
 
   /**
    * @return {boolean}
    * @private
    */
-  shouldShowRelaunch_: function() {
+  computeShowRelaunch_: function() {
     var shouldShow = false;
 <if expr="not chromeos">
     shouldShow = this.checkStatus_(UpdateStatus.NEARLY_UPDATED);
@@ -265,7 +346,7 @@ Polymer({
    * @return {boolean}
    * @private
    */
-  shouldShowRelaunchAndPowerwash_: function() {
+  computeShowRelaunchAndPowerwash_: function() {
     return this.checkStatus_(UpdateStatus.NEARLY_UPDATED) &&
         this.isTargetChannelMoreStable_();
   },
@@ -280,7 +361,7 @@ Polymer({
    * @return {boolean}
    * @private
    */
-  shouldShowCheckUpdates_: function() {
+  computeShowCheckUpdates_: function() {
     return !this.hasCheckedForUpdates_ ||
         this.checkStatus_(UpdateStatus.FAILED);
   },