MD Settings: reduce complexity and overhead of Languages singleton

chrome/browser/resources/settings/languages_page/languages.js

Index: chrome/browser/resources/settings/languages_page/languages.js
diff --git a/chrome/browser/resources/settings/languages_page/languages.js b/chrome/browser/resources/settings/languages_page/languages.js
index 077ad08d2224fe6cfc59aba5f74bd08baddc35b3..63e518971e019aefac6cc73bb59fc5e1935d9a00 100644
--- a/chrome/browser/resources/settings/languages_page/languages.js
+++ b/chrome/browser/resources/settings/languages_page/languages.js
@@ -3,29 +3,13 @@
 // found in the LICENSE file.
 
 /**
- * @fileoverview 'settings-languages' provides convenient access to
- * Chrome's language and input method settings.
- *
- * Instances of this element have a 'languages' property, which reflects the
- * current language settings. The 'languages' property is read-only, meaning
- * hosts using this element cannot change it directly. Instead, changes to
- * language settings should be made using the LanguageHelperImpl singleton.
- *
- * Use upward binding syntax to propagate changes from child to host, so that
- * changes made internally to 'languages' propagate to your host element:
- *
- *     <template>
- *       <settings-languages languages="{{languages}}">
- *       </settings-languages>
- *       <div>[[languages.someProperty]]</div>
- *     </template>
+ * @fileoverview 'settings-languages' handles Chrome's language and input
+ * method settings. The 'languages' property, which reflects the current
+ * language settings, must not be changed directly. Instead, changes to
+ * language settings should be made using settings.LanguageHelperImpl.
  */
 
-var SettingsLanguagesSingletonElement;
-
-cr.exportPath('languageSettings');
-
-(function() {
+cr.define('settings', function() {
 'use strict';

[michaelpg, 2016/08/18 03:05:28]

Everything inside this function will be indented (see previous patchset). But it
completely destroys the diff, so please review with this diff.

[stevenjb, 2016/08/18 16:22:03]

Thank you!

[michaelpg, 2016/08/18 22:32:27]

I've further decided to leave it this way -- the horrible diff and
every-line-blame isn't worth the consistency, since it was incorrect before
anyway.

 
 // Translate server treats some language codes the same.
@@ -51,13 +35,11 @@ var preferredLanguagesPrefName = cr.isChromeOS ?
 
 /**
  * Singleton element that generates the languages model on start-up and
- * updates it whenever Chrome's pref store and other settings change. These
- * updates propagate to each <settings-language> instance so that their
- * 'languages' property updates like any other Polymer property.
+ * updates it whenever Chrome's pref store and other settings change.
  * @implements {LanguageHelper}
  */
-SettingsLanguagesSingletonElement = Polymer({
-  is: 'settings-languages-singleton',
+var SettingsLanguagesElement = Polymer({
+  is: 'settings-languages',
 
   behaviors: [PrefsBehavior],
 
@@ -68,6 +50,7 @@ SettingsLanguagesSingletonElement = Polymer({
     languages: {
       type: Object,
       notify: true,
+      readOnly: true,
     },
 
     /**
@@ -94,32 +77,45 @@ SettingsLanguagesSingletonElement = Polymer({
 
     /** @type {!InputMethodPrivate} */
     inputMethodPrivate: Object,
-  },
 
-  /**
-   * Hash map of supported languages by language codes for fast lookup.
-   * @private {!Map<string, !chrome.languageSettingsPrivate.Language>}
-   */
-  supportedLanguageMap_: new Map(),
+    /**
+     * Hash map of supported languages by language codes for fast lookup.
+     * @private {!Map<string, !chrome.languageSettingsPrivate.Language>}
+     */
+    supportedLanguageMap_: {
+      type: Object,
+      value: function() { return new Map(); },
+    },
 
-  /**
-   * Hash set of enabled language codes for membership testing.
-   * @private {!Set<string>}
-   */
-  enabledLanguageSet_: new Set(),
+    /**
+     * Hash set of enabled language codes for membership testing.
+     * @private {!Set<string>}
+     */
+    enabledLanguageSet_: {
+      type: Object,
+      value: function() { return new Set(); },
+    },
 
-  /**
-   * Hash map of supported input methods by ID for fast lookup.
-   * @private {!Map<string, chrome.languageSettingsPrivate.InputMethod>}
-   */
-  supportedInputMethodMap_: new Map(),
+    /**
+     * Hash map of supported input methods by ID for fast lookup.
+     * @private {!Map<string, chrome.languageSettingsPrivate.InputMethod>}
+     */
+    supportedInputMethodMap_: {
+      type: Object,
+      value: function() { return new Map(); },
+    },
 
-  /**
-   * Hash map of input methods supported for each language.
-   * @type {!Map<string, !Array<!chrome.languageSettingsPrivate.InputMethod>>}
-   * @private
-   */
-  languageInputMethods_: new Map(),
+    /**
+     * Hash map of input methods supported for each language.
+     * @type {!Map<string,
+     *             !Array<!chrome.languageSettingsPrivate.InputMethod>>}
+     * @private
+     */
+    languageInputMethods_: {
+      type: Object,
+      value: function() { return new Map(); },
+    },
+  },
 
   observers: [
     'preferredLanguagesPrefChanged_(' +
@@ -139,12 +135,16 @@ SettingsLanguagesSingletonElement = Polymer({
 
   /** @override */
   created: function() {
+    // This element can now be used through the LanguageHelper API.
+    SettingsLanguagesElement.instance_ = /** @type {!LanguageHelper} */(this);
+
     this.languageSettingsPrivate =
-        languageSettings.languageSettingsPrivateApiForTest ||
-        /** @type {!LanguageSettingsPrivate} */(chrome.languageSettingsPrivate);
+        settings.languageSettingsPrivateApiForTest ||
+        /** @type {!LanguageSettingsPrivate} */(
+            chrome.languageSettingsPrivate);
 
     this.inputMethodPrivate =
-        languageSettings.inputMethodPrivateApiForTest ||
+        settings.inputMethodPrivateApiForTest ||
         /** @type {!InputMethodPrivate} */(chrome.inputMethodPrivate);
 
     var promises = [];
@@ -266,8 +266,8 @@ SettingsLanguagesSingletonElement = Polymer({
         inputMethod.enabled = !!inputMethod.enabled;
         // Add the input method to the map of IDs.
         this.supportedInputMethodMap_.set(inputMethod.id, inputMethod);
-        // Add the input method to the list of input methods for each language
-        // it supports.
+        // Add the input method to the list of input methods for each
+        // language it supports.
         for (var languageCode of inputMethod.languageCodes) {
           if (!this.supportedLanguageMap_.has(languageCode))
             continue;
@@ -280,7 +280,9 @@ SettingsLanguagesSingletonElement = Polymer({
     }
 
     // Create a list of enabled languages from the supported languages.
-    var enabledLanguageStates = this.getEnabledLanguageStates_(translateTarget);
+    var enabledLanguageStates =
+        this.getEnabledLanguageStates_(translateTarget);
+
     // Populate the hash set of enabled languages.
     for (var languageState of enabledLanguageStates)
       this.enabledLanguageSet_.add(languageState.language.code);
@@ -299,12 +301,12 @@ SettingsLanguagesSingletonElement = Polymer({
     }
 
     // Initialize the Polymer languages model.
-    this.languages = model;
+    this._setLanguages(model);
   },
 
   /**
-   * Returns a list of LanguageStates for each enabled language in the supported
-   * languages list.
+   * Returns a list of LanguageStates for each enabled language in the
+   * supported languages list.
    * @param {string} translateTarget Language code of the default translate
    *     target language.
    * @return {!Array<!LanguageState>}
@@ -333,8 +335,8 @@ SettingsLanguagesSingletonElement = Polymer({
       var languageState = /** @type {LanguageState} */({});
       languageState.language = language;
       languageState.spellCheckEnabled = !!spellCheckSet.has(code);
-      // Translate is considered disabled if this language maps to any translate
-      // language that is blocked.
+      // Translate is considered disabled if this language maps to any
+      // translate language that is blocked.
       var translateCode = this.convertLanguageCodeForTranslate(code);
       languageState.translateEnabled = !!language.supportsTranslate &&
           !translateBlockedSet.has(translateCode) &&
@@ -374,7 +376,8 @@ SettingsLanguagesSingletonElement = Polymer({
 
     for (var i = 0; i < this.languages.inputMethods.supported.length; i++) {
       this.set('languages.inputMethods.supported.' + i + '.enabled',
-          enabledInputMethodSet.has(this.languages.inputMethods.supported[i]));
+          enabledInputMethodSet.has(
+              this.languages.inputMethods.supported[i]));
     }
     this.set('languages.inputMethods.enabled', enabledInputMethods);
   },
@@ -423,8 +426,8 @@ SettingsLanguagesSingletonElement = Polymer({
   },
 
   /**
-   * Sets the prospective UI language to the chosen language. This won't affect
-   * the actual UI language until a restart.
+   * Sets the prospective UI language to the chosen language. This won't
+   * affect the actual UI language until a restart.
    * @param {string} languageCode
    */
   setUILanguage: function(languageCode) {
@@ -487,8 +490,8 @@ SettingsLanguagesSingletonElement = Polymer({
     // Remove the language from spell check.
     this.deletePrefListItem('spellcheck.dictionaries', languageCode);
 
+    // Remove input methods that don't support any other enabled language.
     if (cr.isChromeOS) {
-      // Remove input methods that don't support any other enabled language.
       var inputMethods = this.languageInputMethods_.get(languageCode) || [];
       for (var inputMethod of inputMethods) {
         var supportsOtherEnabledLanguages = inputMethod.languageCodes.some(
@@ -537,7 +540,8 @@ SettingsLanguagesSingletonElement = Polymer({
           var otherLanguageCode = languageState.language.code;
           if (otherLanguageCode == languageCode)
             return false;
-          var inputMethods = this.languageInputMethods_.get(otherLanguageCode);
+          var inputMethods =
+              this.languageInputMethods_.get(otherLanguageCode);
           return inputMethods && inputMethods.some(function(inputMethod) {
             return this.isComponentIme(inputMethod) &&
                 this.supportedInputMethodMap_.get(inputMethod.id).enabled;
@@ -549,8 +553,8 @@ SettingsLanguagesSingletonElement = Polymer({
   /**
    * Moves the language in the list of enabled languages by the given offset.
    * @param {string} languageCode
-   * @param {number} offset Negative offset moves the language toward the front
-   *     of the list. A Positive one moves the language toward the back.
+   * @param {number} offset Negative offset moves the language toward the
+   *     front of the list. A Positive one moves the language toward the back.
    */
   moveLanguage: function(languageCode, offset) {
     if (!CrSettingsPrefs.isInitialized)
@@ -561,8 +565,10 @@ SettingsLanguagesSingletonElement = Polymer({
 
     var originalIndex = languageCodes.indexOf(languageCode);
     var newIndex = originalIndex + offset;
-    if (originalIndex == -1 || newIndex < 0 || newIndex >= languageCodes.length)
+    if (originalIndex == -1 || newIndex < 0 ||
+        newIndex >= languageCodes.length) {
       return;
+    }
 
     languageCodes.splice(originalIndex, 1);
     languageCodes.splice(newIndex, 0, languageCode);
@@ -686,7 +692,7 @@ SettingsLanguagesSingletonElement = Polymer({
   },
 
   /**
-   * param {string} languageCode
+   * @param {string} languageCode
    * @return {!Array<!chrome.languageSettingsPrivate.InputMethod>}
    */
   getInputMethodsForLanguage: function(languageCode) {
@@ -726,57 +732,15 @@ SettingsLanguagesSingletonElement = Polymer({
     this.updateEnabledInputMethods_();
   },
 });
-})();
-
-/**
- * A reference to the singleton under the guise of a LanguageHelper
- * implementation. This provides a limited API but implies the singleton
- * should not be used directly for data binding.
- */
-var LanguageHelperImpl = SettingsLanguagesSingletonElement;
-cr.addSingletonGetter(LanguageHelperImpl);
-
-/**
- * This element has a reference to the singleton, exposing the singleton's
- * |languages| model to the host of this element.
- */
-Polymer({
-  is: 'settings-languages',
 
-  properties: {
-    /**
-     * A reference to the languages model from the singleton, exposed as a
-     * read-only property so hosts can bind to it, but not change it.
-     * @type {LanguagesModel|undefined}
-     */
-    languages: {
-      type: Object,
-      notify: true,
-      readOnly: true,
-    },
-  },
-
-  ready: function() {
-    var singleton = /** @type {!SettingsLanguagesSingletonElement} */
-        (LanguageHelperImpl.getInstance());
-    singleton.whenReady().then(function() {
-      // Set the 'languages' property to reference the singleton's model.
-      this._setLanguages(singleton.languages);
-      // Listen for changes to the singleton's languages property, so we know
-      // when to notify hosts of changes to (our reference to) the property.
-      this.listen(singleton, 'languages-changed', 'singletonLanguagesChanged_');
-    }.bind(this));
-  },
-
-  /**
-   * Takes changes reported by the singleton and forwards them to the host,
-   * manually sending a change notification for our 'languages' property (since
-   * it's the same object as the singleton's property, but isn't bound by
-   * Polymer).
-   * @private
-   */
-  singletonLanguagesChanged_: function(e) {
-    // Forward the change notification to the host.
-    this.fire(e.type, e.detail, {bubbles: false});
-  },
+return {
+  LanguageHelperImpl: SettingsLanguagesElement,
+};
 });
+
+/** @return {!LanguageHelper} */
+settings.LanguageHelperImpl.getInstance = function() {
+  // <settings-languages> must have been created before this is called, so it
+  // can set itself as the LanguageHelperImpl instance.
+  return assert(settings.LanguageHelperImpl.instance_);
+};