Bug 1462839 - Switch L10nRegistry to cache FTLResource objects, prefetch resources and return incomplete contexts.
MozReview-Commit-ID: GYQ0YfWkrLL
--- a/intl/l10n/L10nRegistry.jsm
+++ b/intl/l10n/L10nRegistry.jsm
@@ -1,11 +1,11 @@
const { AppConstants } = ChromeUtils.import("resource://gre/modules/AppConstants.jsm", {});
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm", {});
-const { MessageContext } = ChromeUtils.import("resource://gre/modules/MessageContext.jsm", {});
+const { MessageContext, FTLResource } = ChromeUtils.import("resource://gre/modules/MessageContext.jsm", {});
Cu.importGlobalProperties(["fetch"]);
/**
* L10nRegistry is a localization resource management system for Gecko.
*
* It manages the list of resource sources provided with the app and allows
* for additional sources to be added and updated.
*
@@ -73,34 +73,53 @@ Cu.importGlobalProperties(["fetch"]);
*
* If during the life-cycle of the app a new source is added, the generator can be called again
* and will produce a new set of permutations placing the language pack provided resources
* at the top.
*/
const L10nRegistry = {
sources: new Map(),
- ctxCache: new Map(),
bootstrap: null,
/**
* Based on the list of requested languages and resource Ids,
* this function returns an lazy iterator over message context permutations.
*
* @param {Array} requestedLangs
* @param {Array} resourceIds
* @returns {AsyncIterator<MessageContext>}
*/
async* generateContexts(requestedLangs, resourceIds) {
+ for await (let [locale, set] of this.prefetch(requestedLangs, resourceIds, null)) {
+ let dataSet = await Promise.all(set);
+
+ if (dataSet.every(d => d === false)) {
+ continue;
+ }
+ const ctx = new MessageContext(locale, MSG_CONTEXT_OPTIONS);
+ for (const data of dataSet) {
+ if (data !== false) {
+ ctx.loadResource(data);
+ }
+ }
+ yield ctx;
+ }
+ },
+
+ async* prefetch(requestedLangs, resourceIds, limit = 2) {
if (this.bootstrap !== null) {
await this.bootstrap;
}
const sourcesOrder = Array.from(this.sources.keys()).reverse();
for (const locale of requestedLangs) {
- yield * generateContextsForLocale(locale, sourcesOrder, resourceIds);
+ if (limit !== null && --limit === 0) {
+ return;
+ }
+ yield * generateResourceSetsForLocale(locale, sourcesOrder, resourceIds);
}
},
/**
* Adds a new resource source to the L10nRegistry.
*
* @param {FileSource} source
*/
@@ -120,17 +139,16 @@ const L10nRegistry = {
*
* @param {FileSource} source
*/
updateSource(source) {
if (!this.sources.has(source.name)) {
throw new Error(`Source with name "${source.name}" is not registered.`);
}
this.sources.set(source.name, source);
- this.ctxCache.clear();
Services.locale.setAvailableLocales(this.getAvailableLocales());
},
/**
* Removes a source from the L10nRegistry.
*
* @param {String} sourceId
*/
@@ -181,45 +199,35 @@ function generateContextID(locale, sourc
* sources order.
*
* @param {String} locale
* @param {Array} sourcesOrder
* @param {Array} resourceIds
* @param {Array} [resolvedOrder]
* @returns {AsyncIterator<MessageContext>}
*/
-async function* generateContextsForLocale(locale, sourcesOrder, resourceIds, resolvedOrder = []) {
+function* generateResourceSetsForLocale(locale, sourcesOrder, resourceIds, resolvedOrder = []) {
const resolvedLength = resolvedOrder.length;
const resourcesLength = resourceIds.length;
// Inside that loop we have a list of resources and the sources for them, like this:
// ['test.ftl', 'menu.ftl', 'foo.ftl']
// ['app', 'platform', 'app']
for (const sourceName of sourcesOrder) {
const order = resolvedOrder.concat(sourceName);
- // We bail only if the hasFile returns a strict false here,
- // because for FileSource it may also return undefined, which means
- // that we simply don't know if the source contains the file and we'll
- // have to perform the I/O to learn.
- if (L10nRegistry.sources.get(sourceName).hasFile(locale, resourceIds[resolvedOrder.length]) === false) {
- continue;
- }
-
// If the number of resolved sources equals the number of resources,
// create the right context and return it if it loads.
if (resolvedLength + 1 === resourcesLength) {
- const ctx = await generateContext(locale, order, resourceIds);
- if (ctx !== null) {
- yield ctx;
- }
+ const set = generateResourceSet(locale, order, resourceIds);
+ yield [locale, set];
} else if (resolvedLength < resourcesLength) {
// otherwise recursively load another generator that walks over the
// partially resolved list of sources.
- yield * generateContextsForLocale(locale, sourcesOrder, resourceIds, order);
+ yield * generateResourceSetsForLocale(locale, sourcesOrder, resourceIds, order);
}
}
}
const MSG_CONTEXT_OPTIONS = {
// Temporarily disable bidi isolation due to Microsoft not supporting FSI/PDI.
// See bug 1439018 for details.
useIsolating: Services.prefs.getBoolPref("intl.l10n.enable-bidi-marks", false),
@@ -253,41 +261,23 @@ const MSG_CONTEXT_OPTIONS = {
* This allows the caller to be an async generator without using
* try/catch clauses.
*
* @param {String} locale
* @param {Array} sourcesOrder
* @param {Array} resourceIds
* @returns {Promise<MessageContext>}
*/
-function generateContext(locale, sourcesOrder, resourceIds) {
+function generateResourceSet(locale, sourcesOrder, resourceIds) {
const ctxId = generateContextID(locale, sourcesOrder, resourceIds);
- if (L10nRegistry.ctxCache.has(ctxId)) {
- return L10nRegistry.ctxCache.get(ctxId);
- }
- const fetchPromises = resourceIds.map((resourceId, i) => {
+ return resourceIds.map((resourceId, i) => {
return L10nRegistry.sources.get(sourcesOrder[i]).fetchFile(locale, resourceId);
});
- const ctxPromise = Promise.all(fetchPromises).then(
- dataSets => {
- const ctx = new MessageContext(locale, MSG_CONTEXT_OPTIONS);
- for (const data of dataSets) {
- if (data === null) {
- return null;
- }
- ctx.addMessages(data);
- }
- return ctx;
- },
- () => null
- );
- L10nRegistry.ctxCache.set(ctxId, ctxPromise);
- return ctxPromise;
}
/**
* This is a basic Source for L10nRegistry.
* It registers its own locales and a pre-path, and when asked for a file
* it attempts to download and cache it.
*
* The Source caches the downloaded files so any consecutive loads will
@@ -348,38 +338,38 @@ class FileSource {
if (this.cache[fullPath].then) {
return undefined;
}
return true;
}
fetchFile(locale, path) {
if (!this.locales.includes(locale)) {
- return Promise.reject(`The source has no resources for locale "${locale}"`);
+ return false;
}
const fullPath = this.getPath(locale, path);
if (this.cache.hasOwnProperty(fullPath)) {
if (this.cache[fullPath] === false) {
- return Promise.reject(`The source has no resources for path "${fullPath}"`);
+ return false;
}
if (this.cache[fullPath].then) {
return this.cache[fullPath];
}
} else if (this.indexed) {
- return Promise.reject(`The source has no resources for path "${fullPath}"`);
- }
+ return false;
+ }
return this.cache[fullPath] = L10nRegistry.load(fullPath).then(
data => {
- return this.cache[fullPath] = data;
+ return this.cache[fullPath] = new FTLResource(data);
},
err => {
this.cache[fullPath] = false;
- return Promise.reject(err);
+ return false;
}
);
}
}
/**
* This is an extension of the FileSource which should be used
* for sources that can provide the list of files available in the source.
@@ -413,17 +403,17 @@ class IndexedFileSource extends FileSour
*
* @param {string} url
*
* @returns {Promise<string>}
*/
L10nRegistry.load = function(url) {
return fetch(url).then(response => {
if (!response.ok) {
- return Promise.reject(response.statusText);
+ return false;
}
return response.text();
});
};
this.L10nRegistry = L10nRegistry;
this.FileSource = FileSource;
this.IndexedFileSource = IndexedFileSource;
--- a/intl/l10n/MessageContext.jsm
+++ b/intl/l10n/MessageContext.jsm
@@ -21,16 +21,22 @@
/* eslint no-magic-numbers: [0] */
const MAX_PLACEABLES = 100;
const entryIdentifierRe = /-?[a-zA-Z][a-zA-Z0-9_-]*/y;
const identifierRe = /[a-zA-Z][a-zA-Z0-9_-]*/y;
const functionIdentifierRe = /^[A-Z][A-Z_?-]*$/;
+class FTLResource {
+ constructor(source) {
+ const [entries, errors] = parse(source);
+ this._entries = entries;
+ }
+}
/**
* The `Parser` class is responsible for parsing FTL resources.
*
* It's only public method is `getResource(source)` which takes an FTL string
* and returns a two element Array with an Object of entries generated from the
* source as the first element and an array of SyntaxError objects as the
* second.
*
@@ -1770,16 +1776,27 @@ class MessageContext {
* Parsed entities should be formatted with the `format` method in case they
* contain logic (references, select expressions etc.).
*
* @param {string} source - Text resource with translations.
* @returns {Array<Error>}
*/
addMessages(source) {
const [entries, errors] = parse(source);
+
+ this.loadEntries(entries, errors);
+
+ return errors;
+ }
+
+ loadResource(res) {
+ this.loadEntries(res._entries);
+ }
+
+ loadEntries(entries, errors = []) {
for (const id in entries) {
if (id.startsWith("-")) {
// Identifiers starting with a dash (-) define terms. Terms are private
// and cannot be retrieved from MessageContext.
if (this._terms.has(id)) {
errors.push(`Attempt to override an existing term: "${id}"`);
continue;
}
@@ -1787,17 +1804,16 @@ class MessageContext {
} else {
if (this._messages.has(id)) {
errors.push(`Attempt to override an existing message: "${id}"`);
continue;
}
this._messages.set(id, entries[id]);
}
}
-
return errors;
}
/**
* Format a message to a string or null.
*
* Format a raw `message` from the context into a string (or a null if it has
* a null value). `args` will be used to resolve references to external
@@ -1854,9 +1870,10 @@ class MessageContext {
this._intls.set(ctor, cache);
}
return cache[id];
}
}
this.MessageContext = MessageContext;
-var EXPORTED_SYMBOLS = ["MessageContext"];
+this.FTLResource = FTLResource;
+var EXPORTED_SYMBOLS = ["MessageContext", "FTLResource"];
--- a/intl/l10n/l10n.js
+++ b/intl/l10n/l10n.js
@@ -1,11 +1,13 @@
{
const { DOMLocalization } =
ChromeUtils.import("resource://gre/modules/DOMLocalization.jsm", {});
+ const { L10nRegistry } = ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm", {});
+ const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm", {});
/**
* Polyfill for document.ready polyfill.
* See: https://github.com/whatwg/html/issues/127 for details.
*
* @returns {Promise}
*/
function documentReady() {
@@ -42,16 +44,17 @@
);
}
const resourceIds = getResourceLinks(document.head || document);
document.l10n = new DOMLocalization(window, resourceIds);
// Trigger the first two contexts to be loaded eagerly.
- document.l10n.ctxs.touchNext(2);
+ const appLocales = Services.locale.getAppLocalesAsBCP47();
+ L10nRegistry.prefetch(appLocales, resourceIds, 2);
document.l10n.ready = documentReady().then(() => {
document.l10n.registerObservers();
document.l10n.connectRoot(document.documentElement);
return document.l10n.translateRoots();
});
}
--- a/intl/l10n/test/test_l10nregistry.js
+++ b/intl/l10n/test/test_l10nregistry.js
@@ -35,34 +35,32 @@ add_task(async function test_empty_resou
const ctxs = L10nRegistry.generateContexts(["en-US"], []);
const done = (await ctxs.next()).done;
equal(done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* Test that passing empty sources list works.
*/
add_task(async function test_empty_sources() {
fs = {};
const ctxs = L10nRegistry.generateContexts(["en-US"], []);
const done = (await ctxs.next()).done;
equal(done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test tests generation of a proper context for a single
* source scenario
*/
add_task(async function test_methods_calling() {
fs = {
@@ -75,17 +73,16 @@ add_task(async function test_methods_cal
const ctxs = L10nRegistry.generateContexts(["en-US"], ["/browser/menu.ftl"]);
const ctx = (await ctxs.next()).value;
equal(ctx.hasMessage("key"), true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that the public methods return expected values
* for the single source scenario
*/
add_task(async function test_has_one_source() {
let oneSource = new FileSource("app", ["en-US"], "./app/data/locales/{locale}/");
@@ -113,17 +110,16 @@ add_task(async function test_has_one_sou
// returns no contexts for missing locale
ctxs = L10nRegistry.generateContexts(["pl"], ["test.ftl"]);
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that public methods return expected values
* for the dual source scenario.
*/
add_task(async function test_has_two_sources() {
let oneSource = new FileSource("platform", ["en-US"], "./platform/data/locales/{locale}/");
@@ -170,17 +166,16 @@ add_task(async function test_has_two_sou
equal(ctx1.hasMessage("key"), true);
let msg1 = ctx1.getMessage("key");
equal(ctx1.format(msg1), "platform value");
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that behavior specific to the IndexedFileSource
* works correctly.
*
* In particular it tests that IndexedFileSource correctly returns
* missing files as `false` instead of `undefined`.
@@ -198,17 +193,16 @@ add_task(async function test_indexed() {
equal(L10nRegistry.sources.has("langpack-pl"), true);
equal(oneSource.getPath("pl", "test.ftl"), "/data/locales/pl/test.ftl");
equal(oneSource.hasFile("pl", "test.ftl"), true);
equal(oneSource.hasFile("pl", "missing.ftl"), false);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test checks if the correct order of contexts is used for
* scenarios where a new file source is added on top of the default one.
*/
add_task(async function test_override() {
let fileSource = new FileSource("app", ["pl"], "/app/data/locales/{locale}/");
@@ -239,17 +233,16 @@ add_task(async function test_override()
equal(ctx1.hasMessage("key"), true);
let msg1 = ctx1.getMessage("key");
equal(ctx1.format(msg1), "value");
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that new contexts are returned
* after source update.
*/
add_task(async function test_updating() {
let oneSource = new IndexedFileSource("langpack-pl", ["pl"], "/data/locales/{locale}/", [
@@ -277,17 +270,16 @@ add_task(async function test_updating()
equal(L10nRegistry.sources.size, 1);
ctxs = L10nRegistry.generateContexts(["pl"], ["test.ftl"]);
ctx0 = (await ctxs.next()).value;
msg0 = ctx0.getMessage("key");
equal(ctx0.format(msg0), "new value");
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that generated contexts return correct values
* after sources are being removed.
*/
add_task(async function test_removing() {
let fileSource = new FileSource("app", ["pl"], "/app/data/locales/{locale}/");
@@ -343,17 +335,16 @@ add_task(async function test_removing()
equal(L10nRegistry.sources.size, 0);
ctxs = L10nRegistry.generateContexts(["pl"], ["test.ftl"]);
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that the logic works correctly when there's a missing
* file in the FileSource scenario.
*/
add_task(async function test_missing_file() {
let oneSource = new FileSource("app", ["en-US"], "./app/data/locales/{locale}/");
@@ -373,25 +364,26 @@ add_task(async function test_missing_fil
equal(L10nRegistry.sources.size, 2);
equal(L10nRegistry.sources.has("app"), true);
equal(L10nRegistry.sources.has("platform"), true);
// returns a single context
let ctxs = L10nRegistry.generateContexts(["en-US"], ["test.ftl", "test2.ftl"]);
- (await ctxs.next());
- (await ctxs.next());
+ (await ctxs.next()); // [platform, platform] - both present
+ (await ctxs.next()); // [platform, app] - second missing
+ (await ctxs.next()); // [app, platform] - both present
+ (await ctxs.next()); // [app, app] - second missing
equal((await ctxs.next()).done, true);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
});
/**
* This test verifies that each file is that all files requested
* by a single context are fetched at the same time, even
* if one I/O is slow.
*/
add_task(async function test_parallel_io() {
@@ -442,11 +434,10 @@ add_task(async function test_parallel_io
equal((await ctxs.next()).done, true);
// When requested again, the cache should make the load operation not
// increase the fetchedIndex count
L10nRegistry.generateContexts(["en-US"], ["test.ftl", "test2.ftl", "slow-file.ftl"]);
// cleanup
L10nRegistry.sources.clear();
- L10nRegistry.ctxCache.clear();
L10nRegistry.load = originalLoad;
});