Bug 1283329 - Perceptible pause when quickly typing and hitting enter in the urlbar. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 15 Sep 2016 18:55:12 +0200
changeset 355555 89d35548d4fc7868351c5e7c6cc59048b66ff530
parent 355554 e2a46e7754adb0917ed703ad55cf30aae9d3c0eb
child 355556 60d0e1dbddc00e146462d067c56d79b7b51faf3f
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1283329
milestone51.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 1283329 - Perceptible pause when quickly typing and hitting enter in the urlbar. r=adw MozReview-Commit-ID: 2NdTvY9H25Z
browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
browser/base/content/urlbarBindings.xml
toolkit/components/places/UnifiedComplete.js
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
+++ b/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
@@ -64,27 +64,59 @@ add_task(function* test_disabled_ac() {
   Preferences.set("browser.urlbar.suggest.openpages", false);
 
   Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
                                        "http://example.com/?q={searchTerms}");
   let engine = Services.search.getEngineByName("MozSearch");
   let originalEngine = Services.search.currentEngine;
   Services.search.currentEngine = engine;
 
-  registerCleanupFunction(function* () {
+  function* cleanup() {
     Preferences.set("browser.urlbar.suggest.history", suggestHistory);
     Preferences.set("browser.urlbar.suggest.bookmark", suggestBookmarks);
     Preferences.set("browser.urlbar.suggest.openpage", suggestOpenPages);
 
     Services.search.currentEngine = originalEngine;
     let engine = Services.search.getEngineByName("MozSearch");
-    Services.search.removeEngine(engine);
-  });
+    if (engine) {
+      Services.search.removeEngine(engine);
+    }
+  }
+  registerCleanupFunction(cleanup);
 
   gURLBar.focus();
   gURLBar.value = "e";
   EventUtils.synthesizeKey("x", {});
   EventUtils.synthesizeKey("VK_RETURN", {});
 
   info("wait for the page to load");
   yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
                                        false, "http://example.com/?q=ex");
+  yield cleanup();
 });
+
+add_task(function* test_delay() {
+  const TIMEOUT = 10000;
+  // Set a large delay.
+  let delay = Preferences.get("browser.urlbar.delay");
+  Preferences.set("browser.urlbar.delay", TIMEOUT);
+
+  registerCleanupFunction(function* () {
+    Preferences.set("browser.urlbar.delay", delay);
+  });
+
+  // This is needed to clear the current value, otherwise autocomplete may think
+  // the user removed text from the end.
+  let start = Date.now();
+  yield promiseAutocompleteResultPopup("");
+  Assert.ok((Date.now() - start) < TIMEOUT);
+
+  start = Date.now();
+  gURLBar.closePopup();
+  gURLBar.focus();
+  gURLBar.value = "e";
+  EventUtils.synthesizeKey("x", {});
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  info("wait for the page to load");
+  yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
+                                       false, "http://example.com/");
+  Assert.ok((Date.now() - start) < TIMEOUT);
+});
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1499,17 +1499,17 @@ file, You can obtain one at http://mozil
         <body><![CDATA[
           if (this.mPopupOpen) {
             return;
           }
 
           this.mInput = aInput;
           this.selectedIndex = this._isFirstResultHeuristic ? 0 : -1;
           this.view = aInput.controller.QueryInterface(Components.interfaces.nsITreeView);
-          this.invalidate();
+          this._invalidate();
 
           var rect = window.document.documentElement.getBoundingClientRect();
           var width = rect.right - rect.left;
           this.setAttribute("width", width);
 
           // Adjust the direction of the autocomplete popup list based on the textbox direction, bug 649840
           var popupDirection = aElement.ownerDocument.defaultView.getComputedStyle(aElement).direction;
           this.style.direction = popupDirection;
@@ -1769,25 +1769,26 @@ file, You can obtain one at http://mozil
               this.selectedIndex = 0;
               this.richlistbox.suppressMenuItemEvent = false;
             }
 
             this.input.gotResultForCurrentQuery = true;
 
             // Check if we should perform a delayed handleEnter.
             if (this.input.handleEnterInstance) {
-              try {
+              let instance = this.input.handleEnterInstance;
+              this.input.handleEnterInstance = null;
+              // Don't handle this immediately or we could cause a recursive
+              // loop where the controller sets popupOpen and re-enters here.
+              setTimeout(() => {
                 // Safety check: handle only if the search string didn't change.
-                let instance = this.input.handleEnterInstance;
                 if (this.input.mController.searchString == instance.searchString) {
                   this.input.mController.handleEnter(false, instance.event);
                 }
-              } finally {
-                this.input.handleEnterInstance = null;
-              }
+              }, 0);
             }
           ]]>
         </body>
       </method>
 
       <method name="_onSearchBegin">
         <body><![CDATA[
           // Set the selected index to 0 (heuristic) until a result comes back
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -891,25 +891,31 @@ Search.prototype = {
       queries.push(this._switchToTabQuery);
     }
     queries.push(this._searchQuery);
 
     // Add the first heuristic result, if any.  Set _addingHeuristicFirstMatch
     // to true so that when the result is added, "heuristic" can be included in
     // its style.
     this._addingHeuristicFirstMatch = true;
-    yield this._matchFirstHeuristicResult(conn);
+    let hasHeuristic = yield this._matchFirstHeuristicResult(conn);
     this._addingHeuristicFirstMatch = false;
+    if (!this.pending)
+      return;
 
     // We sleep a little between adding the heuristicFirstMatch and matching
     // any other searches so we aren't kicking off potentially expensive
     // searches on every keystroke.
-    yield this._sleep(Prefs.delay);
-    if (!this.pending)
-      return;
+    // Though, if there's no heuristic result, we start searching immediately,
+    // since autocomplete may be waiting for us.
+    if (hasHeuristic) {
+      yield this._sleep(Prefs.delay);
+      if (!this.pending)
+        return;
+    }
 
     if (this._enableActions) {
       yield this._matchSearchSuggestions();
       if (!this.pending)
         return;
     }
 
     for (let [query, params] of queries) {
@@ -945,68 +951,70 @@ Search.prototype = {
   *_matchFirstHeuristicResult(conn) {
     // We always try to make the first result a special "heuristic" result.  The
     // heuristics below determine what type of result it will be, if any.
 
     if (this._searchTokens.length > 0) {
       // This may be a Places keyword.
       let matched = yield this._matchPlacesKeyword();
       if (matched) {
-        return;
+        return true;
       }
     }
 
     if (this.pending && this._enableActions) {
       // If it's not a Places keyword, then it may be a search engine
       // with an alias - which works like a keyword.
       let matched = yield this._matchSearchEngineAlias();
       if (matched) {
-        return;
+        return true;
       }
     }
 
     let shouldAutofill = this._shouldAutofill;
     if (this.pending && shouldAutofill) {
       // It may also look like a URL we know from the database.
       let matched = yield this._matchKnownUrl(conn);
       if (matched) {
-        return;
+        return true;
       }
     }
 
     if (this.pending && shouldAutofill) {
       // Or it may look like a URL we know about from search engines.
       let matched = yield this._matchSearchEngineUrl();
       if (matched) {
-        return;
+        return true;
       }
     }
 
     if (this.pending && this._enableActions) {
       // If we don't have a result that matches what we know about, then
       // we use a fallback for things we don't know about.
 
       // We may not have auto-filled, but this may still look like a URL.
       // However, even if the input is a valid URL, we may not want to use
       // it as such. This can happen if the host would require whitelisting,
       // but isn't in the whitelist.
       let matched = yield this._matchUnknownUrl();
       if (matched) {
-        return;
+        return true;
       }
     }
 
     if (this.pending && this._enableActions && this._originalSearchString) {
       // When all else fails, and the search string is non-empty, we search
       // using the current search engine.
       let matched = yield this._matchCurrentSearchEngine();
       if (matched) {
-        return;
+        return true;
       }
     }
+
+    return false;
   },
 
   *_matchSearchSuggestions() {
     // Limit the string sent for search suggestions to a maximum length.
     let searchString = this._searchTokens.join(" ")
                            .substr(0, Prefs.maxCharsForSearchSuggestions);
     // Avoid fetching suggestions if they are not required, private browsing
     // mode is enabled, or the search string may expose sensitive information.
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1113,17 +1113,23 @@ extends="chrome://global/content/binding
 
           // Update the richlistbox height.
           if (this._adjustHeightTimeout) {
             clearTimeout(this._adjustHeightTimeout);
           }
           if (this._shrinkTimeout) {
             clearTimeout(this._shrinkTimeout);
           }
-          this._adjustHeightTimeout = setTimeout(() => this.adjustHeight(), 0);
+
+          if (this.mPopupOpen) {
+            delete this._adjustHeightOnPopupShown;
+            this._adjustHeightTimeout = setTimeout(() => this.adjustHeight(), 0);
+          } else {
+            this._adjustHeightOnPopupShown = true;
+          }
 
           this._currentIndex = 0;
           if (this._appendResultTimeout) {
             clearTimeout(this._appendResultTimeout);
           }
           this._appendCurrentResult(reason);
         ]]>
         </body>
@@ -1301,20 +1307,23 @@ extends="chrome://global/content/binding
             }
             else {
               // set the class at the end so we can use the attributes
               // in the xbl constructor
               item.className = "autocomplete-richlistitem";
               this.richlistbox.appendChild(item);
             }
 
-            let changed = item.adjustSiteIconStart(this._siteIconStart);
-            if (changed) {
-              item.handleOverUnderflow();
-            }
+            // The binding may have not been applied yet.
+            setTimeout(() => {
+              let changed = item.adjustSiteIconStart(this._siteIconStart);
+              if (changed) {
+                item.handleOverUnderflow();
+              }
+            }, 0);
 
             this._currentIndex++;
           }
 
           if (typeof this.onResultsAdded == "function")
             this.onResultsAdded();
 
           if (this._currentIndex < matchCount) {
@@ -1372,16 +1381,26 @@ extends="chrome://global/content/binding
         document.getAnonymousElementByAttribute(this, "anonid", "richlistbox");
       </field>
 
       <property name="view"
                 onget="return this.mInput.controller;"
                 onset="return val;"/>
 
     </implementation>
+    <handlers>
+      <handler event="popupshown">
+        <![CDATA[
+          if (this._adjustHeightOnPopupShown) {
+            delete this._adjustHeightOnPopupShown;
+            this.adjustHeight();
+          }
+      ]]>
+      </handler>
+    </handlers>
   </binding>
 
   <binding id="autocomplete-richlistitem" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
 
     <content align="center"
              onoverflow="this._onOverflow();"
              onunderflow="this._onUnderflow();">
       <xul:image anonid="type-icon"