Bug 720792 - Implement a better solution to start selected searches without a timeout.

r=gavin

browser/base/content/test/Makefile.in

@@ -93,7 +93,6 @@ endif
 # browser_drag.js is disabled, as it needs to be updated for the new behavior from bug 320638.
 
 # browser_bug321000.js is disabled because newline handling is shaky (bug 592528)
-# browser_urlbarAutoFillTrimURLs.js is disabled till bug 720792 is fixed
 
 _BROWSER_FILES = \
                  head.js \
@@ -223,6 +222,7 @@ _BROWSER_FILES = \
                  browser_tabfocus.js \
                  browser_tabs_isActive.js \
                  browser_tabs_owner.js \
+                 browser_urlbarAutoFillTrimURLs.js \
                  browser_urlbarCopying.js \
                  browser_urlbarEnter.js \
                  browser_urlbarRevert.js \

toolkit/components/autocomplete/nsAutoCompleteController.cpp

@@ -85,7 +85,9 @@ nsAutoCompleteController::nsAutoCompleteController() :
   mSearchStatus(nsAutoCompleteController::STATUS_NONE),
   mRowCount(0),
   mSearchesOngoing(0),
-  mFirstSearchResult(false)
+  mSearchesFailed(0),
+  mFirstSearchResult(false),
+  mImmediateSearchesCount(0)
 {
 }
 
@@ -161,6 +163,7 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
   mResults.SetCapacity(searchCount);
   mSearches.SetCapacity(searchCount);
   mMatchCounts.SetLength(searchCount);
+  mImmediateSearchesCount = 0;
 
   const char *searchCID = kAutoCompleteSearchCID;
 
@@ -173,8 +176,17 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
 
     // Use the created cid to get a pointer to the search service and store it for later
     nsCOMPtr<nsIAutoCompleteSearch> search = do_GetService(cid.get());
-    if (search)
+    if (search) {
       mSearches.AppendObject(search);
+
+      // Count immediate searches.
+      PRUint16 searchType = nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_DELAYED;
+      nsCOMPtr<nsIAutoCompleteSearchDescriptor> searchDesc =
+        do_QueryInterface(search);
+      if (searchDesc && NS_SUCCEEDED(searchDesc->GetSearchType(&searchType)) &&
+          searchType == nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_IMMEDIATE)
+        mImmediateSearchesCount++;
+    }
   }
 
   return NS_OK;
@@ -184,7 +196,7 @@ NS_IMETHODIMP
 nsAutoCompleteController::StartSearch(const nsAString &aSearchString)
 {
   mSearchString = aSearchString;
-  StartSearchTimer();
+  StartSearches();
   return NS_OK;
 }
 
@@ -262,7 +274,7 @@ nsAutoCompleteController::HandleText()
     return NS_OK;
   }
 
-  StartSearchTimer();
+  StartSearches();
 
   return NS_OK;
 }
@@ -488,7 +500,7 @@ nsAutoCompleteController::HandleKeyNavigation(PRUint32 aKey, bool *_retval)
             return NS_OK;
           }
 
-          StartSearchTimer();
+          StartSearches();
         }
       }
     }
@@ -727,7 +739,16 @@ NS_IMETHODIMP
 nsAutoCompleteController::Notify(nsITimer *timer)
 {
   mTimer = nsnull;
-  StartSearch();
+
+  if (mImmediateSearchesCount == 0) {
+    // If there were no immediate searches, BeforeSearches has not yet been
+    // called, so do it now.
+    nsresult rv = BeforeSearches();
+    if (NS_FAILED(rv))
+      return rv;
+  }
+  StartSearch(nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_DELAYED);
+  AfterSearches();
   return NS_OK;
 }
 
@@ -1002,31 +1023,50 @@ nsAutoCompleteController::ClosePopup()
 }
 
 nsresult
-nsAutoCompleteController::StartSearch()
+nsAutoCompleteController::BeforeSearches()
 {
   NS_ENSURE_STATE(mInput);
-  nsCOMPtr<nsIAutoCompleteInput> input(mInput);
+
   mSearchStatus = nsIAutoCompleteController::STATUS_SEARCHING;
   mDefaultIndexCompleted = false;
 
-  // Cache the current results so that we can pass these through to all the
-  // searches without losing them
-  nsCOMArray<nsIAutoCompleteResult> resultCache;
-  if (!resultCache.AppendObjects(mResults)) {
+  // The first search result will clear mResults array, though we should pass
+  // the previous result to each search to allow them to reuse it.  So we
+  // temporarily cache current results till AfterSearches().
+  if (!mResultCache.AppendObjects(mResults)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  PRUint32 count = mSearches.Count();
-  mSearchesOngoing = count;
+  mSearchesOngoing = mSearches.Count();
+  mSearchesFailed = 0;
   mFirstSearchResult = true;
 
   // notify the input that the search is beginning
-  input->OnSearchBegin();
+  mInput->OnSearchBegin();
 
-  PRUint32 searchesFailed = 0;
-  for (PRUint32 i = 0; i < count; ++i) {
+  return NS_OK;
+}
+
+nsresult
+nsAutoCompleteController::StartSearch(PRUint16 aSearchType)
+{
+  NS_ENSURE_STATE(mInput);
+  nsCOMPtr<nsIAutoCompleteInput> input = mInput;
+
+  for (PRInt32 i = 0; i < mSearches.Count(); ++i) {
     nsCOMPtr<nsIAutoCompleteSearch> search = mSearches[i];
-    nsIAutoCompleteResult *result = resultCache.SafeObjectAt(i);
+
+    // Filter on search type.  Not all the searches implement this interface,
+    // in such a case just consider them delayed.
+    PRUint16 searchType = nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_DELAYED;
+    nsCOMPtr<nsIAutoCompleteSearchDescriptor> searchDesc =
+      do_QueryInterface(search);
+    if (searchDesc)
+      searchDesc->GetSearchType(&searchType);
+    if (searchType != aSearchType)
+      continue;
+
+    nsIAutoCompleteResult *result = mResultCache.SafeObjectAt(i);
 
     if (result) {
       PRUint16 searchResult;
@@ -1044,7 +1084,7 @@ nsAutoCompleteController::StartSearch()
 
     rv = search->StartSearch(mSearchString, searchParam, result, static_cast<nsIAutoCompleteObserver *>(this));
     if (NS_FAILED(rv)) {
-      ++searchesFailed;
+      ++mSearchesFailed;
       --mSearchesOngoing;
     }
     // Because of the joy of nested event loops (which can easily happen when some
@@ -1058,12 +1098,17 @@ nsAutoCompleteController::StartSearch()
     }
   }
 
-  if (searchesFailed == count)
-    PostSearchCleanup();
-
   return NS_OK;
 }
 
+void
+nsAutoCompleteController::AfterSearches()
+{
+  mResultCache.Clear();
+  if (mSearchesFailed == mSearches.Count())
+    PostSearchCleanup();
+}
+
 NS_IMETHODIMP
 nsAutoCompleteController::StopSearch()
 {
@@ -1087,7 +1132,7 @@ nsAutoCompleteController::StopSearch()
 }
 
 nsresult
-nsAutoCompleteController::StartSearchTimer()
+nsAutoCompleteController::StartSearches()
 {
   // Don't create a new search timer if we're already waiting for one to fire.
   // If we don't check for this, we won't be able to cancel the original timer
@@ -1095,9 +1140,37 @@ nsAutoCompleteController::StartSearchTimer()
   if (mTimer || !mInput)
     return NS_OK;
 
+  // Get the timeout for delayed searches.
   PRUint32 timeout;
   mInput->GetTimeout(&timeout);
 
+  PRUint32 immediateSearchesCount = mImmediateSearchesCount;
+  if (timeout == 0) {
+    // All the searches should be executed immediately.
+    immediateSearchesCount = mSearches.Count();
+  }
+
+  if (immediateSearchesCount > 0) {
+    nsresult rv = BeforeSearches();
+    if (NS_FAILED(rv))
+      return rv;
+    StartSearch(nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_IMMEDIATE);
+
+    if (mSearches.Count() == immediateSearchesCount) {
+      // Either all searches are immediate, or the timeout is 0.  In the
+      // latter case we still have to execute the delayed searches, otherwise
+      // this will be a no-op.
+      StartSearch(nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_DELAYED);
+
+      // All the searches have been started, just finish.
+      AfterSearches();
+      return NS_OK;
+    }
+  }
+
+  MOZ_ASSERT(timeout > 0, "Trying to delay searches with a 0 timeout!");
+
+  // Now start the delayed searches.
   nsresult rv;
   mTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
   if (NS_FAILED(rv))

toolkit/components/autocomplete/nsAutoCompleteController.h

@@ -74,9 +74,11 @@ protected:
   nsresult OpenPopup();
   nsresult ClosePopup();
 
-  nsresult StartSearch();
+  nsresult StartSearch(PRUint16 aSearchType);
 
-  nsresult StartSearchTimer();
+  nsresult BeforeSearches();
+  nsresult StartSearches();
+  void AfterSearches();
   nsresult ClearSearchTimer();
 
   nsresult ProcessResult(PRInt32 aSearchIndex, nsIAutoCompleteResult *aResult);
@@ -111,7 +113,13 @@ protected:
 
   nsCOMArray<nsIAutoCompleteSearch> mSearches;
   nsCOMArray<nsIAutoCompleteResult> mResults;
+  // Caches the match counts for the current ongoing results to allow
+  // incremental results to keep the rowcount up to date.
   nsTArray<PRUint32> mMatchCounts;
+  // Temporarily keeps the results alive while invoking startSearch() for each
+  // search.  This is needed to allow the searches to reuse the previous result,
+  // since otherwise the first search clears mResults.
+  nsCOMArray<nsIAutoCompleteResult> mResultCache;
 
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsITreeSelection> mSelection;
@@ -126,7 +134,9 @@ protected:
   PRUint16 mSearchStatus;
   PRUint32 mRowCount;
   PRUint32 mSearchesOngoing;
+  PRUint32 mSearchesFailed;
   bool mFirstSearchResult;
+  PRUint32 mImmediateSearchesCount;
 };
 
 #endif /* __nsAutoCompleteController__ */

toolkit/components/autocomplete/nsIAutoCompleteSearch.idl

@@ -82,3 +82,20 @@ interface nsIAutoCompleteObserver : nsISupports
    */
   void onUpdateSearchResult(in nsIAutoCompleteSearch search, in nsIAutoCompleteResult result);
 };
+
+[scriptable, uuid(02314d6e-b730-40cc-a215-221554d77064)]
+interface nsIAutoCompleteSearchDescriptor : nsISupports
+{
+  // The search is started after the timeout specified by the corresponding
+  // nsIAutoCompleteInput implementation.
+  const unsigned short SEARCH_TYPE_DELAYED = 0;
+  // The search is started synchronously, before any delayed searches.
+  const unsigned short SEARCH_TYPE_IMMEDIATE = 1;
+
+  /**
+   * Identifies the search behavior.
+   * Should be one of the SEARCH_TYPE_* constants above.
+   * Defaults to SEARCH_TYPE_DELAYED.
+   */
+  readonly attribute unsigned short searchType;
+};

toolkit/components/autocomplete/tests/unit/test_immediate_search.js

@@ -0,0 +1,160 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+
+function AutoCompleteImmediateSearch(aName, aResult) {
+  this.name = aName;
+  this._result = aResult;
+}
+AutoCompleteImmediateSearch.prototype = Object.create(AutoCompleteSearchBase.prototype);
+AutoCompleteImmediateSearch.prototype.searchType =
+  Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE;
+AutoCompleteImmediateSearch.prototype.QueryInterface =
+  XPCOMUtils.generateQI([Ci.nsIFactory,
+                         Ci.nsIAutoCompleteSearch,
+                         Ci.nsIAutoCompleteSearchDescriptor]);
+
+function AutoCompleteDelayedSearch(aName, aResult) {
+  this.name = aName;
+  this._result = aResult;
+}
+AutoCompleteDelayedSearch.prototype = Object.create(AutoCompleteSearchBase.prototype);
+
+function AutoCompleteResult(aValues, aDefaultIndex) {
+  this._values = aValues;
+  this.defaultIndex = aDefaultIndex;
+}
+AutoCompleteResult.prototype = Object.create(AutoCompleteResultBase.prototype);
+
+function run_test() {
+  run_next_test();
+}
+
+/**
+ * An immediate search should be executed synchronously.
+ */
+add_test(function test_immediate_search() {
+  let immediateResults = ["mozillaTest"];
+  let inputStr = "moz";
+
+  let immediateSearch = new AutoCompleteImmediateSearch(
+    "immediate", new AutoCompleteResult(["moz-immediate"], 0));
+  registerAutoCompleteSearch(immediateSearch);
+  let delayedSearch = new AutoCompleteDelayedSearch(
+    "delayed", new AutoCompleteResult(["moz-delayed"], 0));
+  registerAutoCompleteSearch(delayedSearch);
+
+  let controller = Cc["@mozilla.org/autocomplete/controller;1"].
+                   getService(Ci.nsIAutoCompleteController);  
+
+  let input = new AutoCompleteInputBase([delayedSearch.name,
+                                         immediateSearch.name]);
+  input.completeDefaultIndex = true;
+  input.textValue = inputStr;
+
+  // Caret must be at the end. Autofill doesn't happen unless you're typing
+  // characters at the end.
+  let strLen = inputStr.length;
+  input.selectTextRange(strLen, strLen);
+
+  controller.input = input;
+  controller.startSearch(inputStr);
+
+  // Immediately check the result, the immediate search should have finished.
+  do_check_eq(input.textValue, "moz-immediate");
+
+  // Wait for both queries to finish.
+  input.onSearchComplete = function() {
+    // Sanity check.
+    do_check_eq(input.textValue, "moz-immediate");
+
+    unregisterAutoCompleteSearch(immediateSearch);
+    unregisterAutoCompleteSearch(delayedSearch);
+    run_next_test();
+  };
+});
+
+/**
+ * An immediate search should be executed before any delayed search.
+ */
+add_test(function test_immediate_search_notimeout() {
+  let immediateResults = ["mozillaTest"];
+  let inputStr = "moz";
+
+  let immediateSearch = new AutoCompleteImmediateSearch(
+    "immediate", new AutoCompleteResult(["moz-immediate"], 0));
+  registerAutoCompleteSearch(immediateSearch);
+
+  let delayedSearch = new AutoCompleteDelayedSearch(
+    "delayed", new AutoCompleteResult(["moz-delayed"], 0));
+  registerAutoCompleteSearch(delayedSearch);
+
+  let controller = Cc["@mozilla.org/autocomplete/controller;1"].
+                   getService(Ci.nsIAutoCompleteController);  
+
+  let input = new AutoCompleteInputBase([delayedSearch.name,
+                                         immediateSearch.name]);
+  input.completeDefaultIndex = true;
+  input.textValue = inputStr;
+  input.timeout = 0;
+
+  // Caret must be at the end. Autofill doesn't happen unless you're typing
+  // characters at the end.
+  let strLen = inputStr.length;
+  input.selectTextRange(strLen, strLen);
+
+  controller.input = input;
+  let complete = false;
+  input.onSearchComplete = function() {
+    complete = true;
+  };
+  controller.startSearch(inputStr);
+  do_check_true(complete);
+
+  // Immediately check the result, the immediate search should have finished.
+  do_check_eq(input.textValue, "moz-immediate");
+
+  unregisterAutoCompleteSearch(immediateSearch);
+  unregisterAutoCompleteSearch(delayedSearch);
+  run_next_test();
+});
+
+/**
+ * A delayed search should be executed synchronously with a zero timeout.
+ */
+add_test(function test_delayed_search_notimeout() {
+  let immediateResults = ["mozillaTest"];
+  let inputStr = "moz";
+
+  let delayedSearch = new AutoCompleteDelayedSearch(
+    "delayed", new AutoCompleteResult(["moz-delayed"], 0));
+  registerAutoCompleteSearch(delayedSearch);
+
+  let controller = Cc["@mozilla.org/autocomplete/controller;1"].
+                   getService(Ci.nsIAutoCompleteController);  
+
+  let input = new AutoCompleteInputBase([delayedSearch.name]);
+  input.completeDefaultIndex = true;
+  input.textValue = inputStr;
+  input.timeout = 0;
+
+  // Caret must be at the end. Autofill doesn't happen unless you're typing
+  // characters at the end.
+  let strLen = inputStr.length;
+  input.selectTextRange(strLen, strLen);
+
+  controller.input = input;
+  let complete = false;
+  input.onSearchComplete = function() {
+    complete = true;
+  };
+  controller.startSearch(inputStr);
+  do_check_true(complete);
+
+  // Immediately check the result, the delayed search should have finished.
+  do_check_eq(input.textValue, "moz-delayed");
+
+  unregisterAutoCompleteSearch(delayedSearch);
+  run_next_test();
+});

toolkit/components/autocomplete/tests/unit/xpcshell.ini

@@ -12,5 +12,6 @@ tail =
 [test_badDefaultIndex.js]
 [test_completeDefaultIndex_casing.js]
 [test_hiddenResult.js]
+[test_immediate_search.js]
 [test_previousResult.js]
 [test_stopSearch.js]

toolkit/components/places/nsPlacesAutoComplete.js

@@ -1477,6 +1477,10 @@ urlInlineComplete.prototype = {
     }
   },
 
+  //////////////////////////////////////////////////////////////////////////////
+  //// nsIAutoCompleteSearchDescriptor
+  get searchType() Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE,
+
   //////////////////////////////////////////////////////////////////////////////
   //// mozIStorageStatementCallback
 
@@ -1609,6 +1613,7 @@ urlInlineComplete.prototype = {
 
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsIAutoCompleteSearch,
+    Ci.nsIAutoCompleteSearchDescriptor,
     Ci.mozIStorageStatementCallback,
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference,