Bug 1370518 - Don't completely detach/attach the autocomplete controller on TabSelect. r=mconley

Due to recent changes to tabbrowser focus behavior, now the "focus" event to the location bar
happens before the "TabSelect" event. On "focus" we would like to open the location bar popup,
but detaching the controller would immediately close it. Thus we don't want "TabSelect" to
detach the controller just to reset its internal state. Moreover, this should be cheaper.

MozReview-Commit-ID: 5NZ1TTI9NFW

--HG--
extra : rebase_source : 48e5e755c304f3963f328da06ca300e98304f38d

browser/base/content/test/urlbar/browser_urlbarSearchSuggestions_opt-out.js

@@ -62,6 +62,22 @@ add_task(async function focus() {
   Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
 });
 
+add_task(async function new_tab() {
+  // Opening a new tab when the urlbar is unfocused, should focusing it and thus
+  // open the popup in order to show the notification.
+  setupVisibleHint();
+  gURLBar.blur();
+  let popupPromise = promisePopupShown(gURLBar.popup);
+  // openNewForegroundTab doesn't focus the urlbar.
+  await BrowserTestUtils.synthesizeKey("t", { accelKey: true }, gBrowser.selectedBrowser);
+  await popupPromise;
+  Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
+  assertVisible(true);
+  assertFooterVisible(false);
+  Assert.equal(gURLBar.popup._matchCount, 0, "popup should have no results");
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+  Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
+});
 
 add_task(async function privateWindow() {
   // Since suggestions are disabled in private windows, the notification should

browser/base/content/urlbarBindings.xml

@@ -1163,8 +1163,7 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
               this._hideURLTooltip();
               break;
             case "TabSelect":
-              this.detachController();
-              this.attachController();
+              this.controller.resetInternalState();
               break;
           }
         ]]></body>

toolkit/components/autocomplete/nsAutoCompleteController.cpp

@@ -20,9 +20,12 @@
 #include "nsIDOMKeyEvent.h"
 #include "mozilla/Services.h"
 #include "mozilla/ModuleUtils.h"
+#include "mozilla/Unused.h"
 
 static const char *kAutoCompleteSearchCID = "@mozilla.org/autocomplete/search;1?name=";
 
+using namespace mozilla;
+
 namespace {
 
 void
@@ -130,40 +133,29 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
   if (mInput == aInput)
     return NS_OK;
 
-  // Clear out the current search context
+  Unused << ResetInternalState();
   if (mInput) {
-    // Stop all searches in case they are async.
-    StopSearch();
-    ClearResults();
-    ClosePopup();
     mSearches.Clear();
+    ClosePopup();
   }
 
   mInput = aInput;
 
   // Nothing more to do if the input was just being set to null.
-  if (!aInput)
+  if (!mInput) {
     return NS_OK;
+  }
+  nsCOMPtr<nsIAutoCompleteInput> input(mInput);
 
-  nsAutoString newValue;
-  aInput->GetTextValue(newValue);
+  // Reset the current search string.
+  input->GetTextValue(mSearchString);
 
   // Clear out this reference in case the new input's popup has no tree
   mTree = nullptr;
 
-  // Reset all search state members to default values
-  mSearchString = newValue;
-  mPlaceholderCompletionString.Truncate();
-  mDefaultIndexCompleted = false;
-  mProhibitAutoFill = false;
-  mSearchStatus = nsIAutoCompleteController::STATUS_NONE;
-  mRowCount = 0;
-  mSearchesOngoing = 0;
-  mCompletedSelectionIndex = -1;
-
   // Initialize our list of search objects
   uint32_t searchCount;
-  aInput->GetSearchCount(&searchCount);
+  input->GetSearchCount(&searchCount);
   mResults.SetCapacity(searchCount);
   mSearches.SetCapacity(searchCount);
   mImmediateSearchesCount = 0;
@@ -176,7 +168,7 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
   for (uint32_t i = 0; i < searchCount; ++i) {
     // Use the search name to create the contract id string for the search service
     nsAutoCString searchName;
-    aInput->GetSearchAt(i, searchName);
+    input->GetSearchAt(i, searchName);
     nsAutoCString cid(searchCID);
     cid.Append(searchName);
 
@@ -205,6 +197,29 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsAutoCompleteController::ResetInternalState()
+{
+  // Clear out the current search context
+  if (mInput) {
+    nsAutoString value;
+    mInput->GetTextValue(value);
+    // Stop all searches in case they are async.
+    Unused << StopSearch();
+    Unused << ClearResults();
+    mSearchString = value;
+  }
+
+  mPlaceholderCompletionString.Truncate();
+  mDefaultIndexCompleted = false;
+  mProhibitAutoFill = false;
+  mSearchStatus = nsIAutoCompleteController::STATUS_NONE;
+  mRowCount = 0;
+  mCompletedSelectionIndex = -1;
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsAutoCompleteController::StartSearch(const nsAString &aSearchString)
 {
@@ -1543,8 +1558,7 @@ nsAutoCompleteController::EnterMatch(bool aIsPopupSelection,
     }
   }
 
-  nsCOMPtr<nsIObserverService> obsSvc =
-    mozilla::services::GetObserverService();
+  nsCOMPtr<nsIObserverService> obsSvc = services::GetObserverService();
   NS_ENSURE_STATE(obsSvc);
   obsSvc->NotifyObservers(input, "autocomplete-will-enter-text", nullptr);
 
@@ -1579,8 +1593,7 @@ nsAutoCompleteController::RevertTextValue()
   input->OnTextReverted(&cancel);
 
   if (!cancel) {
-    nsCOMPtr<nsIObserverService> obsSvc =
-      mozilla::services::GetObserverService();
+    nsCOMPtr<nsIObserverService> obsSvc = services::GetObserverService();
     NS_ENSURE_STATE(obsSvc);
     obsSvc->NotifyObservers(input, "autocomplete-will-revert-text", nullptr);
 

toolkit/components/autocomplete/nsIAutoCompleteController.idl

@@ -170,4 +170,11 @@ interface nsIAutoCompleteController : nsISupports
    * tracking variables of features like completeSelectedIndex.
    */
   void setInitiallySelectedIndex(in long index);
+
+  /*
+   * Reset controller internal caches for cases where the input doesn't change
+   * but its context resets, thus it is about to start a completely new search
+   * session.
+   */
+  void resetInternalState();
 };