MD Settings: About page, implementing update status.

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 44da2984db37bcaad55952fe8a64205d4ebaae3e..267e395cf8f6c0aa8c37842caeea4be2c7e6cbda 100644
--- a/chrome/browser/resources/settings/about_page/about_page.js
+++ b/chrome/browser/resources/settings/about_page/about_page.js
@@ -9,7 +9,7 @@
 Polymer({
   is: 'settings-about-page',
 
-  behaviors: [RoutableBehavior],
+  behaviors: [WebUIListenerBehavior, RoutableBehavior, I18nBehavior],
 
   properties: {
     /**
@@ -19,6 +19,20 @@ Polymer({
       type: Object,
       notify: true,
     },
+
+    /** @private {?UpdateStatusChangedEvent} */
+    currentUpdateStatusEvent_: Object,
+
+<if expr="chromeos">
+    /**
+     * Whether the current and target channel is different.
+     * @private
+     */
+    channelsDiffer_: Boolean,
+
+    /** @private {!BrowserChannel} */
+    targetChannel_: String,
+</if>
   },
 
   /** @private {?settings.AboutPageBrowserProxy} */
@@ -34,6 +48,32 @@ Polymer({
   /** @override */
   ready: function() {
     this.browserProxy_ = settings.AboutPageBrowserProxyImpl.getInstance();
+    this.browserProxy_.pageReady();
+
+<if expr="chromeos">
+    Promise.all([
+      this.browserProxy_.getCurrentChannel(),
+      this.browserProxy_.getTargetChannel(),
+    ]).then(function(channels) {
+      this.targetChannel_ = channels[1];
+      this.channelsDiffer_ = channels[0] != this.targetChannel_;
+      this.startListening_();
+    }.bind(this));
+</if>
+<if expr="not chromeos">
+    this.startListening_();
+</if>
+  },
+
+  /** @private */
+  startListening_: function() {
+    this.addWebUIListener(
+        'update-status-changed',
+        /** @param {!UpdateStatusChangedEvent} event */
+        function(event) {
+          this.currentUpdateStatusEvent_ = event;
+        }.bind(this));
+    this.browserProxy_.refreshUpdateStatus();
   },
 
    /** @override */
@@ -46,6 +86,95 @@ Polymer({
     this.browserProxy_.openHelpPage();
   },
 
+  /**
+   * @return {boolean}
+   * @private
+   */
+  shouldShowUpdateStatus_: function() {

[tommycli, 2016/05/19 20:44:48]

optional suggestion: maybe isUpdateStatusShown? (or isUpdateStatusHidden)

[dpapad, 2016/05/19 21:31:05]

I find the suggested naming a bit backward compared to the logic.

This method is called from the HTML to determine whether to show/hide a node.
Naming it isFooShown/isFooHidden makes it sound like this method queries the UI
to figure out if a node is currently shown/hidden. I believe current naming is
more appropriate since it makes it clear that the UI queries the "model" to
update itself accordingly.

+    return this.currentUpdateStatusEvent_.status != UpdateStatus.DISABLED;
+  },
+
+  /**
+   * @return {string}
+   * @private
+   */
+  getUpdateStatusMessage_: function() {

[tommycli, 2016/05/19 20:44:48]

Arg... yeah this is complicated and i can't think of a better way to do it.

My only suggestion: Maybe get rid of the messageId and channelDisplayName
variables, and just directly do: return this.i18n('aboutUpdateCheckStarted')
directly from there? It just gets rid of some state...

And you could just return this.currentUpdateStatusEvent_.message if it falls
through all the way to the end.

[dpapad, 2016/05/19 21:31:05]

Done.

+    var messageId = null;
+    var channelDisplayName = null;
+
+    switch (this.currentUpdateStatusEvent_.status) {
+      case UpdateStatus.CHECKING:
+        messageId = 'aboutUpgradeCheckStarted';
+        break;
+      case UpdateStatus.NEARLY_UPDATED:
+        messageId = 'aboutUpgradeRelaunch';
+<if expr="chromeos">
+        if (this.channelsDiffer_)
+          messageId = 'aboutUpgradeSuccessChannelSwitch';
+</if>
+        break;
+      case UpdateStatus.UPDATED:
+        messageId = 'aboutUpgradeUpToDate';
+        break;
+      case UpdateStatus.UPDATING:
+        messageId = 'aboutUpgradeUpdating';
+<if expr="chromeos">
+        if (this.channelsDiffer_) {
+          messageId = 'aboutUpgradeUpdatingChannelSwitch';
+          channelDisplayName = this.i18n(
+              settings.browserChannelToI18nId(this.targetChannel_));
+        }
+</if>
+        break;
+    }
+
+    return messageId == null ?
+        this.currentUpdateStatusEvent_.message :
+        this.i18n(messageId, channelDisplayName || undefined);
+  },
+
+  /**
+   * @return {string}
+   * @private
+   */
+  getIconCssClass_: function() {
+    switch (this.currentUpdateStatusEvent_.status) {
+      case UpdateStatus.FAILED: return 'error-icon';

[tommycli, 2016/05/19 20:44:48]

formatting nit: maybe put these returns on the separate line? So the FAILED is
separated from the UPDATED and NEARLY_UPDATED?

[dpapad, 2016/05/19 21:31:05]

Done.

+      case UpdateStatus.UPDATED:
+      case UpdateStatus.NEARLY_UPDATED:
+          return 'check-icon';
+      default: return '';

[tommycli, 2016/05/19 20:44:48]

nit: same here

[dpapad, 2016/05/19 21:31:05]

Done.

+    }
+  },
+
+  /**
+   * @return {?string}
+   * @private
+   */
+  getIcon_: function() {
+    switch (this.currentUpdateStatusEvent_.status) {
+      case UpdateStatus.DISABLED_BY_ADMIN: return 'cr:domain';

[tommycli, 2016/05/19 20:44:48]

formatting nit: same here

[dpapad, 2016/05/19 21:31:05]

Done.

+      case UpdateStatus.FAILED: return 'settings:error';
+      case UpdateStatus.UPDATED:
+      case UpdateStatus.NEARLY_UPDATED:
+          return 'settings:check-circle';
+      default: return null;
+    }
+  },
+
+  /**
+   * @return {?string}
+   * @private
+   */
+  getIconSrc_: function() {
+    switch (this.currentUpdateStatusEvent_.status) {
+      case UpdateStatus.CHECKING:
+      case UpdateStatus.UPDATING:
+        return 'chrome://resources/images/throbber_small.svg';
+      default: return null;
+    }
+  },
+
 <if expr="chromeos">
   /** @private */
   onDetailedBuildInfoTap_: function() {