From 2a163b0acae48d6450185ae0345c0272bdd7945e Mon Sep 17 00:00:00 2001 From: Zibi Braniecki Date: Sat, 30 May 2020 09:37:02 +0000 Subject: [PATCH] Bug 1631593 - Cache bundles in Localization C++. r=jfkthame,smaug Differential Revision: https://phabricator.services.mozilla.com/D71815 --- dom/l10n/DOMLocalization.cpp | 4 +- dom/l10n/DOMLocalization.h | 2 + intl/l10n/Localization.cpp | 39 +++++++--- intl/l10n/Localization.h | 2 + intl/l10n/Localization.jsm | 131 +++++++++++---------------------- intl/l10n/mozILocalization.idl | 18 ++--- 6 files changed, 87 insertions(+), 109 deletions(-) diff --git a/dom/l10n/DOMLocalization.cpp b/dom/l10n/DOMLocalization.cpp index fd4c258ed39d..68993788df34 100644 --- a/dom/l10n/DOMLocalization.cpp +++ b/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 diff --git a/dom/l10n/DOMLocalization.h b/dom/l10n/DOMLocalization.h index 4c65a646439e..3cceecb3439c 100644 --- a/dom/l10n/DOMLocalization.h +++ b/dom/l10n/DOMLocalization.h @@ -25,6 +25,8 @@ class DOMLocalization : public intl::Localization { explicit DOMLocalization(nsIGlobalObject* aGlobal); + void Destroy(); + static already_AddRefed Constructor( const GlobalObject& aGlobal, const Sequence& aResourceIds, const bool aSync, const BundleGenerator& aBundleGenerator, diff --git a/intl/l10n/Localization.cpp b/intl/l10n/Localization.cpp index 29beefe288f4..acf06e6d2095 100644 --- a/intl/l10n/Localization.cpp +++ b/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 generateBundlesJS(cx, mGenerateBundles); JS::Rooted generateBundlesSyncJS(cx, mGenerateBundlesSync); - mLocalization->Activate(mResourceIds, mIsSync, aEager, generateBundlesJS, - generateBundlesSyncJS); + JS::Rooted bundlesJS(cx); + 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 generateBundlesJS(cx, mGenerateBundles); JS::Rooted generateBundlesSyncJS(cx, mGenerateBundlesSync); - mLocalization->OnChange(mResourceIds, mIsSync, generateBundlesJS, - generateBundlesSyncJS); + JS::Rooted bundlesJS(cx); + mLocalization->GenerateBundles(mResourceIds, mIsSync, false, + generateBundlesJS, generateBundlesSyncJS, + &bundlesJS); + mBundles.set(bundlesJS); } } @@ -232,7 +240,8 @@ already_AddRefed Localization::FormatValue( } RefPtr promise; - nsresult rv = mLocalization->FormatValue(mResourceIds, aId, args, + JS::Rooted 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 Localization::FormatValues( } RefPtr promise; - aRv = mLocalization->FormatValues(mResourceIds, jsKeys, + JS::Rooted 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 Localization::FormatMessages( } RefPtr promise; - aRv = mLocalization->FormatMessages(mResourceIds, jsKeys, + JS::Rooted 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 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 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 messages; SequenceRooter messagesRooter(aCx, &messages); - aRv = mLocalization->FormatMessagesSync(mResourceIds, jsKeys, messages); + JS::Rooted bundlesJS(aCx, mBundles); + aRv = mLocalization->FormatMessagesSync(mResourceIds, bundlesJS, jsKeys, + messages); if (NS_WARN_IF(aRv.Failed())) { return; } diff --git a/intl/l10n/Localization.h b/intl/l10n/Localization.h index 1c364cb57443..8e00813ced26 100644 --- a/intl/l10n/Localization.h +++ b/intl/l10n/Localization.h @@ -97,6 +97,8 @@ class Localization : public nsIObserver, bool mIsSync; nsTArray mResourceIds; + + JS::Heap mBundles; JS::Heap mGenerateBundles; JS::Heap mGenerateBundlesSync; }; diff --git a/intl/l10n/Localization.jsm b/intl/l10n/Localization.jsm index f6bb6d00d36b..410043035388 100644 --- a/intl/l10n/Localization.jsm +++ b/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 { - /** - * `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} 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); - } - +const Localization = { 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} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {Array} keys - Translation keys to format. * @param {Function} method - Formatting function. * @returns {Promise>} * @private */ - async formatWithFallback(resourceIds, keys, method) { - if (!this.bundles) { + async formatWithFallback(resourceIds, bundles, keys, method) { + 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} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {Array} keys - Translation keys to format. * @param {Function} method - Formatting function. * @returns {Array} * @private */ - formatWithFallbackSync(resourceIds, keys, method) { - if (!this.bundles) { + formatWithFallbackSync(resourceIds, bundles, keys, method) { + 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} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {Array} keys - Translation keys to format. * @returns {Promise>} * @private */ - formatMessages(resourceIds, keys) { - return this.formatWithFallback(resourceIds, keys, messageFromBundle); - } + formatMessages(resourceIds, bundles, keys) { + return this.formatWithFallback(resourceIds, bundles, keys, messageFromBundle); + }, /** * Sync version of `formatMessages`. @@ -369,13 +345,14 @@ class Localization { * * @param {Array} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {Array} keys - Translation keys to format. * @returns {Array<{value: string, attributes: Object}?>} * @private */ - formatMessagesSync(resourceIds, keys) { - return this.formatWithFallbackSync(resourceIds, keys, messageFromBundle); - } + formatMessagesSync(resourceIds, bundles, keys) { + return this.formatWithFallbackSync(resourceIds, bundles, keys, messageFromBundle); + }, /** * Retrieve translations corresponding to the passed keys. @@ -395,12 +372,13 @@ class Localization { * * @param {Array} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {Array} keys - Translation keys to format. * @returns {Promise>} */ - formatValues(resourceIds, keys) { - return this.formatWithFallback(resourceIds, keys, valueFromBundle); - } + formatValues(resourceIds, bundles, keys) { + return this.formatWithFallback(resourceIds, bundles, keys, valueFromBundle); + }, /** * Sync version of `formatValues`. @@ -409,13 +387,14 @@ class Localization { * * @param {Array} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {Array} keys - Translation keys to format. * @returns {Array} * @private */ - formatValuesSync(resourceIds, keys) { - return this.formatWithFallbackSync(resourceIds, keys, valueFromBundle); - } + formatValuesSync(resourceIds, bundles, keys) { + return this.formatWithFallbackSync(resourceIds, bundles, keys, valueFromBundle); + }, /** * Retrieve the translation corresponding to the `id` identifier. @@ -437,14 +416,15 @@ class Localization { * * @param {Array} resourceIds - List of resource ids used by this * localization. + * @param {Iter} bundles - Iterator over bundles. * @param {string} id - Identifier of the translation to format * @param {Object} [args] - Optional external arguments * @returns {Promise} */ - async formatValue(resourceIds, id, args) { - const [val] = await this.formatValues(resourceIds, [{id, args}]); + async formatValue(resourceIds, bundles, 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} resourceIds - List of resource ids used by this * localization. + * @param {Iter} 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) { - const [val] = this.formatValuesSync(resourceIds, [{id, args}]); + formatValueSync(resourceIds, bundles, id, args) { + const [val] = this.formatValuesSync(resourceIds, bundles, [{id, args}]); return val; - } - - /** - * @param {Array} 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} */ - 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"]; diff --git a/intl/l10n/mozILocalization.idl b/intl/l10n/mozILocalization.idl index 35587fda494d..a65a36435e9b 100644 --- a/intl/l10n/mozILocalization.idl +++ b/intl/l10n/mozILocalization.idl @@ -29,21 +29,19 @@ [scriptable, uuid(7d468600-551f-4fe0-98c9-92a53b63ec8d)] interface mozILocalization : nsISupports { - void activate(in Array aResourceIds, in bool aIsSync, in bool aEager, in jsval aGenerateBundles, in jsval aGenerateBundlesSync); + jsval generateBundles(in Array aResourceIds, in bool aIsSync, in bool eager, in jsval aGenerateBundles, in jsval aGenerateBundlesSync); - Promise formatMessages(in Array aResourceIds, in Array aKeys); - Promise formatValues(in Array aResourceIds, in Array aKeys); - Promise formatValue(in Array aResourceIds, in AUTF8String aId, [optional] in jsval aArgs); + Promise formatMessages(in Array aResourceIds, in jsval aBundles, in Array aKeys); + Promise formatValues(in Array aResourceIds, in jsval aBundles, in Array aKeys); + Promise formatValue(in Array aResourceIds, in jsval aBundles, in AUTF8String aId, [optional] in jsval aArgs); - AUTF8String formatValueSync(in Array aResourceIds, in AUTF8String aId, [optional] in jsval aArgs); - Array formatValuesSync(in Array aResourceIds, in Array aKeys); - Array formatMessagesSync(in Array aResourceIds, in Array aKeys); - - void onChange(in Array aResourceIds, in bool aIsSync, in jsval aGenerateBundles, in jsval aGenerateBundlesSync); + AUTF8String formatValueSync(in Array aResourceIds, in jsval aBundles, in AUTF8String aId, [optional] in jsval aArgs); + Array formatValuesSync(in Array aResourceIds, in jsval aBundles, in Array aKeys); + Array formatMessagesSync(in Array aResourceIds, in jsval aBundles, in Array aKeys); }; [scriptable, uuid(96632d26-1422-12e9-b1ce-9bb586acd241)] interface mozILocalizationJSM : nsISupports { - mozILocalization getLocalization(); + readonly attribute mozILocalization Localization; };