Bug 1631593 - Move generateBundles to be stored on Localization C++. r=smaug

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

dom/base/Document.cpp

@@ -10506,6 +10506,10 @@ void Document::Destroy() {
 
   // To break cycles.
   mPreloadService.ClearAllPreloads();
+
+  if (mDocumentL10n) {
+    mDocumentL10n->Destroy();
+  }
 }
 
 void Document::RemovedFromDocShell() {

intl/l10n/Localization.cpp

@@ -25,12 +25,19 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Localization)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mGlobal)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
   NS_IMPL_CYCLE_COLLECTION_UNLINK_WEAK_REFERENCE
+  tmp->Destroy();
+  mozilla::DropJSObjects(tmp);
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(Localization)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLocalization)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mGlobal)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
-NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(Localization)
+
+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_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(Localization)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(Localization)
@@ -55,26 +62,26 @@ Localization::Localization(nsIGlobalObject* aGlobal)
 void Localization::Activate(const bool aSync, const bool aEager,
                             const BundleGenerator& aBundleGenerator) {
   AutoJSContext cx;
-  JS::Rooted<JS::Value> generateBundlesJS(cx);
-  JS::Rooted<JS::Value> generateBundlesSyncJS(cx);
 
   if (aBundleGenerator.mGenerateBundles.WasPassed()) {
     GenerateBundles& generateBundles =
         aBundleGenerator.mGenerateBundles.Value();
-    generateBundlesJS.set(JS::ObjectValue(*generateBundles.CallbackOrNull()));
+    mGenerateBundles.setObject(*generateBundles.CallbackOrNull());
   }
   if (aBundleGenerator.mGenerateBundlesSync.WasPassed()) {
     GenerateBundlesSync& generateBundlesSync =
         aBundleGenerator.mGenerateBundlesSync.Value();
-    generateBundlesSyncJS.set(
+    mGenerateBundlesSync.setObject(*generateBundlesSync.CallbackOrNull());
-        JS::ObjectValue(*generateBundlesSync.CallbackOrNull()));
   }
   mIsSync = aSync;
 
-  mLocalization->Activate(mResourceIds, aSync, aEager, generateBundlesJS,
+  JS::Rooted<JS::Value> generateBundlesJS(cx, mGenerateBundles);
+  JS::Rooted<JS::Value> generateBundlesSyncJS(cx, mGenerateBundlesSync);
+  mLocalization->Activate(mResourceIds, mIsSync, aEager, generateBundlesJS,
                           generateBundlesSyncJS);
 
   RegisterObservers();
+  mozilla::HoldJSObjects(this);
 }
 
 already_AddRefed<Localization> Localization::Constructor(
@@ -112,6 +119,14 @@ Localization::~Localization() {
   }
 
   Preferences::RemoveObservers(this, kObservedPrefs);
+
+  Destroy();
+  mozilla::DropJSObjects(this);
+}
+
+void Localization::Destroy() {
+  mGenerateBundles.setUndefined();
+  mGenerateBundlesSync.setUndefined();
 }
 
 /* Protected */
@@ -145,7 +160,11 @@ Localization::Observe(nsISupports* aSubject, const char* aTopic,
 
 void Localization::OnChange() {
   if (mLocalization) {
-    mLocalization->OnChange(mResourceIds, mIsSync);
+    AutoJSContext cx;
+    JS::Rooted<JS::Value> generateBundlesJS(cx, mGenerateBundles);
+    JS::Rooted<JS::Value> generateBundlesSyncJS(cx, mGenerateBundlesSync);
+    mLocalization->OnChange(mResourceIds, mIsSync, generateBundlesJS,
+                            generateBundlesSyncJS);
   }
 }
 
@@ -213,7 +232,8 @@ already_AddRefed<Promise> Localization::FormatValue(
   }
 
   RefPtr<Promise> promise;
-  nsresult rv = mLocalization->FormatValue(aId, args, getter_AddRefs(promise));
+  nsresult rv = mLocalization->FormatValue(mResourceIds, aId, args,
+                                           getter_AddRefs(promise));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     aRv.Throw(rv);
     return nullptr;
@@ -237,7 +257,8 @@ already_AddRefed<Promise> Localization::FormatValues(
   }
 
   RefPtr<Promise> promise;
-  aRv = mLocalization->FormatValues(jsKeys, getter_AddRefs(promise));
+  aRv = mLocalization->FormatValues(mResourceIds, jsKeys,
+                                    getter_AddRefs(promise));
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
@@ -259,7 +280,8 @@ already_AddRefed<Promise> Localization::FormatMessages(
   }
 
   RefPtr<Promise> promise;
-  aRv = mLocalization->FormatMessages(jsKeys, getter_AddRefs(promise));
+  aRv = mLocalization->FormatMessages(mResourceIds, jsKeys,
+                                      getter_AddRefs(promise));
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
@@ -286,7 +308,7 @@ void Localization::FormatValueSync(JSContext* aCx, const nsACString& aId,
     args = JS::UndefinedValue();
   }
 
-  aRv = mLocalization->FormatValueSync(aId, args, aRetVal);
+  aRv = mLocalization->FormatValueSync(mResourceIds, aId, args, aRetVal);
 }
 
 void Localization::FormatValuesSync(JSContext* aCx,
@@ -309,7 +331,7 @@ void Localization::FormatValuesSync(JSContext* aCx,
     jsKeys.AppendElement(jsKey);
   }
 
-  aRv = mLocalization->FormatValuesSync(jsKeys, aRetVal);
+  aRv = mLocalization->FormatValuesSync(mResourceIds, jsKeys, aRetVal);
 }
 
 void Localization::FormatMessagesSync(JSContext* aCx,
@@ -335,7 +357,7 @@ void Localization::FormatMessagesSync(JSContext* aCx,
   nsTArray<JS::Value> messages;
 
   SequenceRooter<JS::Value> messagesRooter(aCx, &messages);
-  aRv = mLocalization->FormatMessagesSync(jsKeys, messages);
+  aRv = mLocalization->FormatMessagesSync(mResourceIds, jsKeys, messages);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }

intl/l10n/Localization.h

@@ -36,6 +36,8 @@ class Localization : public nsIObserver,
   void Activate(const bool aSync, const bool aEager,
                 const BundleGenerator& aBundleGenerator);
 
+  void Destroy();
+
   static already_AddRefed<Localization> Constructor(
       const GlobalObject& aGlobal, const Sequence<nsString>& aResourceIds,
       const bool aSync, const BundleGenerator& aBundleGenerator,
@@ -92,8 +94,11 @@ class Localization : public nsIObserver,
 
   nsCOMPtr<nsIGlobalObject> mGlobal;
   nsCOMPtr<mozILocalization> mLocalization;
+
   bool mIsSync;
   nsTArray<nsString> mResourceIds;
+  JS::Heap<JS::Value> mGenerateBundles;
+  JS::Heap<JS::Value> mGenerateBundlesSync;
 };
 
 }  // namespace intl

intl/l10n/Localization.jsm

@@ -234,13 +234,8 @@ class Localization {
    * @param {Function}     generateBundles - Custom FluentBundle asynchronous generator.
    * @param {Function} generateBundlesSync - Custom FluentBundle generator.
    */
-  activate(resourceIds, isSync, eager, generateBundles = defaultGenerateBundles, generateBundlesSync = defaultGenerateBundlesSync) {
+  activate(resourceIds, isSync, eager, generateBundles, generateBundlesSync) {
-    if (this.bundles) {
+    this.regenerateBundles(resourceIds, isSync, eager, generateBundles, generateBundlesSync);
-      throw new Error("Attempt to initialize an already initialized instance.");
-    }
-    this.generateBundles = generateBundles;
-    this.generateBundlesSync = generateBundlesSync;
-    this.regenerateBundles(resourceIds, isSync, eager);
   }
 
   cached(iterable, isSync) {
@@ -258,12 +253,14 @@ class Localization {
    * Localization. In case of errors, fetch the next context in the
    * fallback chain.
    *
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @param   {Function}              method  - Formatting function.
    * @returns {Promise<Array<string?|Object?>>}
    * @private
    */
-  async formatWithFallback(keys, method) {
+  async formatWithFallback(resourceIds, keys, method) {
     if (!this.bundles) {
       throw new Error("Attempt to format on an uninitialized instance.");
     }
@@ -284,7 +281,7 @@ class Localization {
     }
 
     if (!hasAtLeastOneBundle) {
-      maybeReportErrorToGecko(`[fluent] Request for keys failed because no resource bundles got generated.\n keys: ${JSON.stringify(keys)}.\n resourceIds: ${JSON.stringify(this.resourceIds)}.`);
+      maybeReportErrorToGecko(`[fluent] Request for keys failed because no resource bundles got generated.\n keys: ${JSON.stringify(keys)}.\n resourceIds: ${JSON.stringify(resourceIds)}.`);
     }
 
     return translations;
@@ -297,12 +294,14 @@ class Localization {
    * Localization. In case of errors, fetch the next context in the
    * fallback chain.
    *
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @param   {Function}              method  - Formatting function.
    * @returns {Array<string|Object>}
    * @private
    */
-  formatWithFallbackSync(keys, method) {
+  formatWithFallbackSync(resourceIds, keys, method) {
     if (!this.bundles) {
       throw new Error("Attempt to format on an uninitialized instance.");
     }
@@ -324,7 +323,7 @@ class Localization {
     }
 
     if (!hasAtLeastOneBundle) {
-      maybeReportErrorToGecko(`[fluent] Request for keys failed because no resource bundles got generated.\n keys: ${JSON.stringify(keys)}.\n resourceIds: ${JSON.stringify(this.resourceIds)}.`);
+      maybeReportErrorToGecko(`[fluent] Request for keys failed because no resource bundles got generated.\n keys: ${JSON.stringify(keys)}.\n resourceIds: ${JSON.stringify(resourceIds)}.`);
     }
 
     return translations;
@@ -353,12 +352,14 @@ class Localization {
    *
    * Returns a Promise resolving to an array of the translation messages.
    *
-   * @param   {Array<Object>} keys
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
+   * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Promise<Array<{value: string, attributes: Object}?>>}
    * @private
    */
-  formatMessages(keys) {
+  formatMessages(resourceIds, keys) {
-    return this.formatWithFallback(keys, messageFromBundle);
+    return this.formatWithFallback(resourceIds, keys, messageFromBundle);
   }
 
   /**
@@ -366,12 +367,14 @@ class Localization {
    *
    * Returns an array of the translation messages.
    *
-   * @param   {Array<Object>} keys
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
+   * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Array<{value: string, attributes: Object}?>}
    * @private
    */
-  formatMessagesSync(keys) {
+  formatMessagesSync(resourceIds, keys) {
-    return this.formatWithFallbackSync(keys, messageFromBundle);
+    return this.formatWithFallbackSync(resourceIds, keys, messageFromBundle);
   }
 
   /**
@@ -390,11 +393,13 @@ class Localization {
    *
    * Returns a Promise resolving to an array of the translation strings.
    *
-   * @param   {Array<Object>} keys
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
+   * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Promise<Array<string?>>}
    */
-  formatValues(keys) {
+  formatValues(resourceIds, keys) {
-    return this.formatWithFallback(keys, valueFromBundle);
+    return this.formatWithFallback(resourceIds, keys, valueFromBundle);
   }
 
   /**
@@ -402,12 +407,14 @@ class Localization {
    *
    * Returns an array of the translation strings.
    *
-   * @param   {Array<Object>} keys
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
+   * @param   {Array<Object>}         keys    - Translation keys to format.
    * @returns {Array<string?>}
    * @private
    */
-  formatValuesSync(keys) {
+  formatValuesSync(resourceIds, keys) {
-    return this.formatWithFallbackSync(keys, valueFromBundle);
+    return this.formatWithFallbackSync(resourceIds, keys, valueFromBundle);
   }
 
   /**
@@ -428,12 +435,14 @@ class Localization {
    * retranslated when the user changes their language preferences, e.g. in
    * notifications.
    *
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
    * @param   {string}                 id     - Identifier of the translation to format
    * @param   {Object}                 [args] - Optional external arguments
    * @returns {Promise<string?>}
    */
-  async formatValue(id, args) {
+  async formatValue(resourceIds, id, args) {
-    const [val] = await this.formatValues([{id, args}]);
+    const [val] = await this.formatValues(resourceIds, [{id, args}]);
     return val;
   }
 
@@ -442,22 +451,29 @@ class Localization {
    *
    * Returns a translation string.
    *
-   * @param   {Array<Object>} keys
+   * @param   {Array<String>}     resourceIds - List of resource ids used by this
+   *                                            localization.
+   * @param   {string}                 id     - Identifier of the translation to format
+   * @param   {Object}                 [args] - Optional external arguments
    * @returns {string?}
    * @private
    */
-  formatValueSync(id, args) {
+  formatValueSync(resourceIds, id, args) {
-    const [val] = this.formatValuesSync([{id, args}]);
+    const [val] = this.formatValuesSync(resourceIds, [{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) {
+  onChange(resourceIds, isSync, generateBundles, generateBundlesSync) {
     if (this.bundles) {
-      this.regenerateBundles(resourceIds, isSync, false);
+      this.regenerateBundles(resourceIds, isSync, false, generateBundles, generateBundlesSync);
     }
   }
 
@@ -467,13 +483,16 @@ class Localization {
    *
    * @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 I/O for new context should begin eagerly
+   * @param {Function}     generateBundles - Custom FluentBundle asynchronous generator.
+   * @param {Function} generateBundlesSync - Custom FluentBundle generator.
    */
-  regenerateBundles(resourceIds, isSync, eager = false) {
+  regenerateBundles(resourceIds, isSync, eager = false, generateBundles = defaultGenerateBundles, generateBundlesSync = defaultGenerateBundlesSync) {
     // Store for error reporting from `formatWithFallback`.
-    this.resourceIds = resourceIds;
+    let generateMessages = isSync ? generateBundlesSync : generateBundles;
-    let generateMessages = isSync ? this.generateBundlesSync : this.generateBundles;
+    this.bundles = this.cached(generateMessages(resourceIds), isSync);
-    this.bundles = this.cached(generateMessages(this.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

intl/l10n/mozILocalization.idl

@@ -11,22 +11,35 @@
  */
 #include "nsISupports.idl"
 
+/**
+ * mozILocalization is an internal API used by Localization class for formatting of translation
+ * units.
+ *
+ * There are three main formatting methods:
+ *    - formatMessages - formats a list of messages based on the requested keys.
+ *    - formatValues   - formats a list of values of messages based on the requested keys.
+ *    - formatValue    - formats a single value based on the requested id and args.
+ *
+ * Each method has a `Sync` variant that can be used if the instance is in the `sync` state.
+ * This mode is enabled via passing `true` to `aIsSync` either in `activate` or `onChange` method.
+ *
+ * When this value is set to `false`, those methods will throw.
+ */
+
 [scriptable, uuid(7d468600-551f-4fe0-98c9-92a53b63ec8d)]
 interface mozILocalization : nsISupports
 {
-  void activate(in Array<AString> aResourceIds, in bool aSync, in bool aEager, in jsval aGenerateBundles, in jsval aGenerateBundlesSync);
+  void activate(in Array<AString> aResourceIds, in bool aIsSync, in bool aEager, in jsval aGenerateBundles, in jsval aGenerateBundlesSync);
 
-  void setIsSync(in boolean isSync);
+  Promise formatMessages(in Array<AString> aResourceIds, in Array<jsval> aKeys);
+  Promise formatValues(in Array<AString> aResourceIds, in Array<jsval> aKeys);
+  Promise formatValue(in Array<AString> aResourceIds, in AUTF8String aId, [optional] in jsval aArgs);
 
-  Promise formatMessages(in Array<jsval> aKeys);
+  AUTF8String formatValueSync(in Array<AString> aResourceIds, in AUTF8String aId, [optional] in jsval aArgs);
-  Promise formatValues(in Array<jsval> aKeys);
+  Array<AUTF8String> formatValuesSync(in Array<AString> aResourceIds, in Array<jsval> aKeys);
-  Promise formatValue(in AUTF8String aId, [optional] in jsval aArgs);
+  Array<jsval> formatMessagesSync(in Array<AString> aResourceIds, in Array<jsval> aKeys);
 
-  AUTF8String formatValueSync(in AUTF8String aId, [optional] in jsval aArgs);
+  void onChange(in Array<AString> aResourceIds, in bool aIsSync, in jsval aGenerateBundles, in jsval aGenerateBundlesSync);
-  Array<AUTF8String> formatValuesSync(in Array<jsval> aKeys);
-  Array<jsval> formatMessagesSync(in Array<jsval> aKeys);
-
-  void onChange(in Array<AString> aResourceIds, in bool aIsSync);
 };
 
 [scriptable, uuid(96632d26-1422-12e9-b1ce-9bb586acd241)]