[MD settings] theme name shown in sub-label, wallpaper chrome os only

chrome/browser/resources/settings/appearance_page/appearance_page.js

Index: chrome/browser/resources/settings/appearance_page/appearance_page.js
diff --git a/chrome/browser/resources/settings/appearance_page/appearance_page.js b/chrome/browser/resources/settings/appearance_page/appearance_page.js
index d4e2da64705ffca1afbb424a9efea08e7dfba561..c9ae54f0fad0de0e16611a8a653c43a8078cbd4a 100644
--- a/chrome/browser/resources/settings/appearance_page/appearance_page.js
+++ b/chrome/browser/resources/settings/appearance_page/appearance_page.js
@@ -17,6 +17,8 @@
 Polymer({
   is: 'settings-appearance-page',
 
+  behaviors: [I18nBehavior],
+
   properties: {
     /**
      * The current active route.
@@ -99,13 +101,16 @@ Polymer({
         {value: 500, name: '500%'},
       ],
     },
-  },
 
-  behaviors: [
-    I18nBehavior,
-  ],
+    /** @private */
+    themeSublabel_: {
+      type: String,
+      value: function() { return this.i18n('chooseFromWebStore'); },

[michaelpg, 2016/03/31 15:48:21]

does this duplicate what happens in themeChanged_, or does that not get called
initially when the value is empty?

[dschuyler, 2016/03/31 19:13:36]

Done.

+    },
+  },
 
   observers: [
+    'themeChanged_(prefs.extensions.theme.id.value)',
     'zoomLevelChanged_(defaultZoomLevel_.value)',
   ],
 
@@ -141,13 +146,6 @@ Polymer({
     return homepage || this.i18n('exampleDotCom');
   },
 
-  /**
-   * @param {boolean} enabled Whether the theme reset is available.
-   */
-  setResetThemeEnabled: function(enabled) {
-    this.allowResetTheme_ = enabled;
-  },
-
   /** @private */
   onCustomizeFontsTap_: function() {
     this.$.pages.setSubpageChain(['appearance-fonts']);
@@ -158,17 +156,48 @@ Polymer({
     window.open(loadTimeData.getString('themesGalleryUrl'));
   },
 
+  /**
+   * ChromeOS only.
+   * @private
+   * */

[michaelpg, 2016/03/31 15:48:21]

nit: extra ' *'

[dschuyler, 2016/03/31 19:13:36]

Done.

+  openWallpaperGallery_: function() {
+    chrome.send('openWallpaperManager');

[michaelpg, 2016/03/31 15:48:21]

does this exist in a settings handler?

[michaelpg, 2016/03/31 15:48:21]

Dan just said to always use a browser proxy instead of chrome.send. I guess that
would also apply to cr.sendWithPromise (and maybe cr.addWebUIListener?) so
that's a larger scoped CL than you should do here.

[Dan Beam, 2016/03/31 17:53:21]

i didn't necesarilly say "always", but I do agree with michaelpg@ that this
class should use a browser proxy when possible.  whether that's before, during,
or after that CL is up to y'all.

Dave: you know my vote as that's what I pre-did:
https://codereview.chromium.org/1845473002/

before hooking up validation:
https://codereview.chromium.org/1849543006/

[dschuyler, 2016/03/31 19:13:36]

Acknowledged.

[dschuyler, 2016/03/31 19:13:36]

Done.

[dschuyler, 2016/03/31 19:13:36]

I'm on board with the conversion.  I'm just doing
it it a different order.

+  },
+
   /** @private */
   resetTheme_: function() {
     chrome.send('resetTheme');
   },
 
+  /**
+   * @param {boolean} enabled Whether the theme reset is available.
+   */
+  setResetThemeEnabled: function(enabled) {

[michaelpg, 2016/03/31 15:48:21]

I assume you have a reason for moving this from one location between two private
functions to another location between two private functions.

[dschuyler, 2016/03/31 19:13:36]

Done.

+    this.allowResetTheme_ = enabled;
+  },
+
   /** @private */
   showFontsPage_: function() {
     return this.currentRoute.subpage[0] == 'appearance-fonts';
   },
 
   /**
+   * @param {string} themeId The theme ID.
+   * @private
+   */
+  themeChanged_: function(themeId) {
+    if (themeId) {
+      var self = this;

[michaelpg, 2016/03/31 15:48:21]

nit: .bind(this) instead, as that's more common?

[dschuyler, 2016/03/31 19:13:36]

Done.

+      chrome.management.get(themeId,
+          function(info) {
+              self.themeSublabel_ = info.name;

[michaelpg, 2016/03/31 15:48:21]

2-space indent

[dschuyler, 2016/03/31 19:13:36]

Done.

+          });
+    } else {
+      this.themeSublabel_ = this.i18n('chooseFromWebStore');
+    }
+  },
+
+  /**
    * @param {number} percent The integer percentage of the page zoom.
    * @private
    */