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 346052 517c553ad647
parent 346051 f4caa69ef5f2
child 346053 6583496f169c
child 346088 940c4595cfeb
child 346210 7b87ddd1f8fd
push id31456
push usercbook@mozilla.com
push dateMon, 06 Mar 2017 16:03:28 +0000
treeherdermozilla-central@517c553ad647 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1335877, 1344760
milestone54.0a1
backs out0bd17b868a31
first release with
nightly mac
517c553ad647 / 54.0a1 / 20170306080637 / files
nightly win32
517c553ad647 / 54.0a1 / 20170306080637 / files
nightly win64
517c553ad647 / 54.0a1 / 20170306080637 / files
nightly linux32
nightly linux64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly mac
nightly win32
nightly win64
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"],