Bug 1230661 - avoid calling the search service's currentEngine setter during startup, r=mak, a=sylvestre
authorFlorian Quèze <florian@queze.net>
Wed, 23 Dec 2015 16:01:13 +0100
changeset 310574 31a67ea4e0e8bbfea2afbee12b91eb9c7c1aea4c
parent 310573 9d72a8e7ecbef8a4d46b771c1c9f731e534a8cc0
child 310575 a7d36d3c727cc8691a8e81cff04401254051ebc0
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, sylvestre
bugs1230661
milestone45.0a2
Bug 1230661 - avoid calling the search service's currentEngine setter during startup, r=mak, a=sylvestre
toolkit/components/search/nsSearchService.js
toolkit/components/search/tests/xpcshell/test_geodefaults.js
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -4128,51 +4128,56 @@ SearchService.prototype = {
       setLocalizedPref(defPref, this._defaultEngine.name);
     }
 
     notifyAction(this._defaultEngine, SEARCH_ENGINE_DEFAULT);
   },
 
   get currentEngine() {
     this._ensureInitialized();
-    let currentEngine = this._currentEngine;
-    if (!currentEngine) {
+    if (!this._currentEngine) {
       let name = this.getGlobalAttr("current");
       let engine = this.getEngineByName(name);
       if (engine && (this.getGlobalAttr("hash") == getVerificationHash(name) ||
                      engine._isDefault)) {
         // If the current engine is a default one, we can relax the
         // verification hash check to reduce the annoyance for users who
         // backup/sync their profile in custom ways.
-        currentEngine = engine;
+        this._currentEngine = engine;
       }
+      if (!name)
+        this._currentEngine = this._originalDefaultEngine;
     }
 
-    if (!currentEngine || currentEngine.hidden)
-      currentEngine = this._originalDefaultEngine;
-    if (!currentEngine || currentEngine.hidden)
-      currentEngine = this._getSortedEngines(false)[0];
-
-    if (!currentEngine) {
-      // Last resort fallback: unhide the original default engine.
-      currentEngine = this._originalDefaultEngine;
-      if (currentEngine)
-        currentEngine.hidden = false;
-    }
-
-    if (currentEngine) {
+    // If the current engine is not set or hidden, we fallback...
+    if (!this._currentEngine || this._currentEngine.hidden) {
+      // first to the original default engine
+      let originalDefault = this._originalDefaultEngine;
+      if (!originalDefault || originalDefault.hidden) {
+        // then to the first visible engine
+        let firstVisible = this._getSortedEngines(false)[0];
+        if (firstVisible && !firstVisible.hidden) {
+          this.currentEngine = firstVisible;
+          return firstVisible;
+        }
+        // and finally as a last resort we unhide the original default engine.
+        if (originalDefault)
+          originalDefault.hidden = false;
+      }
+      if (!originalDefault)
+        return null;
+
       // If the current engine wasn't set or was hidden, we used a fallback
       // to pick a new current engine. As soon as we return it, this new
       // current engine will become user-visible, so we should persist it.
-      // Calling the setter achieves this, and is a no-op when we haven't
-      // actually changed the current engine.
-      this.currentEngine = currentEngine;
+      // by calling the setter.
+      this.currentEngine = originalDefault;
     }
 
-    return currentEngine;
+    return this._currentEngine;
   },
 
   set currentEngine(val) {
     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 (!(val instanceof Ci.nsISearchEngine) && !(val instanceof Engine))
--- a/toolkit/components/search/tests/xpcshell/test_geodefaults.js
+++ b/toolkit/components/search/tests/xpcshell/test_geodefaults.js
@@ -172,19 +172,17 @@ add_task(function* should_recheck_when_b
 
   // The current default engine shouldn't change during a session.
   do_check_eq(Services.search.currentEngine.name, getDefaultEngineName(false));
 
   // After another restart, the current engine should be back to the geo default,
   // without doing yet another request.
   yield asyncReInit();
   checkNoRequest();
-  commitPromise = promiseAfterCache();
   do_check_eq(Services.search.currentEngine.name, kTestEngineName);
-  yield commitPromise;
 });
 
 add_task(function* should_remember_cohort_id() {
   // Check that initially the cohort pref doesn't exist.
   const cohortPref = "browser.search.cohort";
   do_check_eq(Services.prefs.getPrefType(cohortPref), Services.prefs.PREF_INVALID);
 
   // Make the server send a cohort id.