Extract language settings methods into a LanguageHelper interface

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

 // Copyright 2015 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-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 this element's public functions.
+ * language settings should be made using the LanguageHelperImpl singleton.
  *
- * Use two-way binding syntax to propagate changes from child to host, so that
+ * 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>
  *
  * @group Chrome Settings Elements
  * @element settings-languages
  */
 
-/** @typedef {{spellCheckEnabled: boolean, translateEnabled: boolean}} */
-var LanguageState;
-
-/**
- * @typedef {{language: !chrome.languageSettingsPrivate.Language,
- *            state: !LanguageState}}
- */
-var LanguageInfo;
-
-/**
- * supportedLanguages: an array of languages, ordered alphabetically.
- * enabledLanguages: an array of enabled language info and state, ordered by
- *     preference.
- * @typedef {{
- *      supportedLanguages: !Array<!chrome.languageSettingsPrivate.Language>,
- *      enabledLanguages: !Array<!LanguageInfo>,
- *      translateTarget: string
- *  }}
- */
-var LanguagesModel;
+var SettingsLanguagesSingletonElement;

[michaelpg, 2015/11/25 06:20:51]

Warning: the last patch had a bug where LanguageHelper was defined inside an
IIFE. This is a hacky fix: end the IIFE before defining LanguageHelper, and
declare SettingsLanguagesSingletonElement outside the IIFE.

Alternatives:
* don't use an IIFE, just have the constants below in the global scope
* declare LanguageHelper here instead of SettingsLanguagesSingletonElement, then
set it to SettingsLanguagesSingletonElement in the IIFE (meh)
* define LanguageHelper on a namespace like settings (back to where we
started...)

[Dan Beam, 2015/12/01 04:17:44]

this is fine

 
 (function() {
 'use strict';
 
 // Translate server treats some language codes the same.
 // See also: components/translate/core/common/translate_util.cc.
 var kLanguageCodeToTranslateCode = {
   'nb': 'no',
   'fil': 'tl',
   'zh-HK': 'zh-TW',
   'zh-MO': 'zh-TW',
   'zh-SG': 'zh-CN',
 };
 
 // Some ISO 639 language codes have been renamed, e.g. "he" to "iw", but
 // Translate still uses the old versions. TODO(michaelpg): Chrome does too.
 // Follow up with Translate owners to understand the right thing to do.
 var kTranslateLanguageSynonyms = {
   'he': 'iw',
   'jv': 'jw',
 };
 
-/**
- * This element has a reference to the singleton, exposing the singleton's
- * language model to the host of this element as the 'languages' property.
- */
-Polymer({
-  is: 'settings-languages',
-
-  properties: {
-    /**
-     * Singleton element created at startup which provides the languages model.
-     * @type {!Element}
-     */
-    singleton_: {
-      type: Object,
-      value: document.createElement('settings-languages-singleton'),
-    },
-
-    /**
-     * 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() {
-    // Set the 'languages' property to reference the singleton's model.
-    this._setLanguages(this.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(
-        this.singleton_, 'languages-changed', 'singletonLanguagesChanged_');
-  },
-
-  /**
-   * 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});
-  },
-
-  // Forward public methods to the singleton.
-
-  /** @param {string} languageCode */
-  setUILanguage: function(languageCode) {
-    if (cr.isWindows || cr.isChromeOS)
-      this.singleton_.setUILanguage(languageCode);
-  },
-
-  resetUILanguage: function() {
-    if (cr.isWindows || cr.isChromeOS)
-      this.singleton_.resetUILanguage();
-  },
-
-  /** @return {string} */
-  getProspectiveUILanguage: function() {
-    return this.singleton_.getProspectiveUILanguage();
-  },
-
-  /** @param {string} languageCode */
-  enableLanguage: function(languageCode) {
-    this.singleton_.enableLanguage(languageCode);
-  },
-
-  /** @param {string} languageCode */
-  disableLanguage: function(languageCode) {
-    this.singleton_.disableLanguage(languageCode);
-  },
-
-  /** @param {string} languageCode */
-  enableTranslateLanguage: function(languageCode) {
-    this.singleton_.enableTranslateLanguage(languageCode);
-  },
-
-  /** @param {string} languageCode */
-  disableTranslateLanguage: function(languageCode) {
-    this.singleton_.disableTranslateLanguage(languageCode);
-  },
-
-  /**
-   * @param {string} languageCode
-   * @return {boolean}
-   */
-  isEnabled: function(languageCode) {
-    return this.singleton_.isEnabled(languageCode);
-  },
-
-  /**
-   * @param {string} languageCode
-   * @param {boolean} enable
-   */
-  toggleSpellCheck: function(languageCode, enable) {
-    this.singleton_.toggleSpellCheck(languageCode, enable);
-  },
-
-  /**
-   * @param {string} languageCode
-   * @return {string}
-   */
-  convertLanguageCodeForTranslate: function(languageCode) {
-    return this.singleton_.convertLanguageCodeForTranslate(languageCode);
-  },
-});
-
 var preferredLanguagesPrefName = cr.isChromeOS ?
     'settings.language.preferred_languages' : 'intl.accept_languages';
 
 /**
- * Singleton element created when settings-languages is registered.
- * 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.
+ * 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.
+ * @implements {LanguageHelper}
  */
-Polymer({
+SettingsLanguagesSingletonElement = Polymer({
   is: 'settings-languages-singleton',
 
   behaviors: [PrefsBehavior],
 
   properties: {
     /**
      * @type {LanguagesModel|undefined}
      */
     languages: {
       type: Object,
@@ skipping 57 matching lines @@
               resolve();
             });
       }),
     ]).then(function() {
       this.createModel_(languageList, translateTarget);
       this.initialized_ = true;
     }.bind(this));
   },
 
   /**
+   * Updates the list of enabled languages from the preferred languages pref.
+   * @private
+   */
+  preferredLanguagesPrefChanged_: function() {
+    if (!this.initialized_)
+      return;

[Dan Beam, 2015/12/01 04:17:44]

nit: \n

[michaelpg, 2015/12/02 21:06:05]

Done.

+    var enabledLanguages =
+        this.getEnabledLanguages_(this.languages.translateTarget);
+
+    // Reset the enabled language map before updating
+    // languages.enabledLanguages.
+    this.enabledLanguageMap_ = {};
+    for (var i = 0; i < enabledLanguages.length; i++) {
+      var languageInfo = enabledLanguages[i];
+      this.enabledLanguageMap_[languageInfo.language.code] = languageInfo;
+    }
+    this.set('languages.enabledLanguages', enabledLanguages);
+  },
+
+  /**
+   * Updates the spellCheckEnabled state of each enabled language.
+   * @private
+   */
+  spellCheckDictionariesPrefChanged_: function() {
+    if (!this.initialized_)
+      return;
+
+    var spellCheckMap = this.makeMapFromArray_(/** @type {!Array<string>} */(
+        this.getPref('spellcheck.dictionaries').value));
+    for (var i = 0; i < this.languages.enabledLanguages.length; i++) {
+      var languageCode = this.languages.enabledLanguages[i].language.code;
+      this.set('languages.enabledLanguages.' + i + '.state.spellCheckEnabled',
+               !!spellCheckMap[languageCode]);
+    }
+  },
+
+  /** @private */
+  translateLanguagesPrefChanged_: function() {
+    if (!this.initialized_)
+      return;
+
+    var translateBlockedPref = this.getPref('translate_blocked_languages');
+    var translateBlockedMap = this.makeMapFromArray_(
+        /** @type {!Array<string>} */(translateBlockedPref.value));
+
+    for (var i = 0; i < this.languages.enabledLanguages.length; i++) {
+      var translateCode = this.convertLanguageCodeForTranslate(
+          this.languages.enabledLanguages[i].language.code);
+      this.set(
+          'languages.enabledLanguages.' + i + '.state.translateEnabled',
+          !translateBlockedMap[translateCode]);
+    }
+  },
+
+  /**
    * Constructs the languages model.
    * @param {!Array<!chrome.languageSettingsPrivate.Language>}
    *     supportedLanguages
    * @param {string} translateTarget Language code of the default translate
    *     target language.
+   * @private
    */
   createModel_: function(supportedLanguages, translateTarget) {
     // Populate the hash map of supported languages.
     for (var i = 0; i < supportedLanguages.length; i++) {
       var language = supportedLanguages[i];
       language.supportsUI = !!language.supportsUI;
       language.supportsTranslate = !!language.supportsTranslate;
       language.supportsSpellcheck = !!language.supportsSpellcheck;
       this.supportedLanguageMap_[language.code] = language;
     }
@@ skipping 33 matching lines @@
     var spellCheckMap = this.makeMapFromArray_(/** @type {!Array<string>} */(
         spellCheckPref.value));
 
     var translateBlockedPref = this.getPref('translate_blocked_languages');
     var translateBlockedMap = this.makeMapFromArray_(
         /** @type {!Array<string>} */(translateBlockedPref.value));
 
     for (var i = 0; i < enabledLanguageCodes.length; i++) {
       var code = enabledLanguageCodes[i];
       var language = this.supportedLanguageMap_[code];
+      // Skip unsupported languages.
       if (!language)
         continue;
-      var state = {};
+      var state = /** @type {LanguageState} */({});
       state.spellCheckEnabled = !!spellCheckMap[code];
       // Translate is considered disabled if this language maps to any translate
       // language that is blocked.
       var translateCode = this.convertLanguageCodeForTranslate(code);
-      state.translateEnabled = language.supportsTranslate &&
+      state.translateEnabled = !!language.supportsTranslate &&
           !translateBlockedMap[translateCode] &&
           translateCode != translateTarget;
-      enabledLanguages.push({language: language, state: state});
+      enabledLanguages.push(/** @type {LanguageInfo} */(
+          {language: language, state: state}));
     }
     return enabledLanguages;
   },
 
   /**
    * Creates an object whose keys are the elements of the list.
    * @param {!Array<string>} list
    * @return {!Object<boolean>}
+   * @private
    */
   makeMapFromArray_: function(list) {
     var map = {};
     for (var i = 0; i < list.length; i++)
       map[list[i]] = true;
     return map;
   },
 
+  // LanguageHelper implementation.
+  // TODO(michaelpg): replace duplicate docs with @override once b/24294625
+  // is fixed.
+
+<if expr="chromeos or is_win">
   /**
-   * Updates the list of enabled languages from the preferred languages pref.
-   * @private
-   * */
-  preferredLanguagesPrefChanged_: function() {
-    if (!this.initialized_)
-      return;
-
-    var enabledLanguages =
-        this.getEnabledLanguages_(this.languages.translateTarget);
-    // Reset the enabled language map. Do this before notifying of the change
-    // via languages.enabledLanguages.
-    this.enabledLanguageMap_ = {};
-    for (var i = 0; i < enabledLanguages.length; i++) {
-      var languageInfo = enabledLanguages[i];
-      this.enabledLanguageMap_[languageInfo.language.code] = languageInfo;
-    }
-    this.set('languages.enabledLanguages', enabledLanguages);
-  },
-
-  /**
-   * Updates the spellCheckEnabled state of each enabled language.
-   * @private
-   */
-  spellCheckDictionariesPrefChanged_: function() {
-    if (!this.initialized_)
-      return;
-
-    var spellCheckMap = this.makeMapFromArray_(/** @type {!Array<string>} */(
-        this.getPref('spellcheck.dictionaries').value));
-    for (var i = 0; i < this.languages.enabledLanguages.length; i++) {
-      var languageCode = this.languages.enabledLanguages[i].language.code;
-      this.set('languages.enabledLanguages.' + i + '.state.spellCheckEnabled',
-               !!spellCheckMap[languageCode]);
-    }
-  },
-
-  translateLanguagesPrefChanged_: function() {
-    if (!this.initialized_)
-      return;
-
-    var translateBlockedPref = this.getPref('translate_blocked_languages');
-    var translateBlockedMap = this.makeMapFromArray_(
-        /** @type {!Array<string>} */(translateBlockedPref.value));
-
-    for (var i = 0; i < this.languages.enabledLanguages.length; i++) {
-      var translateCode = this.convertLanguageCodeForTranslate(
-          this.languages.enabledLanguages[i].language.code);
-      this.set(
-          'languages.enabledLanguages.' + i + '.state.translateEnabled',
-          !translateBlockedMap[translateCode]);
-    }
-  },
-
-  /**
-   * Deletes the given item from the pref at the given key if the item is found.
-   * Asserts if the pref itself is not found or is not an Array type.
-   * @param {string} key
-   * @param {*} item
-   */
-  deletePrefItem_: function(key, item) {
-    assert(this.getPref(key).type == chrome.settingsPrivate.PrefType.LIST);
-    this.arrayDelete('prefs.' + key + '.value', item);
-  },
-
-  /**
-   * Windows and Chrome OS only: Sets the prospective UI language to the chosen
-   * language. This dosen'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) {
     chrome.send('setUILanguage', [languageCode]);
   },
 
-  /**
-   * Windows and Chrome OS only: Resets the prospective UI language back to the
-   * actual UI language.
-   */
+  /** Resets the prospective UI language back to the actual UI language. */
   resetUILanguage: function() {
     chrome.send('setUILanguage', [navigator.language]);
   },
 
   /**
    * Returns the "prospective" UI language, i.e. the one to be used on next
    * restart. If the pref is not set, the current UI language is also the
    * "prospective" language.
    * @return {string} Language code of the prospective UI language.
-   * @private
    */
   getProspectiveUILanguage: function() {
     return /** @type {string} */(this.getPref('intl.app_locale').value) ||
         navigator.language;
   },
+</if>
+
+  /**
+   * @param {string} languageCode
+   * @return {boolean} True if the language is enabled.
+   */
+  isLanguageEnabled: function(languageCode) {
+    return !!this.enabledLanguageMap_[languageCode];
+  },
 
   /**
    * Enables the language, making it available for spell check and input.
    * @param {string} languageCode
    */
   enableLanguage: function(languageCode) {
     if (!CrSettingsPrefs.isInitialized)
       return;
 
     var languageCodes =
         this.getPref(preferredLanguagesPrefName).value.split(',');
     if (languageCodes.indexOf(languageCode) > -1)
       return;
     languageCodes.push(languageCode);
     chrome.languageSettingsPrivate.setLanguageList(languageCodes);
     this.disableTranslateLanguage(languageCode);
   },
 
   /**
    * Disables the language.
    * @param {string} languageCode
    */
   disableLanguage: function(languageCode) {
     if (!CrSettingsPrefs.isInitialized)
       return;
 
-    // Cannot disable the UI language.
-    assert(languageCode != this.getProspectiveUILanguage());
+    assert(this.canDisableLanguage(languageCode));
 
-    // Cannot disable the only enabled language.
+    // Remove the language from spell check.
+    this.arrayDelete('prefs.spellcheck.dictionaries.value', languageCode);
+
+    // Remove the language from preferred languages.
     var languageCodes =
         this.getPref(preferredLanguagesPrefName).value.split(',');
-    assert(languageCodes.length > 1);
-
-    // Remove the language from spell check.
-    this.deletePrefItem_('spellcheck.dictionaries', languageCode);
-
     var languageIndex = languageCodes.indexOf(languageCode);
     if (languageIndex == -1)
       return;
     languageCodes.splice(languageIndex, 1);
     chrome.languageSettingsPrivate.setLanguageList(languageCodes);
     this.enableTranslateLanguage(languageCode);
   },
 
   /**
-   * @param {string} languageCode
-   * @return {boolean} True if the language is enabled.
+   * @param {string} languageCode Language code for an enabled language.
+   * @return {boolean}
    */
-  isEnabled: function(languageCode) {
-    return !!this.enabledLanguageMap_[languageCode];
+  canDisableLanguage: function(languageCode) {
+    // Cannot disable the prospective UI language.
+    if ((cr.isChromeOS || cr.isWindows) &&
+        languageCode == this.getProspectiveUILanguage()) {
+      return false;
+    }
+
+    // Cannot disable the only enabled language.
+    if (this.languages.enabledLanguages.length == 1)
+      return false;
+
+    return true;
   },
 
   /**
    * Enables translate for the given language by removing the translate
    * language from the blocked languages preference.
    * @param {string} languageCode
    */
   enableTranslateLanguage: function(languageCode) {
     languageCode = this.convertLanguageCodeForTranslate(languageCode);
     this.arrayDelete('prefs.translate_blocked_languages.value', languageCode);
@@ skipping 29 matching lines @@
       this.arrayDelete('prefs.spellcheck.dictionaries.value', languageCode);
     }
   },
 
   /**
    * Converts the language code for translate. There are some differences
    * between the language set the Translate server uses and that for
    * Accept-Language.
    * @param {string} languageCode
    * @return {string} The converted language code.
-   * @private
    */
   convertLanguageCodeForTranslate: function(languageCode) {
     if (languageCode in kLanguageCodeToTranslateCode)
       return kLanguageCodeToTranslateCode[languageCode];
 
     var main = languageCode.split('-')[0];
     if (main == 'zh') {
       // In Translate, general Chinese is not used, and the sub code is
       // necessary as a language code for the Translate server.
       return languageCode;
     }
     if (main in kTranslateLanguageSynonyms)
       return kTranslateLanguageSynonyms[main];
 
     return main;
   },
+
+  /**
+   * @param {string} languageCode
+   * @return {!chrome.languageSettingsPrivate.Language|undefined}
+   */
+  getLanguage: function(languageCode) {
+    return this.supportedLanguageMap_[languageCode];
+  },
 });
 })();
+
+/**
+ * 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;

[Dan Beam, 2015/12/01 04:17:44]

i still don't really understand why we need this step... it just seems to
introduce an extra name for no reason

[michaelpg, 2015/12/02 21:06:06]

It's not a dealbreaker for the CL. But the reason for the extra name is to
minimize the perceived surface area of the element when it's used by other
elements, providing the LanguageHelper interface via a LanguageHelper "pointer"
but hiding the PolymerElement interface. For example:

SettingsLanguageSingletonElement.getInstance().set('languages.enabled', [...]);

is not how the element is supposed to be used, but:

LanguageHelperImpl.getInstance().enableLanguage('ar');

works.

Also, if we decide in the future to not use this weird singleton polymer
pattern, we can still keep the interface and not have to change the other files
that use LanguageHelperImpl. (Spoiler alert: this will probably happen to some
degree.)

[Dan Beam, 2015/12/02 22:21:15]

so basically you want both names to be the most technically correct (because you
want to name it "Element" because Polymer() creates things that are instanceof
Element but also "Helper").  is this really necessary?  Like I mentioned: the
fact that Polymer() is used to implement the Helper[Impl] seems unimportant.

[michaelpg, 2015/12/02 22:32:56]

Oh, no, the reason I want to name it "Element" is because this is the type name
Closure's PolymerPass would create by default if I didn't assign the Polymer
call to a value, and I don't want to stray from that pattern. I don't
particularly care that the thing derives from Polymer.Base or whatever.

settings-languages does require the thing to be implemented as a Polymer
element, but that's internal to this file, so i'm not concerned about its name
here.

[Dan Beam, 2015/12/02 23:18:44]

but we are assigning it to a value?

+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: {
+    /**
+     * Singleton element created at startup which provides the languages model.
+     * @type {!SettingsLanguagesSingletonElement}
+     */
+    singleton_: {
+      type: Object,
+      value: LanguageHelperImpl.getInstance(),
+    },
+
+    /**
+     * 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() {
+    // Set the 'languages' property to reference the singleton's model.
+    this._setLanguages(this.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(
+        this.singleton_, 'languages-changed', 'singletonLanguagesChanged_');
+  },
+
+  /**
+   * 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});
+  },
+});