Backed out changeset 43ead86af68b (bug 1591059) as requested by Standard8 for xpcshell failures. CLOSED TREE

toolkit/components/search/SearchService.jsm

@@ -3134,12 +3134,6 @@ SearchService.prototype = {
   },
 
   set defaultPrivateEngine(newEngine) {
-    if (!this._separatePrivateDefaultPrefValue) {
-      Services.prefs.setBoolPref(
-        SearchUtils.BROWSER_SEARCH_PREF + "separatePrivateDefault",
-        true
-      );
-    }
     this._setEngineDefault(this._separatePrivateDefault, newEngine);
   },
 

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

@@ -145,7 +145,7 @@ add_task(async function test_defaultPrivateEngine_turned_off() {
 
   promise = promiseDefaultNotification("normal");
   let privatePromise = promiseDefaultNotification("private");
-  Services.search.defaultEngine = engine1;
+  Services.search.defaultPrivateEngine = engine1;
   Assert.equal(
     await promise,
     engine1,
@@ -166,7 +166,7 @@ add_task(async function test_defaultPrivateEngine_turned_off() {
     engine1,
     "Should keep the default engine in sync with the pref off"
   );
-  promise = promiseDefaultNotification("private");
+  promise = promiseDefaultNotification("normal");
   Services.search.defaultPrivateEngine = engine2;
   Assert.equal(
     await promise,
@@ -180,18 +180,10 @@ add_task(async function test_defaultPrivateEngine_turned_off() {
   );
   Assert.equal(
     Services.search.defaultEngine,
-    engine1,
-    "Should not change the normal mode default engine"
+    engine2,
+    "Should keep the default engine in sync with the pref off"
   );
-  Assert.equal(
-    Services.prefs.getBoolPref(
-      SearchUtils.BROWSER_SEARCH_PREF + "separatePrivateDefault",
-      false
-    ),
-    true,
-    "Should have set the separate private default pref to true"
-  );
-  promise = promiseDefaultNotification("private");
+  promise = promiseDefaultNotification("normal");
   Services.search.defaultPrivateEngine = engine1;
   Assert.equal(
     await promise,
@@ -208,6 +200,49 @@ add_task(async function test_defaultPrivateEngine_turned_off() {
     engine1,
     "Should keep the default engine in sync with the pref off"
   );
+
+  // Test that hiding the currently-default engine affects the defaultEngine getter
+  // We fallback first to the original default...
+  engine1.hidden = true;
+  Assert.equal(
+    Services.search.defaultPrivateEngine,
+    originalDefault,
+    "Should reset to the original engine"
+  );
+  Assert.equal(
+    Services.search.defaultEngine,
+    originalDefault,
+    "Should also reset the normal default to the original engine"
+  );
+
+  // ... and then to the first visible engine in the list, so move our second
+  // engine to that position.
+  await Services.search.moveEngine(engine2, 0);
+  originalDefault.hidden = true;
+  Assert.equal(
+    Services.search.defaultPrivateEngine,
+    engine2,
+    "Should correctly set the second engine as private default"
+  );
+  Assert.equal(
+    Services.search.defaultEngine,
+    engine2,
+    "Should also set the normal default to the second engine"
+  );
+
+  // Test that setting defaultPrivateEngine to an already-hidden engine works, but
+  // doesn't change the return value of the getter
+  Services.search.defaultPrivateEngine = engine1;
+  Assert.equal(
+    Services.search.defaultPrivateEngine,
+    engine2,
+    "Should not change anything if attempted to be set to a hidden engine"
+  );
+  Assert.equal(
+    Services.search.defaultEngine,
+    engine2,
+    "Should also keep the normal default if attempted to be set to a hidden engine"
+  );
 });
 
 add_task(async function test_defaultPrivateEngine_ui_turned_off() {