Bug 1050683 - UnifiedComplete is missing nsIAutoCompleteSearchDescriptor. r=ttaubert
authorMarco Bonardo <mbonardo@mozilla.com>
Sun, 10 Aug 2014 23:02:14 +0200
changeset 198826 eae2c3cb8a1951d38728d168a2853cff0165340d
parent 198777 70be728521e32ff8bfa276fe275c0238c722cf1a
child 198827 86afc84418569bf7783e2f1ab90cf27ff033aab1
push id47500
push usernigelbabu@gmail.com
push dateMon, 11 Aug 2014 06:52:48 +0000
treeherdermozilla-inbound@bb954a9a154f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert
bugs1050683
milestone34.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 1050683 - UnifiedComplete is missing nsIAutoCompleteSearchDescriptor. r=ttaubert
toolkit/components/places/UnifiedComplete.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -1305,35 +1305,47 @@ UnifiedComplete.prototype = {
                             }, ex => { dump("Query failed: " + ex + "\n");
                                        Cu.reportError(ex); });
   },
 
   stopSearch: function () {
     if (this._currentSearch) {
       this._currentSearch.cancel();
     }
+    // Don't notify since we are canceling this search.  This also means we
+    // won't fire onSearchComplete for this search.
     this.finishSearch();
   },
 
   /**
    * Properly cleans up when searching is completed.
    *
    * @param notify [optional]
    *        Indicates if we should notify the AutoComplete listener about our
    *        results or not.
    */
   finishSearch: function (notify=false) {
-    // Notify about results if we are supposed to.
-    if (notify) {
-      this._currentSearch.notifyResults(false);
-    }
+    TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT);
+    // Clear state now to avoid race conditions, see below.
+    let search = this._currentSearch;
+    delete this._currentSearch;
+
+    if (!notify)
+      return;
 
-    // Clear our state
-    TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT);
-    delete this._currentSearch;
+    // There is a possible race condition here.
+    // When a search completes it calls finishSearch that notifies results
+    // here.  When the controller gets the last result it fires
+    // onSearchComplete.
+    // If onSearchComplete immediately starts a new search it will set a new
+    // _currentSearch, and on return the execution will continue here, after
+    // notifyResults.
+    // Thus, ensure that notifyResults is the last call in this method,
+    // otherwise you might be touching the wrong search.
+    search.notifyResults(false);
   },
 
   //////////////////////////////////////////////////////////////////////////////
   //// nsIAutoCompleteSimpleResultListener
 
   onValueRemoved: function (result, spec, removeFromDB) {
     if (removeFromDB) {
       PlacesUtils.history.removePage(NetUtil.newURI(spec));
@@ -1350,15 +1362,16 @@ UnifiedComplete.prototype = {
 
   classID: Components.ID("f964a319-397a-4d21-8be6-5cdd1ee3e3ae"),
 
   _xpcom_factory: XPCOMUtils.generateSingletonFactory(UnifiedComplete),
 
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsIAutoCompleteSearch,
     Ci.nsIAutoCompleteSimpleResultListener,
+    Ci.nsIAutoCompleteSearchDescriptor,
     Ci.mozIPlacesAutoComplete,
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference
   ])
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([UnifiedComplete]);