Bug 1631593 - Cache bundles in Localization C++. r=jfkthame,smaug

Differential Revision: https://phabricator.services.mozilla.com/D71815

dom/l10n/DOMLocalization.cpp

@@ -63,7 +63,9 @@ JSObject* DOMLocalization::WrapObject(JSContext* aCx,
   return DOMLocalization_Binding::Wrap(aCx, this, aGivenProto);
 }
 
-DOMLocalization::~DOMLocalization() { DisconnectMutations(); }
+void DOMLocalization::Destroy() { DisconnectMutations(); }
+
+DOMLocalization::~DOMLocalization() { Destroy(); }
 
 /**
  * DOMLocalization API

dom/l10n/DOMLocalization.h

@@ -25,6 +25,8 @@ class DOMLocalization : public intl::Localization {
 
   explicit DOMLocalization(nsIGlobalObject* aGlobal);
 
+  void Destroy();
+
   static already_AddRefed<DOMLocalization> Constructor(
       const GlobalObject& aGlobal, const Sequence<nsString>& aResourceIds,
       const bool aSync, const BundleGenerator& aBundleGenerator,

intl/l10n/Localization.cpp

@@ -19,7 +19,7 @@ static const char* kObservedPrefs[] = {L10N_PSEUDO_PREF, INTL_UI_DIRECTION_PREF,
 using namespace mozilla::intl;
 using namespace mozilla::dom;
 
-NS_IMPL_CYCLE_COLLECTION_CLASS(Localization)
+NS_IMPL_CYCLE_COLLECTION_MULTI_ZONE_JSHOLDER_CLASS(Localization)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Localization)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLocalization)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mGlobal)
@@ -37,6 +37,7 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(Localization)
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mGenerateBundles)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mGenerateBundlesSync)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mBundles)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(Localization)
@@ -77,8 +78,11 @@ void Localization::Activate(const bool aSync, const bool aEager,
 
   JS::Rooted<JS::Value> generateBundlesJS(cx, mGenerateBundles);
   JS::Rooted<JS::Value> generateBundlesSyncJS(cx, mGenerateBundlesSync);
-  mLocalization->Activate(mResourceIds, mIsSync, aEager, generateBundlesJS,
+  JS::Rooted<JS::Value> bundlesJS(cx);
-                          generateBundlesSyncJS);
+  mLocalization->GenerateBundles(mResourceIds, mIsSync, aEager,
+                                 generateBundlesJS, generateBundlesSyncJS,
+                                 &bundlesJS);
+  mBundles.set(bundlesJS);
 
   RegisterObservers();
   mozilla::HoldJSObjects(this);
@@ -127,6 +131,7 @@ Localization::~Localization() {
 void Localization::Destroy() {
   mGenerateBundles.setUndefined();
   mGenerateBundlesSync.setUndefined();
+  mBundles.setUndefined();
 }
 
 /* Protected */
@@ -163,8 +168,11 @@ void Localization::OnChange() {
     AutoJSContext cx;
     JS::Rooted<JS::Value> generateBundlesJS(cx, mGenerateBundles);
     JS::Rooted<JS::Value> generateBundlesSyncJS(cx, mGenerateBundlesSync);
-    mLocalization->OnChange(mResourceIds, mIsSync, generateBundlesJS,
+    JS::Rooted<JS::Value> bundlesJS(cx);
-                            generateBundlesSyncJS);
+    mLocalization->GenerateBundles(mResourceIds, mIsSync, false,
+                                   generateBundlesJS, generateBundlesSyncJS,
+                                   &bundlesJS);
+    mBundles.set(bundlesJS);
   }
 }
 
@@ -232,7 +240,8 @@ already_AddRefed<Promise> Localization::FormatValue(
   }
 
   RefPtr<Promise> promise;
-  nsresult rv = mLocalization->FormatValue(mResourceIds, aId, args,
+  JS::Rooted<JS::Value> bundlesJS(aCx, mBundles);
+  nsresult rv = mLocalization->FormatValue(mResourceIds, bundlesJS, aId, args,
                                            getter_AddRefs(promise));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     aRv.Throw(rv);
@@ -257,7 +266,8 @@ already_AddRefed<Promise> Localization::FormatValues(
   }
 
   RefPtr<Promise> promise;
-  aRv = mLocalization->FormatValues(mResourceIds, jsKeys,
+  JS::Rooted<JS::Value> bundlesJS(aCx, mBundles);
+  aRv = mLocalization->FormatValues(mResourceIds, bundlesJS, jsKeys,
                                     getter_AddRefs(promise));
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
@@ -280,7 +290,8 @@ already_AddRefed<Promise> Localization::FormatMessages(
   }
 
   RefPtr<Promise> promise;
-  aRv = mLocalization->FormatMessages(mResourceIds, jsKeys,
+  JS::Rooted<JS::Value> bundlesJS(aCx, mBundles);
+  aRv = mLocalization->FormatMessages(mResourceIds, bundlesJS, jsKeys,
                                       getter_AddRefs(promise));
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
@@ -308,7 +319,9 @@ void Localization::FormatValueSync(JSContext* aCx, const nsACString& aId,
     args = JS::UndefinedValue();
   }
 
-  aRv = mLocalization->FormatValueSync(mResourceIds, aId, args, aRetVal);
+  JS::Rooted<JS::Value> bundlesJS(aCx, mBundles);
+  aRv = mLocalization->FormatValueSync(mResourceIds, bundlesJS, aId, args,
+                                       aRetVal);
 }
 
 void Localization::FormatValuesSync(JSContext* aCx,
@@ -331,7 +344,9 @@ void Localization::FormatValuesSync(JSContext* aCx,
     jsKeys.AppendElement(jsKey);
   }
 
-  aRv = mLocalization->FormatValuesSync(mResourceIds, jsKeys, aRetVal);
+  JS::Rooted<JS::Value> bundlesJS(aCx, mBundles);
+  aRv =
+      mLocalization->FormatValuesSync(mResourceIds, bundlesJS, jsKeys, aRetVal);
 }
 
 void Localization::FormatMessagesSync(JSContext* aCx,
@@ -357,7 +372,9 @@ void Localization::FormatMessagesSync(JSContext* aCx,
   nsTArray<JS::Value> messages;
 
   SequenceRooter<JS::Value> messagesRooter(aCx, &messages);
-  aRv = mLocalization->FormatMessagesSync(mResourceIds, jsKeys, messages);
+  JS::Rooted<JS::Value> bundlesJS(aCx, mBundles);
+  aRv = mLocalization->FormatMessagesSync(mResourceIds, bundlesJS, jsKeys,
+                                          messages);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }

intl/l10n/Localization.h

@@ -97,6 +97,8 @@ class Localization : public nsIObserver,
 
   bool mIsSync;
   nsTArray<nsString> mResourceIds;
+
+  JS::Heap<JS::Value> mBundles;
   JS::Heap<JS::Value> mGenerateBundles;
   JS::Heap<JS::Value> mGenerateBundlesSync;
 };

intl/l10n/Localization.jsm

@@ -209,42 +209,14 @@ function maybeReportErrorToGecko(error) {
  * It combines language negotiation, FluentBundle and I/O to
  * provide a scriptable API to format translations.
  */
-class Localization {
+const Localization = {
-  /**
-   * `Activate` has to be called for this object to be usable.
-   *
-   * @returns {Localization}
-   */
-  constructor() {
-    this.resourceIds = [];
-    this.generateBundles = undefined;
-    this.generateBundlesSync = undefined;
-    this.bundles = undefined;
-  }
-
-  /**
-   * Activate the instance of the `Localization` class.
-   *
-   * @param {Array<String>}    resourceIds - List of resource ids used by this
-   *                                         localization.
-   * @param {bool}                  isSync - Whether the instance should be
-   *                                         synchronous.
-   * @param {bool}                   eager - Whether the initial bundles should be
-   *                                         fetched eagerly.
-   * @param {Function}     generateBundles - Custom FluentBundle asynchronous generator.
-   * @param {Function} generateBundlesSync - Custom FluentBundle generator.
-   */
-  activate(resourceIds, isSync, eager, generateBundles, generateBundlesSync) {
-    this.regenerateBundles(resourceIds, isSync, eager, generateBundles, generateBundlesSync);
-  }
-
   cached(iterable, isSync) {
     if (isSync) {
       return CachedSyncIterable.from(iterable);
     } else {
       return CachedAsyncIterable.from(iterable);
     }
-  }
+  },
 
   /**
    * Format translations and handle fallback if needed.
@@ -255,19 +227,21 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @param   {Function}              method  - Formatting function.
    * @returns {Promise<Array<string?|Object?>>}
    * @private
    */
-  async formatWithFallback(resourceIds, keys, method) {
+  async formatWithFallback(resourceIds, bundles, keys, method) {
-    if (!this.bundles) {
+    if (!bundles) {
       throw new Error("Attempt to format on an uninitialized instance.");
     }
+
     const translations = new Array(keys.length).fill(null);
     let hasAtLeastOneBundle = false;
 
-    for await (const bundle of this.bundles) {
+    for await (const bundle of bundles) {
       hasAtLeastOneBundle = true;
       const missingIds = keysFromBundle(method, bundle, keys, translations);
 
@@ -285,7 +259,7 @@ class Localization {
     }
 
     return translations;
-  }
+  },
 
   /**
    * Format translations and handle fallback if needed.
@@ -296,20 +270,21 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @param   {Function}              method  - Formatting function.
    * @returns {Array<string|Object>}
    * @private
    */
-  formatWithFallbackSync(resourceIds, keys, method) {
+  formatWithFallbackSync(resourceIds, bundles, keys, method) {
-    if (!this.bundles) {
+    if (!bundles) {
       throw new Error("Attempt to format on an uninitialized instance.");
     }
 
     const translations = new Array(keys.length).fill(null);
     let hasAtLeastOneBundle = false;
 
-    for (const bundle of this.bundles) {
+    for (const bundle of bundles) {
       hasAtLeastOneBundle = true;
       const missingIds = keysFromBundle(method, bundle, keys, translations);
 
@@ -327,7 +302,7 @@ class Localization {
     }
 
     return translations;
-  }
+  },
 
 
   /**
@@ -354,13 +329,14 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Promise<Array<{value: string, attributes: Object}?>>}
    * @private
    */
-  formatMessages(resourceIds, keys) {
+  formatMessages(resourceIds, bundles, keys) {
-    return this.formatWithFallback(resourceIds, keys, messageFromBundle);
+    return this.formatWithFallback(resourceIds, bundles, keys, messageFromBundle);
-  }
+  },
 
   /**
    * Sync version of `formatMessages`.
@@ -369,13 +345,14 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Array<{value: string, attributes: Object}?>}
    * @private
    */
-  formatMessagesSync(resourceIds, keys) {
+  formatMessagesSync(resourceIds, bundles, keys) {
-    return this.formatWithFallbackSync(resourceIds, keys, messageFromBundle);
+    return this.formatWithFallbackSync(resourceIds, bundles, keys, messageFromBundle);
-  }
+  },
 
   /**
    * Retrieve translations corresponding to the passed keys.
@@ -395,12 +372,13 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Promise<Array<string?>>}
    */
-  formatValues(resourceIds, keys) {
+  formatValues(resourceIds, bundles, keys) {
-    return this.formatWithFallback(resourceIds, keys, valueFromBundle);
+    return this.formatWithFallback(resourceIds, bundles, keys, valueFromBundle);
-  }
+  },
 
   /**
    * Sync version of `formatValues`.
@@ -409,13 +387,14 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Array<string?>}
    * @private
    */
-  formatValuesSync(resourceIds, keys) {
+  formatValuesSync(resourceIds, bundles, keys) {
-    return this.formatWithFallbackSync(resourceIds, keys, valueFromBundle);
+    return this.formatWithFallbackSync(resourceIds, bundles, keys, valueFromBundle);
-  }
+  },
 
   /**
    * Retrieve the translation corresponding to the `id` identifier.
@@ -437,14 +416,15 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {string}                 id     - Identifier of the translation to format
    * @param   {Object}                 [args] - Optional external arguments
    * @returns {Promise<string?>}
    */
-  async formatValue(resourceIds, id, args) {
+  async formatValue(resourceIds, bundles, id, args) {
-    const [val] = await this.formatValues(resourceIds, [{id, args}]);
+    const [val] = await this.formatValues(resourceIds, bundles, [{id, args}]);
     return val;
-  }
+  },
 
   /**
    * Sync version of `formatValue`.
@@ -453,29 +433,16 @@ class Localization {
    *
    * @param   {Array<String>}     resourceIds - List of resource ids used by this
    *                                            localization.
+   * @param   {Iter<FluentBundle>}    bundles - Iterator over bundles.
    * @param   {string}                 id     - Identifier of the translation to format
    * @param   {Object}                 [args] - Optional external arguments
    * @returns {string?}
    * @private
    */
-  formatValueSync(resourceIds, id, args) {
+  formatValueSync(resourceIds, bundles, id, args) {
-    const [val] = this.formatValuesSync(resourceIds, [{id, args}]);
+    const [val] = this.formatValuesSync(resourceIds, bundles, [{id, args}]);
     return val;
-  }
+  },
-
-  /**
-   * @param {Array<String>}    resourceIds - List of resource ids used by this
-   *                                         localization.
-   * @param {bool}                  isSync - Whether the instance should be
-   *                                         synchronous.
-   * @param {Function}     generateBundles - Custom FluentBundle asynchronous generator.
-   * @param {Function} generateBundlesSync - Custom FluentBundle generator.
-   */
-  onChange(resourceIds, isSync, generateBundles, generateBundlesSync) {
-    if (this.bundles) {
-      this.regenerateBundles(resourceIds, isSync, false, generateBundles, generateBundlesSync);
-    }
-  }
 
   /**
    * This method should be called when there's a reason to believe
@@ -488,11 +455,12 @@ class Localization {
    * @param {bool}                   eager - whether the I/O for new context should begin eagerly
    * @param {Function}     generateBundles - Custom FluentBundle asynchronous generator.
    * @param {Function} generateBundlesSync - Custom FluentBundle generator.
+   * @returns {Iter<FluentBundle>}
    */
-  regenerateBundles(resourceIds, isSync, eager = false, generateBundles = defaultGenerateBundles, generateBundlesSync = defaultGenerateBundlesSync) {
+  generateBundles(resourceIds, isSync, eager = false, generateBundles = defaultGenerateBundles, generateBundlesSync = defaultGenerateBundlesSync) {
     // Store for error reporting from `formatWithFallback`.
     let generateMessages = isSync ? generateBundlesSync : generateBundles;
-    this.bundles = this.cached(generateMessages(resourceIds), isSync);
+    let bundles = this.cached(generateMessages(resourceIds), isSync);
     if (eager) {
       // If the first app locale is the same as last fallback
       // it means that we have all resources in this locale, and
@@ -502,15 +470,12 @@ class Localization {
       const appLocale = Services.locale.appLocaleAsBCP47;
       const lastFallback = Services.locale.lastFallbackLocale;
       const prefetchCount = appLocale === lastFallback ? 1 : 2;
-      this.bundles.touchNext(prefetchCount);
+      bundles.touchNext(prefetchCount);
-    }
     }
+    return bundles;
+  },
 }
 
-Localization.prototype.QueryInterface = ChromeUtils.generateQI([
-  Ci.nsISupportsWeakReference,
-]);
-
 /**
  * Format the value of a message into a string or `null`.
  *
@@ -630,13 +595,5 @@ function keysFromBundle(method, bundle, keys, translations) {
   return missingIds;
 }
 
-/**
- * Helper function which allows us to construct a new
- * Localization from Localization.
- */
-var getLocalization = () => {
-  return new Localization();
-};
-
 this.Localization = Localization;
-var EXPORTED_SYMBOLS = ["Localization", "getLocalization"];
+var EXPORTED_SYMBOLS = ["Localization"];

intl/l10n/mozILocalization.idl

@@ -29,21 +29,19 @@
 [scriptable, uuid(7d468600-551f-4fe0-98c9-92a53b63ec8d)]
 interface mozILocalization : nsISupports
 {
-  void activate(in Array<AString> aResourceIds, in bool aIsSync, in bool aEager, in jsval aGenerateBundles, in jsval aGenerateBundlesSync);
+  jsval generateBundles(in Array<AString> aResourceIds, in bool aIsSync, in bool eager, in jsval aGenerateBundles, in jsval aGenerateBundlesSync);
 
-  Promise formatMessages(in Array<AString> aResourceIds, in Array<jsval> aKeys);
+  Promise formatMessages(in Array<AString> aResourceIds, in jsval aBundles, in Array<jsval> aKeys);
-  Promise formatValues(in Array<AString> aResourceIds, in Array<jsval> aKeys);
+  Promise formatValues(in Array<AString> aResourceIds, in jsval aBundles, in Array<jsval> aKeys);
-  Promise formatValue(in Array<AString> aResourceIds, in AUTF8String aId, [optional] in jsval aArgs);
+  Promise formatValue(in Array<AString> aResourceIds, in jsval aBundles, in AUTF8String aId, [optional] in jsval aArgs);
 
-  AUTF8String formatValueSync(in Array<AString> aResourceIds, in AUTF8String aId, [optional] in jsval aArgs);
+  AUTF8String formatValueSync(in Array<AString> aResourceIds, in jsval aBundles, in AUTF8String aId, [optional] in jsval aArgs);
-  Array<AUTF8String> formatValuesSync(in Array<AString> aResourceIds, in Array<jsval> aKeys);
+  Array<AUTF8String> formatValuesSync(in Array<AString> aResourceIds, in jsval aBundles, in Array<jsval> aKeys);
-  Array<jsval> formatMessagesSync(in Array<AString> aResourceIds, in Array<jsval> aKeys);
+  Array<jsval> formatMessagesSync(in Array<AString> aResourceIds, in jsval aBundles, in Array<jsval> aKeys);
-
-  void onChange(in Array<AString> aResourceIds, in bool aIsSync, in jsval aGenerateBundles, in jsval aGenerateBundlesSync);
 };
 
 [scriptable, uuid(96632d26-1422-12e9-b1ce-9bb586acd241)]
 interface mozILocalizationJSM : nsISupports
 {
-  mozILocalization getLocalization();
+  readonly attribute mozILocalization Localization;
 };