From dfd9659f8036d23213e5ff765d47f596868821ae Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Fri, 11 Sep 2020 20:06:39 +0000 Subject: [PATCH] Bug 1662944 - Set availableLocales at the start of search service tests to avoid maybeReloadEngines being called twice. r=daleharvey Depends on D89497 Differential Revision: https://phabricator.services.mozilla.com/D89652 --- toolkit/components/search/SearchService.jsm | 53 ++++++++++++------- .../search/tests/xpcshell/head_search.js | 7 ++- .../tests/xpcshell/test_list_json_locale.js | 7 ++- .../search/tests/xpcshell/test_sort_orders.js | 5 ++ .../xpcshell/test_webextensions_install.js | 5 ++ 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/toolkit/components/search/SearchService.jsm b/toolkit/components/search/SearchService.jsm index ccdc4663bfdc..b323fc6928e9 100644 --- a/toolkit/components/search/SearchService.jsm +++ b/toolkit/components/search/SearchService.jsm @@ -136,6 +136,12 @@ SearchService.prototype = { // or not). _initialized: false, + // Indicates if we're already waiting for maybeReloadEngines to be called. + _maybeReloadDebounce: false, + + // Indicates if we're currently in maybeReloadEngines. + _reloadingEngines: false, + // The engine selector singleton that is managing the engine configuration. _engineSelector: null, @@ -721,32 +727,40 @@ SearchService.prototype = { * the service has already been initialized. */ async _maybeReloadEngines() { - if (!this._initialized) { - if (this._maybeReloadDebounce) { - logConsole.debug( - "We're already waiting for init to finish and reload engines after." - ); - return; - } + if (this._maybeReloadDebounce) { + logConsole.debug("We're already waiting to reload engines."); + return; + } + + if (!this._initialized || this._reloadingEngines) { this._maybeReloadDebounce = true; - // Schedule a reload to happen at most 10 seconds after initialization of - // the service finished, during an idle moment. - this._initObservers.promise.then(() => { - Services.tm.idleDispatchToMainThread(() => { - if (!this._maybeReloadDebounce) { - return; - } - delete this._maybeReloadDebounce; - this._maybeReloadEngines().catch(Cu.reportError); - }, 10000); - }); + // Schedule a reload to happen at most 10 seconds after the current run. + Services.tm.idleDispatchToMainThread(() => { + if (!this._maybeReloadDebounce) { + return; + } + this._maybeReloadDebounce = false; + this._maybeReloadEngines().catch(Cu.reportError); + }, 10000); logConsole.debug( "Post-poning maybeReloadEngines() as we're currently initializing." ); return; } - logConsole.debug("Running maybeReloadEngines"); + logConsole.debug("Running maybeReloadEngines"); + this._reloadingEngines = true; + + try { + await this._reloadEngines(); + } catch (ex) { + logConsole.error("maybeReloadEngines failed", ex); + } + this._reloadingEngines = false; + logConsole.debug("maybeReloadEngines complete"); + }, + + async _reloadEngines() { // Capture the current engine state, in case we need to notify below. const prevCurrentEngine = this._currentEngine; const prevPrivateEngine = this._currentPrivateEngine; @@ -944,7 +958,6 @@ SearchService.prototype = { SearchUtils.TOPIC_SEARCH_SERVICE, "engines-reloaded" ); - logConsole.debug("maybeReloadEngines complete"); }, /** diff --git a/toolkit/components/search/tests/xpcshell/head_search.js b/toolkit/components/search/tests/xpcshell/head_search.js index 46fb1e78a343..e9da82780638 100644 --- a/toolkit/components/search/tests/xpcshell/head_search.js +++ b/toolkit/components/search/tests/xpcshell/head_search.js @@ -175,8 +175,13 @@ async function promiseSetHomeRegion(region) { * The locale to set. */ async function promiseSetLocale(locale) { + if (!Services.locale.availableLocales.includes(locale)) { + throw new Error( + `"${locale}" needs to be included in Services.locales.availableLocales at the start of the test.` + ); + } + let promise = SearchTestUtils.promiseSearchNotification("engines-reloaded"); - Services.locale.availableLocales = [locale]; Services.locale.requestedLocales = [locale]; await promise; } diff --git a/toolkit/components/search/tests/xpcshell/test_list_json_locale.js b/toolkit/components/search/tests/xpcshell/test_list_json_locale.js index 60322c5507fb..a02e07b9d258 100644 --- a/toolkit/components/search/tests/xpcshell/test_list_json_locale.js +++ b/toolkit/components/search/tests/xpcshell/test_list_json_locale.js @@ -8,6 +8,12 @@ add_task(async function setup() { await SearchTestUtils.useTestEngines(); + Services.locale.availableLocales = [ + ...Services.locale.availableLocales, + "de", + "fr", + ]; + Services.prefs.setBoolPref( SearchUtils.BROWSER_SEARCH_PREF + "separatePrivateDefault.ui.enabled", true @@ -20,7 +26,6 @@ add_task(async function setup() { }); add_task(async function test_listJSONlocale() { - Services.locale.availableLocales = ["de"]; Services.locale.requestedLocales = ["de"]; await AddonTestUtils.promiseStartupManager(); diff --git a/toolkit/components/search/tests/xpcshell/test_sort_orders.js b/toolkit/components/search/tests/xpcshell/test_sort_orders.js index 653ce697c349..5c305999f6ec 100644 --- a/toolkit/components/search/tests/xpcshell/test_sort_orders.js +++ b/toolkit/components/search/tests/xpcshell/test_sort_orders.js @@ -26,6 +26,11 @@ add_task(async function setup() { await SearchTestUtils.useTestEngines(); + Services.locale.availableLocales = [ + ...Services.locale.availableLocales, + "gd", + ]; + Services.prefs.setBoolPref( SearchUtils.BROWSER_SEARCH_PREF + "separatePrivateDefault", true diff --git a/toolkit/components/search/tests/xpcshell/test_webextensions_install.js b/toolkit/components/search/tests/xpcshell/test_webextensions_install.js index 0c66c02d66ae..39f85188d584 100644 --- a/toolkit/components/search/tests/xpcshell/test_webextensions_install.js +++ b/toolkit/components/search/tests/xpcshell/test_webextensions_install.js @@ -25,6 +25,11 @@ add_task(async function setup() { await SearchTestUtils.useTestEngines("test-extensions"); await promiseStartupManager(); + Services.locale.availableLocales = [ + ...Services.locale.availableLocales, + "af", + ]; + registerCleanupFunction(async () => { await promiseShutdownManager(); Services.prefs.clearUserPref("browser.search.region");