Bug 1402286 - chunk notifyResults calls so that we don't run all the autocomplete js for each match. r=adw

MozReview-Commit-ID: GuYew5B30WQ

--HG--
extra : rebase_source : 236f6fe83728ddbb9d73c4f7606aca2187e267b5

browser/base/content/test/performance/browser_urlbar_keyed_search_reflows.js

@@ -45,6 +45,7 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
       "adjustHeight@chrome://global/content/bindings/autocomplete.xml",
       "_invalidate/this._adjustHeightTimeout<@chrome://global/content/bindings/autocomplete.xml",
     ],
+    minTimes: 39, // This number should only ever go down - never up.
     times: 51, // This number should only ever go down - never up.
   },
 

browser/base/content/test/performance/browser_urlbar_search_reflows.js

@@ -57,7 +57,7 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
       "_invalidate@chrome://global/content/bindings/autocomplete.xml",
       "invalidate@chrome://global/content/bindings/autocomplete.xml"
     ],
-    times: 60, // This number should only ever go down - never up.
+    times: 36, // This number should only ever go down - never up.
   },
 
   {
@@ -102,6 +102,18 @@ const EXPECTED_REFLOWS_SECOND_OPEN = [
     times: 3, // This number should only ever go down - never up.
   },
 
+  {
+    stack: [
+      "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
+      "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
+      "_reuseAcItem@chrome://global/content/bindings/autocomplete.xml",
+      "_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml",
+      "_invalidate@chrome://global/content/bindings/autocomplete.xml",
+      "invalidate@chrome://global/content/bindings/autocomplete.xml"
+    ],
+    times: 24, // This number should only ever go down - never up.
+  },
+
   // Bug 1359989
   {
     stack: [

browser/base/content/test/urlbar/browser_urlbarOneOffs.js

@@ -36,6 +36,7 @@ add_task(async function history() {
   gURLBar.focus();
   EventUtils.synthesizeKey("VK_DOWN", {});
   await promisePopupShown(gURLBar.popup);
+  await waitForAutocompleteResultAt(gMaxResults - 1)
 
   assertState(-1, -1, "");
 
@@ -106,7 +107,7 @@ add_task(async function() {
   // trigger autofill since that would complicate the test.
   let typedValue = "browser_urlbarOneOffs";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-
+  await waitForAutocompleteResultAt(gMaxResults - 1);
   assertState(0, -1, typedValue);
 
   // Key down through each result.  The first result is already selected, which
@@ -158,7 +159,7 @@ add_task(async function() {
 add_task(async function searchWith() {
   let typedValue = "foo";
   await promiseAutocompleteResultPopup(typedValue);
-
+  await waitForAutocompleteResultAt(0);
   assertState(0, -1, typedValue);
 
   let item = gURLBar.popup.richlistbox.firstChild;
@@ -190,7 +191,7 @@ add_task(async function oneOffClick() {
   // stricter. Even if it looks like a url, we should search.
   let typedValue = "foo.bar";
   await promiseAutocompleteResultPopup(typedValue);
-
+  await waitForAutocompleteResultAt(1);
   assertState(0, -1, typedValue);
 
   let oneOffs = gURLBar.popup.oneOffSearchButtons.getSelectableButtons(true);
@@ -211,7 +212,7 @@ add_task(async function oneOffReturn() {
   // stricter. Even if it looks like a url, we should search.
   let typedValue = "foo.bar";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-
+  await waitForAutocompleteResultAt(1);
   assertState(0, -1, typedValue);
 
   // Alt+Down to select the first one-off.

browser/base/content/test/urlbar/head.js

@@ -320,5 +320,7 @@ async function waitForAutocompleteResultAt(index) {
     () => gURLBar.popup.richlistbox.children.length > index &&
           gURLBar.popup.richlistbox.children[index].getAttribute("ac-text") == searchString,
     `Waiting for the autocomplete result for "${searchString}" at [${index}] to appear`);
+  // Ensure the addition is complete, for proper mouse events on the entries.
+  await new Promise(resolve => window.requestIdleCallback(resolve, {timeout: 1000}));
   return gURLBar.popup.richlistbox.children[index];
 }

browser/components/extensions/test/browser/browser_ext_omnibox.js

@@ -84,21 +84,35 @@ add_task(async function() {
     }
   }
 
-  async function startInputSession() {
+  async function waitForAutocompleteResultAt(index) {
+    let searchString = gURLBar.controller.searchString;
+    await BrowserTestUtils.waitForCondition(
+      () => gURLBar.popup.richlistbox.children.length > index &&
+            gURLBar.popup.richlistbox.children[index].getAttribute("ac-text") == searchString,
+      `Waiting for the autocomplete result for "${searchString}" at [${index}] to appear`);
+    // Ensure the addition is complete, for proper mouse events on the entries.
+    await new Promise(resolve => window.requestIdleCallback(resolve, {timeout: 1000}));
+    return gURLBar.popup.richlistbox.children[index];
+  }
+
+  let inputSessionSerial = 0;
+  async function startInputSession(indexToWaitFor) {
     gURLBar.focus();
     gURLBar.value = keyword;
     EventUtils.synthesizeKey(" ", {});
     await expectEvent("on-input-started-fired");
-    EventUtils.synthesizeKey("t", {});
-    await expectEvent("on-input-changed-fired", {text: "t"});
+    // Always use a different input at every invokation, so that
+    // waitForAutocompleteResultAt can distinguish different cases.
+    let char = ((inputSessionSerial++) % 10).toString();
+    EventUtils.synthesizeKey(char, {});
+
+    await expectEvent("on-input-changed-fired", {text: char});
     // Wait for the autocomplete search. Note that we cannot wait for the search
     // to be complete, since the add-on doesn't communicate when it's done, so
     // just check matches count.
-    await BrowserTestUtils.waitForCondition(
-      () => gURLBar.controller.matchCount >= 2 &&
-            gURLBar.popup.richlistbox.children[1].getAttribute("ac-text") == gURLBar.controller.searchString,
-      "waiting urlbar search to complete");
-    return "t";
+    await waitForAutocompleteResultAt(indexToWaitFor);
+
+    return char;
   }
 
   async function testInputEvents() {
@@ -174,7 +188,7 @@ add_task(async function() {
       await extension.awaitMessage("default-suggestion-set");
     }
 
-    let text = await startInputSession();
+    let text = await startInputSession(0);
 
     let item = gURLBar.popup.richlistbox.children[0];
 
@@ -193,7 +207,7 @@ add_task(async function() {
   }
 
   async function testDisposition(suggestionIndex, expectedDisposition, expectedText) {
-    await startInputSession();
+    await startInputSession(suggestionIndex);
 
     // Select the suggestion.
     for (let i = 0; i < suggestionIndex; i++) {
@@ -229,7 +243,7 @@ add_task(async function() {
         `Expected suggestion to have displayurl: "${keyword} ${content}".`);
     }
 
-    let text = await startInputSession();
+    let text = await startInputSession(info.suggestions.length - 1);
 
     extension.sendMessage(info.test);
     await extension.awaitMessage("test-ready");
@@ -274,12 +288,10 @@ add_task(async function() {
   // Test adding suggestions asynchronously.
   await testSuggestions({
     test: "test-multiple-suggest-calls",
-    skipHeuristic: true,
     suggestions,
   });
   await testSuggestions({
     test: "test-suggestions-after-delay",
-    skipHeuristic: true,
     suggestions,
   });
 

toolkit/components/places/UnifiedComplete.js

@@ -82,7 +82,7 @@ const FRECENCY_DEFAULT = 1000;
 const MAXIMUM_ALLOWED_EXTENSION_MATCHES = 6;
 
 // After this time, we'll give up waiting for the extension to return matches.
-const MAXIMUM_ALLOWED_EXTENSION_TIME_MS = 5000;
+const MAXIMUM_ALLOWED_EXTENSION_TIME_MS = 3000;
 
 // A regex that matches "single word" hostnames for whitelisting purposes.
 // The hostname will already have been checked for general validity, so we
@@ -95,6 +95,9 @@ const REGEXP_USER_CONTEXT_ID = /(?:^| )user-context-id:(\d+)/;
 // Regex used to match one or more whitespace.
 const REGEXP_SPACES = /\s+/;
 
+// The result is notified on a delay, to avoid rebuilding the panel at every match.
+const NOTIFYRESULT_DELAY_MS = 16;
+
 // Sqlite result row index constants.
 const QUERYINDEX_QUERYTYPE     = 0;
 const QUERYINDEX_URL           = 1;
@@ -330,6 +333,7 @@ XPCOMUtils.defineLazyServiceGetter(this, "textURIService",
 function setTimeout(callback, ms) {
   let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
   timer.initWithCallback(callback, ms, timer.TYPE_ONE_SHOT);
+  return timer;
 }
 
 function convertBucketsCharPrefToArray(str) {
@@ -992,6 +996,9 @@ Search.prototype = {
     // Avoid multiple calls or re-entrance.
     if (!this.pending)
       return;
+    if (this._notifyTimer)
+      this._notifyTimer.cancel();
+    this._notifyDelaysCount = 0;
     if (this._sleepTimer)
       this._sleepTimer.cancel();
     if (this._sleepResolve) {
@@ -1127,7 +1134,6 @@ Search.prototype = {
           // We're done if we're restricting to search suggestions.
           // Notify the result completion then stop the search.
           this._autocompleteSearch.finishSearch(true);
-          this.stop();
           return;
         }
       }
@@ -1667,9 +1673,8 @@ Search.prototype = {
 
     // Since the extension has no way to signale when it's done pushing
     // results, we add a timeout racing with the addition.
-    let timeoutPromise = new Promise((resolve, reject) => {
-      setTimeout(() => reject(new Error("timeout waiting for the extension to add its results to the location bar")),
-                 MAXIMUM_ALLOWED_EXTENSION_TIME_MS);
+    let timeoutPromise = new Promise(resolve => {
+      setTimeout(resolve, MAXIMUM_ALLOWED_EXTENSION_TIME_MS);
     });
     return Promise.race([timeoutPromise, promise]).catch(Cu.reportError);
   },
@@ -1909,7 +1914,7 @@ Search.prototype = {
       TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT, this);
     if (this._currentMatchCount == 6)
       TelemetryStopwatch.finish(TELEMETRY_6_FIRST_RESULTS, this);
-    this.notifyResults(true);
+    this.notifyResult(true, match.type == MATCHTYPE.HEURISTIC);
   },
 
   _getInsertIndexForMatch(match) {
@@ -2009,7 +2014,7 @@ Search.prototype = {
       }
     }
     if (changed && notify) {
-      this.notifyResults(true);
+      this.notifyResult(true);
     }
   },
 
@@ -2364,24 +2369,47 @@ Search.prototype = {
     return query;
   },
 
+  // The result is notified to the search listener on a timer, to chunk multiple
+  // match updates together and avoid rebuilding the popup at every new match.
+  _notifyTimer: null,
+
   /**
-   * Notifies the listener about results.
+   * Notifies the current result to the listener.
    *
    * @param searchOngoing
-   *        Indicates whether the search is ongoing.
+   *        Indicates whether the search result should be marked as ongoing.
+   * @param skipDelay
+   *        Whether to notify immediately.
    */
-  notifyResults(searchOngoing) {
-    let result = this._result;
+  _notifyDelaysCount: 0,
+  notifyResult(searchOngoing, skipDelay = false) {
+    let notify = () => {
+      this._notifyDelaysCount = 0;
       let resultCode = this._currentMatchCount ? "RESULT_SUCCESS" : "RESULT_NOMATCH";
       if (searchOngoing) {
         resultCode += "_ONGOING";
       }
+      let result = this._result;
       result.setSearchResult(Ci.nsIAutoCompleteResult[resultCode]);
       this._listener.onSearchResult(this._autocompleteSearch, result);
       if (!searchOngoing) {
         // Break possible cycles.
         this._listener = null;
         this._autocompleteSearch = null;
+        this.stop();
+      }
+    };
+    if (this._notifyTimer) {
+      this._notifyTimer.cancel();
+    }
+    // In the worst case, we may get evenly spaced matches that would end up
+    // delaying the UI by N_MATCHES * NOTIFYRESULT_DELAY_MS. Thus, we clamp the
+    // number of times we may delay matches.
+    if (skipDelay || this._notifyDelaysCount > 3) {
+      notify();
+    } else {
+      this._notifyDelaysCount++;
+      this._notifyTimer = setTimeout(notify, NOTIFYRESULT_DELAY_MS);
     }
   },
 };
@@ -2579,7 +2607,6 @@ UnifiedComplete.prototype = {
     if (!notify || !search.pending)
       return;
 
-
     // If we are in restrict mode and we reused the previous search results,
     // it's possible we didn't go through all the cleanup methods due to early
     // bailouts. Thus we could still have nonmatching results to remove.
@@ -2591,10 +2618,10 @@ UnifiedComplete.prototype = {
     // 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,
+    // notifyResult.
+    // Thus, ensure that notifyResult is the last call in this method,
     // otherwise you might be touching the wrong search.
-    search.notifyResults(false);
+    search.notifyResult(false);
   },
 
   // nsIAutoCompleteSearchDescriptor

toolkit/components/places/tests/unifiedcomplete/test_extension_matches.js

@@ -195,7 +195,10 @@ add_task(async function test_removes_suggestion_if_its_content_is_typed_in() {
           {content: "bar", description: "second suggestion"},
           {content: "baz", description: "third suggestion"},
         ]);
+        // The API doesn't have a way to notify when addition is complete.
+        do_timeout(1000, () => {
           controller.stopSearch();
+        });
       }
     }
   };
@@ -302,8 +305,11 @@ add_task(async function test_setting_the_default_suggestion() {
     emit(message, text, id) {
       if (message === ExtensionSearchHandler.MSG_INPUT_CHANGED) {
         ExtensionSearchHandler.addSuggestions(keyword, id, []);
-      }
+        // The API doesn't have a way to notify when addition is complete.
+        do_timeout(1000, () => {
           controller.stopSearch();
+        });
+      }
     }
   };
 
@@ -358,7 +364,10 @@ add_task(async function test_maximum_number_of_suggestions_is_enforced() {
           {content: "i", description: "ninth suggestion"},
           {content: "j", description: "tenth suggestion"},
         ]);
+        // The API doesn't have a way to notify when addition is complete.
+        do_timeout(1000, () => {
           controller.stopSearch();
+        });
       }
     }
   };