Back out 423d565f5637 (bug 493051) for xpcshell bustage

browser/components/search/content/search.xml

@@ -488,22 +488,17 @@
     <handlers>
       <handler event="command"><![CDATA[
         const target = event.originalTarget;
-        if (target.engine) {
+        if (target.classList.contains("addengine-item")) {
-          this.currentEngine = target.engine;
-        } else if (target.classList.contains("addengine-item")) {
           var searchService =
             Components.classes["@mozilla.org/browser/search-service;1"]
                       .getService(Components.interfaces.nsIBrowserSearchService);
           // We only detect OpenSearch files
           var type = Components.interfaces.nsISearchEngine.DATA_XML;
-          // Select the installed engine if the installation succeeds
-          var installCallback = {
-            onSuccess: engine => this.currentEngine = engine
-          }
           searchService.addEngine(target.getAttribute("uri"), type,
-                                  target.getAttribute("src"), false,
+                                  target.getAttribute("src"), false);
-                                  installCallback);
         }
+        else if (target.engine)
+          this.currentEngine = target.engine;
         else
           return;
 

browser/components/search/test/browser_426329.js

@@ -30,7 +30,8 @@ function test() {
       case "engine-added":
         var engine = ss.getEngineByName("Bug 426329");
         ok(engine, "Engine was added.");
-        ss.currentEngine = engine;
+        //XXX Bug 493051
+        //ss.currentEngine = engine;
         break;
       case "engine-current":
         ok(ss.currentEngine.name == "Bug 426329", "currentEngine set");

browser/components/search/test/browser_contextmenu.js

@@ -16,7 +16,8 @@ function test() {
       case "engine-added":
         var engine = ss.getEngineByName(ENGINE_NAME);
         ok(engine, "Engine was added.");
-        ss.currentEngine = engine;
+        //XXX Bug 493051
+        //ss.currentEngine = engine;
         break;
       case "engine-current":
         is(ss.currentEngine.name, ENGINE_NAME, "currentEngine set");

browser/components/search/test/browser_private_search_perwindowpb.js

@@ -42,18 +42,20 @@ function test() {
   }
 
   function addEngine(aCallback) {
-    let installCallback = {
+    function observer(aSub, aTopic, aData) {
-      onSuccess: function (engine) {
+      switch (aData) {
-        Services.search.currentEngine = engine;
+        case "engine-current":
-        aCallback();
+          ok(Services.search.currentEngine.name == "Bug 426329",
-      },
+             "currentEngine set");
-      onError: function (errorCode) {
+          aCallback();
-        ok(false, "failed to install engine: " + errorCode);
+          break;
       }
-    };
+    }
-    Services.search.addEngine(engineURL + "426329.xml",
+
-                              Ci.nsISearchEngine.DATA_XML,
+    Services.obs.addObserver(observer, "browser-search-engine-modified", false);
-                              "data:image/x-icon,%00", false, installCallback);
+    Services.search.addEngine(
+      engineURL + "426329.xml", Ci.nsISearchEngine.DATA_XML,
+      "data:image/x-icon,%00", false);
   }
 
   function testOnWindow(aIsPrivate, aCallback) {

netwerk/base/public/nsIBrowserSearchService.idl

@@ -140,30 +140,6 @@ interface nsISearchEngine : nsISupports
 
 };
 
-[scriptable, uuid(9fc39136-f08b-46d3-b232-96f4b7b0e235)]
-interface nsISearchInstallCallback : nsISupports
-{
-  const unsigned long ERROR_UNKNOWN_FAILURE = 0x1;
-  const unsigned long ERROR_DUPLICATE_ENGINE = 0x2;
-
-  /**
-   * Called to indicate that the engine addition process succeeded.
-   *
-   * @param engine
-   *        The nsISearchEngine object that was added (will not be null).
-   */
-  void onSuccess(in nsISearchEngine engine);
-
-  /**
-   * Called to indicate that the engine addition process failed.
-   *
-   * @param errorCode
-   *        One of the ERROR_* values described above indicating the cause of
-   *        the failure.
-   */
-  void onError(in unsigned long errorCode);
-};
-
 /**
  * Callback for asynchronous initialization of nsIBrowserSearchService
  */
@@ -210,7 +186,8 @@ interface nsIBrowserSearchService : nsISupports
    * Adds a new search engine from the file at the supplied URI, optionally
    * asking the user for confirmation first.  If a confirmation dialog is
    * shown, it will offer the option to begin using the newly added engine
-   * right away.
+   * right away; if no confirmation dialog is shown, the new engine will be
+   * used right away automatically.
    *
    * @param engineURL
    *        The URL to the search engine's description file.
@@ -230,16 +207,11 @@ interface nsIBrowserSearchService : nsISupports
    *        value is false, the engine will be added to the list upon successful
    *        load, but it will not be selected as the current engine.
    *
-   * @param callback
-   *        A nsISearchInstallCallback that will be notified when the
-   *        addition is complete, or if the addition fails. It will not be
-   *        called if addEngine throws an exception.
-   *
    * @throws NS_ERROR_FAILURE if the type is invalid, or if the description
    *         file cannot be successfully loaded.
    */
   void addEngine(in AString engineURL, in long dataType, in AString iconURL,
-                 in boolean confirm, [optional] in nsISearchInstallCallback callback);
+                 in boolean confirm);
 
   /**
    * Adds a new search engine, without asking the user for confirmation and

toolkit/components/search/nsSearchService.js

@@ -5,7 +5,6 @@
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cr = Components.results;
-const Cu = Components.utils;
 
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 Components.utils.import("resource://gre/modules/Services.jsm");
@@ -1151,10 +1150,7 @@ Engine.prototype = {
   _confirm: false,
   // Whether to set this as the current engine as soon as it is loaded.  This
   // is only used when the engine is first added to the list.
-  _useNow: false,
+  _useNow: true,
-  // A function to be invoked when this engine object's addition completes (or
-  // fails). Only used for installation via addEngine.
-  _installCallback: null,
   // Where the engine was loaded from. Can be one of: SEARCH_APP_DIR,
   // SEARCH_PROFILE_DIR, SEARCH_IN_EXTENSION.
   __installLocation: null,
@@ -1308,19 +1304,10 @@ Engine.prototype = {
    */
   _onLoad: function SRCH_ENG_onLoad(aBytes, aEngine) {
     /**
-     * Handle an error during the load of an engine by notifying the engine's
+     * Handle an error during the load of an engine by prompting the user to
-     * error callback, if any.
+     * notify him that the load failed.
      */
-    function onError(errorCode = Ci.nsISearchInstallCallback.ERROR_UNKNOWN_FAILURE) {
+    function onError(aErrorString, aTitleString) {
-      // Notify the callback of the failure
-      if (aEngine._installCallback) {
-        aEngine._installCallback(errorCode);
-      }
-    }
-
-    function promptError(strings = {}, error = undefined) {
-      onError(error);
-
       if (aEngine._engineToUpdate) {
         // We're in an update, so just fail quietly
         LOG("updating " + aEngine._engineToUpdate.name + " failed");
@@ -1330,8 +1317,8 @@ Engine.prototype = {
       var brandName = brandBundle.GetStringFromName("brandShortName");
 
       var searchBundle = Services.strings.createBundle(SEARCH_BUNDLE);
-      var msgStringName = strings.error || "error_loading_engine_msg2";
+      var msgStringName = aErrorString || "error_loading_engine_msg2";
-      var titleStringName = strings.title || "error_loading_engine_title";
+      var titleStringName = aTitleString || "error_loading_engine_title";
       var title = searchBundle.GetStringFromName(titleStringName);
       var text = searchBundle.formatStringFromName(msgStringName,
                                                    [brandName, aEngine._location],
@@ -1341,7 +1328,7 @@ Engine.prototype = {
     }
 
     if (!aBytes) {
-      promptError();
+      onError();
       return;
     }
 
@@ -1364,7 +1351,7 @@ Engine.prototype = {
         aEngine._data = aBytes;
         break;
       default:
-        promptError();
+        onError();
         LOG("_onLoad: Bogus engine _dataType: \"" + this._dataType + "\"");
         return;
     }
@@ -1375,7 +1362,7 @@ Engine.prototype = {
     } catch (ex) {
       LOG("_onLoad: Failed to init engine!\n" + ex);
       // Report an error to the user
-      promptError();
+      onError();
       return;
     }
 
@@ -1384,9 +1371,9 @@ Engine.prototype = {
     // otherwise we fail silently.
     if (!engineToUpdate) {
       if (Services.search.getEngineByName(aEngine.name)) {
-        promptError({ error: "error_duplicate_engine_msg",
+        if (aEngine._confirm)
-                      title: "error_invalid_engine_title"
+          onError("error_duplicate_engine_msg", "error_invalid_engine_title");
-                    }, Ci.nsISearchInstallCallback.ERROR_DUPLICATE_ENGINE);
+
         LOG("_onLoad: duplicate engine found, bailing");
         return;
       }
@@ -1399,10 +1386,8 @@ Engine.prototype = {
       var confirmation = aEngine._confirmAddEngine();
       LOG("_onLoad: confirm is " + confirmation.confirmed +
           "; useNow is " + confirmation.useNow);
-      if (!confirmation.confirmed) {
+      if (!confirmation.confirmed)
-        onError();
         return;
-      }
       aEngine._useNow = confirmation.useNow;
     }
 
@@ -1429,7 +1414,6 @@ Engine.prototype = {
           if (!newSelfURL || !newSelfURL._hasRelation("self")) {
             LOG("_onLoad: updateURL missing in updated engine for " +
                 aEngine.name + " aborted");
-            onError();
             return;
           }
           newUpdateURL = newSelfURL.template;
@@ -1437,7 +1421,6 @@ Engine.prototype = {
 
         if (oldUpdateURL != newUpdateURL) {
           LOG("_onLoad: updateURLs do not match! Update of " + aEngine.name + " aborted");
-          onError();
           return;
         }
       }
@@ -1445,6 +1428,10 @@ Engine.prototype = {
       // Set the new engine's icon, if it doesn't yet have one.
       if (!aEngine._iconURI && engineToUpdate._iconURI)
         aEngine._iconURI = engineToUpdate._iconURI;
+
+      // Clear the "use now" flag since we don't want to be changing the
+      // current engine for an update.
+      aEngine._useNow = false;
     }
 
     // Write the engine to file. For readOnly engines, they'll be stored in the
@@ -1455,11 +1442,6 @@ Engine.prototype = {
     // Notify the search service of the successful load. It will deal with
     // updates by checking aEngine._engineToUpdate.
     notifyAction(aEngine, SEARCH_ENGINE_LOADED);
-
-    // Notify the callback if needed
-    if (aEngine._installCallback) {
-      aEngine._installCallback();
-    }
   },
 
   /**
@@ -3359,31 +3341,14 @@ SearchService.prototype = {
   },
 
   addEngine: function SRCH_SVC_addEngine(aEngineURL, aDataType, aIconURL,
-                                         aConfirm, aCallback) {
+                                         aConfirm) {
     LOG("addEngine: Adding \"" + aEngineURL + "\".");
     this._ensureInitialized();
     try {
       var uri = makeURI(aEngineURL);
       var engine = new Engine(uri, aDataType, false);
-      if (aCallback) {
-        engine._installCallback = function (errorCode) {
-          try {
-            if (errorCode == null)
-              aCallback.onSuccess(engine);
-            else
-              aCallback.onError(errorCode);
-          } catch (ex) {
-            Cu.reportError("Error invoking addEngine install callback: " + ex);
-          }
-          // Clear the reference to the callback now that it's been invoked.
-          engine._installCallback = null;
-        };
-      }
       engine._initFromURI();
     } catch (ex) {
-      // Drop the reference to the callback, if set
-      if (engine)
-        engine._installCallback = null;
       FAIL("addEngine: Error adding engine:\n" + ex, Cr.NS_ERROR_FAILURE);
     }
     engine._setIcon(aIconURL, false);

toolkit/components/search/tests/xpcshell/head_search.js

@@ -97,7 +97,7 @@ function afterCache(callback)
   Services.obs.addObserver(obs, "browser-search-service", false);
 }
 
-function parseJsonFromStream(aInputStream) {
+function  parseJsonFromStream(aInputStream) {
   const json = Cc["@mozilla.org/dom/json;1"].createInstance(Components.interfaces.nsIJSON);
   const data = json.decodeFromStream(aInputStream, aInputStream.available());
   return data;

toolkit/components/search/tests/xpcshell/test_addEngine_callback.js

@@ -1,89 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- *    http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/*
- * Tests covering nsIBrowserSearchService::addEngine's optional callback.
- */
-
-"use strict";
-
-const Ci = Components.interfaces;
-
-Components.utils.import("resource://testing-common/httpd.js");
-
-// First test inits the search service
-add_test(function init_search_service() {
-  Services.search.init(function (status) {
-    if (!Components.isSuccessCode(status))
-      do_throw("Failed to initialize search service");
-
-    run_next_test();
-  });
-});
-
-// Simple test of the search callback
-add_test(function simple_callback_test() {
-  let searchCallback = {
-    onSuccess: function (engine) {
-      do_check_true(!!engine);
-      do_check_neq(engine.name, Services.search.defaultEngine.name);
-      run_next_test();
-    },
-    onError: function (errorCode) {
-      do_throw("search callback returned error: " + errorCode);
-    }
-  }
-  Services.search.addEngine("http://localhost:4444/data/engine.xml",
-                            Ci.nsISearchEngine.DATA_XML,
-                            null, false, searchCallback);
-});
-
-// Test of the search callback on duplicate engine failures
-add_test(function duplicate_failure_test() {
-  let searchCallback = {
-    onSuccess: function (engine) {
-      do_throw("this addition should not have succeeded");
-    },
-    onError: function (errorCode) {
-      do_check_true(!!errorCode);
-      do_check_eq(errorCode, Ci.nsISearchInstallCallback.ERROR_DUPLICATE_ENGINE);
-      run_next_test();
-    }
-  }
-  // Re-add the same engine added in the previous test
-  Services.search.addEngine("http://localhost:4444/data/engine.xml",
-                            Ci.nsISearchEngine.DATA_XML,
-                            null, false, searchCallback);
-});
-
-// Test of the search callback on failure to load the engine failures
-add_test(function load_failure_test() {
-  let searchCallback = {
-    onSuccess: function (engine) {
-      do_throw("this addition should not have succeeded");
-    },
-    onError: function (errorCode) {
-      do_check_true(!!errorCode);
-      do_check_eq(errorCode, Ci.nsISearchInstallCallback.ERROR_UNKNOWN_FAILURE);
-      run_next_test();
-    }
-  }
-  // Try adding an engine that doesn't exist
-  Services.search.addEngine("http://invalid/data/engine.xml",
-                            Ci.nsISearchEngine.DATA_XML,
-                            null, false, searchCallback);
-});
-
-function run_test() {
-  updateAppInfo();
-
-  let httpServer = new HttpServer();
-  httpServer.start(4444);
-  httpServer.registerDirectory("/", do_get_cwd());
-
-  do_register_cleanup(function cleanup() {
-    httpServer.stop(function() {});
-  });
-
-  run_next_test();
-}

toolkit/components/search/tests/xpcshell/test_notifications.js

@@ -40,7 +40,8 @@ function search_observer(subject, topic, data) {
       let retrievedEngine = Services.search.getEngineByName("Test search engine");
       do_check_eq(engine, retrievedEngine);
       Services.search.defaultEngine = engine;
-      Services.search.currentEngine = engine;
+      // XXX bug 493051
+      // Services.search.currentEngine = engine;
       do_execute_soon(function () {
         Services.search.removeEngine(engine);
       });

toolkit/components/search/tests/xpcshell/xpcshell.ini

@@ -19,4 +19,4 @@ skip-if = debug && os == "linux"
 [test_defaultEngine.js]
 [test_prefSync.js]
 [test_notifications.js]
-[test_addEngine_callback.js]
+