Bug 1596766 - Change the Search Engine Selector to return a string rather than array of locales. r=mikedeboer
authorMark Banner <standard8@mozilla.com>
Thu, 02 Jan 2020 13:11:58 +0000
changeset 508578 1a45094b7aa14dfb9c420d4645a0c284699fd94f
parent 508577 330661a663e8619e5ed63dc0fac751cb7f81baba
child 508579 69bad46cb54f77f42cb0f010545d17d92c5d3f8a
push id36973
push userncsoregi@mozilla.com
push dateThu, 02 Jan 2020 21:50:15 +0000
treeherdermozilla-central@7d0cec2c6c40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1596766
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1596766 - Change the Search Engine Selector to return a string rather than array of locales. r=mikedeboer The configuration may specify multiple locales for a search engine. The search engine selector already splits these out into different engine objects, but keeps the locale field as an array. This patch changes the 'locales' array into a 'locale' string and applies appropriate simplifications to the search service. Differential Revision: https://phabricator.services.mozilla.com/D58510
toolkit/components/search/SearchEngineSelector.jsm
toolkit/components/search/SearchService.jsm
toolkit/components/search/nsISearchService.idl
toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js
toolkit/components/search/tests/xpcshell/test_engine_selector.js
--- a/toolkit/components/search/SearchEngineSelector.jsm
+++ b/toolkit/components/search/SearchEngineSelector.jsm
@@ -94,26 +94,25 @@ class SearchEngineSelector {
 
         if (
           "webExtension" in baseConfig &&
           "locales" in baseConfig.webExtension
         ) {
           for (const webExtensionLocale of baseConfig.webExtension.locales) {
             const engine = { ...baseConfig };
             engine.webExtension = { ...baseConfig.webExtension };
-            engine.webExtension.locales = [
-              webExtensionLocale == USER_LOCALE ? locale : webExtensionLocale,
-            ];
+            delete engine.webExtension.locales;
+            engine.webExtension.locale =
+              webExtensionLocale == USER_LOCALE ? locale : webExtensionLocale;
             engines.push(engine);
           }
         } else {
           const engine = { ...baseConfig };
-          (engine.webExtension = engine.webExtension || {}).locales = [
-            SearchUtils.DEFAULT_TAG,
-          ];
+          (engine.webExtension = engine.webExtension || {}).locale =
+            SearchUtils.DEFAULT_TAG;
           engines.push(engine);
         }
       }
     }
 
     engines = this._filterEngines(engines);
 
     let defaultEngine;
--- a/toolkit/components/search/SearchService.jsm
+++ b/toolkit/components/search/SearchService.jsm
@@ -1179,17 +1179,17 @@ SearchService.prototype = {
 
     let enginesCorrupted = false;
     if (!rebuildCache) {
       if (gModernConfig) {
         const notInCacheEngines = engine => {
           return !cache.builtInEngineList.find(details => {
             return (
               engine.webExtension.id == details.id &&
-              engine.webExtension.locales[0] == details.locale
+              engine.webExtension.locale == details.locale
             );
           });
         };
 
         rebuildCache =
           !cache.builtInEngineList ||
           cache.builtInEngineList.length != engines.length ||
           engines.some(notInCacheEngines);
@@ -1284,18 +1284,18 @@ SearchService.prototype = {
 
     SearchUtils.log("_loadEngines: done using rebuilt cache");
   },
 
   async _loadEnginesFromConfig(engineConfigs, isReload = false) {
     SearchUtils.log("_loadEnginesFromConfig");
     let engines = [];
     for (let config of engineConfigs) {
-      let newEngines = await this.makeEnginesFromConfig(config, isReload);
-      engines = engines.concat(newEngines);
+      let engine = await this.makeEngineFromConfig(config, isReload);
+      engines.push(engine);
     }
     return engines;
   },
 
   /**
    * Ensures a built in search WebExtension is installed, installing
    * it if necessary.
    *
@@ -1829,19 +1829,18 @@ SearchService.prototype = {
     let { engines, privateDefault } = engineSelector.fetchEngineConfiguration(
       locale,
       region,
       channel
     );
 
     const defaultEngine = engines[0];
     function getLocale(engineInfo) {
-      return "webExtension" in engineInfo &&
-        "locales" in engineInfo.webExtension
-        ? engineInfo.webExtension.locales[0]
+      return "webExtension" in engineInfo && "locale" in engineInfo.webExtension
+        ? engineInfo.webExtension.locale
         : SearchUtils.DEFAULT_TAG;
     }
     this._searchDefault = {
       id: defaultEngine.webExtension.id,
       locale: getLocale(defaultEngine),
     };
     this._searchOrder = engines.map(e => {
       return {
@@ -2556,22 +2555,22 @@ SearchService.prototype = {
   /**
    * Create an engine object from the search configuration details.
    *
    * @param {object} config
    *   The configuration object that defines the details of the engine
    *   webExtensionId etc.
    * @param {boolean} isReload (optional)
    *   Is being called as part of maybeReloadEngines.
-   * @returns {Array}
-   *   Returns an array of nsISearchEngine objects.
+   * @returns {nsISearchEngine}
+   *   Returns the search engine object.
    */
-  async makeEnginesFromConfig(config, isReload = false) {
+  async makeEngineFromConfig(config, isReload = false) {
     if (SearchUtils.loggingEnabled) {
-      SearchUtils.log("makeEnginesFromConfig: " + JSON.stringify(config));
+      SearchUtils.log("makeEngineFromConfig: " + JSON.stringify(config));
     }
     let id = config.webExtension.id;
     let policy = WebExtensionPolicy.getByID(id);
     if (!policy) {
       let idPrefix = id.split("@")[0];
       let path = `resource://search-extensions/${idPrefix}/`;
       await AddonManager.installBuiltinAddon(path);
       policy = WebExtensionPolicy.getByID(id);
@@ -2601,51 +2600,47 @@ SearchService.prototype = {
         }
       }
     }
 
     if ("telemetryId" in config) {
       params.telemetryId = config.telemetryId;
     }
 
-    let locales =
-      "locales" in config.webExtension
-        ? config.webExtension.locales
+    let locale =
+      "locale" in config.webExtension
+        ? config.webExtension.locale
         : [SearchUtils.DEFAULT_TAG];
 
-    let engines = [];
-    for (let locale of locales) {
-      let manifest = policy.extension.manifest;
-      if (locale != "default") {
-        manifest = await policy.extension.getLocalizedManifest(locale);
-      }
-
-      let engineParams = await Services.search.getEngineParams(
-        policy.extension,
-        manifest,
-        locale,
-        params
-      );
-
-      let engine = new SearchEngine({
-        name: engineParams.name,
-        readOnly: engineParams.isBuiltin,
-        sanitizeName: true,
-      });
-      engine._initFromMetadata(engineParams.name, engineParams);
-      engine._loadPath = "[other]addEngineWithDetails";
-      if (engineParams.extensionID) {
-        engine._loadPath += ":" + engineParams.extensionID;
-      }
-      if (isReload && this._engines.has(engine.name)) {
-        engine._engineToUpdate = this._engines.get(engine.name);
-      }
-      engines.push(engine);
+    let manifest = policy.extension.manifest;
+    if (locale != "default") {
+      manifest = await policy.extension.getLocalizedManifest(locale);
     }
-    return engines;
+
+    let engineParams = await Services.search.getEngineParams(
+      policy.extension,
+      manifest,
+      locale,
+      params
+    );
+
+    let engine = new SearchEngine({
+      name: engineParams.name,
+      readOnly: engineParams.isBuiltin,
+      sanitizeName: true,
+    });
+    engine._initFromMetadata(engineParams.name, engineParams);
+    engine._loadPath = "[other]addEngineWithDetails";
+    if (engineParams.extensionID) {
+      engine._loadPath += ":" + engineParams.extensionID;
+    }
+    if (isReload && this._engines.has(engine.name)) {
+      engine._engineToUpdate = this._engines.get(engine.name);
+    }
+    return engine;
   },
 
   async _installExtensionEngine(extension, locales, initEngine, isReload) {
     SearchUtils.log("installExtensionEngine: " + extension.id);
 
     let installLocale = async locale => {
       let manifest =
         locale == SearchUtils.DEFAULT_TAG
--- a/toolkit/components/search/nsISearchService.idl
+++ b/toolkit/components/search/nsISearchService.idl
@@ -221,17 +221,17 @@ interface nsISearchService : nsISupports
    */
   Promise init([optional] in boolean skipRegionCheck);
 
   /**
    * Exposed for testing.
    */
   void reInit([optional] in boolean skipRegionCheck);
   void reset();
-  Promise makeEnginesFromConfig(in jsval config);
+  Promise makeEngineFromConfig(in jsval config);
   Promise ensureBuiltinExtension(in AString id,
                                 [optional] in jsval locales);
 
   /**
    * Determine whether initialization has been completed.
    *
    * Clients of the service can use this attribute to quickly determine whether
    * initialization is complete, and decide to trigger some immediate treatment,
--- a/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js
+++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js
@@ -170,21 +170,18 @@ class SearchConfigTest {
       let configs = await engineSelector.fetchEngineConfiguration(
         locale,
         region,
         AppConstants.MOZ_APP_VERSION_DISPLAY.endsWith("esr")
           ? "esr"
           : AppConstants.MOZ_UPDATE_CHANNEL
       );
       for (let config of configs.engines) {
-        let engine = await Services.search.makeEnginesFromConfig(config);
-        // Currently wikipedia is the only engine that uses multiple
-        // locales and that isn't a tested engine so for now pick
-        // the first (only) locale.
-        engines.push(engine[0]);
+        let engine = await Services.search.makeEngineFromConfig(config);
+        engines.push(engine);
       }
       return engines;
     }
     return Services.search.getVisibleEngines();
   }
 
   /**
    * Causes re-initialization of the SearchService with the new region and locale.
--- a/toolkit/components/search/tests/xpcshell/test_engine_selector.js
+++ b/toolkit/components/search/tests/xpcshell/test_engine_selector.js
@@ -85,19 +85,19 @@ add_task(async function() {
   );
   Assert.equal(
     privateDefault.engineName,
     "altavista",
     "Should set altavista as privateDefault"
   );
   let names = engines.map(obj => obj.engineName);
   Assert.deepEqual(names, ["lycos", "altavista", "aol"], "Correct order");
-  Assert.deepEqual(
-    engines[2].webExtension.locales,
-    ["en-US"],
+  Assert.equal(
+    engines[2].webExtension.locale,
+    "en-US",
     "Subsequent matches in applies to can override default"
   );
 
   ({ engines, privateDefault } = engineSelector.fetchEngineConfiguration(
     "zh-CN",
     "kz",
     "default"
   ));