From 80544101c76212c8d16c6e2b368893b83de96863 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Tue, 19 Sep 2023 03:19:30 +0300 Subject: [PATCH] Backed out changeset f0a2aa2ffe6d (bug 1832704) for causing xpc failures in toolkit/components/search/tests/xpcshell/test_async.js --- .../components/extensions/parent/.eslintrc.js | 1 + .../extensions/parent/ext-browser.js | 14 ++++++++++ .../parent/ext-chrome-settings-overrides.js | 7 ++--- .../extensions/parent/ext-search.js | 6 ++-- .../test_ext_settings_overrides_shutdown.js | 2 +- .../search-detection/extension/api.js | 13 ++++++++- .../components/search/SearchService.sys.mjs | 28 ++++++------------- .../components/search/SearchSettings.sys.mjs | 7 +---- .../components/search/nsISearchService.idl | 10 ------- .../app/TelemetryEnvironment.sys.mjs | 20 +++++++------ 10 files changed, 54 insertions(+), 54 deletions(-) diff --git a/browser/components/extensions/parent/.eslintrc.js b/browser/components/extensions/parent/.eslintrc.js index b7c3113e4958..1efdd83ff4bb 100644 --- a/browser/components/extensions/parent/.eslintrc.js +++ b/browser/components/extensions/parent/.eslintrc.js @@ -24,6 +24,7 @@ module.exports = { openOptionsPage: true, pageActionFor: true, replaceUrlInTab: true, + searchInitialized: true, sidebarActionFor: true, tabTracker: true, waitForTabLoaded: true, diff --git a/browser/components/extensions/parent/ext-browser.js b/browser/components/extensions/parent/ext-browser.js index 5443ed71d478..4906a42a0084 100644 --- a/browser/components/extensions/parent/ext-browser.js +++ b/browser/components/extensions/parent/ext-browser.js @@ -239,6 +239,20 @@ global.TabContext = class extends EventEmitter { } }; +// This promise is used to wait for the search service to be initialized. +// None of the code in the WebExtension modules requests that initialization. +// It is assumed that it is started at some point. That might never happen, +// e.g. if the application shuts down before the search service initializes. +ChromeUtils.defineLazyGetter(global, "searchInitialized", () => { + if (Services.search.isInitialized) { + return Promise.resolve(); + } + return ExtensionUtils.promiseObserved( + "browser-search-service", + (_, data) => data == "init-complete" + ); +}); + class WindowTracker extends WindowTrackerBase { addProgressListener(window, listener) { window.gBrowser.addTabsProgressListener(listener); diff --git a/browser/components/extensions/parent/ext-chrome-settings-overrides.js b/browser/components/extensions/parent/ext-chrome-settings-overrides.js index 2e3a28501419..aa5202ba2de7 100644 --- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js +++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js @@ -284,9 +284,8 @@ this.chrome_settings_overrides = class extends ExtensionAPI { } if (manifest.chrome_settings_overrides.search_provider) { // Registering a search engine can potentially take a long while, - // or not complete at all (when Services.search.promiseInitialized is - // never resolved), so we are deliberately not awaiting the returned - // promise here. + // or not complete at all (when searchInitialized is never resolved), + // so we are deliberately not awaiting the returned promise here. let searchStartupPromise = this.processSearchProviderManifestEntry().finally(() => { if ( @@ -406,7 +405,7 @@ this.chrome_settings_overrides = class extends ExtensionAPI { return; } - await Services.search.promiseInitialized; + await searchInitialized; if (!this.extension) { Cu.reportError( `Extension shut down before search provider was registered` diff --git a/browser/components/extensions/parent/ext-search.js b/browser/components/extensions/parent/ext-search.js index 62db70219416..be51f5a17ae3 100644 --- a/browser/components/extensions/parent/ext-search.js +++ b/browser/components/extensions/parent/ext-search.js @@ -35,7 +35,7 @@ this.search = class extends ExtensionAPI { return { search: { async get() { - await Services.search.promiseInitialized; + await searchInitialized; let visibleEngines = await Services.search.getVisibleEngines(); let defaultEngine = await Services.search.getDefault(); return Promise.all( @@ -69,7 +69,7 @@ this.search = class extends ExtensionAPI { }, async search(searchProperties) { - await Services.search.promiseInitialized; + await searchInitialized; let engine; if (searchProperties.engine) { @@ -97,7 +97,7 @@ this.search = class extends ExtensionAPI { }, async query(queryProperties) { - await Services.search.promiseInitialized; + await searchInitialized; let { tab, where } = getTarget({ tabId: queryProperties.tabId, diff --git a/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_shutdown.js b/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_shutdown.js index 851efd6b2a8a..fdb9baf25ae9 100644 --- a/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_shutdown.js +++ b/browser/components/extensions/test/xpcshell/test_ext_settings_overrides_shutdown.js @@ -46,7 +46,7 @@ add_task(async function shutdown_during_search_provider_startup() { }); let initialized = false; - Services.search.promiseInitialized.then(() => { + ExtensionParent.apiManager.global.searchInitialized.then(() => { initialized = true; }); diff --git a/browser/extensions/search-detection/extension/api.js b/browser/extensions/search-detection/extension/api.js index dd15b080abcb..ae3336930406 100644 --- a/browser/extensions/search-detection/extension/api.js +++ b/browser/extensions/search-detection/extension/api.js @@ -21,6 +21,17 @@ ChromeUtils.defineESModuleGetters(lazy, { // eslint-disable-next-line mozilla/reject-importGlobalProperties XPCOMUtils.defineLazyGlobalGetters(this, ["ChannelWrapper"]); +ChromeUtils.defineLazyGetter(this, "searchInitialized", () => { + if (Services.search.isInitialized) { + return Promise.resolve(); + } + + return ExtensionUtils.promiseObserved( + "browser-search-service", + (_, data) => data === "init-complete" + ); +}); + const SEARCH_TOPIC_ENGINE_MODIFIED = "browser-search-engine-modified"; this.addonsSearchDetection = class extends ExtensionAPI { @@ -45,7 +56,7 @@ this.addonsSearchDetection = class extends ExtensionAPI { const patterns = {}; try { - await Services.search.promiseInitialized; + await searchInitialized; const visibleEngines = await Services.search.getEngines(); visibleEngines.forEach(engine => { diff --git a/toolkit/components/search/SearchService.sys.mjs b/toolkit/components/search/SearchService.sys.mjs index a042bfd23af5..90dec7518dac 100644 --- a/toolkit/components/search/SearchService.sys.mjs +++ b/toolkit/components/search/SearchService.sys.mjs @@ -23,7 +23,6 @@ ChromeUtils.defineESModuleGetters(lazy, { SearchSettings: "resource://gre/modules/SearchSettings.sys.mjs", SearchStaticData: "resource://gre/modules/SearchStaticData.sys.mjs", SearchUtils: "resource://gre/modules/SearchUtils.sys.mjs", - TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.sys.mjs", UserSearchEngine: "resource://gre/modules/UserSearchEngine.sys.mjs", }); @@ -161,6 +160,7 @@ const gEmptyParseSubmissionResult = Object.freeze( */ export class SearchService { constructor() { + this.#initObservers = PromiseUtils.defer(); // this._engines is prefixed with _ rather than # because it is called from // a test. this._engines = new Map(); @@ -277,18 +277,6 @@ export class SearchService { return this.#initializationStatus == "success"; } - /** - * A promise that is resolved when initialization has finished, but does not - * trigger initialization to begin. - * - * @returns {Promise} - * Resolved when initalization has successfully finished, and rejected if it - * has failed. - */ - get promiseInitialized() { - return this.#initObservers.promise; - } - getDefaultEngineInfo() { let [telemetryId, defaultSearchEngineData] = this.#getEngineInfo( this.defaultEngine @@ -1006,12 +994,7 @@ export class SearchService { } // end engine iteration } - /** - * A deferred promise that is resolved when initialization completes. - * - * @type {Deferred} - */ - #initObservers = PromiseUtils.defer(); + #initObservers; #currentEngine; #currentPrivateEngine; #queuedIdle; @@ -1399,9 +1382,14 @@ export class SearchService { this.#initObservers.reject(result); } - lazy.TelemetryEnvironment.startSearchEngineDataRecording(); this.#recordTelemetryData(); + Services.obs.notifyObservers( + null, + lazy.SearchUtils.TOPIC_SEARCH_SERVICE, + "init-complete" + ); + lazy.logConsole.debug("Completed #init"); // It is possible that Nimbus could have called onUpdate before diff --git a/toolkit/components/search/SearchSettings.sys.mjs b/toolkit/components/search/SearchSettings.sys.mjs index fed0dd1808d6..625070ba71c3 100644 --- a/toolkit/components/search/SearchSettings.sys.mjs +++ b/toolkit/components/search/SearchSettings.sys.mjs @@ -28,12 +28,6 @@ const SETTINGS_FILENAME = "search.json.mozlz4"; export class SearchSettings { constructor(searchService) { this.#searchService = searchService; - - // Once the search service has initialized, schedule a write to ensure - // that any settings that may have changed or need updating are handled. - searchService.promiseInitialized.then(() => { - this._delayedWrite(); - }); } QueryInterface = ChromeUtils.generateQI([Ci.nsIObserver]); @@ -502,6 +496,7 @@ export class SearchSettings { break; case lazy.SearchUtils.TOPIC_SEARCH_SERVICE: switch (verb) { + case "init-complete": case "engines-reloaded": this._delayedWrite(); break; diff --git a/toolkit/components/search/nsISearchService.idl b/toolkit/components/search/nsISearchService.idl index dccbe617556c..fc98833678cd 100644 --- a/toolkit/components/search/nsISearchService.idl +++ b/toolkit/components/search/nsISearchService.idl @@ -281,16 +281,6 @@ interface nsISearchService : nsISupports */ Promise init(); - /** - * A promise that is resolved when initialization has finished, but does not - * trigger initialization to begin. - * - * @returns {Promise} - * Resolved when initalization has successfully finished, and rejected if it - * has failed. - */ - readonly attribute Promise promiseInitialized; - /** * Determine whether initialization has been completed. * diff --git a/toolkit/components/telemetry/app/TelemetryEnvironment.sys.mjs b/toolkit/components/telemetry/app/TelemetryEnvironment.sys.mjs index 4479c166cd07..fbb17d65bf1f 100644 --- a/toolkit/components/telemetry/app/TelemetryEnvironment.sys.mjs +++ b/toolkit/components/telemetry/app/TelemetryEnvironment.sys.mjs @@ -155,15 +155,6 @@ export var TelemetryEnvironment = { return result; }, - /** - * Intended to be called by the search service once initialization is complete. - */ - startSearchEngineDataRecording() { - let globalEnvironment = getGlobal(); - globalEnvironment._canQuerySearch = true; - globalEnvironment._updateSearchEngine(); - }, - shutdown() { return getGlobal().shutdown(); }, @@ -388,6 +379,7 @@ const DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC = "distribution-customization-complete"; const GFX_FEATURES_READY_TOPIC = "gfx-features-ready"; const SEARCH_ENGINE_MODIFIED_TOPIC = "browser-search-engine-modified"; +const SEARCH_SERVICE_TOPIC = "browser-search-service"; const SESSIONSTORE_WINDOWS_RESTORED_TOPIC = "sessionstore-windows-restored"; const PREF_CHANGED_TOPIC = "nsPref:changed"; const GMP_PROVIDER_REGISTERED_TOPIC = "gmp-provider-registered"; @@ -1332,6 +1324,7 @@ EnvironmentCache.prototype = { Services.obs.addObserver(this, DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC); Services.obs.addObserver(this, GFX_FEATURES_READY_TOPIC); Services.obs.addObserver(this, SEARCH_ENGINE_MODIFIED_TOPIC); + Services.obs.addObserver(this, SEARCH_SERVICE_TOPIC); Services.obs.addObserver(this, AUTO_UPDATE_PREF_CHANGE_TOPIC); Services.obs.addObserver(this, BACKGROUND_UPDATE_PREF_CHANGE_TOPIC); Services.obs.addObserver(this, SERVICES_INFO_CHANGE_TOPIC); @@ -1349,6 +1342,7 @@ EnvironmentCache.prototype = { } catch (ex) {} Services.obs.removeObserver(this, GFX_FEATURES_READY_TOPIC); Services.obs.removeObserver(this, SEARCH_ENGINE_MODIFIED_TOPIC); + Services.obs.removeObserver(this, SEARCH_SERVICE_TOPIC); Services.obs.removeObserver(this, AUTO_UPDATE_PREF_CHANGE_TOPIC); Services.obs.removeObserver(this, BACKGROUND_UPDATE_PREF_CHANGE_TOPIC); Services.obs.removeObserver(this, SERVICES_INFO_CHANGE_TOPIC); @@ -1375,6 +1369,14 @@ EnvironmentCache.prototype = { // Record the new default search choice and send the change notification. this._onSearchEngineChange(); break; + case SEARCH_SERVICE_TOPIC: + if (aData != "init-complete") { + return; + } + // Now that the search engine init is complete, record the default search choice. + this._canQuerySearch = true; + this._updateSearchEngine(); + break; case GFX_FEATURES_READY_TOPIC: case COMPOSITOR_CREATED_TOPIC: // Full graphics information is not available until we have created at