Bug 1548817 - Quantum Bar controller notifications may arrive out of order. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 08 May 2019 21:14:33 +0000
changeset 531945 876559fca157d87c6b39f57ee4995dc09fff0db2
parent 531944 175f567cd163eb70fd015e585bb0ae74f60c2cfe
child 531946 d6756af108596e5a27548d397f1dd72d44c45854
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1548817
milestone68.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 1548817 - Quantum Bar controller notifications may arrive out of order. r=adw Differential Revision: https://phabricator.services.mozilla.com/D30082
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/UrlbarUtils.jsm
browser/components/urlbar/UrlbarView.jsm
browser/components/urlbar/tests/UrlbarTestUtils.jsm
browser/components/urlbar/tests/browser/browser_urlbarOneOffs.js
browser/components/urlbar/tests/unit/test_UrlbarController_integration.js
browser/components/urlbar/tests/unit/test_UrlbarController_telemetry.js
browser/components/urlbar/tests/unit/test_UrlbarController_unit.js
browser/docs/AddressBar.rst
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -8,17 +8,16 @@ var EXPORTED_SYMBOLS = [
   "UrlbarController",
 ];
 
 const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 XPCOMUtils.defineLazyModuleGetters(this, {
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
-  ExtensionSearchHandler: "resource://gre/modules/ExtensionSearchHandler.jsm",
   PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm",
   UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
   URLBAR_SELECTED_RESULT_TYPES: "resource:///modules/BrowserUsageTelemetry.jsm",
 });
 
 const TELEMETRY_1ST_RESULT = "PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS";
@@ -90,52 +89,63 @@ class UrlbarController {
    * Takes a query context and starts the query based on the user input.
    *
    * @param {UrlbarQueryContext} queryContext The query details.
    */
   async startQuery(queryContext) {
     // Cancel any running query.
     this.cancelQuery();
 
-    this._lastQueryContext = queryContext;
+    // Wrap the external queryContext, to track a unique object, in case
+    // the external consumer reuses the same context multiple times.
+    // This also allows to add properties without polluting the context.
+    // Note this can't be null-ed or deleted once a query is done, because it's
+    // used by handleDeleteEntry and handleKeyNavigation, that can run after
+    // a query is cancelled or finished.
+    let contextWrapper = this._lastQueryContextWrapper = {queryContext};
 
     queryContext.lastResultCount = 0;
     TelemetryStopwatch.start(TELEMETRY_1ST_RESULT, queryContext);
     TelemetryStopwatch.start(TELEMETRY_6_FIRST_RESULTS, queryContext);
 
+    // For proper functionality we must ensure this notification is fired
+    // synchronously, as soon as startQuery is invoked, but after any
+    // notifications related to the previous query.
     this._notify("onQueryStarted", queryContext);
     await this.manager.startQuery(queryContext, this);
-    this._notify("onQueryFinished", queryContext);
+    // If the query has been cancelled, onQueryFinished was notified already.
+    // Note this._lastQueryContextWrapper may have changed in the meanwhile.
+    if (contextWrapper === this._lastQueryContextWrapper &&
+        !contextWrapper.done) {
+      contextWrapper.done = true;
+      // TODO (Bug 1549936) this is necessary to avoid leaks in PB tests.
+      this.manager.cancelQuery(queryContext);
+      this._notify("onQueryFinished", queryContext);
+    }
     return queryContext;
   }
 
   /**
    * Cancels an in-progress query. Note, queries may continue running if they
    * can't be cancelled.
-   *
-   * @param {UrlbarUtils.CANCEL_REASON} [reason]
-   *   The reason the query was cancelled.
    */
-  cancelQuery(reason) {
-    if (!this._lastQueryContext ||
-        this._lastQueryContext._cancelled) {
+  cancelQuery() {
+    // If the query finished already, don't handle cancel.
+    if (!this._lastQueryContextWrapper || this._lastQueryContextWrapper.done) {
       return;
     }
 
-    TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT, this._lastQueryContext);
-    TelemetryStopwatch.cancel(TELEMETRY_6_FIRST_RESULTS, this._lastQueryContext);
+    this._lastQueryContextWrapper.done = true;
 
-    this.manager.cancelQuery(this._lastQueryContext);
-    this._lastQueryContext._cancelled = true;
-    this._notify("onQueryCancelled", this._lastQueryContext);
-
-    if (reason == UrlbarUtils.CANCEL_REASON.BLUR &&
-        ExtensionSearchHandler.hasActiveInputSession()) {
-      ExtensionSearchHandler.handleInputCancelled();
-    }
+    let {queryContext} = this._lastQueryContextWrapper;
+    TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT, queryContext);
+    TelemetryStopwatch.cancel(TELEMETRY_6_FIRST_RESULTS, queryContext);
+    this.manager.cancelQuery(queryContext);
+    this._notify("onQueryCancelled", queryContext);
+    this._notify("onQueryFinished", queryContext);
   }
 
   /**
    * Receives results from a query.
    *
    * @param {UrlbarQueryContext} queryContext The query details.
    */
   receiveResults(queryContext) {
@@ -250,27 +260,25 @@ class UrlbarController {
         (event.key == "n" || event.key == "p")) {
       if (executeAction) {
         this.view.selectBy(1, { reverse: event.key == "p" });
       }
       event.preventDefault();
       return;
     }
 
-    if (this.view.isOpen && executeAction) {
-      let queryContext = this._lastQueryContext;
-      if (queryContext) {
-        let handled = this.view.oneOffSearchButtons.handleKeyPress(
-          event,
-          queryContext.results.length,
-          this.view.allowEmptySelection,
-          queryContext.searchString);
-        if (handled) {
-          return;
-        }
+    if (this.view.isOpen && executeAction && this._lastQueryContextWrapper) {
+      let {queryContext} = this._lastQueryContextWrapper;
+      let handled = this.view.oneOffSearchButtons.handleKeyPress(
+        event,
+        queryContext.results.length,
+        this.view.allowEmptySelection,
+        queryContext.searchString);
+      if (handled) {
+        return;
       }
     }
 
     switch (event.keyCode) {
       case KeyEvent.DOM_VK_ESCAPE:
         if (executeAction) {
           this.input.handleRevert();
         }
@@ -491,34 +499,35 @@ class UrlbarController {
 
   /**
    * Internal function handling deletion of entries. We only support removing
    * of history entries - other result sources will be ignored.
    *
    * @returns {boolean} Returns true if the deletion was acted upon.
    */
   _handleDeleteEntry() {
-    if (!this._lastQueryContext) {
+    if (!this._lastQueryContextWrapper) {
       Cu.reportError("Cannot delete - the latest query is not present");
       return false;
     }
 
     const selectedResult = this.input.view.selectedResult;
     if (!selectedResult ||
         selectedResult.source != UrlbarUtils.RESULT_SOURCE.HISTORY) {
       return false;
     }
 
-    let index = this._lastQueryContext.results.indexOf(selectedResult);
+    let {queryContext} = this._lastQueryContextWrapper;
+    let index = queryContext.results.indexOf(selectedResult);
     if (!index) {
       Cu.reportError("Failed to find the selected result in the results");
       return false;
     }
 
-    this._lastQueryContext.results.splice(index, 1);
+    queryContext.results.splice(index, 1);
     this._notify("onQueryResultRemoved", index);
 
     PlacesUtils.history.remove(selectedResult.payload.url).catch(Cu.reportError);
     return true;
   }
 
   /**
    * Internal function to notify listeners of results.
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -1265,20 +1265,27 @@ class UrlbarInput {
   // Event handlers below.
 
   _on_blur(event) {
     // In certain cases, like holding an override key and confirming an entry,
     // we don't key a keyup event for the override key, thus we make this
     // additional cleanup on blur.
     this._clearActionOverride();
     this.formatValue();
+
+    // The extension input sessions depends more on blur than on the fact we
+    // actually cancel a running query, so we do it here.
+    if (ExtensionSearchHandler.hasActiveInputSession()) {
+      ExtensionSearchHandler.handleInputCancelled();
+    }
+
     // Respect the autohide preference for easier inspecting/debugging via
     // the browser toolbox.
     if (!UrlbarPrefs.get("ui.popup.disable_autohide")) {
-      this.view.close(UrlbarUtils.CANCEL_REASON.BLUR);
+      this.view.close();
     }
     // We may have hidden popup notifications, show them again if necessary.
     if (this.getAttribute("pageproxystate") != "valid") {
       this.window.UpdatePopupNotificationsVisibility();
     }
     this._resetSearchState();
   }
 
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -116,22 +116,16 @@ var UrlbarUtils = {
   // IME composition states.
   COMPOSITION: {
     NONE: 1,
     COMPOSING: 2,
     COMMIT: 3,
     CANCELED: 4,
   },
 
-  // This defines possible reasons for canceling a query.
-  CANCEL_REASON: {
-    // 1 is intentionally left in case we want a none/undefined/other later.
-    BLUR: 2,
-  },
-
   // Limit the length of titles and URLs we display so layout doesn't spend too
   // much time building text runs.
   MAX_TEXT_LENGTH: 255,
 
   /**
    * Adds a url to history as long as it isn't in a private browsing window,
    * and it is valid.
    *
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -175,38 +175,39 @@ class UrlbarView {
   }
 
   removeAccessibleFocus() {
     this._setAccessibleFocus(null);
   }
 
   /**
    * Closes the autocomplete popup, cancelling the query if necessary.
-   *
-   * @param {UrlbarUtils.CANCEL_REASON} [cancelReason]
-   *   Indicates if this close is being triggered as a result of a user action
-   *   which would cancel a query, e.g. on blur.
    */
-  close(cancelReason) {
-    this.controller.cancelQuery(cancelReason);
+  close() {
+    this.controller.cancelQuery();
     this.panel.hidePopup();
   }
 
   // UrlbarController listener methods.
   onQueryStarted(queryContext) {
+    this._queryWasCancelled = false;
     this._startRemoveStaleRowsTimer();
   }
 
   onQueryCancelled(queryContext) {
+    this._queryWasCancelled = true;
     this._cancelRemoveStaleRowsTimer();
   }
 
   onQueryFinished(queryContext) {
     this._cancelRemoveStaleRowsTimer();
-    this._removeStaleRows();
+    // If the query has not been canceled, remove stale rows immediately.
+    if (!this._queryWasCancelled) {
+      this._removeStaleRows();
+    }
   }
 
   onQueryResults(queryContext) {
     this._queryContext = queryContext;
 
     this._updateResults(queryContext);
 
     let isFirstPreselectedResult = false;
@@ -748,16 +749,17 @@ class UrlbarView {
     this.input.inputField.setAttribute("aria-expanded", "true");
   }
 
   _on_popuphiding() {
     this.controller.cancelQuery();
     this.window.removeEventListener("resize", this);
     this.removeAccessibleFocus();
     this.input.inputField.setAttribute("aria-expanded", "false");
+    this._rows.textContent = "";
   }
 
   _on_resize() {
     // Close the popup as it would be wrongly sized. This can
     // happen when using special OS resize functions like Win+Arrow.
     this.close();
   }
 }
--- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm
+++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm
@@ -451,31 +451,26 @@ class UrlbarAbstraction {
         return UrlbarUtils.RESULT_TYPE.REMOTE_TAB;
       } else if (action && action.type == "switchtab") {
         return UrlbarUtils.RESULT_TYPE.TAB_SWITCH;
       }
       return UrlbarUtils.RESULT_TYPE.URL;
     }
     let details = {};
     if (this.quantumbar) {
-      let context = await this.urlbar.lastQueryContextPromise;
-      if (index >= context.results.length) {
-        throw new Error("Requested index not found in results");
-      }
-      let {url, postData} = UrlbarUtils.getUrlFromResult(context.results[index]);
+      let result = element.result;
+      let {url, postData} = UrlbarUtils.getUrlFromResult(result);
       details.url = url;
       details.postData = postData;
-      details.type = context.results[index].type;
-      details.heuristic = context.results[index].heuristic;
-      details.autofill = !!context.results[index].autofill;
+      details.type = result.type;
+      details.heuristic = result.heuristic;
+      details.autofill = !!result.autofill;
       details.image = element.getElementsByClassName("urlbarView-favicon")[0].src;
-      details.title = context.results[index].title;
-      details.tags = "tags" in context.results[index].payload ?
-        context.results[index].payload.tags :
-        [];
+      details.title = result.title;
+      details.tags = "tags" in result.payload ? result.payload.tags : [];
       let actions = element.getElementsByClassName("urlbarView-action");
       let urls = element.getElementsByClassName("urlbarView-url");
       let typeIcon = element.querySelector(".urlbarView-type-icon");
       let typeIconStyle = this.window.getComputedStyle(typeIcon);
       details.displayed = {
         title: element.getElementsByClassName("urlbarView-title")[0].textContent,
         action: actions.length > 0 ? actions[0].textContent : null,
         url: urls.length > 0 ? urls[0].textContent : null,
@@ -485,23 +480,23 @@ class UrlbarAbstraction {
         action: element.getElementsByClassName("urlbarView-action")[0],
         row: element,
         separator: element.getElementsByClassName("urlbarView-title-separator")[0],
         title: element.getElementsByClassName("urlbarView-title")[0],
         url: element.getElementsByClassName("urlbarView-url")[0],
       };
       if (details.type == UrlbarUtils.RESULT_TYPE.SEARCH) {
         details.searchParams = {
-          engine: context.results[index].payload.engine,
-          keyword: context.results[index].payload.keyword,
-          query: context.results[index].payload.query,
-          suggestion: context.results[index].payload.suggestion,
+          engine: result.payload.engine,
+          keyword: result.payload.keyword,
+          query: result.payload.query,
+          suggestion: result.payload.suggestion,
         };
       } else if (details.type == UrlbarUtils.RESULT_TYPE.KEYWORD) {
-        details.keyword = context.results[index].payload.keyword;
+        details.keyword = result.payload.keyword;
       }
     } else {
       details.url = this.urlbar.controller.getFinalCompleteValueAt(index);
 
       let style = this.urlbar.controller.getStyleAt(index);
       let action = PlacesUtils.parseActionUrl(this.urlbar.controller.getValueAt(index));
       details.type = getType(style, action);
       details.heuristic = style.includes("heuristic");
--- a/browser/components/urlbar/tests/browser/browser_urlbarOneOffs.js
+++ b/browser/components/urlbar/tests/browser/browser_urlbarOneOffs.js
@@ -193,17 +193,17 @@ add_task(async function searchWith() {
 // Clicks a one-off.
 add_task(async function oneOffClick() {
   gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
 
   // We are explicitly using something that looks like a url, to make the test
   // stricter. Even if it looks like a url, we should search.
   let typedValue = "foo.bar";
   await promiseAutocompleteResultPopup(typedValue);
-  await waitForAutocompleteResultAt(1);
+  await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
   assertState(0, -1, typedValue);
 
   let oneOffs = oneOffSearchButtons.getSelectableButtons(true);
   let resultsPromise =
     BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
                                    "http://mochi.test:8888/?terms=foo.bar");
   EventUtils.synthesizeMouseAtCenter(oneOffs[0], {});
   await resultsPromise;
@@ -214,17 +214,17 @@ add_task(async function oneOffClick() {
 // Presses the Return key when a one-off is selected.
 add_task(async function oneOffReturn() {
   gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
 
   // We are explicitly using something that looks like a url, to make the test
   // stricter. Even if it looks like a url, we should search.
   let typedValue = "foo.bar";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-  await waitForAutocompleteResultAt(1);
+  await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
   assertState(0, -1, typedValue);
 
   // Alt+Down to select the first one-off.
   EventUtils.synthesizeKey("KEY_ArrowDown", { altKey: true });
   assertState(0, 0, typedValue);
 
   let resultsPromise =
     BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
@@ -241,45 +241,43 @@ add_task(async function collapsedOneOffs
   let defaultEngine = await Services.search.getDefault();
   let engines = (await Services.search.getVisibleEngines()).filter(e => e.name != defaultEngine.name);
   await SpecialPowers.pushPrefEnv({"set": [
     [ "browser.search.hiddenOneOffs", engines.map(e => e.name).join(",") ],
   ]});
 
   let typedValue = "foo";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-  await waitForAutocompleteResultAt(0);
+  await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
   assertState(0, -1);
   Assert.ok(oneOffSearchButtons.buttons.collapsed,
     "The one-off buttons should be collapsed");
   EventUtils.synthesizeKey("KEY_ArrowUp");
   assertState(1, -1);
   await hidePopup();
 });
 
-
 // The one-offs should be hidden when searching with an "@engine" search engine
 // alias.
 add_task(async function hiddenWhenUsingSearchAlias() {
   let typedValue = "@example";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-  await waitForAutocompleteResultAt(0);
+  await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
   Assert.equal(UrlbarTestUtils.getOneOffSearchButtonsVisible(window), false,
     "Should not be showing the one-off buttons");
   await hidePopup();
 
   typedValue = "not an engine alias";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-  await waitForAutocompleteResultAt(0);
+  await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
   Assert.equal(UrlbarTestUtils.getOneOffSearchButtonsVisible(window), true,
     "Should be showing the one-off buttons");
   await hidePopup();
 });
 
-
 function assertState(result, oneOff, textValue = undefined) {
   Assert.equal(UrlbarTestUtils.getSelectedIndex(window), result,
     "Expected result should be selected");
   Assert.equal(oneOffSearchButtons.selectedButtonIndex,
     oneOff, "Expected one-off should be selected");
   if (textValue !== undefined) {
     Assert.equal(gURLBar.textValue, textValue, "Expected textValue");
   }
--- a/browser/components/urlbar/tests/unit/test_UrlbarController_integration.js
+++ b/browser/components/urlbar/tests/unit/test_UrlbarController_integration.js
@@ -63,17 +63,17 @@ add_task(async function test_basic_searc
 add_task(async function test_cancel_search() {
   let providerCanceledDeferred = PromiseUtils.defer();
   let providerName = registerBasicTestProvider([match], providerCanceledDeferred.resolve);
   const context = createContext(TEST_URL, {providers: [providerName]});
 
   let startedPromise = promiseControllerNotification(controller, "onQueryStarted");
   let cancelPromise = promiseControllerNotification(controller, "onQueryCancelled");
 
-  await controller.startQuery(context);
+  controller.startQuery(context);
 
   let params = await startedPromise;
 
   controller.cancelQuery(context);
 
   Assert.equal(params[0], context);
 
   info("Should tell the provider the query is canceled");
--- a/browser/components/urlbar/tests/unit/test_UrlbarController_telemetry.js
+++ b/browser/components/urlbar/tests/unit/test_UrlbarController_telemetry.js
@@ -34,26 +34,35 @@ class DelayedProvider extends UrlbarProv
     return UrlbarUtils.PROVIDER_TYPE.PROFILE;
   }
   get sources() {
     return [UrlbarUtils.RESULT_SOURCE.TABS];
   }
   async startQuery(context, add) {
     Assert.ok(context, "context is passed-in");
     Assert.equal(typeof add, "function", "add is a callback");
-    this._context = context;
     this._add = add;
+    await new Promise(resolve => {
+      this._resultsAdded = resolve;
+    });
   }
   cancelQuery(context) {
-    Assert.ok(false, "cancelQuery should not be called");
+    // Nothing.
   }
-  addResults(matches) {
+  async addResults(matches, finish = true) {
+    // startQuery may have not been invoked yet, so wait for it
+    await TestUtils.waitForCondition(() => !!this._add,
+                                     "Waiting for the _add callback");
     for (const match of matches) {
       this._add(this, match);
     }
+    if (finish) {
+      this._add = null;
+      this._resultsAdded();
+    }
   }
 }
 
 /**
  * Returns the number of reports sent recorded within the histogram results.
  *
  * @param {object} results a snapshot of histogram results to check.
  * @returns {number} The count of reports recorded in the histogram.
@@ -88,17 +97,17 @@ add_task(async function test_n_autocompl
   UrlbarProvidersManager.registerProvider(provider);
   const context = createContext(TEST_URL, {providers: [provider.name]});
 
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_1ST_RESULT, context),
     "Should not have started first result stopwatch");
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_6_FIRST_RESULTS, context),
     "Should not have started first 6 results stopwatch");
 
-  await controller.startQuery(context);
+  controller.startQuery(context);
 
   Assert.ok(TelemetryStopwatch.running(TELEMETRY_1ST_RESULT, context),
     "Should have started first result stopwatch");
   Assert.ok(TelemetryStopwatch.running(TELEMETRY_6_FIRST_RESULTS, context),
     "Should have started first 6 results stopwatch");
 
   controller.cancelQuery(context);
 
@@ -125,24 +134,24 @@ add_task(async function test_n_autocompl
 
   let resultsPromise = promiseControllerNotification(controller, "onQueryResults");
 
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_1ST_RESULT, context),
     "Should not have started first result stopwatch");
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_6_FIRST_RESULTS, context),
     "Should not have started first 6 results stopwatch");
 
-  await controller.startQuery(context);
+  controller.startQuery(context);
 
   Assert.ok(TelemetryStopwatch.running(TELEMETRY_1ST_RESULT, context),
     "Should have started first result stopwatch");
   Assert.ok(TelemetryStopwatch.running(TELEMETRY_6_FIRST_RESULTS, context),
     "Should have started first 6 results stopwatch");
 
-  provider.addResults([MATCH]);
+  await provider.addResults([MATCH], false);
   await resultsPromise;
 
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_1ST_RESULT, context),
     "Should have stopped the first stopwatch");
   Assert.ok(TelemetryStopwatch.running(TELEMETRY_6_FIRST_RESULTS, context),
     "Should have kept the first 6 results stopwatch running");
 
   let firstResults = firstHistogram.snapshot();
@@ -150,21 +159,21 @@ add_task(async function test_n_autocompl
   Assert.equal(getHistogramReportsCount(firstResults), 1,
     "Should have recorded one time for the first result");
   Assert.equal(getHistogramReportsCount(first6Results), 0,
     "Should not have recorded any times (first 6 results)");
 
   // Now add 5 more results, so that the first 6 results is triggered.
   for (let i = 0; i < 5; i++) {
     resultsPromise = promiseControllerNotification(controller, "onQueryResults");
-    provider.addResults([
+    await provider.addResults([
       new UrlbarResult(UrlbarUtils.RESULT_TYPE.TAB_SWITCH,
                        UrlbarUtils.RESULT_SOURCE.TABS,
                        { url: TEST_URL + "/i" }),
-    ]);
+    ], false);
     await resultsPromise;
   }
 
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_1ST_RESULT, context),
     "Should have stopped the first stopwatch");
   Assert.ok(!TelemetryStopwatch.running(TELEMETRY_6_FIRST_RESULTS, context),
     "Should have stopped the first 6 results stopwatch");
 
@@ -172,17 +181,17 @@ add_task(async function test_n_autocompl
   let updated6Results = sixthHistogram.snapshot();
   Assert.deepEqual(updatedResults, firstResults,
     "Should not have changed the histogram for the first result");
   Assert.equal(getHistogramReportsCount(updated6Results), 1,
     "Should have recorded one time for the first 6 results");
 
   // Add one more, to check neither are updated.
   resultsPromise = promiseControllerNotification(controller, "onQueryResults");
-  provider.addResults([
+  await provider.addResults([
     new UrlbarResult(UrlbarUtils.RESULT_TYPE.TAB_SWITCH,
                      UrlbarUtils.RESULT_SOURCE.TABS,
                      { url: TEST_URL + "/6" }),
   ]);
   await resultsPromise;
 
   let secondUpdateResults = firstHistogram.snapshot();
   let secondUpdate6Results = sixthHistogram.snapshot();
--- a/browser/components/urlbar/tests/unit/test_UrlbarController_unit.js
+++ b/browser/components/urlbar/tests/unit/test_UrlbarController_unit.js
@@ -144,21 +144,21 @@ add_task(function test_handle_query_star
   Assert.equal(generalListener.onQueryStarted.callCount, 1,
     "Should have called onQueryStarted for the listener");
   Assert.deepEqual(generalListener.onQueryStarted.args[0], [context],
     "Should have called onQueryStarted with the context");
 
   sandbox.resetHistory();
 });
 
-add_task(function test_handle_query_starts_search_sets_allowAutofill() {
+add_task(async function test_handle_query_starts_search_sets_allowAutofill() {
   let originalValue = Services.prefs.getBoolPref("browser.urlbar.autoFill");
   Services.prefs.setBoolPref("browser.urlbar.autoFill", !originalValue);
 
-  controller.startQuery(createContext());
+  await controller.startQuery(createContext());
 
   Assert.equal(fPM.startQuery.callCount, 1,
     "Should have called startQuery once");
   Assert.equal(fPM.startQuery.args[0].length, 2,
     "Should have called startQuery with two arguments");
 
   assertContextMatches(fPM.startQuery.args[0][0], {
     allowAutofill: !originalValue,
@@ -167,19 +167,16 @@ add_task(function test_handle_query_star
     "Should have passed the controller as the second argument");
 
   sandbox.resetHistory();
 
   Services.prefs.clearUserPref("browser.urlbar.autoFill");
 });
 
 add_task(function test_cancel_query() {
-  // Ensure the controller doesn't have any previous queries.
-  delete controller._lastQueryContext;
-
   const context = createContext();
   controller.startQuery(context);
 
   controller.cancelQuery();
 
   Assert.equal(fPM.cancelQuery.callCount, 1,
     "Should have called cancelQuery once");
   Assert.equal(fPM.cancelQuery.args[0].length, 1,
@@ -200,8 +197,49 @@ add_task(function test_receiveResults() 
 
   Assert.equal(generalListener.onQueryResults.callCount, 1,
     "Should have called onQueryResults for the listener");
   Assert.deepEqual(generalListener.onQueryResults.args[0], [context],
     "Should have called onQueryResults with the context");
 
   sandbox.resetHistory();
 });
+
+add_task(async function test_notifications_order() {
+  // Clear any pending notifications.
+  const context = createContext();
+  await controller.startQuery(context);
+
+  // Check that when multiple queries are executed, the notifications arrive
+  // in the proper order.
+  let collectingListener = new Proxy({}, {
+    _notifications: [],
+    get(target, name) {
+      if (name == "notifications") {
+        return this._notifications;
+      }
+      return () => {
+        this._notifications.push(name);
+      };
+    },
+  });
+  controller.addQueryListener(collectingListener);
+  controller.startQuery(context);
+  Assert.deepEqual(["onQueryStarted"], collectingListener.notifications,
+                   "Check onQueryStarted is fired synchronously");
+  controller.startQuery(context);
+  Assert.deepEqual(["onQueryStarted", "onQueryCancelled", "onQueryFinished",
+                    "onQueryStarted"],
+                   collectingListener.notifications,
+                   "Check order of notifications");
+  controller.cancelQuery();
+  Assert.deepEqual(["onQueryStarted", "onQueryCancelled", "onQueryFinished",
+                    "onQueryStarted", "onQueryCancelled", "onQueryFinished"],
+                   collectingListener.notifications,
+                   "Check order of notifications");
+  await controller.startQuery(context);
+  controller.cancelQuery();
+  Assert.deepEqual(["onQueryStarted", "onQueryCancelled", "onQueryFinished",
+                    "onQueryStarted", "onQueryCancelled", "onQueryFinished",
+                    "onQueryStarted", "onQueryFinished"],
+                   collectingListener.notifications,
+                   "Check order of notifications");
+});
--- a/browser/docs/AddressBar.rst
+++ b/browser/docs/AddressBar.rst
@@ -149,96 +149,96 @@ implementation details may vary deeply a
 .. note::
 
   Internal providers can access the Places database through the
   *PlacesUtils.promiseLargeCacheDBConnection* utility.
 
 .. highlight:: JavaScript
 .. code::
 
-class UrlbarProvider {
-  /**
-   * Unique name for the provider, used by the context to filter on providers.
-   * Not using a unique name will cause the newest registration to win.
-   * @abstract
-   */
-  get name() {
-    return "UrlbarProviderBase";
-  }
-  /**
-   * The type of the provider, must be one of UrlbarUtils.PROVIDER_TYPE.
-   * @abstract
-   */
-  get type() {
-    throw new Error("Trying to access the base class, must be overridden");
+  class UrlbarProvider {
+    /**
+    * Unique name for the provider, used by the context to filter on providers.
+    * Not using a unique name will cause the newest registration to win.
+    * @abstract
+    */
+    get name() {
+      return "UrlbarProviderBase";
+    }
+    /**
+    * The type of the provider, must be one of UrlbarUtils.PROVIDER_TYPE.
+    * @abstract
+    */
+    get type() {
+      throw new Error("Trying to access the base class, must be overridden");
+    }
+    /**
+    * List of UrlbarUtils.RESULT_SOURCE, representing the data sources used by
+    * the provider.
+    * @abstract
+    */
+    get sources() {
+      throw new Error("Trying to access the base class, must be overridden");
+    }
+    /**
+    * Starts querying.
+    * @param {UrlbarQueryContext} queryContext The query context object
+    * @param {function} addCallback Callback invoked by the provider to add a new
+    *        result. A UrlbarResult should be passed to it.
+    * @note Extended classes should return a Promise resolved when the provider
+    *       is done searching AND returning results.
+    * @abstract
+    */
+    startQuery(queryContext, addCallback) {
+      throw new Error("Trying to access the base class, must be overridden");
+    }
+    /**
+    * Cancels a running query,
+    * @param {UrlbarQueryContext} queryContext The query context object to cancel
+    *        query for.
+    * @abstract
+    */
+    cancelQuery(queryContext) {
+      throw new Error("Trying to access the base class, must be overridden");
+    }
   }
-  /**
-   * List of UrlbarUtils.RESULT_SOURCE, representing the data sources used by
-   * the provider.
-   * @abstract
-   */
-  get sources() {
-    throw new Error("Trying to access the base class, must be overridden");
-  }
-  /**
-   * Starts querying.
-   * @param {UrlbarQueryContext} queryContext The query context object
-   * @param {function} addCallback Callback invoked by the provider to add a new
-   *        result. A UrlbarResult should be passed to it.
-   * @note Extended classes should return a Promise resolved when the provider
-   *       is done searching AND returning results.
-   * @abstract
-   */
-  startQuery(queryContext, addCallback) {
-    throw new Error("Trying to access the base class, must be overridden");
-  }
-  /**
-   * Cancels a running query,
-   * @param {UrlbarQueryContext} queryContext The query context object to cancel
-   *        query for.
-   * @abstract
-   */
-  cancelQuery(queryContext) {
-    throw new Error("Trying to access the base class, must be overridden");
-  }
-}
 
 UrlbarMuxer
 -----------
 
 The *Muxer* is responsible for sorting results based on their importance and
 additional rules that depend on the UrlbarQueryContext. The muxer to use is
 indicated by the UrlbarQueryContext.muxer property.
 
 .. caution::
 
   The Muxer is a replaceable component, as such what is described here is a
   reference for the default View, but may not be valid for other implementations.
 
 .. highlight:: JavaScript
 .. code::
 
-class UrlbarMuxer {
-  /**
-   * Unique name for the muxer, used by the context to sort results.
-   * Not using a unique name will cause the newest registration to win.
-   * @abstract
-   */
-  get name() {
-    return "UrlbarMuxerBase";
+  class UrlbarMuxer {
+    /**
+    * Unique name for the muxer, used by the context to sort results.
+    * Not using a unique name will cause the newest registration to win.
+    * @abstract
+    */
+    get name() {
+      return "UrlbarMuxerBase";
+    }
+    /**
+    * Sorts UrlbarQueryContext results in-place.
+    * @param {UrlbarQueryContext} queryContext the context to sort results for.
+    * @abstract
+    */
+    sort(queryContext) {
+      throw new Error("Trying to access the base class, must be overridden");
+    }
   }
-  /**
-   * Sorts UrlbarQueryContext results in-place.
-   * @param {UrlbarQueryContext} queryContext the context to sort results for.
-   * @abstract
-   */
-  sort(queryContext) {
-    throw new Error("Trying to access the base class, must be overridden");
-  }
-}
 
 
 The Controller
 ==============
 
 `UrlbarController <https://dxr.mozilla.org/mozilla-central/source/browser/components/urlbar/UrlbarController.jsm>`_
 is the component responsible for reacting to user's input, by communicating
 proper course of action to the Model (e.g. starting/stopping a query) and the
@@ -340,17 +340,18 @@ Represents the base *View* implementatio
     open();
     close();
     // Invoked when the query starts.
     onQueryStarted(queryContext);
     // Invoked when new results are available.
     onQueryResults(queryContext);
     // Invoked when the query has been canceled.
     onQueryCancelled(queryContext);
-    // Invoked when the query is done.
+    // Invoked when the query is done. This is invoked in any case, even if the
+    // query was canceled earlier.
     onQueryFinished(queryContext);
     // Invoked when the view context changed, so that cached information about
     // the latest search is no more relevant and can be dropped.
     onViewContextChanged();
   }
 
 
 UrlbarResult