Bug 1632303 - Replace SearchUtils.fail with direct throws. r=daleharvey
authorMark Banner <standard8@mozilla.com>
Thu, 04 Jun 2020 21:54:47 +0000
changeset 534066 dd60c4a35ba721d262d696328427e830944ec7e0
parent 534065 0a0feb5361c96fbaefe8db6e821d92409623ea7d
child 534067 99efc194746ad150bc902ea9cb5a358c4d6ba6d7
push id37482
push usernbeleuzu@mozilla.com
push dateFri, 05 Jun 2020 14:35:19 +0000
treeherdermozilla-central@c835da226e6d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaleharvey
bugs1632303
milestone79.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 1632303 - Replace SearchUtils.fail with direct throws. r=daleharvey Depends on D77649 Differential Revision: https://phabricator.services.mozilla.com/D77650
toolkit/components/search/SearchEngine.jsm
toolkit/components/search/SearchService.jsm
toolkit/components/search/SearchUtils.jsm
--- a/toolkit/components/search/SearchEngine.jsm
+++ b/toolkit/components/search/SearchEngine.jsm
@@ -273,20 +273,16 @@ function getVerificationHash(name) {
  * Gets a directory from the directory service.
  * @param {string} key
  *   The directory service key indicating the directory to get.
  * @param {nsIIDRef} iface
  *   The expected interface type of the directory information.
  * @returns {object}
  */
 function getDir(key, iface) {
-  if (!key) {
-    SearchUtils.fail("getDir requires a directory key!");
-  }
-
   return Services.dirsvc.get(key, iface || Ci.nsIFile);
 }
 
 /**
  * Sanitizes a name so that it can be used as a filename. If it cannot be
  * sanitized (e.g. no valid characters), then it returns a random name.
  *
  * @param {string} name
@@ -357,17 +353,20 @@ class QueryParameter {
    *  The value of the parameter. May be an empty string, must not be null or
    *  undefined.
    * @param {string} purpose
    *   The search purpose for which matches when this parameter should be
    *   applied, e.g. "searchbar", "contextmenu".
    */
   constructor(name, value, purpose) {
     if (!name || value == null) {
-      SearchUtils.fail("missing name or value for QueryParameter!");
+      throw Components.Exception(
+        "missing name or value for QueryParameter!",
+        Cr.NS_ERROR_INVALID_ARG
+      );
     }
 
     this.name = name;
     this._value = value;
     this.purpose = purpose;
   }
 
   get value() {
@@ -554,49 +553,55 @@ function getInternalAliases(engine) {
  *   The root domain for this URL.  Defaults to the template's host.
  *
  * @see http://opensearch.a9.com/spec/1.1/querysyntax/#urltag
  *
  * @throws NS_ERROR_NOT_IMPLEMENTED if aType is unsupported.
  */
 function EngineURL(mimeType, requestMethod, template, resultDomain) {
   if (!mimeType || !requestMethod || !template) {
-    SearchUtils.fail("missing mimeType, method or template for EngineURL!");
+    throw Components.Exception(
+      "missing mimeType, method or template for EngineURL!",
+      Cr.NS_ERROR_INVALID_ARG
+    );
   }
 
   var method = requestMethod.toUpperCase();
   var type = mimeType.toLowerCase();
 
   if (method != "GET" && method != "POST") {
-    SearchUtils.fail('method passed to EngineURL must be "GET" or "POST"');
+    throw Components.Exception(
+      'method passed to EngineURL must be "GET" or "POST"',
+      Cr.NS_ERROR_INVALID_ARG
+    );
   }
 
   this.type = type;
   this.method = method;
   this.params = [];
   this.rels = [];
 
   var templateURI = SearchUtils.makeURI(template);
   if (!templateURI) {
-    SearchUtils.fail(
+    throw Components.Exception(
       "new EngineURL: template is not a valid URI!",
       Cr.NS_ERROR_FAILURE
     );
   }
 
   switch (templateURI.scheme) {
     case "http":
     case "https":
       // Disable these for now, see bug 295018
       // case "file":
       // case "resource":
       this.template = template;
       break;
     default:
-      SearchUtils.fail(
+      throw Components.Exception(
         "new EngineURL: template uses invalid scheme!",
         Cr.NS_ERROR_FAILURE
       );
   }
 
   this.templateHost = templateURI.host;
   // If no resultDomain was specified in the engine definition file, use the
   // host from the template.
@@ -915,17 +920,17 @@ SearchEngine.prototype = {
    * Retrieves the data from the engine's file asynchronously.
    * The document element is placed in the engine's data field.
    *
    * @param {nsIFile} file
    *   The file to load the search plugin from.
    */
   async _initFromFile(file) {
     if (!file || !(await OS.File.exists(file.path))) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "File must exist before calling initFromFile!",
         Cr.NS_ERROR_UNEXPECTED
       );
     }
 
     let fileURI = Services.io.newFileURI(file);
     await this._retrieveSearchXMLData(fileURI.spec);
 
@@ -1386,17 +1391,17 @@ SearchEngine.prototype = {
       (element.localName == OPENSEARCH_LOCALNAME &&
         OPENSEARCH_NAMESPACES.includes(element.namespaceURI))
     ) {
       logConsole.debug("Initing search plugin from", this._location);
 
       this._parse();
     } else {
       Cu.reportError("Invalid search plugin due to namespace not matching.");
-      SearchUtils.fail(
+      throw Components.Exception(
         this._location + " is not a valid search plugin.",
         Cr.NS_ERROR_FILE_CORRUPTED
       );
     }
     // No need to keep a ref to our data (which in some cases can be a document
     // element) past this point
     this._data = null;
   },
@@ -1608,17 +1613,17 @@ SearchEngine.prototype = {
     // Support an alternate suggestion type, see bug 1425827 for details.
     if (type == "application/json" && rels.includes("suggestions")) {
       type = SearchUtils.URL_TYPE.SUGGEST_JSON;
     }
 
     try {
       var url = new EngineURL(type, method, template, resultDomain);
     } catch (ex) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "_parseURL: failed to add " + template + " as a URL",
         Cr.NS_ERROR_FAILURE
       );
     }
 
     if (rels.length) {
       url.rels = rels;
     }
@@ -1750,20 +1755,23 @@ SearchEngine.prototype = {
           this._iconUpdateURL = child.textContent;
           break;
         case "ExtensionID":
           this._extensionID = child.textContent;
           break;
       }
     }
     if (!this.name || !this._urls.length) {
-      SearchUtils.fail("_parse: No name, or missing URL!", Cr.NS_ERROR_FAILURE);
+      throw Components.Exception(
+        "_parse: No name, or missing URL!",
+        Cr.NS_ERROR_FAILURE
+      );
     }
     if (!this.supportsResponseType(SearchUtils.URL_TYPE.SEARCH)) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "_parse: No text/html result type!",
         Cr.NS_ERROR_FAILURE
       );
     }
   },
 
   /**
    * Init from a JSON record.
@@ -2151,30 +2159,33 @@ SearchEngine.prototype = {
       return this._queryCharset;
     }
     return (this._queryCharset = "windows-1252"); // the default
   },
 
   // from nsISearchEngine
   addParam(name, value, responseType) {
     if (!name || value == null) {
-      SearchUtils.fail("missing name or value for nsISearchEngine::addParam!");
+      throw Components.Exception(
+        "missing name or value for nsISearchEngine::addParam",
+        Cr.NS_ERROR_INVALID_ARG
+      );
     }
     ENSURE_WARN(
       !this._isBuiltin,
       "called nsISearchEngine::addParam on a built-in engine!",
       Cr.NS_ERROR_FAILURE
     );
     if (!responseType) {
       responseType = SearchUtils.URL_TYPE.SEARCH;
     }
 
     var url = this._getURLOfType(responseType);
     if (!url) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "Engine object has no URL for response type " + responseType,
         Cr.NS_ERROR_FAILURE
       );
     }
 
     url.addParam(name, value);
   },
 
--- a/toolkit/components/search/SearchService.jsm
+++ b/toolkit/components/search/SearchService.jsm
@@ -2380,58 +2380,64 @@ SearchService.prototype = {
       ) {
         return engine;
       }
     }
     return null;
   },
 
   async addEngineWithDetails(name, details, isReload = false) {
+    if (!name) {
+      throw Components.Exception(
+        "Empty name passed to addEngineWithDetails!",
+        Cr.NS_ERROR_INVALID_ARG
+      );
+    }
+    let params = details;
+    if (!params.template) {
+      throw Components.Exception(
+        "Empty template passed to addEngineWithDetails!",
+        Cr.NS_ERROR_INVALID_ARG
+      );
+    }
     logConsole.debug("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) {
       await this.init();
     }
-    if (!name) {
-      SearchUtils.fail("Invalid name passed to addEngineWithDetails!");
-    }
-    if (!params.template) {
-      SearchUtils.fail("Invalid template passed to addEngineWithDetails!");
-    }
     let existingEngine = this._engines.get(name);
     // In the modern configuration, distributions are app-provided engines,
     // so we don't need this separate check.
     if (
       !gModernConfig &&
       existingEngine &&
       existingEngine._loadPath.startsWith("[distribution]")
     ) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "Not loading engine due to having a distribution engine with the same name",
         Cr.NS_ERROR_FILE_ALREADY_EXISTS
       );
     }
     if (!isReload && existingEngine) {
       if (
         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(
+        throw Components.Exception(
           "An engine with that name already exists!",
           Cr.NS_ERROR_FILE_ALREADY_EXISTS
         );
       }
     }
 
     let newEngine = new SearchEngine({
       name,
@@ -2722,17 +2728,17 @@ SearchService.prototype = {
       if (errCode) {
         throw errCode;
       }
     } catch (ex) {
       // Drop the reference to the callback, if set
       if (engine) {
         engine._installCallback = null;
       }
-      SearchUtils.fail(
+      throw Components.Exception(
         "addEngine: Error adding engine:\n" + ex,
         errCode || Cr.NS_ERROR_FAILURE
       );
     }
     return engine;
   },
 
   async removeWebExtensionEngine(id) {
@@ -2740,28 +2746,31 @@ SearchService.prototype = {
     for (let engine of await this.getEnginesByExtensionID(id)) {
       await this.removeEngine(engine);
     }
   },
 
   async removeEngine(engine) {
     await this.init();
     if (!engine) {
-      SearchUtils.fail("no engine passed to removeEngine!");
+      throw Components.Exception(
+        "no engine passed to removeEngine!",
+        Cr.NS_ERROR_INVALID_ARG
+      );
     }
 
     var engineToRemove = null;
     for (var e of this._engines.values()) {
       if (engine.wrappedJSObject == e) {
         engineToRemove = e;
       }
     }
 
     if (!engineToRemove) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "removeEngine: Can't find engine to remove!",
         Cr.NS_ERROR_FILE_NOT_FOUND
       );
     }
 
     if (engineToRemove == this.defaultEngine) {
       this._currentEngine = null;
     }
@@ -2792,17 +2801,17 @@ SearchService.prototype = {
           file.remove(false);
         }
         engineToRemove._filePath = null;
       }
 
       // Remove the engine from _sortedEngines
       var index = this._sortedEngines.indexOf(engineToRemove);
       if (index == -1) {
-        SearchUtils.fail(
+        throw Components.Exception(
           "Can't find engine to remove in _sortedEngines!",
           Cr.NS_ERROR_FAILURE
         );
       }
       this.__sortedEngines.splice(index, 1);
 
       // Remove the engine from the internal store
       this._engines.delete(engineToRemove.name);
@@ -2811,36 +2820,39 @@ SearchService.prototype = {
       this._saveSortedEngineList();
     }
     SearchUtils.notifyAction(engineToRemove, SearchUtils.MODIFIED_TYPE.REMOVED);
   },
 
   async moveEngine(engine, newIndex) {
     await this.init();
     if (newIndex > this._sortedEngines.length || newIndex < 0) {
-      SearchUtils.fail("moveEngine: Index out of bounds!");
+      throw Components.Exception("moveEngine: Index out of bounds!");
     }
     if (
       !(engine instanceof Ci.nsISearchEngine) &&
       !(engine instanceof SearchEngine)
     ) {
-      SearchUtils.fail("moveEngine: Invalid engine passed to moveEngine!");
+      throw Components.Exception(
+        "moveEngine: Invalid engine passed to moveEngine!",
+        Cr.NS_ERROR_INVALID_ARG
+      );
     }
     if (engine.hidden) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "moveEngine: Can't move a hidden engine!",
         Cr.NS_ERROR_FAILURE
       );
     }
 
     engine = engine.wrappedJSObject;
 
     var currentIndex = this._sortedEngines.indexOf(engine);
     if (currentIndex == -1) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "moveEngine: Can't find engine to move!",
         Cr.NS_ERROR_UNEXPECTED
       );
     }
 
     // Our callers only take into account non-hidden engines when calculating
     // newIndex, but we need to move it in the array of all engines, so we
     // need to adjust newIndex accordingly. To do this, we count the number
@@ -2849,17 +2861,17 @@ SearchService.prototype = {
     // we were supposed to replace) and then iterating through the complete
     // engine list until we reach it, increasing newIndex for each hidden
     // engine we find on our way there.
     //
     // This could be further simplified by having our caller pass in
     // newIndexEngine directly instead of newIndex.
     var newIndexEngine = this._getSortedEngines(false)[newIndex];
     if (!newIndexEngine) {
-      SearchUtils.fail(
+      throw Components.Exception(
         "moveEngine: Can't find engine to replace!",
         Cr.NS_ERROR_UNEXPECTED
       );
     }
 
     for (var i = 0; i < this._sortedEngines.length; ++i) {
       if (newIndexEngine == this._sortedEngines[i]) {
         break;
@@ -2983,22 +2995,28 @@ SearchService.prototype = {
     this._ensureInitialized();
     // Sometimes we get wrapped nsISearchEngine objects (external XPCOM callers),
     // and sometimes we get raw Engine JS objects (callers in this file), so
     // handle both.
     if (
       !(newEngine instanceof Ci.nsISearchEngine) &&
       !(newEngine instanceof SearchEngine)
     ) {
-      SearchUtils.fail("Invalid argument passed to defaultEngine setter");
+      throw Components.Exception(
+        "Invalid argument passed to defaultEngine setter",
+        Cr.NS_ERROR_INVALID_ARG
+      );
     }
 
     const newCurrentEngine = this.getEngineByName(newEngine.name);
     if (!newCurrentEngine) {
-      SearchUtils.fail("Can't find engine in store!", Cr.NS_ERROR_UNEXPECTED);
+      throw Components.Exception(
+        "Can't find engine in store!",
+        Cr.NS_ERROR_UNEXPECTED
+      );
     }
 
     if (!newCurrentEngine.isAppProvided) {
       // If a non default engine is being set as the current engine, ensure
       // its loadPath has a verification hash.
       if (!newCurrentEngine._loadPath) {
         newCurrentEngine._loadPath = "[other]unknown";
       }
--- a/toolkit/components/search/SearchUtils.jsm
+++ b/toolkit/components/search/SearchUtils.jsm
@@ -104,31 +104,16 @@ var SearchUtils = {
   notifyAction(engine, verb) {
     if (Services.search.isInitialized) {
       logConsole.debug("NOTIFY: Engine:", engine.name, "Verb:", verb);
       Services.obs.notifyObservers(engine, this.TOPIC_ENGINE_MODIFIED, verb);
     }
   },
 
   /**
-   * Logs the failure message (if browser.search.log is enabled) and throws.
-   * @param {string} message
-   *   A message to display
-   * @param {number} resultCode
-   *   The NS_ERROR_* value to throw.
-   * @throws resultCode or NS_ERROR_INVALID_ARG if resultCode isn't specified.
-   */
-  fail(message, resultCode) {
-    if (SearchUtils.loggingEnabled) {
-      Services.console.logStringMessage(message);
-    }
-    throw Components.Exception(message, resultCode || Cr.NS_ERROR_INVALID_ARG);
-  },
-
-  /**
    * 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.
    */
   makeURI(urlSpec) {
     try {
       return Services.io.newURI(urlSpec);