Backed out changeset 0bd17b868a31 (bug 1335877) for causing regression bug 1344760
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Mon, 06 Mar 2017 17:03:11 +0100
changeset 375079 517c553ad64746c479456653ce11b04ab8e4977f
parent 375078 f4caa69ef5f2b363944b62225d3daa68d1687a3b
child 375080 6583496f169cd8a13c531ed16e98e8bf313eda8e
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1335877, 1344760
milestone54.0a1
backs out0bd17b868a31ce1e53b87f7a619974ad8c796f84
Backed out changeset 0bd17b868a31 (bug 1335877) for causing regression bug 1344760
browser/base/content/browser-syncui.js
browser/base/content/test/general/browser_misused_characters_in_strings.js
services/cloudsync/CloudSyncLocal.jsm
services/common/moz.build
services/common/stringbundle.js
services/common/tests/unit/test_load_modules.js
services/sync/modules/engines/clients.js
services/sync/modules/util.js
services/sync/tests/unit/test_utils_lazyStrings.js
services/sync/tests/unit/xpcshell.ini
tools/lint/eslint/modules.json
--- a/browser/base/content/browser-syncui.js
+++ b/browser/base/content/browser-syncui.js
@@ -1,14 +1,13 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
 
 if (AppConstants.MOZ_SERVICES_CLOUDSYNC) {
   XPCOMUtils.defineLazyModuleGetter(this, "CloudSync",
                                     "resource://gre/modules/CloudSync.jsm");
 }
 
 XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
                                   "resource://gre/modules/FxAccounts.jsm");
@@ -36,16 +35,18 @@ var gSyncUI = {
 
   _unloaded: false,
   // The last sync start time. Used to calculate the leftover animation time
   // once syncing completes (bug 1239042).
   _syncStartTime: 0,
   _syncAnimationTimer: 0,
 
   init() {
+    Cu.import("resource://services-common/stringbundle.js");
+
     // Proceed to set up the UI if Sync has already started up.
     // Otherwise we'll do it when Sync is firing up.
     if (this.weaveService.ready) {
       this.initUI();
       return;
     }
 
     // Sync isn't ready yet, but we can still update the UI with an initial
@@ -218,19 +219,18 @@ var gSyncUI = {
     this.updateUI();
   },
 
   onLogout: function SUI_onLogout() {
     this.updateUI();
   },
 
   _getAppName() {
-    let brand = Services.strings.createBundle(
-      "chrome://branding/locale/brand.properties");
-    return brand.GetStringFromName("brandShortName");
+    let brand = new StringBundle("chrome://branding/locale/brand.properties");
+    return brand.get("brandShortName");
   },
 
   // Commands
   // doSync forces a sync - it *does not* return a promise as it is called
   // via the various UI components.
   doSync() {
     this._needsSetup().then(needsSetup => {
       if (!needsSetup) {
@@ -470,18 +470,19 @@ var gSyncUI = {
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference
   ])
 };
 
 XPCOMUtils.defineLazyGetter(gSyncUI, "_stringBundle", function() {
   // XXXzpao these strings should probably be moved from /services to /browser... (bug 583381)
   //        but for now just make it work
-  return Services.strings.createBundle(
-    "chrome://weave/locale/services/sync.properties");
+  return Cc["@mozilla.org/intl/stringbundle;1"].
+         getService(Ci.nsIStringBundleService).
+         createBundle("chrome://weave/locale/services/sync.properties");
 });
 
 XPCOMUtils.defineLazyGetter(gSyncUI, "log", function() {
   return Log.repository.getLogger("browserwindow.syncui");
 });
 
 XPCOMUtils.defineLazyGetter(gSyncUI, "weaveService", function() {
   return Components.classes["@mozilla.org/weave/service;1"]
--- a/browser/base/content/test/general/browser_misused_characters_in_strings.js
+++ b/browser/base/content/test/general/browser_misused_characters_in_strings.js
@@ -187,21 +187,19 @@ function* getAllTheFiles(extension) {
 add_task(function* checkAllTheProperties() {
   // This asynchronously produces a list of URLs (sadly, mostly sync on our
   // test infrastructure because it runs against jarfiles there, and
   // our zipreader APIs are all sync)
   let uris = yield getAllTheFiles(".properties");
   ok(uris.length, `Found ${uris.length} .properties files to scan for misused characters`);
 
   for (let uri of uris) {
-    let bundle = Services.strings.createBundle(uri.spec);
-    let enumerator = bundle.getSimpleEnumeration();
-
-    while (enumerator.hasMoreElements()) {
-      let entity = enumerator.getNext().QueryInterface(Ci.nsIPropertyElement);
+    let bundle = new StringBundle(uri.spec);
+    let entities = bundle.getAll();
+    for (let entity of entities) {
       testForErrors(uri.spec, entity.key, entity.value);
     }
   }
 });
 
 var checkDTD = Task.async(function* (aURISpec) {
   let rawContents = yield fetchFile(aURISpec);
   // The regular expression below is adapted from:
--- a/services/cloudsync/CloudSyncLocal.jsm
+++ b/services/cloudsync/CloudSyncLocal.jsm
@@ -6,24 +6,24 @@
 
 this.EXPORTED_SYMBOLS = ["Local"];
 
 const Cu = Components.utils;
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://services-common/stringbundle.js");
 Cu.import("resource://services-common/utils.js");
 Cu.import("resource://services-crypto/utils.js");
 Cu.import("resource://gre/modules/Preferences.jsm");
 
 function lazyStrings(name) {
-  return () => Services.strings.createBundle(
-    `chrome://weave/locale/services//${name}.properties`);
+  let bundle = "chrome://weave/locale/services/" + name + ".properties";
+  return () => new StringBundle(bundle);
 }
 
 this.Str = {};
 XPCOMUtils.defineLazyGetter(Str, "errors", lazyStrings("errors"));
 XPCOMUtils.defineLazyGetter(Str, "sync", lazyStrings("sync"));
 
 function makeGUID() {
   return CommonUtils.encodeBase64URL(CryptoUtils.generateRandomBytes(9));
@@ -53,36 +53,35 @@ Local.prototype = {
       return clientName;
     }
 
     // Generate a client name if we don't have a useful one yet
     let env = Cc["@mozilla.org/process/environment;1"]
                 .getService(Ci.nsIEnvironment);
     let user = env.get("USER") || env.get("USERNAME");
     let appName;
-    let brand = Services.strings.createBundle(
-      "chrome://branding/locale/brand.properties");
-    let brandName = brand.GetStringFromName("brandShortName");
+    let brand = new StringBundle("chrome://branding/locale/brand.properties");
+    let brandName = brand.get("brandShortName");
 
     try {
-      let syncStrings = Services.strings.createBundle("chrome://browser/locale/sync.properties");
-      appName = syncStrings.formatStringFromName("sync.defaultAccountApplication", [brandName], 1);
+      let syncStrings = new StringBundle("chrome://browser/locale/sync.properties");
+      appName = syncStrings.getFormattedString("sync.defaultAccountApplication", [brandName]);
     } catch (ex) {
     }
 
     appName = appName || brandName;
 
     let system =
       // 'device' is defined on unix systems
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("device") ||
       // hostname of the system, usually assigned by the user or admin
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("host") ||
       // fall back on ua info string
       Cc["@mozilla.org/network/protocol;1?name=http"].getService(Ci.nsIHttpProtocolHandler).oscpu;
 
-    return this.name = Str.sync.formatStringFromName("client.name2", [user, appName, system], 3);
+    return this.name = Str.sync.get("client.name2", [user, appName, system]);
   },
 
   set name(value) {
     this.prefs.set("client.name", value);
   },
 };
 
--- a/services/common/moz.build
+++ b/services/common/moz.build
@@ -18,16 +18,17 @@ EXTRA_JS_MODULES['services-common'] += [
     'blocklist-clients.js',
     'blocklist-updater.js',
     'kinto-http-client.js',
     'kinto-offline-client.js',
     'kinto-storage-adapter.js',
     'logmanager.js',
     'observers.js',
     'rest.js',
+    'stringbundle.js',
     'utils.js',
 ]
 
 if CONFIG['MOZ_WIDGET_TOOLKIT'] != 'android':
     EXTRA_JS_MODULES['services-common'] += [
         'hawkclient.js',
         'hawkrequest.js',
         'tokenserverclient.js',
new file mode 100644
--- /dev/null
+++ b/services/common/stringbundle.js
@@ -0,0 +1,201 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+this.EXPORTED_SYMBOLS = ["StringBundle"];
+
+var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
+
+/**
+ * A string bundle.
+ *
+ * This object presents two APIs: a deprecated one that is equivalent to the API
+ * for the stringbundle XBL binding, to make it easy to switch from that binding
+ * to this module, and a new one that is simpler and easier to use.
+ *
+ * The benefit of this module over the XBL binding is that it can also be used
+ * in JavaScript modules and components, not only in chrome JS.
+ *
+ * To use this module, import it, create a new instance of StringBundle,
+ * and then use the instance's |get| and |getAll| methods to retrieve strings
+ * (you can get both plain and formatted strings with |get|):
+ *
+ *   let strings =
+ *     new StringBundle("chrome://example/locale/strings.properties");
+ *   let foo = strings.get("foo");
+ *   let barFormatted = strings.get("bar", [arg1, arg2]);
+ *   for (let string of strings.getAll())
+ *     dump (string.key + " = " + string.value + "\n");
+ *
+ * @param url {String}
+ *        the URL of the string bundle
+ */
+this.StringBundle = function StringBundle(url) {
+  this.url = url;
+}
+
+StringBundle.prototype = {
+  /**
+   * the locale associated with the application
+   * @type nsILocale
+   * @private
+   */
+  get _appLocale() {
+    try {
+      return Cc["@mozilla.org/intl/nslocaleservice;1"].
+             getService(Ci.nsILocaleService).
+             getApplicationLocale();
+    } catch (ex) {
+      return null;
+    }
+  },
+
+  /**
+   * the wrapped nsIStringBundle
+   * @type nsIStringBundle
+   * @private
+   */
+  get _stringBundle() {
+    let stringBundle = Cc["@mozilla.org/intl/stringbundle;1"].
+                       getService(Ci.nsIStringBundleService).
+                       createBundle(this.url, this._appLocale);
+    this.__defineGetter__("_stringBundle", () => stringBundle);
+    return this._stringBundle;
+  },
+
+
+  // the new API
+
+  /**
+   * the URL of the string bundle
+   * @type String
+   */
+  _url: null,
+  get url() {
+    return this._url;
+  },
+  set url(newVal) {
+    this._url = newVal;
+    delete this._stringBundle;
+  },
+
+  /**
+   * Get a string from the bundle.
+   *
+   * @param key {String}
+   *        the identifier of the string to get
+   * @param args {array} [optional]
+   *        an array of arguments that replace occurrences of %S in the string
+   *
+   * @returns {String} the value of the string
+   */
+  get(key, args) {
+    if (args)
+      return this.stringBundle.formatStringFromName(key, args, args.length);
+    return this.stringBundle.GetStringFromName(key);
+  },
+
+  /**
+   * Get all the strings in the bundle.
+   *
+   * @returns {Array}
+   *          an array of objects with key and value properties
+   */
+  getAll() {
+    let strings = [];
+
+    // FIXME: for performance, return an enumerable array that wraps the string
+    // bundle's nsISimpleEnumerator (does JavaScript already support this?).
+
+    let enumerator = this.stringBundle.getSimpleEnumeration();
+
+    while (enumerator.hasMoreElements()) {
+      // We could simply return the nsIPropertyElement objects, but I think
+      // it's better to return standard JS objects that behave as consumers
+      // expect JS objects to behave (f.e. you can modify them dynamically).
+      let string = enumerator.getNext().QueryInterface(Ci.nsIPropertyElement);
+      strings.push({ key: string.key, value: string.value });
+    }
+
+    return strings;
+  },
+
+
+  // the deprecated XBL binding-compatible API
+
+  /**
+   * the URL of the string bundle
+   * @deprecated because its name doesn't make sense outside of an XBL binding
+   * @type String
+   */
+  get src() {
+    return this.url;
+  },
+  set src(newVal) {
+    this.url = newVal;
+  },
+
+  /**
+   * the locale associated with the application
+   * @deprecated because it has never been used outside the XBL binding itself,
+   * and consumers should obtain it directly from the locale service anyway.
+   * @type nsILocale
+   */
+  get appLocale() {
+    return this._appLocale;
+  },
+
+  /**
+   * the wrapped nsIStringBundle
+   * @deprecated because this module should provide all necessary functionality
+   * @type nsIStringBundle
+   *
+   * If you do ever need to use this, let the authors of this module know why
+   * so they can surface functionality for your use case in the module itself
+   * and you don't have to access this underlying XPCOM component.
+   */
+  get stringBundle() {
+    return this._stringBundle;
+  },
+
+  /**
+   * Get a string from the bundle.
+   * @deprecated use |get| instead
+   *
+   * @param key {String}
+   *        the identifier of the string to get
+   *
+   * @returns {String}
+   *          the value of the string
+   */
+  getString(key) {
+    return this.get(key);
+  },
+
+  /**
+   * Get a formatted string from the bundle.
+   * @deprecated use |get| instead
+   *
+   * @param key {string}
+   *        the identifier of the string to get
+   * @param args {array}
+   *        an array of arguments that replace occurrences of %S in the string
+   *
+   * @returns {String}
+   *          the formatted value of the string
+   */
+  getFormattedString(key, args) {
+    return this.get(key, args);
+  },
+
+  /**
+   * Get an enumeration of the strings in the bundle.
+   * @deprecated use |getAll| instead
+   *
+   * @returns {nsISimpleEnumerator}
+   *          a enumeration of the strings in the bundle
+   */
+  get strings() {
+    return this.stringBundle.getSimpleEnumeration();
+  }
+}
--- a/services/common/tests/unit/test_load_modules.js
+++ b/services/common/tests/unit/test_load_modules.js
@@ -3,16 +3,17 @@
 
 Components.utils.import("resource://gre/modules/AppConstants.jsm");
 
 const MODULE_BASE = "resource://services-common/";
 const shared_modules = [
   "async.js",
   "logmanager.js",
   "rest.js",
+  "stringbundle.js",
   "utils.js",
 ];
 
 const non_android_modules = [
   "tokenserverclient.js",
 ];
 
 const TEST_BASE = "resource://testing-common/services/common/";
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -23,16 +23,17 @@
 this.EXPORTED_SYMBOLS = [
   "ClientEngine",
   "ClientsRec"
 ];
 
 var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://services-common/async.js");
+Cu.import("resource://services-common/stringbundle.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/resource.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
@@ -168,19 +169,18 @@ ClientEngine.prototype = {
     let localID = Svc.Prefs.get("client.GUID", "");
     return localID == "" ? this.localID = Utils.makeGUID() : localID;
   },
   set localID(value) {
     Svc.Prefs.set("client.GUID", value);
   },
 
   get brandName() {
-    let brand = Services.strings.createBundle(
-      "chrome://branding/locale/brand.properties");
-    return brand.GetStringFromName("brandShortName");
+    let brand = new StringBundle("chrome://branding/locale/brand.properties");
+    return brand.get("brandShortName");
   },
 
   get localName() {
     let name = Utils.getDeviceName();
     // If `getDeviceName` returns the default name, set the pref. FxA registers
     // the device before syncing, so we don't need to update the registration
     // in this case.
     Svc.Prefs.set("client.name", name);
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 this.EXPORTED_SYMBOLS = ["XPCOMUtils", "Services", "Utils", "Async", "Svc", "Str"];
 
 var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://services-common/observers.js");
+Cu.import("resource://services-common/stringbundle.js");
 Cu.import("resource://services-common/utils.js");
 Cu.import("resource://services-common/async.js", this);
 Cu.import("resource://services-crypto/utils.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/osfile.jsm", this);
@@ -212,18 +213,18 @@ this.Utils = {
     if (!prot.__lookupSetter__(prop)) {
       prot.__defineSetter__(prop, function(val) {
         this[defer][prop] = val;
       });
     }
   },
 
   lazyStrings: function Weave_lazyStrings(name) {
-    return () => Services.strings.createBundle(
-      `chrome://weave/locale/services/${name}.properties`);
+    let bundle = "chrome://weave/locale/services/" + name + ".properties";
+    return () => new StringBundle(bundle);
   },
 
   deepEquals: function eq(a, b) {
     // If they're triple equals, then it must be equals!
     if (a === b)
       return true;
 
     // If they weren't equal, they must be objects to be different
@@ -455,25 +456,21 @@ this.Utils = {
     if (that._log) {
       that._log.trace("Deleting " + path);
     }
     return OS.File.remove(path, { ignoreAbsent: true });
   },
 
   getErrorString: function Utils_getErrorString(error, args) {
     try {
-      if (args) {
-        return Str.errors.formatStringFromName(error, args, args.length);
-      } else {
-        return Str.errors.GetStringFromName(error);
-      }
+      return Str.errors.get(error, args || null);
     } catch (e) {}
 
     // basically returns "Unknown Error"
-    return Str.errors.GetStringFromName('error.reason.unknown');
+    return Str.errors.get("error.reason.unknown");
   },
 
   /**
    * Generate 26 characters.
    */
   generatePassphrase: function generatePassphrase() {
     // Note that this is a different base32 alphabet to the one we use for
     // other tasks. It's lowercase, uses different letters, and needs to be
@@ -681,24 +678,23 @@ this.Utils = {
     let user = env.get("USER") || env.get("USERNAME") ||
                Svc.Prefs.get("account") || Svc.Prefs.get("username");
     // A little hack for people using the the moz-build environment on Windows
     // which sets USER to the literal "%USERNAME%" (yes, really)
     if (user == "%USERNAME%" && env.get("USERNAME")) {
       user = env.get("USERNAME");
     }
 
-    let brand = Services.strings.createBundle(
-      "chrome://branding/locale/brand.properties");
-    let brandName = brand.GetStringFromName("brandShortName");
+    let brand = new StringBundle("chrome://branding/locale/brand.properties");
+    let brandName = brand.get("brandShortName");
 
     let appName;
     try {
-      let syncStrings = Services.strings.createBundle("chrome://browser/locale/sync.properties");
-      appName = syncStrings.formatStringFromName("sync.defaultAccountApplication", [brandName], 1);
+      let syncStrings = new StringBundle("chrome://browser/locale/sync.properties");
+      appName = syncStrings.getFormattedString("sync.defaultAccountApplication", [brandName]);
     } catch (ex) {}
     appName = appName || brandName;
 
     let system =
       // 'device' is defined on unix systems
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("device") ||
       // hostname of the system, usually assigned by the user or admin
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("host") ||
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_utils_lazyStrings.js
@@ -0,0 +1,14 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://services-common/stringbundle.js");
+Cu.import("resource://services-sync/util.js");
+
+function run_test() {
+    let fn = Utils.lazyStrings("sync");
+    do_check_eq(typeof fn, "function");
+    let bundle = fn();
+    do_check_true(bundle instanceof StringBundle);
+    let url = bundle.url;
+    do_check_eq(url, "chrome://weave/locale/services/sync.properties");
+}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -25,16 +25,17 @@ support-files =
 # util contains a bunch of functionality used throughout.
 [test_utils_catch.js]
 [test_utils_deepEquals.js]
 [test_utils_deferGetSet.js]
 [test_utils_deriveKey.js]
 [test_utils_keyEncoding.js]
 [test_utils_getErrorString.js]
 [test_utils_json.js]
+[test_utils_lazyStrings.js]
 [test_utils_lock.js]
 [test_utils_makeGUID.js]
 [test_utils_notify.js]
 [test_utils_passphrase.js]
 
 # We have a number of other libraries that are pretty much standalone.
 [test_addon_utils.js]
 run-sequentially = Restarts server, can't change pref.
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -200,16 +200,17 @@
   "ShutdownLeaksCollector.jsm": ["ContentCollector"],
   "SignInToWebsite.jsm": ["SignInToWebsiteController"],
   "Social.jsm": ["Social", "OpenGraphBuilder", "DynamicResizeWatcher", "sizeSocialPanelToContent"],
   "SpecialPowersObserver.jsm": ["SpecialPowersObserver", "SpecialPowersObserverFactory"],
   "stack.js": ["findCallerFrame"],
   "StateMachineHelper.jsm": ["State", "CommandType"],
   "status.js": ["Status"],
   "storageserver.js": ["ServerBSO", "StorageServerCallback", "StorageServerCollection", "StorageServer", "storageServerForUsers"],
+  "stringbundle.js": ["StringBundle"],
   "strings.js": ["trim", "vslice"],
   "StructuredLog.jsm": ["StructuredLogger", "StructuredFormatter"],
   "StyleEditorUtil.jsm": ["getString", "assert", "log", "text", "wire", "showFilePicker"],
   "subprocess_common.jsm": ["BaseProcess", "PromiseWorker", "SubprocessConstants"],
   "subprocess_unix.jsm": ["SubprocessImpl"],
   "subprocess_win.jsm": ["SubprocessImpl"],
   "sync.jsm": ["Authentication"],
   "tabs.js": ["TabEngine", "TabSetRecord"],