Bug 1502385 - Filter matches and providers in the Quantum Bar manager. r=adw

Differential Revision: https://phabricator.services.mozilla.com/D10348

--HG--
extra : moz-landing-system : lando

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

@@ -61,7 +61,7 @@ add_task(taskWithNewTab(async function test_disabled_ac() {
   let suggestBookmarks = Preferences.get("browser.urlbar.suggest.bookmark");
   Preferences.set("browser.urlbar.suggest.bookmark", false);
   let suggestOpenPages = Preferences.get("browser.urlbar.suggest.openpage");
-  Preferences.set("browser.urlbar.suggest.openpages", false);
+  Preferences.set("browser.urlbar.suggest.openpage", false);
 
   Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
                                        "http://example.com/?q={searchTerms}");

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

@@ -33,7 +33,9 @@ add_task(function losslessDecode() {
   let urlNoScheme = "example.com/\u30a2\u30a4\u30a6\u30a8\u30aa";
   let url = "http://" + urlNoScheme;
   if (Services.prefs.getBoolPref("browser.urlbar.quantumbar", true)) {
-    const result = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH, {url});
+    const result = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                                   UrlbarUtils.MATCH_SOURCE.TABS,
+                                   { url });
     gURLBar.setValueFromResult(result);
   } else {
     gURLBar.textValue = url;

browser/components/urlbar/UrlbarController.jsm

@@ -79,6 +79,7 @@ class QueryContext {
  * - onQueryStarted(queryContext)
  * - onQueryResults(queryContext)
  * - onQueryCancelled(queryContext)
+ * - onQueryFinished(queryContext)
  */
 class UrlbarController {
   /**
@@ -119,6 +120,8 @@ class UrlbarController {
     this._notify("onQueryStarted", queryContext);
 
     await this.manager.startQuery(queryContext, this);
+
+    this._notify("onQueryFinished", queryContext);
   }
 
   /**

browser/components/urlbar/UrlbarMatch.jsm

@@ -25,13 +25,32 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 class UrlbarMatch {
   /**
    * Creates a match.
-   * @param {integer} matchType one of UrlbarUtils.MATCHTYPE.* values
+   * @param {integer} matchType one of UrlbarUtils.MATCH_TYPE.* values
+   * @param {integer} matchSource one of UrlbarUtils.MATCH_SOURCE.* values
    * @param {object} payload data for this match. A payload should always
    *        contain a way to extract a final url to visit. The url getter
    *        should have a case for each of the types.
    */
-  constructor(matchType, payload) {
+  constructor(matchType, matchSource, payload) {
+    // Type describes the payload and visualization that should be used for
+    // this match.
+    if (!Object.values(UrlbarUtils.MATCH_TYPE).includes(matchType)) {
+      throw new Error("Invalid match type");
+    }
     this.type = matchType;
+
+    // Source describes which data has been used to derive this match. In case
+    // multiple sources are involved, use the more privacy restricted.
+    if (!Object.values(UrlbarUtils.MATCH_SOURCE).includes(matchSource)) {
+      throw new Error("Invalid match source");
+    }
+    this.source = matchSource;
+
+    // The payload contains match data. Some of the data is common across
+    // multiple types, but most of it will vary.
+    if (!payload || (typeof payload != "object") || !payload.url) {
+      throw new Error("Invalid match payload");
+    }
     this.payload = payload;
   }
 

browser/components/urlbar/UrlbarProviderOpenTabs.jsm

@@ -92,6 +92,12 @@ class ProviderOpenTabs {
     return UrlbarUtils.PROVIDER_TYPE.PROFILE;
   }
 
+  get sources() {
+    return [
+      UrlbarUtils.MATCH_SOURCE.TABS,
+    ];
+  }
+
   /**
    * Registers a tab as open.
    * @param {string} url Address of the tab
@@ -149,7 +155,8 @@ class ProviderOpenTabs {
         cancel();
         return;
       }
-      addCallback(this, new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH, {
+      addCallback(this, new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                                        UrlbarUtils.MATCH_SOURCE.TABS, {
         url: row.getResultByName("url"),
         userContextId: row.getResultByName("userContextId"),
       }));

browser/components/urlbar/UrlbarProvidersManager.jsm

@@ -156,6 +156,11 @@ class Query {
     this.started = false;
     this.canceled = false;
     this.complete = false;
+    // Array of acceptable MATCH_SOURCE values for this query. Providers not
+    // returning any of these will be skipped, as well as matches not part of
+    // this subset (Note we still expect the provider to do its own internal
+    // filtering, our additional filtering will be for sanity).
+    this.acceptableSources = [];
   }
 
   /**
@@ -167,13 +172,17 @@ class Query {
     }
     this.started = true;
     UrlbarTokenizer.tokenize(this.context);
+    this.acceptableSources = getAcceptableMatchSources(this.context);
+    logger.debug(`Acceptable sources ${this.acceptableSources}`);
 
     let promises = [];
     for (let provider of this.providers.get(UrlbarUtils.PROVIDER_TYPE.IMMEDIATE).values()) {
       if (this.canceled) {
         break;
       }
-      promises.push(provider.startQuery(this.context, this.add));
+      if (this._providerHasAcceptableSources(provider)) {
+        promises.push(provider.startQuery(this.context, this.add));
+      }
     }
 
     // Tracks the delay timer. We will fire (in this specific case, cancel would
@@ -189,15 +198,20 @@ class Query {
         if (this.canceled) {
           break;
         }
-        promises.push(provider.startQuery(this.context, this.add.bind(this)));
+        if (this._providerHasAcceptableSources(provider)) {
+          promises.push(provider.startQuery(this.context, this.add.bind(this)));
+        }
       }
     }
 
-    await Promise.all(promises.map(p => p.catch(Cu.reportError)));
+    logger.info(`Queried ${promises.length} providers`);
+    if (promises.length) {
+      await Promise.all(promises.map(p => p.catch(Cu.reportError)));
 
-    if (this._chunkTimer) {
-      // All the providers are done returning results, so we can stop chunking.
-      await this._chunkTimer.fire();
+      if (this._chunkTimer) {
+        // All the providers are done returning results, so we can stop chunking.
+        await this._chunkTimer.fire();
+      }
     }
 
     // Nothing should be failing above, since we catch all the promises, thus
@@ -234,11 +248,11 @@ class Query {
    */
   add(provider, match) {
     // Stop returning results as soon as we've been canceled.
-    if (this.canceled) {
+    if (this.canceled || !this.acceptableSources.includes(match.source)) {
       return;
     }
-    this.context.results.push(match);
 
+    this.context.results.push(match);
 
     let notifyResults = () => {
       if (this._chunkTimer) {
@@ -258,6 +272,15 @@ class Query {
       this._chunkTimer = new SkippableTimer(notifyResults, CHUNK_MATCHES_DELAY_MS);
     }
   }
+
+  /**
+   * Returns whether a provider's sources are acceptable for this query.
+   * @param {object} provider A provider object.
+   * @returns {boolean}whether the provider sources are acceptable.
+   */
+  _providerHasAcceptableSources(provider) {
+    return provider.sources.some(s => this.acceptableSources.includes(s));
+  }
 }
 
 /**
@@ -317,3 +340,63 @@ class SkippableTimer {
     return this.fire();
   }
 }
+
+/**
+ * Gets an array of the provider sources accepted for a given QueryContext.
+ * @param {object} context The QueryContext to examine
+ * @returns {array} Array of accepted sources
+ */
+function getAcceptableMatchSources(context) {
+  let acceptedSources = [];
+  // There can be only one restrict token about sources.
+  let restrictToken = context.tokens.find(t => [ UrlbarTokenizer.TYPE.RESTRICT_HISTORY,
+                                                 UrlbarTokenizer.TYPE.RESTRICT_BOOKMARK,
+                                                 UrlbarTokenizer.TYPE.RESTRICT_TAG,
+                                                 UrlbarTokenizer.TYPE.RESTRICT_OPENPAGE,
+                                                 UrlbarTokenizer.TYPE.RESTRICT_SEARCH,
+                                               ].includes(t.type));
+  let restrictTokenType = restrictToken ? restrictToken.type : undefined;
+  for (let source of Object.values(UrlbarUtils.MATCH_SOURCE)) {
+    switch (source) {
+      case UrlbarUtils.MATCH_SOURCE.BOOKMARKS:
+        if (UrlbarPrefs.get("suggest.bookmark") &&
+            (!restrictTokenType ||
+             restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_BOOKMARK ||
+             restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_TAG)) {
+          acceptedSources.push(source);
+        }
+        break;
+      case UrlbarUtils.MATCH_SOURCE.HISTORY:
+        if (UrlbarPrefs.get("suggest.history") &&
+            (!restrictTokenType ||
+             restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_HISTORY)) {
+          acceptedSources.push(source);
+        }
+        break;
+      case UrlbarUtils.MATCH_SOURCE.SEARCHENGINE:
+        if (UrlbarPrefs.get("suggest.searches") &&
+            (!restrictTokenType ||
+             restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_SEARCH)) {
+          acceptedSources.push(source);
+        }
+        break;
+      case UrlbarUtils.MATCH_SOURCE.TABS:
+        if (UrlbarPrefs.get("suggest.openpage") &&
+            (!restrictTokenType ||
+             restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_OPENPAGE)) {
+          acceptedSources.push(source);
+        }
+        break;
+      case UrlbarUtils.MATCH_SOURCE.OTHER_NETWORK:
+        if (!context.isPrivate) {
+          acceptedSources.push(source);
+        }
+        break;
+      case UrlbarUtils.MATCH_SOURCE.OTHER_LOCAL:
+      default:
+        acceptedSources.push(source);
+        break;
+    }
+  }
+  return acceptedSources;
+}

browser/components/urlbar/UrlbarUtils.jsm

@@ -67,6 +67,19 @@ var UrlbarUtils = {
     TAB_SWITCH: 1,
   },
 
+  // This defines the source of matches returned by a provider. Each provider
+  // can return matches from more than one source. This is used by the
+  // ProvidersManager to decide which providers must be queried and which
+  // matches can be returned.
+  MATCH_SOURCE: {
+    BOOKMARKS: 1,
+    HISTORY: 2,
+    SEARCHENGINE: 3,
+    TABS: 4,
+    OTHER_LOCAL: 5,
+    OTHER_NETWORK: 6,
+  },
+
   /**
    * Adds a url to history as long as it isn't in a private browsing window,
    * and it is valid.

browser/components/urlbar/UrlbarView.jsm

@@ -83,6 +83,14 @@ class UrlbarView {
     this._rows.textContent = "";
   }
 
+  onQueryCancelled(queryContext) {
+    // Nothing.
+  }
+
+  onQueryFinished(queryContext) {
+    // Nothing.
+  }
+
   onQueryResults(queryContext) {
     // XXX For now, clear the results for each set received. We should really
     // be updating the existing list.

browser/components/urlbar/tests/browser/browser_UrlbarController_resultOpening.js

@@ -57,7 +57,9 @@ add_task(function test_resultSelected_switchtab() {
 
   const event = new MouseEvent("click", {button: 0});
   const url = "https://example.com/1";
-  const result = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH, {url});
+  const result = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                                 UrlbarUtils.MATCH_SOURCE.TABS,
+                                 { url });
 
   Assert.equal(gURLBar.value, "", "urlbar input is empty before selecting a result");
   if (Services.prefs.getBoolPref("browser.urlbar.quantumbar", true)) {

browser/components/urlbar/tests/unit/head.js

@@ -54,17 +54,22 @@ function createContext(searchString = "foo") {
  *
  * @param {UrlbarController} controller The controller to wait for a response from.
  * @param {string} notification The name of the notification to wait for.
+ * @param {boolean} expected Wether the notification is expected.
  * @returns {Promise} A promise that is resolved with the arguments supplied to
  *   the notification.
  */
-function promiseControllerNotification(controller, notification) {
-  return new Promise(resolve => {
+function promiseControllerNotification(controller, notification, expected = true) {
+  return new Promise((resolve, reject) => {
     let proxifiedObserver = new Proxy({}, {
       get: (target, name) => {
         if (name == notification) {
           return (...args) => {
             controller.removeQueryListener(proxifiedObserver);
-            resolve(args);
+            if (expected) {
+              resolve(args);
+            } else {
+              reject();
+            }
           };
         }
         return () => false;
@@ -74,7 +79,6 @@ function promiseControllerNotification(controller, notification) {
   });
 }
 
-
 /**
  * Helper function to clear the existing providers and register a basic provider
  * that returns only the results given.
@@ -101,6 +105,9 @@ function registerBasicTestProvider(results, cancelCallback) {
     get type() {
       return UrlbarUtils.PROVIDER_TYPE.PROFILE;
     },
+    get sources() {
+      return results.map(r => r.source);
+    },
     async startQuery(context, add) {
       Assert.ok(context, "context is passed-in");
       Assert.equal(typeof add, "function", "add is a callback");

browser/components/urlbar/tests/unit/test_UrlbarController_integration.js

@@ -10,7 +10,9 @@
 ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm");
 
 const TEST_URL = "http://example.com";
-const match = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH, { url: TEST_URL });
+const match = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                              UrlbarUtils.MATCH_SOURCE.TABS,
+                              { url: TEST_URL });
 let controller;
 
 /**

browser/components/urlbar/tests/unit/test_providersManager.js

@@ -4,7 +4,9 @@
 "use strict";
 
 add_task(async function test_providers() {
-  let match = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH, { url: "http://mozilla.org/foo/" });
+  let match = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                              UrlbarUtils.MATCH_SOURCE.TABS,
+                              { url: "http://mozilla.org/foo/" });
   registerBasicTestProvider([match]);
 
   let context = createContext();

browser/components/urlbar/tests/unit/test_providersManager_filtering.js

@@ -0,0 +1,59 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function test_providers() {
+  let match = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                              UrlbarUtils.MATCH_SOURCE.TABS,
+                              { url: "http://mozilla.org/foo/" });
+  registerBasicTestProvider([match]);
+
+  let context = createContext();
+  let controller = new UrlbarController({
+    browserWindow: {
+      location: {
+        href: AppConstants.BROWSER_CHROME_URL,
+      },
+    },
+  });
+
+  info("Disable the only available source, should get no matches");
+  Services.prefs.setBoolPref("browser.urlbar.suggest.openpage", false);
+  let promise = Promise.race([
+    promiseControllerNotification(controller, "onQueryResults", false),
+    promiseControllerNotification(controller, "onQueryFinished"),
+  ]);
+  await controller.startQuery(context);
+  await promise;
+  Services.prefs.clearUserPref("browser.urlbar.suggest.openpage");
+
+  let matches = [
+    match,
+    new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
+                    UrlbarUtils.MATCH_SOURCE.HISTORY,
+                    { url: "http://mozilla.org/foo/" }),
+  ];
+  registerBasicTestProvider(matches);
+
+  info("Disable one of the sources, should get a single match");
+  Services.prefs.setBoolPref("browser.urlbar.suggest.history", false);
+  promise = Promise.all([
+    promiseControllerNotification(controller, "onQueryResults"),
+    promiseControllerNotification(controller, "onQueryFinished"),
+  ]);
+  await controller.startQuery(context, controller);
+  await promise;
+  Assert.deepEqual(context.results, [match]);
+  Services.prefs.clearUserPref("browser.urlbar.suggest.history");
+
+  info("Use a restriction character, should get a single match");
+  context = createContext(`foo ${UrlbarTokenizer.RESTRICT.OPENPAGE}`);
+  promise = Promise.all([
+    promiseControllerNotification(controller, "onQueryResults"),
+    promiseControllerNotification(controller, "onQueryFinished"),
+  ]);
+  await controller.startQuery(context, controller);
+  await promise;
+  Assert.deepEqual(context.results, [match]);
+});

browser/components/urlbar/tests/unit/xpcshell.ini

@@ -4,6 +4,7 @@ firefox-appdir = browser
 
 [test_providerOpenTabs.js]
 [test_providersManager.js]
+[test_providersManager_filtering.js]
 [test_QueryContext.js]
 [test_tokenizer.js]
 [test_UrlbarController_unit.js]

browser/docs/AddressBar.rst

@@ -68,6 +68,7 @@ It is augmented as it progresses through the system, with various information:
     // Properties added by the Model.
     tokens; // {array} tokens extracted from the searchString, each token is an
             // object in the form {type, value}.
+    results; // {array} list of UrlbarMatch objects.
   }
 
 
@@ -143,6 +144,8 @@ implementation details may vary deeply among different providers.
   UrlbarProvider {
     name; // {string} A simple name to track the provider.
     type; // {integer} One of UrlbarUtils.PROVIDER_TYPE.
+    sources; // {array} List of UrlbarUtils.MATCH_SOURCE, representing the
+             // data sources used by this provider.
     // The returned promise should be resolved when the provider is done
     // searching AND returning matches.
     // Each new UrlbarMatch should be passed to the AddCallback function.
@@ -272,6 +275,10 @@ Represents the base *View* implementation, communicates with the *Controller*.
     onQueryStarted(queryContext);
     // Invoked when new matches are available.
     onQueryResults(queryContext);
+    // Invoked when the query has been canceled.
+    onQueryCancelled(queryContext);
+    // Invoked when the query is done.
+    onQueryFinished(queryContext);
   }
 
 UrlbarMatch
@@ -294,6 +301,8 @@ properties, supported by all of the matches.
     constructor(matchType, payload);
 
     // Common properties:
+    type: {integer} One of UrlbarUtils.MATCH_TYPE.
+    source: {integer} One of UrlbarUtils.MATCH_SOURCE.
     url: {string} The url pointed by this match.
     title: {string} A title that may be used as a label for this match.
   }