From 7a246d97fe624486797688e60939778d1fb6ae05 Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Wed, 10 Jul 2019 22:47:02 +0000 Subject: [PATCH] Bug 1556789 - Refactor extension install in searchservice to use promises r=robwu,daleharvey This provides a set of promises that the searchservice resolves once the search engine has been configured Differential Revision: https://phabricator.services.mozilla.com/D33660 --HG-- extra : moz-landing-system : lando --- .../parent/ext-chrome-settings-overrides.js | 53 +-- .../extensions/test/xpcshell/head.js | 6 + ...st_ext_chrome_settings_overrides_update.js | 5 + .../test/xpcshell/test_ext_urlbar.js | 1 + .../preferences/in-content/tests/browser.ini | 3 + .../in-content/tests/browser_engines.js | 3 + .../test/browser/browser_contextmenu.js | 6 +- .../tests/unit/test_distribution.js | 5 + browser/components/urlbar/tests/unit/head.js | 8 + testing/profiles/common/user.js | 4 + .../unifiedcomplete/head_autocomplete.js | 11 + .../test_PlacesSearchAutocompleteProvider.js | 22 +- .../places/tests/unifiedcomplete/xpcshell.ini | 8 +- .../places/tests/unit/head_bookmarks.js | 5 + toolkit/components/search/SearchEngine.jsm | 24 +- toolkit/components/search/SearchService.jsm | 316 ++++++++---------- toolkit/components/search/SearchUtils.jsm | 146 +++++++- .../components/search/nsISearchService.idl | 2 - .../invalid-extension/invalid/manifest.json | 19 ++ .../xpcshell/data/invalid-extension/list.json | 7 + .../search/tests/xpcshell/head_search.js | 4 + .../tests/xpcshell/test_async_distribution.js | 26 +- .../xpcshell/test_list_json_searchorder.js | 6 +- .../test_migrateWebExtensionEngine.js | 130 ++++++- .../tests/xpcshell/test_parseSubmissionURL.js | 191 ++++++----- .../xpcshell/test_remove_profile_engine.js | 3 +- .../test_require_engines_for_cache.js | 16 + .../xpcshell/test_require_engines_in_cache.js | 52 +-- .../tests/xpcshell/test_validate_engines.js | 50 ++- .../test_webextensions_install_failure.js | 30 ++ .../search/tests/xpcshell/xpcshell.ini | 4 + .../harness/telemetry_harness/runner.py | 12 +- .../components/telemetry/tests/unit/head.js | 5 + 33 files changed, 791 insertions(+), 392 deletions(-) create mode 100644 toolkit/components/search/tests/xpcshell/data/invalid-extension/invalid/manifest.json create mode 100644 toolkit/components/search/tests/xpcshell/data/invalid-extension/list.json create mode 100644 toolkit/components/search/tests/xpcshell/test_require_engines_for_cache.js create mode 100644 toolkit/components/search/tests/xpcshell/test_webextensions_install_failure.js diff --git a/browser/components/extensions/parent/ext-chrome-settings-overrides.js b/browser/components/extensions/parent/ext-chrome-settings-overrides.js index b34a9943813e..f20e9210c2ac 100644 --- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js +++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js @@ -310,7 +310,12 @@ this.chrome_settings_overrides = class extends ExtensionAPI { let { extension } = this; let { manifest } = extension; let searchProvider = manifest.chrome_settings_overrides.search_provider; - if (searchProvider.is_default) { + let handleIsDefault = + searchProvider.is_default && !extension.addonData.builtIn; + let engineName = searchProvider.name.trim(); + // Builtin extensions are never marked with is_default. We can safely wait on + // the search service to fully initialize before handling these extensions. + if (handleIsDefault) { await searchInitialized; if (!this.extension) { Cu.reportError( @@ -318,10 +323,6 @@ this.chrome_settings_overrides = class extends ExtensionAPI { ); return; } - } - - let engineName = searchProvider.name.trim(); - if (searchProvider.is_default) { let engine = Services.search.getEngineByName(engineName); let defaultEngines = await Services.search.getDefaultEngines(); if ( @@ -336,7 +337,7 @@ this.chrome_settings_overrides = class extends ExtensionAPI { } } await this.addSearchEngine(); - if (searchProvider.is_default) { + if (handleIsDefault) { if (extension.startupReason === "ADDON_INSTALL") { // Don't ask if it already the current engine let engine = Services.search.getEngineByName(engineName); @@ -417,29 +418,10 @@ this.chrome_settings_overrides = class extends ExtensionAPI { async addSearchEngine() { let { extension } = this; - let isCurrent = false; - let index = -1; - if ( - extension.startupReason === "ADDON_UPGRADE" && - !extension.addonData.builtIn - ) { - let engines = await Services.search.getEnginesByExtensionID(extension.id); - if (engines.length > 0) { - let firstEngine = engines[0]; - let firstEngineName = firstEngine.name; - // There can be only one engine right now - isCurrent = - (await Services.search.getDefault()).name == firstEngineName; - // Get position of engine and store it - index = (await Services.search.getEngines()) - .map(engine => engine.name) - .indexOf(firstEngineName); - await Services.search.removeEngine(firstEngine); - } - } try { + // This is safe to await prior to SearchService.init completing. let engines = await Services.search.addEnginesFromExtension(extension); - if (engines.length > 0) { + if (engines[0]) { await ExtensionSettingsStore.addSetting( extension.id, DEFAULT_SEARCH_STORE_TYPE, @@ -447,26 +429,9 @@ this.chrome_settings_overrides = class extends ExtensionAPI { engines[0].name ); } - if ( - extension.startupReason === "ADDON_UPGRADE" && - !extension.addonData.builtIn - ) { - let engines = await Services.search.getEnginesByExtensionID( - extension.id - ); - let engine = Services.search.getEngineByName(engines[0].name); - if (isCurrent) { - await Services.search.setDefault(engine); - } - if (index != -1) { - await Services.search.moveEngine(engine, index); - } - } } catch (e) { Cu.reportError(e); - return false; } - return true; } }; diff --git a/browser/components/extensions/test/xpcshell/head.js b/browser/components/extensions/test/xpcshell/head.js index 886b95591966..7d5eee569c84 100644 --- a/browser/components/extensions/test/xpcshell/head.js +++ b/browser/components/extensions/test/xpcshell/head.js @@ -20,6 +20,12 @@ XPCOMUtils.defineLazyModuleGetters(this, { TestUtils: "resource://testing-common/TestUtils.jsm", }); +// For search related tests, reduce what is happening. Search tests cover +// these otherwise. +Services.prefs.setCharPref("browser.search.region", "US"); +Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); +Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + Services.prefs.setBoolPref("extensions.webextensions.remote", false); ExtensionTestUtils.init(this); diff --git a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js index 892570536522..34c6d693c5dc 100644 --- a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js +++ b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js @@ -18,7 +18,12 @@ AddonTestUtils.createAppInfo( ); add_task(async function setup() { + Services.prefs.setCharPref("browser.search.region", "US"); + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + await AddonTestUtils.promiseStartupManager(); + await Services.search.init(); }); add_task(async function test_overrides_update_removal() { diff --git a/browser/components/extensions/test/xpcshell/test_ext_urlbar.js b/browser/components/extensions/test/xpcshell/test_ext_urlbar.js index d83b6abf3b04..12a8f81a1e34 100644 --- a/browser/components/extensions/test/xpcshell/test_ext_urlbar.js +++ b/browser/components/extensions/test/xpcshell/test_ext_urlbar.js @@ -30,6 +30,7 @@ add_task(async function startup() { Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); await AddonTestUtils.promiseStartupManager(); + await Services.search.init(true); // Add a test engine and make it default so that when we do searches below, // Firefox doesn't try to include search suggestions from the actual default diff --git a/browser/components/preferences/in-content/tests/browser.ini b/browser/components/preferences/in-content/tests/browser.ini index 52b5269760c3..7df75736a447 100644 --- a/browser/components/preferences/in-content/tests/browser.ini +++ b/browser/components/preferences/in-content/tests/browser.ini @@ -2,6 +2,9 @@ prefs = extensions.formautofill.available='on' extensions.formautofill.creditCards.available=true +# turn off geo updates for search related tests + browser.search.region=US + browser.search.geoSpecificDefaults=false support-files = head.js privacypane_tests_perwindow.js diff --git a/browser/components/preferences/in-content/tests/browser_engines.js b/browser/components/preferences/in-content/tests/browser_engines.js index 8a8114d8c6e0..369d37a2fe0a 100644 --- a/browser/components/preferences/in-content/tests/browser_engines.js +++ b/browser/components/preferences/in-content/tests/browser_engines.js @@ -1,5 +1,8 @@ // Test Engine list add_task(async function() { + // running stand-alone, be sure to wait for init + await Services.search.init(); + let prefs = await openPreferencesViaOpenPreferencesAPI("search", { leaveOpen: true, }); diff --git a/browser/components/search/test/browser/browser_contextmenu.js b/browser/components/search/test/browser/browser_contextmenu.js index 9b493c98b599..5a148f26c165 100644 --- a/browser/components/search/test/browser/browser_contextmenu.js +++ b/browser/components/search/test/browser/browser_contextmenu.js @@ -4,6 +4,10 @@ * Test searching for the selected text using the context menu */ +const { SearchExtensionLoader } = ChromeUtils.import( + "resource://gre/modules/SearchUtils.jsm" +); + const ENGINE_NAME = "mozSearch"; const ENGINE_ID = "mozsearch-engine@search.mozilla.org"; @@ -28,7 +32,7 @@ add_task(async function() { Services.io.newURI("file://" + searchExtensions.path) ); - await Services.search.ensureBuiltinExtension(ENGINE_ID); + await SearchExtensionLoader.installAddons([ENGINE_ID]); let engine = await Services.search.getEngineByName(ENGINE_NAME); ok(engine, "Got a search engine"); diff --git a/browser/components/tests/unit/test_distribution.js b/browser/components/tests/unit/test_distribution.js index e050408d0b21..ba1b5145bde0 100644 --- a/browser/components/tests/unit/test_distribution.js +++ b/browser/components/tests/unit/test_distribution.js @@ -280,6 +280,11 @@ add_task(async function() { "de-DE" ); + // Turn off region updates and timeouts for search service + Services.prefs.setCharPref("browser.search.region", "DE"); + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + await Services.search.init(); var engine = Services.search.getEngineByName("Google"); Assert.equal(engine.description, "override-de-DE"); diff --git a/browser/components/urlbar/tests/unit/head.js b/browser/components/urlbar/tests/unit/head.js index 55ab69c4a735..873b7707b0cf 100644 --- a/browser/components/urlbar/tests/unit/head.js +++ b/browser/components/urlbar/tests/unit/head.js @@ -35,6 +35,11 @@ XPCOMUtils.defineLazyModuleGetters(this, { }); const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm"); +// Turn off region updates and timeouts for search service +Services.prefs.setCharPref("browser.search.region", "US"); +Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); +Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + /** * @param {string} searchString The search string to insert into the context. * @param {object} properties Overrides for the default values. @@ -194,6 +199,9 @@ async function addTestEngine(basename, httpServer = undefined) { } /** + * WARNING: use of this function may result in intermittent failures when tests + * run in parallel due to reliance on port 9000. Duplicated in/from unifiedcomplete. + * * Sets up a search engine that provides some suggestions by appending strings * onto the search query. * diff --git a/testing/profiles/common/user.js b/testing/profiles/common/user.js index b8907ad6b221..b5fc7cd92357 100644 --- a/testing/profiles/common/user.js +++ b/testing/profiles/common/user.js @@ -24,6 +24,10 @@ user_pref("browser.pagethumbnails.capturing_disabled", true); user_pref("browser.search.region", "US"); // This will prevent HTTP requests for region defaults. user_pref("browser.search.geoSpecificDefaults", false); +// Debug builds will timeout on the failsafe timeout for search init, +// we just turn off the load timeout for tests in general. +user_pref("browser.search.addonLoadTimeout", 0); + // Disable webapp updates. Yes, it is supposed to be an integer. user_pref("browser.webapps.checkForUpdates", 0); // We do not wish to display datareporting policy notifications as it might diff --git a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js index 653a30063216..010c2bf107d6 100644 --- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js +++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js @@ -74,7 +74,14 @@ AddonTestUtils.createAppInfo( ); add_task(async function setup() { + // Tell the search service we are running in the US. This also has the + // desired side-effect of preventing our geoip lookup. + Services.prefs.setCharPref("browser.search.region", "US"); + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + await AddonTestUtils.promiseStartupManager(); + await Services.search.init(); }); async function cleanup() { @@ -560,6 +567,9 @@ function addTestEngine(basename, httpServer = undefined) { } /** + * WARNING: use of this function may result in intermittent failures when tests + * run in parallel due to reliance on port 9000. + * * Sets up a search engine that provides some suggestions by appending strings * onto the search query. * @@ -606,6 +616,7 @@ add_task(async function ensure_search_engine() { await Services.search.addEngineWithDetails("MozSearch", { method: "GET", template: "http://s.example.com/search", + isBuiltin: true, }); let engine = Services.search.getEngineByName("MozSearch"); await Services.search.setDefault(engine); diff --git a/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js b/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js index a8856505364c..5ac748791b5d 100644 --- a/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js +++ b/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js @@ -6,12 +6,14 @@ const { PlacesSearchAutocompleteProvider } = ChromeUtils.import( "resource://gre/modules/PlacesSearchAutocompleteProvider.jsm" ); -add_task(async function() { - await Services.search.init(); +add_task(async function setup() { // Tell the search service we are running in the US. This also has the // desired side-effect of preventing our geoip lookup. Services.prefs.setCharPref("browser.search.region", "US"); Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + + await Services.search.init(); Services.search.restoreDefaultEngines(); Services.search.resetToOriginalDefaultEngine(); @@ -38,9 +40,9 @@ add_task(async function hide_search_engine_nomatch() { let engine = await Services.search.getDefault(); let domain = engine.getResultDomain(); let token = domain.substr(0, 1); - let promiseTopic = promiseSearchTopic("engine-changed"); + let promiseTopic = promiseSearchTopic("engine-removed"); await Promise.all([Services.search.removeEngine(engine), promiseTopic]); - Assert.ok(engine.hidden); + Assert.ok(engine.hidden, "engine was hidden rather than removed"); let matchedEngine = await PlacesSearchAutocompleteProvider.engineForDomainPrefix( token ); @@ -163,7 +165,11 @@ add_task(async function test_parseSubmissionURL_basic() { let result = PlacesSearchAutocompleteProvider.parseSubmissionURL( submissionURL ); - Assert.equal(result.engineName, engine.name); + Assert.equal( + result.engineName, + engine.name, + "parsed submissionURL has matching engine name" + ); Assert.equal(result.terms, "terms"); result = PlacesSearchAutocompleteProvider.parseSubmissionURL( @@ -174,8 +180,8 @@ add_task(async function test_parseSubmissionURL_basic() { add_task(async function test_builtin_aliased_search_engine_match() { let engine = await PlacesSearchAutocompleteProvider.engineForAlias("@google"); - Assert.ok(engine); - Assert.equal(engine.name, "Google"); + Assert.ok(engine, "matched an engine with an alias"); + Assert.equal(engine.name, "Google", "correct engine for alias"); let promiseTopic = promiseSearchTopic("engine-changed"); await Promise.all([Services.search.removeEngine(engine), promiseTopic]); let matchedEngine = await PlacesSearchAutocompleteProvider.engineForAlias( @@ -187,7 +193,7 @@ add_task(async function test_builtin_aliased_search_engine_match() { PlacesSearchAutocompleteProvider.engineForAlias("@google") ); engine = await PlacesSearchAutocompleteProvider.engineForAlias("@google"); - Assert.ok(engine); + Assert.ok(engine, "matched an engine with an alias"); }); function promiseSearchTopic(expectedVerb) { diff --git a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini index d93630bc78f5..fd23064d57b1 100644 --- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini +++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini @@ -43,14 +43,18 @@ skip-if = appname == "thunderbird" [test_query_url.js] [test_remote_tab_matches.js] skip-if = !sync -[test_search_engine_alias.js] [test_search_engine_default.js] [test_search_engine_host.js] [test_search_engine_restyle.js] [test_search_suggestions.js] -[test_special_search.js] [test_swap_protocol.js] [test_tab_matches.js] [test_trimming.js] [test_visit_url.js] [test_word_boundary_search.js] +# The following tests use addTestSuggestionsEngine which doesn't +# play well when run in parallel. +[test_search_engine_alias.js] +run-sequentially = Test relies on port 9000, fails intermittently +[test_special_search.js] +run-sequentially = Test relies on port 9000, may fail intermittently diff --git a/toolkit/components/places/tests/unit/head_bookmarks.js b/toolkit/components/places/tests/unit/head_bookmarks.js index e251cfe10582..32646aed6533 100644 --- a/toolkit/components/places/tests/unit/head_bookmarks.js +++ b/toolkit/components/places/tests/unit/head_bookmarks.js @@ -18,6 +18,11 @@ const { AddonTestUtils } = ChromeUtils.import( "resource://testing-common/AddonTestUtils.jsm" ); +// Turn off region updates and timeouts for search service +Services.prefs.setCharPref("browser.search.region", "US"); +Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); +Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + AddonTestUtils.init(this, false); AddonTestUtils.overrideCertDB(); AddonTestUtils.createAppInfo( diff --git a/toolkit/components/search/SearchEngine.jsm b/toolkit/components/search/SearchEngine.jsm index f64f24bf20d4..42e3c6266e0a 100644 --- a/toolkit/components/search/SearchEngine.jsm +++ b/toolkit/components/search/SearchEngine.jsm @@ -48,6 +48,7 @@ const MOZSEARCH_NS_10 = "http://www.mozilla.org/2006/browser/search/"; const MOZSEARCH_LOCALNAME = "SearchPlugin"; const USER_DEFINED = "searchTerms"; +const SEARCH_TERM_PARAM = "{searchTerms}"; // Custom search parameters const MOZ_PARAM_LOCALE = "moz:locale"; @@ -580,8 +581,18 @@ EngineURL.prototype = { }, _getTermsParameterName() { - let queryParam = this.params.find(p => p.value == "{" + USER_DEFINED + "}"); - return queryParam ? queryParam.name : ""; + if (this.params.length > 0) { + let queryParam = this.params.find(p => p.value == SEARCH_TERM_PARAM); + return queryParam ? queryParam.name : ""; + } + // If an engine only used template, then params is empty, fall back to checking the template. + let params = new URL(this.template).searchParams; + for (let [name, value] of params.entries()) { + if (value == SEARCH_TERM_PARAM) { + return name; + } + } + return ""; }, _hasRelation(rel) { @@ -814,6 +825,8 @@ SearchEngine.prototype = { _iconUpdateURL: null, /* The extension ID if added by an extension. */ _extensionID: null, + /* The extension version if added by an extension. */ + _version: null, // Built in search engine extensions. _isBuiltin: false, @@ -1403,6 +1416,7 @@ SearchEngine.prototype = { */ _initFromMetadata(engineName, params) { this._extensionID = params.extensionID; + this._version = params.version; this._isBuiltin = !!params.isBuiltin; this._initEngineURLFromMetaData(SearchUtils.URL_TYPE.SEARCH, { @@ -1684,6 +1698,9 @@ SearchEngine.prototype = { if (json.extensionID) { this._extensionID = json.extensionID; } + if (json.version) { + this._version = json.version; + } for (let i = 0; i < json._urls.length; ++i) { let url = json._urls[i]; let engineURL = new EngineURL( @@ -1742,6 +1759,9 @@ SearchEngine.prototype = { if (this._extensionID) { json.extensionID = this._extensionID; } + if (this._version) { + json.version = this._version; + } return json; }, diff --git a/toolkit/components/search/SearchService.jsm b/toolkit/components/search/SearchService.jsm index 2d05b6ffebb8..8075e0b7df22 100644 --- a/toolkit/components/search/SearchService.jsm +++ b/toolkit/components/search/SearchService.jsm @@ -13,7 +13,6 @@ const { PromiseUtils } = ChromeUtils.import( XPCOMUtils.defineLazyModuleGetters(this, { AppConstants: "resource://gre/modules/AppConstants.jsm", - AddonManager: "resource://gre/modules/AddonManager.jsm", clearTimeout: "resource://gre/modules/Timer.jsm", DeferredTask: "resource://gre/modules/DeferredTask.jsm", ExtensionParent: "resource://gre/modules/ExtensionParent.jsm", @@ -22,6 +21,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { RemoteSettings: "resource://services-settings/remote-settings.js", RemoteSettingsClient: "resource://services-settings/RemoteSettingsClient.jsm", SearchEngine: "resource://gre/modules/SearchEngine.jsm", + SearchExtensionLoader: "resource://gre/modules/SearchUtils.jsm", SearchStaticData: "resource://gre/modules/SearchStaticData.jsm", SearchUtils: "resource://gre/modules/SearchUtils.jsm", Services: "resource://gre/modules/Services.jsm", @@ -53,14 +53,6 @@ XPCOMUtils.defineLazyGetter(this, "gEncoder", function() { // Directory service keys const NS_APP_DISTRIBUTION_SEARCH_DIR_LIST = "SrchPluginsDistDL"; -// We load plugins from EXT_SEARCH_PREFIX, where a list.json -// file needs to exist to list available engines. -const EXT_SEARCH_PREFIX = "resource://search-extensions/"; -const APP_SEARCH_PREFIX = "resource://search-plugins/"; - -// The address we use to sign the built in search extensions with. -const EXT_SIGNING_ADDRESS = "search.mozilla.org"; - const TOPIC_LOCALES_CHANGE = "intl:app-locales-changed"; const QUIT_APPLICATION_TOPIC = "quit-application"; @@ -267,11 +259,12 @@ function fetchRegion(ss) { let endpoint = Services.urlFormatter.formatURLPref( "browser.search.geoip.url" ); - SearchUtils.log("_fetchRegion starting with endpoint " + endpoint); // As an escape hatch, no endpoint means no geoip. if (!endpoint) { return Promise.resolve(); } + SearchUtils.log("_fetchRegion starting with endpoint " + endpoint); + let startTime = Date.now(); return new Promise(resolve => { // Instead of using a timeout on the xhr object itself, we simulate one @@ -569,6 +562,9 @@ const gEmptyParseSubmissionResult = Object.freeze( */ function SearchService() { this._initObservers = PromiseUtils.defer(); + // This deferred promise is resolved once a set of engines have been + // parsed out of list.json, which happens in _loadEngines. + this._extensionLoadReady = PromiseUtils.defer(); } SearchService.prototype = { @@ -640,6 +636,7 @@ SearchService.prototype = { async _init(skipRegionCheck) { SearchUtils.log("_init start"); + TelemetryStopwatch.start("SEARCH_SERVICE_INIT_MS"); try { // See if we have a cache file so we don't have to parse a bunch of XML. let cache = await this._readCacheFile(); @@ -665,15 +662,19 @@ SearchService.prototype = { this._buildCache(); this._addObservers(); } catch (ex) { - this._initRV = ex.result !== undefined ? ex.result : Cr.NS_ERROR_FAILURE; + // If loadEngines has a rejected promise chain, ex is undefined. + this._initRV = + ex && ex.result !== undefined ? ex.result : Cr.NS_ERROR_FAILURE; SearchUtils.log( - "_init: failure initializng search: " + ex + "\n" + ex.stack + "_init: failure initializing search: " + ex + "\n" + (ex && ex.stack) ); } gInitialized = true; if (Components.isSuccessCode(this._initRV)) { + TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS"); this._initObservers.resolve(this._initRV); } else { + TelemetryStopwatch.cancel("SEARCH_SERVICE_INIT_MS"); this._initObservers.reject(this._initRV); } Services.obs.notifyObservers( @@ -683,7 +684,6 @@ SearchService.prototype = { ); SearchUtils.log("_init: Completed _init"); - return this._initRV; }, /** @@ -847,20 +847,16 @@ SearchService.prototype = { return val; }, - _listJSONURL: - (AppConstants.platform == "android" - ? APP_SEARCH_PREFIX - : EXT_SEARCH_PREFIX) + "list.json", + // Some tests need to modify this url, they can do so through SearchUtils. + get _listJSONURL() { + return SearchUtils.LIST_JSON_URL; + }, _engines: {}, __sortedEngines: null, _visibleDefaultEngines: [], _searchDefault: null, _searchOrder: [], - // A Set of installed search extensions reported by AddonManager - // startup before SearchSevice has started. Will be installed - // during init(). - _startupExtensions: new Set(), get _sortedEngines() { if (!this.__sortedEngines) { @@ -1026,12 +1022,15 @@ SearchService.prototype = { this._visibleDefaultEngines.length || this._visibleDefaultEngines.some(notInCacheVisibleEngines); + this._engineLocales = this._enginesToLocales(engines); + this._extensionLoadReady.resolve(); + if (!rebuildCache) { SearchUtils.log("_loadEngines: loading from cache directories"); this._loadEnginesFromCache(cache); if (Object.keys(this._engines).length) { SearchUtils.log("_loadEngines: done using existing cache"); - return; + return Promise.resolve(); } SearchUtils.log( "_loadEngines: No valid engines found in cache. Loading engines from disk." @@ -1049,19 +1048,7 @@ SearchService.prototype = { let enginesFromURLs = await this._loadFromChromeURLs(engines, isReload); enginesFromURLs.forEach(this._addEngineToStore, this); } else { - let engineList = this._enginesToLocales(engines); - for (let [id, locales] of engineList) { - await this.ensureBuiltinExtension(id, locales); - } - - SearchUtils.log( - "_loadEngines: loading " + - this._startupExtensions.size + - " engines reported by AddonManager startup" - ); - for (let extension of this._startupExtensions) { - await this._installExtensionEngine(extension, [DEFAULT_TAG], true); - } + return SearchExtensionLoader.installAddons(this._engineLocales.keys()); } SearchUtils.log( @@ -1072,39 +1059,7 @@ SearchService.prototype = { this._loadEnginesMetadataFromCache(cache); SearchUtils.log("_loadEngines: done using rebuilt cache"); - }, - - /** - * Ensures a built in search WebExtension is installed, installing - * it if necessary. - * - * @param {string} id - * The WebExtension ID. - * @param {Array} locales - * An array of locales to use for the WebExtension. If more than - * one is specified, different versions of the same engine may - * be installed. - */ - async ensureBuiltinExtension(id, locales = [DEFAULT_TAG]) { - SearchUtils.log("ensureBuiltinExtension: " + id); - try { - let policy = WebExtensionPolicy.getByID(id); - if (!policy) { - SearchUtils.log("ensureBuiltinExtension: Installing " + id); - let path = EXT_SEARCH_PREFIX + id.split("@")[0] + "/"; - await AddonManager.installBuiltinAddon(path); - policy = WebExtensionPolicy.getByID(id); - } - // On startup the extension may have not finished parsing the - // manifest, wait for that here. - await policy.readyPromise; - await this._installExtensionEngine(policy.extension, locales); - SearchUtils.log("ensureBuiltinExtension: " + id + " installed."); - } catch (err) { - Cu.reportError( - "Failed to install engine: " + err.message + "\n" + err.stack - ); - } + return Promise.resolve(); }, /** @@ -1113,13 +1068,13 @@ SearchService.prototype = { * * @param {array} engines * An array of engines - * @returns {Map} A Map of extension names + locales. + * @returns {Map} A Map of extension IDs to locales. */ _enginesToLocales(engines) { let engineLocales = new Map(); for (let engine of engines) { let [extensionName, locale] = this._parseEngineName(engine); - let id = extensionName + "@" + EXT_SIGNING_ADDRESS; + let id = SearchUtils.makeExtensionId(extensionName); let locales = engineLocales.get(id) || new Set(); locales.add(locale); engineLocales.set(id, locales); @@ -1202,9 +1157,14 @@ SearchService.prototype = { // Start by clearing the initialized state, so we don't abort early. gInitialized = false; + // Reset any init promises synchronously before the async init below. + this._initObservers = PromiseUtils.defer(); + this._extensionLoadReady = PromiseUtils.defer(); + // If reset is called prior to reinit, be sure to mark init as started. + this._initStarted = true; + (async () => { try { - this._initObservers = PromiseUtils.defer(); if (this._batchTask) { SearchUtils.log("finalizing batch task"); let task = this._batchTask; @@ -1226,6 +1186,7 @@ SearchService.prototype = { this._searchDefault = null; this._searchOrder = []; this._metaData = {}; + this._engineLocales = null; // Tests that want to force a synchronous re-initialization need to // be notified when we are done uninitializing. @@ -1270,6 +1231,7 @@ SearchService.prototype = { SearchUtils.TOPIC_SEARCH_SERVICE, "reinit-failed" ); + this._initObservers.reject(); } finally { gReinitializing = false; Services.obs.notifyObservers( @@ -1293,6 +1255,8 @@ SearchService.prototype = { this._visibleDefaultEngines = []; this._searchOrder = []; this._metaData = {}; + this._extensionLoadReady = PromiseUtils.defer(); + this._engineLocales = null; }, /** @@ -1347,21 +1311,31 @@ SearchService.prototype = { return; } - SearchUtils.log('_addEngineToStore: Adding engine: "' + engine.name + '"'); - // See if there is an existing engine with the same name. However, if this // engine is updating another engine, it's allowed to have the same name. - var hasSameNameAsUpdate = - engine._engineToUpdate && engine.name == engine._engineToUpdate.name; - if (engine.name in this._engines && !hasSameNameAsUpdate) { + var matchingEngineUpdate = + engine._engineToUpdate && + (engine.name == engine._engineToUpdate.name || + (engine._extensionID && + engine._extensionID == engine._engineToUpdate._extensionID)); + if (engine.name in this._engines && !matchingEngineUpdate) { SearchUtils.log("_addEngineToStore: Duplicate engine found, aborting!"); return; } if (engine._engineToUpdate) { + SearchUtils.log( + '_addEngineToStore: Updating engine: "' + engine.name + '"' + ); // We need to replace engineToUpdate with the engine that just loaded. var oldEngine = engine._engineToUpdate; + let index = -1; + if (this.__sortedEngines) { + index = this.__sortedEngines.indexOf(oldEngine); + } + let isCurrent = this._currentEngine == oldEngine; + // Remove the old engine from the hash, since it's keyed by name, and our // name might change (the update might have a new name). delete this._engines[oldEngine.name]; @@ -1380,8 +1354,18 @@ SearchService.prototype = { // Add the engine back this._engines[engine.name] = engine; + if (index >= 0) { + this.__sortedEngines[index] = engine; + this._saveSortedEngineList(); + } + if (isCurrent) { + this._currentEngine = engine; + } SearchUtils.notifyAction(engine, SearchUtils.MODIFIED_TYPE.CHANGED); } else { + SearchUtils.log( + '_addEngineToStore: Adding engine: "' + engine.name + '"' + ); // Not an update, just add the new engine. this._engines[engine.name] = engine; // Only add the engine to the list of sorted engines if the initial list @@ -1534,7 +1518,9 @@ SearchService.prototype = { SearchUtils.log( "_loadFromChromeURLs: loading engine from chrome url: " + url ); - let uri = Services.io.newURI(APP_SEARCH_PREFIX + url + ".xml"); + let uri = Services.io.newURI( + SearchUtils.APP_SEARCH_PREFIX + url + ".xml" + ); let engine = new SearchEngine({ uri, readOnly: true, @@ -1575,10 +1561,10 @@ SearchService.prototype = { let request = new XMLHttpRequest(); request.overrideMimeType("text/plain"); let list = await new Promise(resolve => { - request.onload = function(event) { + request.onload = event => { resolve(event.target.responseText); }; - request.onerror = function(event) { + request.onerror = event => { SearchUtils.log("_findEngines: failed to read " + this._listJSONURL); resolve(); }; @@ -1586,7 +1572,7 @@ SearchService.prototype = { request.send(); }); - return this._parseListJSON(list); + return list !== undefined ? this._parseListJSON(list) : []; }, _parseListJSON(list) { @@ -1742,7 +1728,7 @@ SearchService.prototype = { } if (!this._searchDefault) { - Cu.reportError("parseListJSON: No searchDefault"); + SearchUtils.log("parseListJSON: No searchDefault"); } if ( @@ -1925,38 +1911,18 @@ SearchService.prototype = { // nsISearchService async init(skipRegionCheck = false) { - SearchUtils.log("SearchService.init"); if (this._initStarted) { if (!skipRegionCheck) { await this._ensureKnownRegionPromise; } return this._initObservers.promise; } - - TelemetryStopwatch.start("SEARCH_SERVICE_INIT_MS"); this._initStarted = true; - try { - // Complete initialization by calling asynchronous initializer. - await this._init(skipRegionCheck); - TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS"); - } catch (ex) { - if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) { - // No need to pursue asynchronous because synchronous fallback was - // called and has finished. - TelemetryStopwatch.finish("SEARCH_SERVICE_INIT_MS"); - } else { - this._initObservers.reject(ex.result); - TelemetryStopwatch.cancel("SEARCH_SERVICE_INIT_MS"); - throw ex; - } - } - if (!Components.isSuccessCode(this._initRV)) { - throw Components.Exception( - "SearchService initialization failed", - this._initRV - ); - } - return this._initRV; + SearchUtils.log("SearchService.init"); + + // Don't await on _init, _initObservers is resolved or rejected in _init. + this._init(skipRegionCheck); + return this._initObservers.promise; }, get isInitialized() { @@ -2083,17 +2049,17 @@ SearchService.prototype = { }, async addEngineWithDetails(name, details) { - SearchUtils.log('addEngineWithDetails: Adding "' + name + '".'); - let isCurrent = false; - var params = details; - - let isBuiltin = !!params.isBuiltin; - // We install search extensions during the init phase, both built in - // web extensions freshly installed (via addEnginesFromExtension) or - // user installed extensions being reenabled calling this directly. - if (!gInitialized && !isBuiltin && !params.initEngine) { + // We only enforce init when called via the IDL API. Internally we are adding engines + // during init and do not wait on this. + if (!gInitialized) { await this.init(true); } + return this._addEngineWithDetails(name, details); + }, + + async _addEngineWithDetails(name, params) { + SearchUtils.log('addEngineWithDetails: Adding "' + name + '".'); + if (!name) { SearchUtils.fail("Invalid name passed to addEngineWithDetails!"); } @@ -2102,26 +2068,47 @@ SearchService.prototype = { } let existingEngine = this._engines[name]; if (existingEngine) { - if ( + // Is this a webextension update? If not we're dealing with legacy opensearch or an override attempt. + let webExtUpdate = params.extensionID && - existingEngine._loadPath.startsWith( - `jar:[profile]/extensions/${params.extensionID}` - ) - ) { - // This is a legacy extension engine that needs to be migrated to WebExtensions. - isCurrent = this.defaultEngine == existingEngine; - await this.removeEngine(existingEngine); - } else { - SearchUtils.fail( - "An engine with that name already exists!", - Cr.NS_ERROR_FILE_ALREADY_EXISTS - ); + params.extensionID === existingEngine._extensionID; + if (!webExtUpdate) { + let webExtBuiltin = params.extensionID && params.isBuiltin; + // Is the existing engine a distribution engine? + if ( + webExtBuiltin && + existingEngine._loadPath.startsWith( + `[profile]/distribution/searchplugins/` + ) + ) { + SearchExtensionLoader.reject( + params.extensionID, + new Error( + `${params.extensionID} cannot override distribution engine.` + ) + ); + return null; + } else if ( + params.extensionID && + existingEngine._loadPath.startsWith( + `jar:[profile]/extensions/${params.extensionID}` + ) + ) { + // We uninstall the legacy engine, but we don't need to wait or do anything else here, + // _addEngineToStore will handle updating the engine data we're using. + this._removeEngineInstall(existingEngine); + } else { + SearchUtils.fail( + `An engine with the name ${name} already exists!`, + Cr.NS_ERROR_FILE_ALREADY_EXISTS + ); + } } } let newEngine = new SearchEngine({ name, - readOnly: isBuiltin, + readOnly: !!params.isBuiltin, sanitizeName: true, }); newEngine._initFromMetadata(name, params); @@ -2129,43 +2116,26 @@ SearchService.prototype = { if (params.extensionID) { newEngine._loadPath += ":" + params.extensionID; } + newEngine._engineToUpdate = existingEngine; this._addEngineToStore(newEngine); - if (isCurrent) { - this.defaultEngine = newEngine; - } return newEngine; }, async addEnginesFromExtension(extension) { SearchUtils.log("addEnginesFromExtension: " + extension.id); - if (extension.addonData.builtIn) { - SearchUtils.log("addEnginesFromExtension: Ignoring builtIn engine."); - return []; - } - // If we havent started SearchService yet, store this extension - // to install in SearchService.init(). - if (!gInitialized) { - this._startupExtensions.add(extension); - return []; - } - return this._installExtensionEngine(extension, [DEFAULT_TAG]); - }, - - async _installExtensionEngine(extension, locales, initEngine) { - SearchUtils.log("installExtensionEngine: " + extension.id); + // Wait for the list.json engines to be parsed before + // allowing addEnginesFromExtension to continue. This delays early start + // extensions until we are at a stage that they can be handled. + await this._extensionLoadReady.promise; + let locales = this._engineLocales.get(extension.id) || [DEFAULT_TAG]; let installLocale = async locale => { let manifest = locale === DEFAULT_TAG ? extension.manifest : await extension.getLocalizedManifest(locale); - return this._addEngineForManifest( - extension, - manifest, - locale, - initEngine - ); + return this._addEngineForManifest(extension, manifest, locale); }; let engines = []; @@ -2176,17 +2146,15 @@ SearchService.prototype = { ":" + locale ); - engines.push(await installLocale(locale)); + engines.push(installLocale(locale)); } - return engines; + return Promise.all(engines).then(installedEngines => { + SearchExtensionLoader.resolve(extension.id); + return installedEngines; + }); }, - async _addEngineForManifest( - extension, - manifest, - locale = DEFAULT_TAG, - initEngine = false - ) { + async _addEngineForManifest(extension, manifest, locale = DEFAULT_TAG) { let { IconDetails } = ExtensionParent; // General set of icons for an engine. @@ -2241,10 +2209,10 @@ SearchService.prototype = { suggestGetParams: searchProvider.suggest_url_get_params, queryCharset: searchProvider.encoding || "UTF-8", mozParams: searchProvider.params, - initEngine, + version: extension.version, }; - return this.addEngineWithDetails(params.name, params); + return this._addEngineWithDetails(params.name, params); }, async addEngine(engineURL, iconURL, confirm, extensionID) { @@ -2292,6 +2260,19 @@ SearchService.prototype = { } }, + async _removeEngineInstall(engine) { + // Make sure there is a file and this is not a webextension. + if (!engine._filePath || engine._extensionID) { + return; + } + let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile); + file.persistentDescriptor = engine._filePath; + if (file.exists()) { + file.remove(false); + } + engine._filePath = null; + }, + async removeEngine(engine) { await this.init(true); if (!engine) { @@ -2323,14 +2304,7 @@ SearchService.prototype = { engineToRemove.alias = null; } else { // Remove the engine file from disk if we had a legacy file in the profile. - if (engineToRemove._filePath) { - let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile); - file.persistentDescriptor = engineToRemove._filePath; - if (file.exists()) { - file.remove(false); - } - engineToRemove._filePath = null; - } + this._removeEngineInstall(engineToRemove); // Remove the engine from _sortedEngines var index = this._sortedEngines.indexOf(engineToRemove); diff --git a/toolkit/components/search/SearchUtils.jsm b/toolkit/components/search/SearchUtils.jsm index c79ae39f3abe..2c4bd2d44a66 100644 --- a/toolkit/components/search/SearchUtils.jsm +++ b/toolkit/components/search/SearchUtils.jsm @@ -6,18 +6,36 @@ "use strict"; -var EXPORTED_SYMBOLS = ["SearchUtils"]; +var EXPORTED_SYMBOLS = ["SearchUtils", "SearchExtensionLoader"]; const { XPCOMUtils } = ChromeUtils.import( "resource://gre/modules/XPCOMUtils.jsm" ); XPCOMUtils.defineLazyModuleGetters(this, { + AddonManager: "resource://gre/modules/AddonManager.jsm", + AppConstants: "resource://gre/modules/AppConstants.jsm", + PromiseUtils: "resource://gre/modules/PromiseUtils.jsm", Services: "resource://gre/modules/Services.jsm", + clearTimeout: "resource://gre/modules/Timer.jsm", + setTimeout: "resource://gre/modules/Timer.jsm", }); const BROWSER_SEARCH_PREF = "browser.search."; +const EXT_SEARCH_PREFIX = "resource://search-extensions/"; +const APP_SEARCH_PREFIX = "resource://search-plugins/"; + +// By the time we start loading an extension, it should load much +// faster than 1000ms. This simply ensures we resolve all the +// promises and let search init complete if something happens. +XPCOMUtils.defineLazyPreferenceGetter( + this, + "ADDON_LOAD_TIMEOUT", + BROWSER_SEARCH_PREF + "addonLoadTimeout", + 1000 +); + XPCOMUtils.defineLazyPreferenceGetter( this, "loggingEnabled", @@ -26,9 +44,14 @@ XPCOMUtils.defineLazyPreferenceGetter( ); var SearchUtils = { - APP_SEARCH_PREFIX: "resource://search-plugins/", + APP_SEARCH_PREFIX, BROWSER_SEARCH_PREF, + EXT_SEARCH_PREFIX, + LIST_JSON_URL: + (AppConstants.platform == "android" + ? APP_SEARCH_PREFIX + : EXT_SEARCH_PREFIX) + "list.json", /** * Topic used for events involving the service itself. @@ -95,7 +118,6 @@ var SearchUtils = { */ log(text) { if (loggingEnabled) { - dump("*** Search: " + text + "\n"); Services.console.logStringMessage(text); } }, @@ -150,4 +172,122 @@ var SearchUtils = { return null; }, + + makeExtensionId(name) { + return name + "@search.mozilla.org"; + }, + + getExtensionUrl(id) { + return EXT_SEARCH_PREFIX + id.split("@")[0] + "/"; + }, +}; + +/** + * SearchExtensionLoader provides a simple install function that + * returns a set of promises. The caller (SearchService) must resolve + * each extension id once it has handled the final part of the install + * (creating the SearchEngine). Once they are resolved, the extensions + * are fully functional, in terms of the SearchService, and initialization + * can be completed. + * + * When an extension is installed (that has a search provider), the + * extension system will call ss.addEnginesFromExtension. When that is + * completed, SearchService calls back to resolve the promise. + */ +const SearchExtensionLoader = { + _promises: new Map(), + // strict is used in tests. + _strict: false, + + /** + * Creates a deferred promise for an extension install. + * @param {string} id the extension id. + * @returns {Promise} + */ + _addPromise(id) { + let deferred = PromiseUtils.defer(); + // We never want to have some uncaught problem stop the SearchService + // init from completing, so timeout the promise. + if (ADDON_LOAD_TIMEOUT > 0) { + deferred.timeout = setTimeout(() => { + deferred.reject(id, new Error("addon install timed out.")); + this._promises.delete(id); + }, ADDON_LOAD_TIMEOUT); + } + this._promises.set(id, deferred); + return deferred.promise; + }, + + /** + * @param {string} id the extension id to resolve. + */ + resolve(id) { + if (this._promises.has(id)) { + let deferred = this._promises.get(id); + if (deferred.timeout) { + clearTimeout(deferred.timeout); + } + deferred.resolve(); + this._promises.delete(id); + } + }, + + /** + * @param {string} id the extension id to reject. + * @param {object} error The error to log when rejecting. + */ + reject(id, error) { + if (this._promises.has(id)) { + let deferred = this._promises.get(id); + if (deferred.timeout) { + clearTimeout(deferred.timeout); + } + // We don't want to reject here because that will reject the promise.all + // and stop the searchservice init. Log the error, and resolve the promise. + // strict mode can be used by tests to force an exception to occur. + Cu.reportError(`Addon install for search engine ${id} failed: ${error}`); + if (this._strict) { + deferred.reject(); + } else { + deferred.resolve(); + } + this._promises.delete(id); + } + }, + + _reset() { + SearchUtils.log(`SearchExtensionLoader.reset`); + for (let id of this._promises.keys()) { + this.reject(id, new Error(`installAddons reset during install`)); + } + this._promises = new Map(); + }, + + /** + * Tell AOM to install a set of built-in extensions. If the extension is + * already installed, it will be reinstalled. + * + * @param {Array} engineIDList is an array of extension IDs. + * @returns {Promise} resolved when all engines have finished installation. + */ + async installAddons(engineIDList) { + SearchUtils.log(`SearchExtensionLoader.installAddons`); + // If SearchService calls us again, it is being re-inited. reset ourselves. + this._reset(); + let promises = []; + for (let id of engineIDList) { + promises.push(this._addPromise(id)); + let path = SearchUtils.getExtensionUrl(id); + SearchUtils.log( + `SearchExtensionLoader.installAddons: installing ${id} at ${path}` + ); + // The AddonManager will install the engine asynchronously + AddonManager.installBuiltinAddon(path).catch(error => { + // Catch any install errors and propogate. + this.reject(id, error); + }); + } + + return Promise.all(promises); + }, }; diff --git a/toolkit/components/search/nsISearchService.idl b/toolkit/components/search/nsISearchService.idl index 233cd5b25512..4fc1bfdabf97 100644 --- a/toolkit/components/search/nsISearchService.idl +++ b/toolkit/components/search/nsISearchService.idl @@ -221,8 +221,6 @@ interface nsISearchService : nsISupports */ void reInit([optional] in boolean skipRegionCheck); void reset(); - Promise ensureBuiltinExtension(in AString id, - [optional] in jsval locales); /** * Determine whether initialization has been completed. diff --git a/toolkit/components/search/tests/xpcshell/data/invalid-extension/invalid/manifest.json b/toolkit/components/search/tests/xpcshell/data/invalid-extension/invalid/manifest.json new file mode 100644 index 000000000000..ffdbdc000ac3 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/data/invalid-extension/invalid/manifest.json @@ -0,0 +1,19 @@ +{ + "name": "Invalid", + "description": "Invalid Engine", + "manifest_version": 2, + "version": "1.0", + "applications": { + "gecko": { + "id": "invalid@search.mozilla.org" + } + }, + "hidden": true, + "chrome_settings_overrides": { + "search_provider": { + "name": "Invalid", + "search_url": "ssh://duckduckgo.com/", + "suggest_url": "ssh://ac.duckduckgo.com/ac/q={searchTerms}&type=list" + } + } +} diff --git a/toolkit/components/search/tests/xpcshell/data/invalid-extension/list.json b/toolkit/components/search/tests/xpcshell/data/invalid-extension/list.json new file mode 100644 index 000000000000..ccf5ef836037 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/data/invalid-extension/list.json @@ -0,0 +1,7 @@ +{ + "default": { + "visibleDefaultEngines": [ + "invalid" + ] + } +} diff --git a/toolkit/components/search/tests/xpcshell/head_search.js b/toolkit/components/search/tests/xpcshell/head_search.js index 8046b077a637..45d1dead904c 100644 --- a/toolkit/components/search/tests/xpcshell/head_search.js +++ b/toolkit/components/search/tests/xpcshell/head_search.js @@ -41,6 +41,10 @@ var XULRuntime = Cc["@mozilla.org/xre/runtime;1"].getService(Ci.nsIXULRuntime); // Expand the amount of information available in error logs Services.prefs.setBoolPref("browser.search.log", true); +// Some tests load tons of extensions and will timeout, disable the timeout +// here to allow tests to be slow. +Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + // The geo-specific search tests assume certain prefs are already setup, which // might not be true when run in comm-central etc. So create them here. Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", true); diff --git a/toolkit/components/search/tests/xpcshell/test_async_distribution.js b/toolkit/components/search/tests/xpcshell/test_async_distribution.js index 25dc001d22f5..3ea13d9917ac 100644 --- a/toolkit/components/search/tests/xpcshell/test_async_distribution.js +++ b/toolkit/components/search/tests/xpcshell/test_async_distribution.js @@ -11,19 +11,21 @@ add_task(async function test_async_distribution() { Assert.ok(!Services.search.isInitialized); - return Services.search.init().then(function search_initialized(aStatus) { - Assert.ok(Components.isSuccessCode(aStatus)); - Assert.ok(Services.search.isInitialized); + let aStatus = await Services.search.init(); + Assert.ok(Components.isSuccessCode(aStatus)); + Assert.ok(Services.search.isInitialized); - // test that the engine from the distribution overrides our jar engine - return Services.search.getEngines().then(engines => { - Assert.equal(engines.length, 1); + // test that the engine from the distribution overrides our jar engine + let engines = await Services.search.getEngines(); + Assert.equal(engines.length, 1); - let engine = Services.search.getEngineByName("bug645970"); - Assert.notEqual(engine, null); + let engine = Services.search.getEngineByName("bug645970"); + Assert.ok(!!engine, "engine is installed"); - // check the engine we have is actually the one from the distribution - Assert.equal(engine.description, "override"); - }); - }); + // check the engine we have is actually the one from the distribution + Assert.equal( + engine.description, + "override", + "distribution engine override installed" + ); }); diff --git a/toolkit/components/search/tests/xpcshell/test_list_json_searchorder.js b/toolkit/components/search/tests/xpcshell/test_list_json_searchorder.js index 58a0b487136d..0be90afb5f95 100644 --- a/toolkit/components/search/tests/xpcshell/test_list_json_searchorder.js +++ b/toolkit/components/search/tests/xpcshell/test_list_json_searchorder.js @@ -6,6 +6,10 @@ "use strict"; add_task(async function setup() { + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + Services.prefs.setCharPref("browser.search.geoip.url", ""); + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + await AddonTestUtils.promiseStartupManager(); }); @@ -18,7 +22,7 @@ add_task(async function test_searchOrderJSON() { .QueryInterface(Ci.nsIResProtocolHandler); resProt.setSubstitution("search-extensions", Services.io.newURI(url)); - await asyncReInit(); + await Services.search.init(); Assert.ok(Services.search.isInitialized, "search initialized"); Assert.equal( diff --git a/toolkit/components/search/tests/xpcshell/test_migrateWebExtensionEngine.js b/toolkit/components/search/tests/xpcshell/test_migrateWebExtensionEngine.js index 646995683f2a..005de699a005 100644 --- a/toolkit/components/search/tests/xpcshell/test_migrateWebExtensionEngine.js +++ b/toolkit/components/search/tests/xpcshell/test_migrateWebExtensionEngine.js @@ -3,6 +3,14 @@ "use strict"; +const { ExtensionTestUtils } = ChromeUtils.import( + "resource://testing-common/ExtensionXPCShellUtils.jsm" +); +ExtensionTestUtils.init(this); + +AddonTestUtils.usePrivilegedSignatures = false; +AddonTestUtils.overrideCertDB(); + const kSearchEngineID = "addEngineWithDetails_test_engine"; const kExtensionID = "test@example.com"; @@ -13,16 +21,14 @@ const kSearchEngineDetails = { "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==", suggestURL: "http://example.com/?suggest={searchTerms}", alias: "alias_foo", - extensionID: kExtensionID, }; add_task(async function setup() { await AddonTestUtils.promiseStartupManager(); + await Services.search.init(); }); add_task(async function test_migrateLegacyEngine() { - Assert.ok(!Services.search.isInitialized); - await Services.search.addEngineWithDetails( kSearchEngineID, kSearchEngineDetails @@ -30,19 +36,125 @@ add_task(async function test_migrateLegacyEngine() { // Modify the loadpath so it looks like an legacy plugin loadpath let engine = Services.search.getEngineByName(kSearchEngineID); + Assert.ok(!!engine, "opensearch engine installed"); engine.wrappedJSObject._loadPath = `jar:[profile]/extensions/${kExtensionID}.xpi!/engine.xml`; - engine.wrappedJSObject._extensionID = null; - - // This should replace the existing engine - await Services.search.addEngineWithDetails( - kSearchEngineID, - kSearchEngineDetails + await Services.search.setDefault(engine); + Assert.equal( + engine.name, + Services.search.defaultEngine.name, + "set engine to default" ); + // We assume the default engines are installed, so our position will be after the default engine. + // This sets up the test to later test the engine position after updates. + let allEngines = await Services.search.getEngines(); + Assert.ok( + allEngines.length > 2, + "default engines available " + allEngines.length + ); + let origIndex = allEngines.map(e => e.name).indexOf(kSearchEngineID); + Assert.ok( + origIndex > 1, + "opensearch engine installed at position " + origIndex + ); + await Services.search.moveEngine(engine, origIndex - 1); + let index = (await Services.search.getEngines()) + .map(e => e.name) + .indexOf(kSearchEngineID); + Assert.equal( + origIndex - 1, + index, + "opensearch engine moved to position " + index + ); + + // Replace the opensearch extension with a webextension + let extensionInfo = { + useAddonManager: "permanent", + manifest: { + version: "1.0", + applications: { + gecko: { + id: kExtensionID, + }, + }, + chrome_settings_overrides: { + search_provider: { + name: kSearchEngineID, + search_url: "https://example.com/?q={searchTerms}", + }, + }, + }, + }; + + let extension = ExtensionTestUtils.loadExtension(extensionInfo); + await extension.startup(); + engine = Services.search.getEngineByName(kSearchEngineID); Assert.equal( engine.wrappedJSObject._loadPath, "[other]addEngineWithDetails:" + kExtensionID ); Assert.equal(engine.wrappedJSObject._extensionID, kExtensionID); + Assert.equal(engine.wrappedJSObject._version, "1.0"); + index = (await Services.search.getEngines()) + .map(e => e.name) + .indexOf(kSearchEngineID); + Assert.equal(origIndex - 1, index, "webext position " + index); + Assert.equal( + engine.name, + Services.search.defaultEngine.name, + "engine stil default" + ); + + extensionInfo.manifest.version = "2.0"; + await extension.upgrade(extensionInfo); + await AddonTestUtils.waitForSearchProviderStartup(extension); + + engine = Services.search.getEngineByName(kSearchEngineID); + Assert.equal( + engine.wrappedJSObject._loadPath, + "[other]addEngineWithDetails:" + kExtensionID + ); + Assert.equal(engine.wrappedJSObject._extensionID, kExtensionID); + Assert.equal(engine.wrappedJSObject._version, "2.0"); + index = (await Services.search.getEngines()) + .map(e => e.name) + .indexOf(kSearchEngineID); + Assert.equal(origIndex - 1, index, "webext position " + index); + Assert.equal( + engine.name, + Services.search.defaultEngine.name, + "engine stil default" + ); + + // A different extension cannot use the same name + extensionInfo.manifest.applications.gecko.id = "takeover@search.foo"; + let otherExt = ExtensionTestUtils.loadExtension(extensionInfo); + await otherExt.startup(); + // Verify correct owner + engine = Services.search.getEngineByName(kSearchEngineID); + Assert.equal( + engine.wrappedJSObject._extensionID, + kExtensionID, + "prior search engine could not be overwritten" + ); + // Verify no engine installed + let engines = await Services.search.getEnginesByExtensionID( + "takeover@search.foo" + ); + Assert.equal(engines.length, 0, "no search engines installed"); + await otherExt.unload(); + + // An opensearch engine cannot replace a webextension. + try { + await Services.search.addEngineWithDetails( + kSearchEngineID, + kSearchEngineDetails + ); + Assert.ok(false, "unable to install opensearch over webextension"); + } catch (e) { + Assert.ok(true, "unable to install opensearch over webextension"); + } + + await extension.unload(); }); diff --git a/toolkit/components/search/tests/xpcshell/test_parseSubmissionURL.js b/toolkit/components/search/tests/xpcshell/test_parseSubmissionURL.js index b5f73a3c726f..f598552517a4 100644 --- a/toolkit/components/search/tests/xpcshell/test_parseSubmissionURL.js +++ b/toolkit/components/search/tests/xpcshell/test_parseSubmissionURL.js @@ -18,7 +18,7 @@ add_task(async function test_parseSubmissionURL() { await Services.search.removeEngine(engine); } - let [engine1, engine2, engine3, engine4] = await addTestEngines([ + let engines = await addTestEngines([ { name: "Test search engine", xmlFileName: "engine.xml" }, { name: "Test search engine (fr)", xmlFileName: "engine-fr.xml" }, { @@ -52,113 +52,132 @@ add_task(async function test_parseSubmissionURL() { }, ]); - engine3.addParam("q", "{searchTerms}", null); - engine4.addParam("q", "{searchTerms}", null); + engines[2].addParam("q", "{searchTerms}", null); + engines[3].addParam("q", "{searchTerms}", null); + + function testParseSubmissionURL(url, engine, terms = "", offsetTerm) { + let result = Services.search.parseSubmissionURL(url); + Assert.equal(result.engine.name, engine.name, "engine matches"); + Assert.equal(result.terms, terms, "term matches"); + if (offsetTerm) { + Assert.ok( + url.slice(result.termsOffset).startsWith(offsetTerm), + "offset term matches" + ); + Assert.equal( + result.termsLength, + offsetTerm.length, + "offset term length matches" + ); + } else { + Assert.equal(result.termsOffset, url.length, "no term offset"); + } + } // Test the first engine, whose URLs use UTF-8 encoding. - let url = "http://www.google.com/search?foo=bar&q=caff%C3%A8"; - let result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine1); - Assert.equal(result.terms, "caff\u00E8"); - Assert.ok(url.slice(result.termsOffset).startsWith("caff%C3%A8")); - Assert.equal(result.termsLength, "caff%C3%A8".length); + info("URLs use UTF-8 encoding"); + testParseSubmissionURL( + "http://www.google.com/search?foo=bar&q=caff%C3%A8", + engines[0], + "caff\u00E8", + "caff%C3%A8" + ); // The second engine uses a locale-specific domain that is an alternate domain // of the first one, but the second engine should get priority when matching. // The URL used with this engine uses ISO-8859-1 encoding instead. - url = "http://www.google.fr/search?q=caff%E8"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine2); - Assert.equal(result.terms, "caff\u00E8"); - Assert.ok(url.slice(result.termsOffset).startsWith("caff%E8")); - Assert.equal(result.termsLength, "caff%E8".length); + info("URLs use alternate domain and ISO-8859-1 encoding"); + testParseSubmissionURL( + "http://www.google.fr/search?q=caff%E8", + engines[1], + "caff\u00E8", + "caff%E8" + ); // Test a domain that is an alternate domain of those defined. In this case, // the first matching engine from the ordered list should be returned. - url = "http://www.google.co.uk/search?q=caff%C3%A8"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine1); - Assert.equal(result.terms, "caff\u00E8"); - Assert.ok(url.slice(result.termsOffset).startsWith("caff%C3%A8")); - Assert.equal(result.termsLength, "caff%C3%A8".length); + info("URLs use alternate domain"); + testParseSubmissionURL( + "http://www.google.co.uk/search?q=caff%C3%A8", + engines[0], + "caff\u00E8", + "caff%C3%A8" + ); // We support parsing URLs from a dynamically added engine. Those engines use // windows-1252 encoding by default. - url = "http://www.bacon.test/find?q=caff%E8"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine3); - Assert.equal(result.terms, "caff\u00E8"); - Assert.ok(url.slice(result.termsOffset).startsWith("caff%E8")); - Assert.equal(result.termsLength, "caff%E8".length); - - // Test URLs with unescaped unicode characters. - url = "http://www.google.com/search?q=foo+b\u00E4r"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine1); - Assert.equal(result.terms, "foo b\u00E4r"); - Assert.ok(url.slice(result.termsOffset).startsWith("foo+b\u00E4r")); - Assert.equal(result.termsLength, "foo+b\u00E4r".length); - - // Test search engines with unescaped IDNs. - url = "http://www.b\u00FCcher.ch/search?q=foo+bar"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine4); - Assert.equal(result.terms, "foo bar"); - Assert.ok(url.slice(result.termsOffset).startsWith("foo+bar")); - Assert.equal(result.termsLength, "foo+bar".length); - - // Test search engines with escaped IDNs. - url = "http://www.xn--bcher-kva.ch/search?q=foo+bar"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine4); - Assert.equal(result.terms, "foo bar"); - Assert.ok(url.slice(result.termsOffset).startsWith("foo+bar")); - Assert.equal(result.termsLength, "foo+bar".length); - - // Parsing of parameters from an engine template URL is not supported. - Assert.equal( - Services.search.parseSubmissionURL("http://www.bacon.moz/search?q=").engine, - null - ); - Assert.equal( - Services.search.parseSubmissionURL("https://duckduckgo.com?q=test").engine, - null - ); - Assert.equal( - Services.search.parseSubmissionURL("https://duckduckgo.com/?q=test").engine, - null + info("URLs use windows-1252"); + testParseSubmissionURL( + "http://www.bacon.test/find?q=caff%E8", + engines[2], + "caff\u00E8", + "caff%E8" ); - // HTTP and HTTPS schemes are interchangeable. - url = "https://www.google.com/search?q=caff%C3%A8"; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine1); - Assert.equal(result.terms, "caff\u00E8"); - Assert.ok(url.slice(result.termsOffset).startsWith("caff%C3%A8")); - - // Decoding search terms with multiple spaces should work. - result = Services.search.parseSubmissionURL( - "http://www.google.com/search?q=+with++spaces+" + info("URLs with unescaped unicode characters"); + testParseSubmissionURL( + "http://www.google.com/search?q=foo+b\u00E4r", + engines[0], + "foo b\u00E4r", + "foo+b\u00E4r" ); - Assert.equal(result.engine, engine1); - Assert.equal(result.terms, " with spaces "); - // An empty query parameter should work the same. - url = "http://www.google.com/search?q="; - result = Services.search.parseSubmissionURL(url); - Assert.equal(result.engine, engine1); - Assert.equal(result.terms, ""); - Assert.equal(result.termsOffset, url.length); + info("URLs with unescaped IDNs"); + testParseSubmissionURL( + "http://www.b\u00FCcher.ch/search?q=foo+bar", + engines[3], + "foo bar", + "foo+bar" + ); - // There should be no match when the path is different. - result = Services.search.parseSubmissionURL( + info("URLs with escaped IDNs"); + testParseSubmissionURL( + "http://www.xn--bcher-kva.ch/search?q=foo+bar", + engines[3], + "foo bar", + "foo+bar" + ); + + info("URLs with engines using template params, no value"); + testParseSubmissionURL("http://www.bacon.moz/search?q=", engines[5]); + + info("URLs with engines using template params"); + testParseSubmissionURL( + "https://duckduckgo.com?q=test", + engines[4], + "test", + "test" + ); + + info("HTTP and HTTPS schemes are interchangeable."); + testParseSubmissionURL( + "https://www.google.com/search?q=caff%C3%A8", + engines[0], + "caff\u00E8", + "caff%C3%A8" + ); + + info("Decoding search terms with multiple spaces should work."); + testParseSubmissionURL( + "http://www.google.com/search?q=+with++spaces+", + engines[0], + " with spaces ", + "+with++spaces+" + ); + + info("An empty query parameter should work the same."); + testParseSubmissionURL("http://www.google.com/search?q=", engines[0]); + + // These test slightly different so we don't use testParseSubmissionURL. + info("There should be no match when the path is different."); + let result = Services.search.parseSubmissionURL( "http://www.google.com/search/?q=test" ); Assert.equal(result.engine, null); Assert.equal(result.terms, ""); Assert.equal(result.termsOffset, -1); - // There should be no match when the argument is different. + info("There should be no match when the argument is different."); result = Services.search.parseSubmissionURL( "http://www.google.com/search?q2=test" ); @@ -166,7 +185,7 @@ add_task(async function test_parseSubmissionURL() { Assert.equal(result.terms, ""); Assert.equal(result.termsOffset, -1); - // There should be no match for URIs that are not HTTP or HTTPS. + info("There should be no match for URIs that are not HTTP or HTTPS."); result = Services.search.parseSubmissionURL("file://localhost/search?q=test"); Assert.equal(result.engine, null); Assert.equal(result.terms, ""); diff --git a/toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js b/toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js index 66f45a56a50f..f95de933c5a4 100644 --- a/toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js +++ b/toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js @@ -35,7 +35,8 @@ add_task(async function run_test() { await promiseSaveCacheData(data); - await asyncReInit(); + Services.search.reset(); + await Services.search.init(); // test the engine is loaded ok. let engine = Services.search.getEngineByName("bug645970"); diff --git a/toolkit/components/search/tests/xpcshell/test_require_engines_for_cache.js b/toolkit/components/search/tests/xpcshell/test_require_engines_for_cache.js new file mode 100644 index 000000000000..7d49476333b7 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/test_require_engines_for_cache.js @@ -0,0 +1,16 @@ +"strict"; + +// https://bugzilla.mozilla.org/show_bug.cgi?id=1255605 +add_task(async function skip_writing_cache_without_engines() { + Services.prefs.setCharPref("browser.search.region", "US"); + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + await AddonTestUtils.promiseStartupManager(); + + useTestEngines("no-extensions"); + Assert.strictEqual( + 0, + (await Services.search.getEngines()).length, + "no engines loaded" + ); + Assert.ok(!removeCacheFile(), "empty cache file was not created."); +}); diff --git a/toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js b/toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js index db093d4ebb1f..116f04d1a664 100644 --- a/toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js +++ b/toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js @@ -2,6 +2,8 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ add_task(async function setup() { + Services.prefs.setCharPref("browser.search.region", "US"); + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); configureToLoadJarEngines(); await AddonTestUtils.promiseStartupManager(); }); @@ -9,7 +11,7 @@ add_task(async function setup() { add_task(async function ignore_cache_files_without_engines() { let commitPromise = promiseAfterCache(); let engineCount = (await Services.search.getEngines()).length; - Assert.equal(engineCount, 1); + Assert.equal(engineCount, 1, "one engine installed on search init"); // Wait for the file to be saved to disk, so that we can mess with it. await commitPromise; @@ -22,7 +24,11 @@ add_task(async function ignore_cache_files_without_engines() { // Check that after an async re-initialization, we still have the same engine count. commitPromise = promiseAfterCache(); await asyncReInit(); - Assert.equal(engineCount, (await Services.search.getEngines()).length); + Assert.equal( + engineCount, + (await Services.search.getEngines()).length, + "Search got correct number of engines" + ); await commitPromise; // Check that after a sync re-initialization, we still have the same engine count. @@ -32,41 +38,13 @@ add_task(async function ignore_cache_files_without_engines() { ); let reInitPromise = asyncReInit(); await unInitPromise; - Assert.ok(!Services.search.isInitialized); + Assert.ok(!Services.search.isInitialized, "Search is not initialized"); // Synchronously check the engine count; will force a sync init. - Assert.equal(engineCount, (await Services.search.getEngines()).length); - Assert.ok(Services.search.isInitialized); - await reInitPromise; -}); - -add_task(async function skip_writing_cache_without_engines() { - let unInitPromise = SearchTestUtils.promiseSearchNotification( - "uninit-complete" - ); - let reInitPromise = asyncReInit(); - await unInitPromise; - - // Configure so that no engines will be found. - Assert.ok(removeCacheFile()); - let resProt = Services.io - .getProtocolHandler("resource") - .QueryInterface(Ci.nsIResProtocolHandler); - resProt.setSubstitution( - "search-extensions", - Services.io.newURI("about:blank") - ); - - // Let the async-reInit happen. - await reInitPromise; - Assert.strictEqual(0, (await Services.search.getEngines()).length); - - // Trigger yet another re-init, to flush of any pending cache writing task. - unInitPromise = SearchTestUtils.promiseSearchNotification("uninit-complete"); - reInitPromise = asyncReInit(); - await unInitPromise; - - // Now check that a cache file doesn't exist. - Assert.ok(!removeCacheFile()); - + Assert.equal( + engineCount, + (await Services.search.getEngines()).length, + "Search got correct number of engines" + ); + Assert.ok(Services.search.isInitialized, "Search is initialized"); await reInitPromise; }); diff --git a/toolkit/components/search/tests/xpcshell/test_validate_engines.js b/toolkit/components/search/tests/xpcshell/test_validate_engines.js index 49a9c7bdf8aa..74b17fcaec2b 100644 --- a/toolkit/components/search/tests/xpcshell/test_validate_engines.js +++ b/toolkit/components/search/tests/xpcshell/test_validate_engines.js @@ -9,10 +9,9 @@ Cu.importGlobalProperties(["fetch"]); -const { SearchService } = ChromeUtils.import( - "resource://gre/modules/SearchService.jsm" +const { SearchUtils, SearchExtensionLoader } = ChromeUtils.import( + "resource://gre/modules/SearchUtils.jsm" ); -const LIST_JSON_URL = "resource://search-extensions/list.json"; function traverse(obj, fun) { for (var i in obj) { @@ -23,10 +22,10 @@ function traverse(obj, fun) { } } -const ss = new SearchService(); - -add_task(async function test_validate_engines() { - let engines = await fetch(LIST_JSON_URL).then(req => req.json()); +add_task(async function setup() { + // Read all the builtin engines and locales, create a giant list.json + // that includes everything. + let engines = await fetch(SearchUtils.LIST_JSON_URL).then(req => req.json()); let visibleDefaultEngines = new Set(); traverse(engines, (key, val) => { @@ -40,8 +39,41 @@ add_task(async function test_validate_engines() { visibleDefaultEngines: Array.from(visibleDefaultEngines), }, }; - ss._listJSONURL = "data:application/json," + JSON.stringify(listjson); + SearchUtils.LIST_JSON_URL = + "data:application/json," + JSON.stringify(listjson); + // Set strict so the addon install promise is rejected. This causes + // search.init to throw the error, and this test fails. + SearchExtensionLoader._strict = true; await AddonTestUtils.promiseStartupManager(); - await ss.init(); +}); + +add_task(async function test_validate_engines() { + // All engines should parse and init should work fine. + await Services.search + .init() + .then(() => { + ok(true, "all engines parsed and loaded"); + }) + .catch(() => { + ok(false, "an engine failed to parse and load"); + }); +}); + +add_task(async function test_install_timeout_failure() { + // Set an incredibly unachievable timeout here and make sure + // that init throws. We're loading every engine/locale combo under the + // sun, it's unlikely we could intermittently succeed in loading + // them all. + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 1); + removeCacheFile(); + Services.search.reset(); + await Services.search + .init() + .then(() => { + ok(false, "search init did not time out"); + }) + .catch(error => { + equal(Cr.NS_ERROR_FAILURE, error, "search init timed out"); + }); }); diff --git a/toolkit/components/search/tests/xpcshell/test_webextensions_install_failure.js b/toolkit/components/search/tests/xpcshell/test_webextensions_install_failure.js new file mode 100644 index 000000000000..aa72aa3d36c3 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/test_webextensions_install_failure.js @@ -0,0 +1,30 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ +"use strict"; + +const { SearchExtensionLoader } = ChromeUtils.import( + "resource://gre/modules/SearchUtils.jsm" +); +const { ExtensionTestUtils } = ChromeUtils.import( + "resource://testing-common/ExtensionXPCShellUtils.jsm" +); + +ExtensionTestUtils.init(this); +AddonTestUtils.usePrivilegedSignatures = false; +AddonTestUtils.overrideCertDB(); + +add_task(async function test_install_manifest_failure() { + // Force addon loading to reject on errors + SearchExtensionLoader._strict = true; + useTestEngines("invalid-extension"); + await AddonTestUtils.promiseStartupManager(); + + await Services.search + .init() + .then(() => { + ok(false, "search init did not throw"); + }) + .catch(e => { + equal(Cr.NS_ERROR_FAILURE, e, "search init error"); + }); +}); diff --git a/toolkit/components/search/tests/xpcshell/xpcshell.ini b/toolkit/components/search/tests/xpcshell/xpcshell.ini index 2922db8f39a2..d5ff6014f4db 100644 --- a/toolkit/components/search/tests/xpcshell/xpcshell.ini +++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini @@ -48,6 +48,8 @@ support-files = data/test-extensions/multilocale/manifest.json data/test-extensions/multilocale/_locales/af/messages.json data/test-extensions/multilocale/_locales/an/messages.json + data/invalid-extension/list.json + data/invalid-extension/invalid/manifest.json tags=searchmain [test_nocache.js] @@ -99,6 +101,7 @@ tags = addons [test_geodefaults.js] [test_hidden.js] [test_currentEngine_fallback.js] +[test_require_engines_for_cache.js] [test_require_engines_in_cache.js] skip-if = (verify && !debug && (os == 'linux')) [test_svg_icon.js] @@ -113,4 +116,5 @@ skip-if = (verify && !debug && (os == 'linux')) [test_validate_engines.js] [test_validate_manifests.js] [test_webextensions_install.js] +[test_webextensions_install_failure.js] [test_purpose.js] diff --git a/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/runner.py b/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/runner.py index 6fba69ed2e24..dbfdfca4d765 100644 --- a/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/runner.py +++ b/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/runner.py @@ -25,10 +25,14 @@ class TelemetryTestRunner(BaseMarionetteTestRunner): # Set Firefox Client Telemetry specific preferences prefs.update( { - # Fake the geoip lookup to always return Germany to: - # * avoid net access in tests - # * stabilize browser.search.region to avoid an extra subsession (bug 1545207) - "browser.search.geoip.url": "data:application/json,{\"country_code\": \"DE\"}", + # Force search region to DE and disable geo lookups. + "browser.search.region": "DE", + "browser.search.geoSpecificDefaults": False, + # Turn off timeouts for loading search extensions + "browser.search.addonLoadTimeout": 0, + "browser.search.log": True, + # geoip is skipped if url is empty (bug 1545207) + "browser.search.geoip.url": "", # Disable smart sizing because it changes prefs at startup. (bug 1547750) "browser.cache.disk.smart_size.enabled": False, "toolkit.telemetry.server": "{}/pings".format(SERVER_URL), diff --git a/toolkit/components/telemetry/tests/unit/head.js b/toolkit/components/telemetry/tests/unit/head.js index 3250a0391ec7..1ca856ded33a 100644 --- a/toolkit/components/telemetry/tests/unit/head.js +++ b/toolkit/components/telemetry/tests/unit/head.js @@ -484,6 +484,11 @@ function setEmptyPrefWatchlist() { } if (runningInParent) { + // Turn off region updates and timeouts for search service + Services.prefs.setCharPref("browser.search.region", "US"); + Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false); + Services.prefs.setIntPref("browser.search.addonLoadTimeout", 0); + // Set logging preferences for all the tests. Services.prefs.setCharPref("toolkit.telemetry.log.level", "Trace"); // Telemetry archiving should be on.