Bug 1563559 - Tidy up a few things in SearchService.jsm. r=daleharvey
authorMark Banner <standard8@mozilla.com>
Fri, 05 Jul 2019 16:06:17 +0000
changeset 544353 af32b84056af45f9f08c579432496ebc8e1ae295
parent 544352 95dbf20a4c0482af6b643a0993a4588cab3efdca
child 544354 178a2ab709eebee94aa64621823f6a65249c3d30
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaleharvey
bugs1563559
milestone69.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 1563559 - Tidy up a few things in SearchService.jsm. r=daleharvey * I'd missed removing makeURI/makeChannel when I moved those to SearchUtils.jsm * We've an observer notification for 'find-jar-engines' that I cannot find any uses of. * '_findEngines' is creating the 'uris' array (which isn't uris), and passing it to '_parseListJSON' which fills them in and then '_findEngines' returns them. It can simply return the value direct from '_parseListJSON'. * There's a couple of cases where we create an intermediate variable to return it straight away. Depends on D36914 Differential Revision: https://phabricator.services.mozilla.com/D36927
toolkit/components/search/SearchService.jsm
--- a/toolkit/components/search/SearchService.jsm
+++ b/toolkit/components/search/SearchService.jsm
@@ -509,53 +509,16 @@ var fetchRegionDefault = ss =>
     };
     request.open("GET", endpoint, true);
     request.setRequestHeader("Content-Type", "application/json");
     request.responseType = "json";
     request.send();
   });
 
 /**
- * Wrapper function for nsIIOService::newURI.
- * @param {string} urlSpec
- *        The URL string from which to create an nsIURI.
- * @returns {nsIURI} an nsIURI object, or null if the creation of the URI failed.
- */
-function makeURI(urlSpec) {
-  try {
-    return Services.io.newURI(urlSpec);
-  } catch (ex) {}
-
-  return null;
-}
-
-/**
- * Wrapper function for nsIIOService::newChannel.
- * @param {string|nsIURI} url
- *   The URL string from which to create an nsIChannel.
- * @returns {nsIChannel|null}
- *   A nsIChannel object, or null if the url is invalid.
- */
-function makeChannel(url) {
-  try {
-    let uri = typeof url == "string" ? Services.io.newURI(url) : url;
-    return Services.io.newChannelFromURI(
-      uri,
-      null /* loadingNode */,
-      Services.scriptSecurityManager.getSystemPrincipal(),
-      null /* triggeringPrincipal */,
-      Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL,
-      Ci.nsIContentPolicy.TYPE_OTHER
-    );
-  } catch (ex) {}
-
-  return null;
-}
-
-/**
  * Wrapper for nsIPrefBranch::getComplexValue.
  * @param {string} prefName
  *   The name of the pref to get.
  * @param {*} defaultValue
  *   The value to return if the preference isn't found.
  * @returns {*}
  *   Returns either the preference value, or the default value.
  */
@@ -1012,21 +975,16 @@ SearchService.prototype = {
    *
    * @param {object} cache
    *   An object representing the search engine cache.
    * @param {boolean} isReload
    *   Set to true if this load is happening during a reload.
    */
   async _loadEngines(cache, isReload) {
     SearchUtils.log("_loadEngines: start");
-    Services.obs.notifyObservers(
-      null,
-      SearchUtils.TOPIC_SEARCH_SERVICE,
-      "find-jar-engines"
-    );
     let engines = await this._findEngines();
     SearchUtils.log("_loadEngines: loading - " + engines.join(","));
 
     // Get the non-empty distribution directories into distDirs...
     let distDirs = [];
     let locations;
     try {
       locations = Services.dirsvc.get(
@@ -1600,53 +1558,50 @@ SearchService.prototype = {
    * Loads the list of engines from list.json
    *
    * @returns {Array<string>}
    *   Returns an array of engine names.
    */
   async _findEngines() {
     SearchUtils.log("_findEngines: looking for engines in list.json");
 
-    let chan = makeChannel(this._listJSONURL);
+    let chan = SearchUtils.makeChannel(this._listJSONURL);
     if (!chan) {
       SearchUtils.log(
         "_findEngines: " + this._listJSONURL + " isn't registered"
       );
       return [];
     }
 
-    let uris = [];
-
     // Read list.json to find the engines we need to load.
     let request = new XMLHttpRequest();
     request.overrideMimeType("text/plain");
     let list = await new Promise(resolve => {
       request.onload = function(event) {
         resolve(event.target.responseText);
       };
       request.onerror = function(event) {
         SearchUtils.log("_findEngines: failed to read " + this._listJSONURL);
         resolve();
       };
       request.open("GET", Services.io.newURI(this._listJSONURL).spec, true);
       request.send();
     });
 
-    this._parseListJSON(list, uris);
-    return uris;
+    return this._parseListJSON(list);
   },
 
-  _parseListJSON(list, uris) {
+  _parseListJSON(list) {
     let json;
     try {
       json = JSON.parse(list);
     } catch (e) {
       Cu.reportError("parseListJSON: Failed to parse list.json: " + e);
       dump("parseListJSON: Failed to parse list.json: " + e + "\n");
-      return;
+      return [];
     }
 
     let searchRegion = Services.prefs.getCharPref(
       "browser.search.region",
       null
     );
 
     let searchSettings;
@@ -1654,17 +1609,17 @@ SearchService.prototype = {
     if ("locales" in json && locale in json.locales) {
       searchSettings = json.locales[locale];
     } else {
       // No locales were found, so use the JSON as is.
       // It should have a default section.
       if (!("default" in json)) {
         Cu.reportError("parseListJSON: Missing default in list.json");
         dump("parseListJSON: Missing default in list.json\n");
-        return;
+        return [];
       }
       searchSettings = json;
     }
 
     // Check if we have a useable region specific list of visible default engines.
     // This will only be set if we got the list from the Mozilla search server;
     // it will not be set for distributions.
     let engineNames;
@@ -1766,20 +1721,16 @@ SearchService.prototype = {
       for (let engine in esrOverrides) {
         let index = engineNames.indexOf(engine);
         if (index > -1) {
           engineNames[index] = esrOverrides[engine];
         }
       }
     }
 
-    for (let name of engineNames) {
-      uris.push(name);
-    }
-
     // Store this so that it can be used while writing the cache file.
     this._visibleDefaultEngines = engineNames;
 
     if (
       searchRegion &&
       searchRegion in searchSettings &&
       "searchDefault" in searchSettings[searchRegion]
     ) {
@@ -1800,16 +1751,17 @@ SearchService.prototype = {
       "searchOrder" in searchSettings[searchRegion]
     ) {
       this._searchOrder = searchSettings[searchRegion].searchOrder;
     } else if ("searchOrder" in searchSettings.default) {
       this._searchOrder = searchSettings.default.searchOrder;
     } else if ("searchOrder" in json.default) {
       this._searchOrder = json.default.searchOrder;
     }
+    return [...engineNames];
   },
 
   _saveSortedEngineList() {
     SearchUtils.log("_saveSortedEngineList: starting");
 
     // Set the useDB pref to indicate that from now on we should use the order
     // information stored in the database.
     Services.prefs.setBoolPref(
@@ -2014,25 +1966,23 @@ SearchService.prototype = {
   // reInit is currently only exposed for testing purposes
   async reInit(skipRegionCheck) {
     return this._reInit("test", skipRegionCheck);
   },
 
   async getEngines() {
     await this.init(true);
     SearchUtils.log("getEngines: getting all engines");
-    var engines = this._getSortedEngines(true);
-    return engines;
+    return this._getSortedEngines(true);
   },
 
   async getVisibleEngines() {
     await this.init();
     SearchUtils.log("getVisibleEngines: getting all visible engines");
-    var engines = this._getSortedEngines(false);
-    return engines;
+    return this._getSortedEngines(false);
   },
 
   async getDefaultEngines() {
     await this.init(true);
     function isDefault(engine) {
       return engine._isDefault;
     }
     var engines = this._sortedEngines.filter(isDefault);
@@ -3152,17 +3102,17 @@ var engineUpdateService = {
       return;
     }
 
     let testEngine = null;
     let updateURL = engine._getURLOfType(SearchUtils.URL_TYPE.OPENSEARCH);
     let updateURI =
       updateURL && updateURL._hasRelation("self")
         ? updateURL.getSubmission("", engine).uri
-        : makeURI(engine._updateURL);
+        : SearchUtils.makeURI(engine._updateURL);
     if (updateURI) {
       if (engine._isDefault && !updateURI.schemeIs("https")) {
         this._log("Invalid scheme for default engine update");
         return;
       }
 
       this._log("updating " + engine.name + " from " + updateURI.spec);
       testEngine = new SearchEngine({