diff --git a/toolkit/components/search/SearchCache.jsm b/toolkit/components/search/SearchCache.jsm index 8a7623adefd9..8e3fb8d9b727 100644 --- a/toolkit/components/search/SearchCache.jsm +++ b/toolkit/components/search/SearchCache.jsm @@ -46,9 +46,6 @@ XPCOMUtils.defineLazyGetter(this, "gEncoder", function() { return new TextEncoder(); }); -// Delay for batching invalidation of the JSON cache (ms) -const CACHE_INVALIDATION_DELAY = 1000; - const CACHE_FILENAME = "search.json.mozlz4"; /** @@ -62,6 +59,12 @@ class SearchCache { constructor(searchService) { this._searchService = searchService; } + + QueryInterface = ChromeUtils.generateQI([Ci.nsIObserver]); + + // Delay for batching invalidation of the JSON cache (ms) + static CACHE_INVALIDATION_DELAY = 1000; + /** * A reference to the pending DeferredTask, if there is one. */ @@ -93,14 +96,32 @@ class SearchCache { */ _searchService; + addObservers() { + Services.obs.addObserver(this, SearchUtils.TOPIC_ENGINE_MODIFIED); + Services.obs.addObserver(this, SearchUtils.TOPIC_SEARCH_SERVICE); + } + + /** + * Cleans up, removing observers. + */ + removeObservers() { + Services.obs.removeObserver(this, SearchUtils.TOPIC_ENGINE_MODIFIED); + Services.obs.removeObserver(this, SearchUtils.TOPIC_SEARCH_SERVICE); + } + /** * Reads the cache file. * + * @param {string} origin + * If this parameter is "test", then the cache will not be written. As + * some tests manipulate the cache directly, we allow turning off writing to + * avoid writing stale cache data. * @returns {object} * Returns the cache file data. */ - async get() { + async get(origin = "") { let json; + await this._ensurePendingWritesCompleted(origin); try { let cacheFilePath = OS.Path.join( OS.Constants.Path.profileDir, @@ -135,15 +156,15 @@ class SearchCache { * Queues writing the cache until after CACHE_INVALIDATION_DELAY. If there * is a currently queued task then it will be restarted. */ - delayedWrite() { + _delayedWrite() { if (this._batchTask) { this._batchTask.disarm(); } else { let task = async () => { logConsole.debug("batchTask: Invalidating engine cache"); - await this.write(); + await this._write(); }; - this._batchTask = new DeferredTask(task, CACHE_INVALIDATION_DELAY); + this._batchTask = new DeferredTask(task, this.CACHE_INVALIDATION_DELAY); } this._batchTask.arm(); } @@ -156,26 +177,27 @@ class SearchCache { * some tests manipulate the cache directly, we allow turning off writing to * avoid writing stale cache data. */ - async ensurePendingWritesCompleted(origin = "") { + async _ensurePendingWritesCompleted(origin = "") { // Before we read the cache file, first make sure all pending tasks are clear. - if (this._batchTask) { - logConsole.debug("finalizing batch task"); - let task = this._batchTask; - this._batchTask = null; - // Tests manipulate the cache directly, so let's not double-write with - // stale cache data here. - if (origin == "test") { - task.disarm(); - } else { - await task.finalize(); - } + if (!this._batchTask) { + return; + } + logConsole.debug("finalizing batch task"); + let task = this._batchTask; + this._batchTask = null; + // Tests manipulate the cache directly, so let's not double-write with + // stale cache data here. + if (origin == "test") { + task.disarm(); + } else { + await task.finalize(); } } /** * Writes the cache to disk (no delay). */ - async write() { + async _write() { if (this._batchTask) { this._batchTask.disarm(); } @@ -238,7 +260,7 @@ class SearchCache { */ setAttribute(name, val) { this._metaData[name] = val; - this.delayedWrite(); + this._delayedWrite(); } /** @@ -251,8 +273,10 @@ class SearchCache { * The value to set. */ setVerifiedAttribute(name, val) { - this.setAttribute(name, val); - this.setAttribute(this.getHashName(name), getVerificationHash(val)); + this._metaData[name] = val; + this._metaData[this.getHashName(name)] = getVerificationHash(val); + console.log(this._metaData); + this._delayedWrite(); } /** @@ -329,4 +353,28 @@ class SearchCache { } } } + + // nsIObserver + observe(engine, topic, verb) { + switch (topic) { + case SearchUtils.TOPIC_ENGINE_MODIFIED: + switch (verb) { + case SearchUtils.MODIFIED_TYPE.ADDED: + case SearchUtils.MODIFIED_TYPE.CHANGED: + case SearchUtils.MODIFIED_TYPE.REMOVED: + this._delayedWrite(); + break; + } + break; + case SearchUtils.TOPIC_SEARCH_SERVICE: + switch (verb) { + case "init-complete": + case "reinit-complete": + case "engines-reloaded": + this._delayedWrite(); + break; + } + break; + } + } } diff --git a/toolkit/components/search/SearchService.jsm b/toolkit/components/search/SearchService.jsm index fd53991c27e7..40b5424b7ddc 100644 --- a/toolkit/components/search/SearchService.jsm +++ b/toolkit/components/search/SearchService.jsm @@ -556,7 +556,6 @@ SearchService.prototype = { // Make sure the current list of engines is persisted, without the need to wait. logConsole.debug("_init: engines loaded, writing cache"); - this._cache.write(); this._addObservers(); } catch (ex) { this._initRV = ex.result !== undefined ? ex.result : Cr.NS_ERROR_FAILURE; @@ -1292,8 +1291,6 @@ SearchService.prototype = { return; } - await this._cache.ensurePendingWritesCompleted(); - // Capture the current engine state, in case we need to notify below. const prevCurrentEngine = this._currentEngine; const prevPrivateEngine = this._currentPrivateEngine; @@ -1305,8 +1302,6 @@ SearchService.prototype = { // engine order. this.__sortedEngines = null; await this._loadEngines(await this._cache.get(), true); - // Make sure the current list of engines is persisted. - await this._cache.write(); // If the defaultEngine has changed between the previous load and this one, // dispatch the appropriate notifications. @@ -1371,7 +1366,6 @@ SearchService.prototype = { } this._initObservers = PromiseUtils.defer(); - await this._cache.ensurePendingWritesCompleted(origin); // Clear the engines, too, so we don't stick with the stale ones. this._resetLocalData(); @@ -1384,7 +1378,7 @@ SearchService.prototype = { "uninit-complete" ); - let cache = await this._cache.get(); + let cache = await this._cache.get(origin); // The init flow is not going to block on a fetch from an external service, // but we're kicking it off as soon as possible to prevent UI flickering as // much as possible. @@ -1411,9 +1405,6 @@ SearchService.prototype = { return; } - // Make sure the current list of engines is persisted. - await this._cache.write(); - // Typically we'll re-init as a result of a pref observer, // so signal to 'callers' that we're done. gInitialized = true; @@ -3344,7 +3335,6 @@ SearchService.prototype = { case SearchUtils.MODIFIED_TYPE.ADDED: case SearchUtils.MODIFIED_TYPE.CHANGED: case SearchUtils.MODIFIED_TYPE.REMOVED: - this._cache.delayedWrite(); // Invalidate the map used to parse URLs to search engines. this._parseSubmissionMap = null; break; @@ -3457,6 +3447,8 @@ SearchService.prototype = { Services.obs.addObserver(this, QUIT_APPLICATION_TOPIC); Services.obs.addObserver(this, TOPIC_LOCALES_CHANGE); + this._cache.addObservers(); + // The current stage of shutdown. Used to help analyze crash // signatures in case of shutdown timeout. let shutdownState = { @@ -3509,6 +3501,8 @@ SearchService.prototype = { this._queuedIdle = false; } + this._cache.removeObservers(); + Services.obs.removeObserver(this, SearchUtils.TOPIC_ENGINE_MODIFIED); Services.obs.removeObserver(this, QUIT_APPLICATION_TOPIC); Services.obs.removeObserver(this, TOPIC_LOCALES_CHANGE); diff --git a/toolkit/components/search/tests/xpcshell/head_search.js b/toolkit/components/search/tests/xpcshell/head_search.js index 44035ed0b06b..fd0a4107d9e3 100644 --- a/toolkit/components/search/tests/xpcshell/head_search.js +++ b/toolkit/components/search/tests/xpcshell/head_search.js @@ -12,6 +12,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { Region: "resource://gre/modules/Region.jsm", RemoteSettings: "resource://services-settings/remote-settings.js", RemoteSettingsClient: "resource://services-settings/RemoteSettingsClient.jsm", + SearchCache: "resource://gre/modules/SearchCache.jsm", SearchEngineSelector: "resource://gre/modules/SearchEngineSelector.jsm", SearchTestUtils: "resource://testing-common/SearchTestUtils.jsm", Services: "resource://gre/modules/Services.jsm", @@ -69,6 +70,10 @@ Services.prefs.setBoolPref( true ); +// For tests, allow the cache to write sooner than it would do normally so that +// the tests that need to wait for it can run a bit faster. +SearchCache.CACHE_INVALIDATION_DELAY = 250; + /** * Load engines from test data located in particular folders. * diff --git a/toolkit/components/search/tests/xpcshell/test_json_cache.js b/toolkit/components/search/tests/xpcshell/test_json_cache.js index 469b747a6663..a4fe8987db34 100644 --- a/toolkit/components/search/tests/xpcshell/test_json_cache.js +++ b/toolkit/components/search/tests/xpcshell/test_json_cache.js @@ -156,7 +156,7 @@ add_task(async function test_cache_write() { Services.tm.dispatchToMainThread(() => { // Call the observe method directly to simulate a remove but not actually // remove anything. - Services.search + Services.search.wrappedJSObject._cache .QueryInterface(Ci.nsIObserver) .observe(null, "browser-search-engine-modified", "engine-removed"); }); diff --git a/toolkit/components/search/tests/xpcshell/test_webextensions_builtin_upgrade.js b/toolkit/components/search/tests/xpcshell/test_webextensions_builtin_upgrade.js index e77c6589534f..1609e46b299b 100644 --- a/toolkit/components/search/tests/xpcshell/test_webextensions_builtin_upgrade.js +++ b/toolkit/components/search/tests/xpcshell/test_webextensions_builtin_upgrade.js @@ -113,5 +113,4 @@ add_task(async function basic_multilocale_test() { ); await ext.unload(); - await promiseAfterCache(); });