tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 9533b94cc4eef92d8289e006728345f3e1a31c12
parent 6f884204aa2058cc1b8921dedd34628ba9e978ed
Author: Drew Willcoxon <adw@mozilla.com>
Date:   Mon, 13 Oct 2025 21:19:01 +0000

Bug 1966166 - Update the Rust SharedRemoteSettingsService on locale changes. r=bdk

In addition to fixing the bug, this also makes sure we don't use a
cannot-be-a-base URL for the server during tests. I added a comment about that.

Depends on D268232

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

Diffstat:
Mtoolkit/modules/RustSharedRemoteSettingsService.sys.mjs | 80+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
Atoolkit/modules/tests/xpcshell/test_RustSharedRemoteSettingsService.js | 215+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mtoolkit/modules/tests/xpcshell/xpcshell.toml | 3+++
3 files changed, 288 insertions(+), 10 deletions(-)

diff --git a/toolkit/modules/RustSharedRemoteSettingsService.sys.mjs b/toolkit/modules/RustSharedRemoteSettingsService.sys.mjs @@ -36,7 +36,7 @@ class _SharedRemoteSettingsService { ); this.#config = new RemoteSettingsConfig2({ - server: new RemoteSettingsServer.Custom({ url: Utils.SERVER_URL }), + server: this.#makeServer(Utils.SERVER_URL), bucketName: Utils.actualBucketName("main"), appContext: new RemoteSettingsContext({ formFactor: "desktop", @@ -51,21 +51,44 @@ class _SharedRemoteSettingsService { }); Services.obs.addObserver(this, Region.REGION_TOPIC); + Services.obs.addObserver(this, "intl:app-locales-changed"); this.#rustService = RemoteSettingsService.init(storageDir, this.#config); } /** + * @returns {string} + * The country of the service's app context. + */ + get country() { + return this.#config.appContext.country; + } + + /** + * @returns {string} + * The locale of the service's app context. + */ + get locale() { + return this.#config.appContext.locale; + } + + /** + * @returns {RemoteSettingsServer} + * The service's server. + */ + get server() { + return this.#config.server; + } + + /** * Update the Remote Settings server * * @param {object} opts object with the following fields: * - `url`: server URL (defaults to the production URL) * - `bucketName`: bucket name (defaults to "main") */ - updateServer(opts) { - this.#config.server = opts.url - ? new RemoteSettingsServer.Custom({ url: opts.url }) - : undefined; + updateServer(opts = {}) { + this.#config.server = this.#makeServer(opts.url ?? Utils.SERVER_URL); this.#config.bucketName = opts.bucketName ?? Utils.actualBucketName("main"); this.#rustService.updateConfig(this.#config); } @@ -86,14 +109,51 @@ class _SharedRemoteSettingsService { } observe(subj, topic) { - if (topic == Region.REGION_TOPIC) { - const newCountry = subj.data; - if (newCountry != this.#config.appContext.country) { - this.#config.appContext.country = newCountry; - this.#rustService.updateConfig(this.#config); + switch (topic) { + case Region.REGION_TOPIC: { + const newCountry = subj.data; + if (newCountry != this.#config.appContext.country) { + this.#config.appContext.country = newCountry; + this.#rustService.updateConfig(this.#config); + } + break; + } + case "intl:app-locales-changed": { + const newLocale = Services.locale.appLocaleAsBCP47; + if (newLocale != this.#config.appContext.locale) { + this.#config.appContext.locale = newLocale; + this.#rustService.updateConfig(this.#config); + } + break; } } } + + #makeServer(url) { + // This is annoyingly complex but set `config.server` to a falsey value + // while tests are running and the URL is `Utils.SERVER_URL`. This will + // cause the Rust component to fall back to the production server, but it + // will avoid "cannot-be-a-base" errors. Since remote connections are not + // allowed during testing, consumers will need to avoid using the RS + // service. Ideally we would both handle the cannot-be-a-base errors and + // avoid pinging the production server for them somehow. + // + // Details: + // + // * During normal operation, `Utils.SERVER_URL` is the production URL, but + // during tests, it's a dummy data URI, `data:,#remote-settings-dummy/v1`, + // which is a "cannot-be-a-base" URL. + // + // * `RemoteSettingsService::new` falls back to the production URL when + // `config.server.url` is a cannot-be-a-base URL. So passing in the dummy + // data URI is actually fine for `new`. + // + // * In contrast, `RemoteSettingsService::update_config` returns the error + // when it parses a cannot-be-a-base `config.server.url`. + return !Utils.shouldSkipRemoteActivityDueToTests || url != Utils.SERVER_URL + ? new RemoteSettingsServer.Custom({ url }) + : null; + } } export const SharedRemoteSettingsService = new _SharedRemoteSettingsService(); diff --git a/toolkit/modules/tests/xpcshell/test_RustSharedRemoteSettingsService.js b/toolkit/modules/tests/xpcshell/test_RustSharedRemoteSettingsService.js @@ -0,0 +1,215 @@ +/* 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/. */ + +ChromeUtils.defineESModuleGetters(this, { + Region: "resource://gre/modules/Region.sys.mjs", + SharedRemoteSettingsService: + "resource://gre/modules/RustSharedRemoteSettingsService.sys.mjs", + Utils: "resource://services-settings/Utils.sys.mjs", +}); + +// When the app's region or locale changes, `SharedRemoteSettingsService` should +// update its country and locale. +add_task(async function regionOrLocaleChanged() { + let tests = [ + { country: "US", locale: "en-US" }, + { country: "US", locale: "es-MX" }, + { country: "DE", locale: "de" }, + { country: "DE", locale: "en-US" }, + ]; + + for (let { country, locale } of tests) { + await withRegionAndLocale({ + locale, + region: country, + callback: async () => { + await waitForServiceCountryAndLocale({ country, locale }); + Assert.equal( + SharedRemoteSettingsService.country, + country, + "SharedRemoteSettingsService.country should be the expected country" + ); + Assert.equal( + SharedRemoteSettingsService.locale, + locale, + "SharedRemoteSettingsService.locale should be the expected locale" + ); + }, + }); + } +}); + +// When the app's region or locale changes, the service's server URL should not +// change. This task makes sure we use the correct server URL when we update the +// config in response to region or locale changes. +add_task(async function serverUrl() { + let countriesAndLocales = [ + { newLocale: "de" }, + { newCountry: "DE" }, + { newCountry: "DE", newLocale: "de" }, + ]; + let serverUrls = [ + undefined, + Utils.SERVER_URL, + "https://example.com/test-shared-rs-service-1", + "https://example.com/test-shared-rs-service-2", + ]; + for (let { newCountry, newLocale } of countriesAndLocales) { + for (let url of serverUrls) { + await doServerUrlTest({ newCountry, newLocale, url }); + } + } +}); + +async function doServerUrlTest({ + url, + newCountry = "US", + newLocale = "en-US", +}) { + // First, set an initial app country and locale. + await withRegionAndLocale({ + region: "US", + locale: "en-US", + callback: async () => { + await waitForServiceCountryAndLocale({ country: "US", locale: "en-US" }); + + // Set the initial server URL. + SharedRemoteSettingsService.updateServer({ url }); + + // Now update the app country and locale. + await withRegionAndLocale({ + region: newCountry, + locale: newLocale, + callback: async () => { + await waitForServiceCountryAndLocale({ + country: newCountry, + locale: newLocale, + }); + + // The server URL set above should remain set. + if (!url || url == Utils.SERVER_URL) { + // A falsey URL should fall back to `Utils.SERVER_URL`, and during + // testing `Utils.SERVER_URL` is a cannot-be-a-base URL, so the + // server should be undefined. + Assert.ok( + !SharedRemoteSettingsService.server, + "SharedRemoteSettingsService.server should be undefined" + ); + } else { + Assert.ok( + SharedRemoteSettingsService.server, + "SharedRemoteSettingsService.server should be defined" + ); + Assert.equal( + SharedRemoteSettingsService.server.url, + url, + "SharedRemoteSettingsService.server.url should be as expected" + ); + } + }, + }); + }, + }); +} + +/** + * Sets the app's region (country) and locale, calls your callback, and restores + * the original region and locale. + * + * @param {string} region + * The app region to set. + * @param {string} locale + * The app locale to set. + * @param {function} callback + * Your callback. + */ +async function withRegionAndLocale({ region, locale, callback }) { + let originalRegion = Region.home; + + info("Setting region: " + region); + Region._setHomeRegion(region, true); + + Assert.equal(Region.home, region, "Region should now be the desired region"); + + let { availableLocales, requestedLocales } = Services.locale; + let localePromise = waitForLocaleChange(locale); + + info("Setting locale: " + locale); + Services.locale.availableLocales = [locale]; + Services.locale.requestedLocales = [locale]; + + info("Waiting for locale change"); + await localePromise; + info("Done waiting for locale change"); + + Assert.equal( + Services.locale.appLocaleAsBCP47, + locale, + "App locale should now be the desired locale" + ); + + info("Calling callback"); + await callback(); + info("Done calling callback"); + + // Restore the original region and locales. + info("Resetting region to orginal: " + originalRegion); + Region._setHomeRegion(originalRegion, true); + + Assert.equal( + Region.home, + originalRegion, + "Region should now be the original region" + ); + + let restoreLocalePromise = waitForLocaleChange(requestedLocales[0]); + Services.locale.availableLocales = availableLocales; + Services.locale.requestedLocales = requestedLocales; + + info("Waiting for original locale to be restored"); + await restoreLocalePromise; + info("Done waiting for original locale to be restored"); + + Assert.equal( + Services.locale.appLocaleAsBCP47, + requestedLocales[0], + "App locale should now be the original locale" + ); +} + +/** + * Waits for the app locale to be set to an expected value. + * + * @param {string} locale + * The expected locale. + */ +async function waitForLocaleChange(locale) { + // Nothing happens when the locale doesn't actually change. + if (locale == Services.locale.requestedLocales[0]) { + info("Locale is already set"); + } else { + info("Waiting for intl:app-locales-changed"); + await TestUtils.topicObserved("intl:app-locales-changed"); + info("Got intl:app-locales-changed"); + } +} + +/** + * Waits for the country and locale of the `SharedRemoteSettingsService` to be + * set to some expected values. + * + * @param {string} country + * The expected country. + * @param {string} locale + * The expected locale. + */ +async function waitForServiceCountryAndLocale({ country, locale }) { + await TestUtils.waitForCondition( + () => + SharedRemoteSettingsService.country == country && + SharedRemoteSettingsService.locale == locale, + "Waiting for SharedRemoteSettingsService country and locale to be set: " + + JSON.stringify({ country, locale }) + ); +} diff --git a/toolkit/modules/tests/xpcshell/xpcshell.toml b/toolkit/modules/tests/xpcshell/xpcshell.toml @@ -100,6 +100,9 @@ skip-if = ["os == 'android'"] ["test_Region_geocoding.js"] +["test_RustSharedRemoteSettingsService.js"] +skip-if = ["os == 'android'"] + ["test_Services.js"] ["test_UpdateUtils_updatechannel.js"]