Backed out changeset c516b5d716f1 (bug 1608597) for browser chrome failures on browser_search_tips.

browser/components/extensions/test/xpcshell/test_ext_urlbar.js

@@ -135,7 +135,7 @@ add_task(async function test_registerProvider() {
   );
 
   // Run a query, this should execute the above listeners and checks, plus it
-  // will set the provider's isActive and priority.
+  // will set the provider's isActive and isRestricting.
   let queryContext = new UrlbarQueryContext({
     allowAutofill: false,
     isPrivate: false,
@@ -158,19 +158,11 @@ add_task(async function test_registerProvider() {
       provider.isActive(queryContext),
       "Check active callback"
     );
-    if (restricting) {
-      Assert.notEqual(
-        provider.getPriority(queryContext),
-        0,
-        "Check provider priority"
-      );
-    } else {
-      Assert.equal(
-        provider.getPriority(queryContext),
-        0,
-        "Check provider priority"
-      );
-    }
+    Assert.equal(
+      restricting,
+      provider.isRestricting(queryContext),
+      "Check restrict callback"
+    );
   }
 
   await ext.unload();
@@ -265,9 +257,9 @@ add_task(async function test_onProviderResultsRequested() {
   let controller = UrlbarTestUtils.newMockController();
   await controller.startQuery(context);
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // Check the results.
   let expectedResults = [
@@ -594,11 +586,11 @@ add_task(async function test_activeAndInactiveProviders() {
   let controller = UrlbarTestUtils.newMockController();
   await controller.startQuery(context);
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(active.isActive(context));
   Assert.ok(!inactive.isActive(context));
-  Assert.equal(active.getPriority(context), 0);
-  Assert.equal(inactive.getPriority(context), 0);
+  Assert.ok(!active.isRestricting(context));
+  Assert.ok(!inactive.isRestricting(context));
 
   // Check the results.
   Assert.equal(context.results.length, 2);
@@ -658,10 +650,10 @@ add_task(async function test_threeActiveProviders() {
   let controller = UrlbarTestUtils.newMockController();
   await controller.startQuery(context);
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   for (let provider of providers) {
     Assert.ok(provider.isActive(context));
-    Assert.equal(provider.getPriority(context), 0);
+    Assert.ok(!provider.isRestricting(context));
   }
 
   // Check the results.
@@ -716,10 +708,10 @@ add_task(async function test_threeInactiveProviders() {
   let controller = UrlbarTestUtils.newMockController();
   await controller.startQuery(context);
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   for (let provider of providers) {
     Assert.ok(!provider.isActive(context));
-    Assert.equal(provider.getPriority(context), 0);
+    Assert.ok(!provider.isRestricting(context));
   }
 
   // Check the results.
@@ -785,11 +777,11 @@ add_task(async function test_activeInactiveAndRestrictingProviders() {
 
   // Check isActive and isRestricting.
   Assert.ok(providers.active.isActive(context));
-  Assert.equal(providers.active.getPriority(context), 0);
+  Assert.ok(!providers.active.isRestricting(context));
   Assert.ok(!providers.inactive.isActive(context));
-  Assert.equal(providers.inactive.getPriority(context), 0);
+  Assert.ok(!providers.inactive.isRestricting(context));
   Assert.ok(providers.restricting.isActive(context));
-  Assert.notEqual(providers.restricting.getPriority(context), 0);
+  Assert.ok(providers.restricting.isRestricting(context));
 
   // Check the results.
   Assert.equal(context.results.length, 1);
@@ -942,7 +934,7 @@ add_task(async function test_onResultsRequestedNotImplemented() {
 
   // Check isActive and isRestricting.
   Assert.ok(provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // Check the results.
   Assert.equal(context.results.length, 1);
@@ -1090,9 +1082,9 @@ add_task(async function test_onBehaviorRequestedTimeout() {
   await controller.startQuery(context);
   UrlbarProviderExtension.notificationTimeout = currentTimeout;
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(!provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // Check the results.
   Assert.equal(context.results.length, 1);
@@ -1155,9 +1147,9 @@ add_task(async function test_onResultsRequestedTimeout() {
   await controller.startQuery(context);
   UrlbarProviderExtension.notificationTimeout = currentTimeout;
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // Check the results.
   Assert.equal(context.results.length, 1);
@@ -1194,7 +1186,7 @@ add_task(async function test_privateBrowsing_not_allowed() {
   await ext.startup();
 
   // Run a query, this should execute the above listeners and checks, plus it
-  // will set the provider's isActive and priority.
+  // will set the provider's isActive and isRestricting.
   let queryContext = new UrlbarQueryContext({
     allowAutofill: false,
     isPrivate: true,
@@ -1250,9 +1242,9 @@ add_task(async function test_privateBrowsing_not_allowed_onQueryCanceled() {
   controller.cancelQuery();
   await startPromise;
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(!provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   await ext.unload();
 });
@@ -1297,9 +1289,9 @@ add_task(async function test_privateBrowsing_allowed() {
   let controller = UrlbarTestUtils.newMockController();
   await controller.startQuery(context);
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // The events should have been fired.
   await Promise.all(
@@ -1352,9 +1344,9 @@ add_task(async function test_privateBrowsing_allowed_onQueryCanceled() {
   controller.cancelQuery();
   await startPromise;
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // The events should have been fired.
   await Promise.all(
@@ -1409,9 +1401,9 @@ add_task(async function test_nonPrivateBrowsing() {
   let controller = UrlbarTestUtils.newMockController();
   await controller.startQuery(context);
 
-  // Check isActive and priority.
+  // Check isActive and isRestricting.
   Assert.ok(provider.isActive(context));
-  Assert.equal(provider.getPriority(context), 0);
+  Assert.ok(!provider.isRestricting(context));
 
   // Check the results.
   Assert.equal(context.results.length, 2);

browser/components/urlbar/UrlbarProviderExtension.jsm

@@ -108,18 +108,17 @@ class UrlbarProviderExtension extends UrlbarProvider {
   }
 
   /**
-   * Gets the provider's priority.
+   * Whether this provider wants to restrict results to just itself.  Other
+   * providers won't be invoked, unless this provider doesn't support the
+   * current query.
    *
    * @param {UrlbarQueryContext} context
    *   The query context object.
-   * @returns {number}
-   *   The provider's priority for the given query.
+   * @returns {boolean}
+   *   Whether this provider wants to restrict results.
    */
-  getPriority(context) {
-    // We give restricting extension providers a very high priority so that they
-    // normally override all built-in providers, but not Infinity so that we can
-    // still override them if necessary.
-    return this.behavior == "restricting" ? 999 : 0;
+  isRestricting(context) {
+    return this.behavior == "restricting";
   }
 
   /**

browser/components/urlbar/UrlbarProviderOpenTabs.jsm

@@ -110,6 +110,17 @@ class ProviderOpenTabs extends UrlbarProvider {
     return false;
   }
 
+  /**
+   * Whether this provider wants to restrict results to just itself.
+   * Other providers won't be invoked, unless this provider doesn't
+   * support the current query.
+   * @param {UrlbarQueryContext} queryContext The query context object
+   * @returns {boolean} Whether this provider wants to restrict results.
+   */
+  isRestricting(queryContext) {
+    return false;
+  }
+
   /**
    * Registers a tab as open.
    * @param {string} url Address of the tab

browser/components/urlbar/UrlbarProviderPrivateSearch.jsm

@@ -84,6 +84,17 @@ class ProviderPrivateSearch extends UrlbarProvider {
     );
   }
 
+  /**
+   * Whether this provider wants to restrict results to just itself.
+   * Other providers won't be invoked, unless this provider doesn't
+   * support the current query.
+   * @param {UrlbarQueryContext} queryContext The query context object
+   * @returns {boolean} Whether this provider wants to restrict results.
+   */
+  isRestricting(queryContext) {
+    return false;
+  }
+
   /**
    * Starts querying.
    * @param {object} queryContext The query context object

browser/components/urlbar/UrlbarProviderTopSites.jsm

@@ -71,13 +71,14 @@ class ProviderTopSites extends UrlbarProvider {
   }
 
   /**
-   * Gets the provider's priority.
+   * Whether this provider wants to restrict results to just itself.
+   * Other providers won't be invoked, unless this provider doesn't
+   * support the current query.
    * @param {UrlbarQueryContext} queryContext The query context object
-   * @returns {number} The provider's priority for the given query.
+   * @returns {boolean} Whether this provider wants to restrict results.
    */
-  getPriority(queryContext) {
-    // Top sites are prioritized over other built-in providers.
-    return 1;
+  isRestricting(queryContext) {
+    return true;
   }
 
   /**

browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm

@@ -72,6 +72,17 @@ class ProviderUnifiedComplete extends UrlbarProvider {
     return true;
   }
 
+  /**
+   * Whether this provider wants to restrict results to just itself.
+   * Other providers won't be invoked, unless this provider doesn't
+   * support the current query.
+   * @param {UrlbarQueryContext} queryContext The query context object
+   * @returns {boolean} Whether this provider wants to restrict results.
+   */
+  isRestricting(queryContext) {
+    return false;
+  }
+
   /**
    * Starts querying.
    * @param {object} queryContext The query context object

browser/components/urlbar/UrlbarProvidersManager.jsm

@@ -287,23 +287,14 @@ class Query {
     this.started = true;
 
     // Check which providers should be queried.
-    let providers = [];
-    let maxPriority = -1;
-    for (let provider of this.providers) {
-      if (provider.isActive(this.context)) {
-        let priority = provider.getPriority(this.context);
-        if (priority >= maxPriority) {
-          // The provider's priority is at least as high as the max.
-          if (priority > maxPriority) {
-            // The provider's priority is higher than the max.  Remove all
-            // previously added providers, since their priority is necessarily
-            // lower, by setting length to zero.
-            providers.length = 0;
-            maxPriority = priority;
-          }
-          providers.push(provider);
-        }
-      }
+    let providers = this.providers.filter(p => p.isActive(this.context));
+
+    // Check if any of the remaining providers wants to restrict the search.
+    let restrictProviders = providers.filter(p =>
+      p.isRestricting(this.context)
+    );
+    if (restrictProviders.length) {
+      providers = restrictProviders;
     }
 
     // Start querying providers.

browser/components/urlbar/UrlbarUtils.jsm

@@ -661,17 +661,15 @@ class UrlbarProvider {
   }
 
   /**
-   * Gets the provider's priority.  Priorities are numeric values starting at
-   * zero and increasing in value.  Smaller values are lower priorities, and
-   * larger values are higher priorities.  For a given query, `startQuery` is
-   * called on only the active and highest-priority providers.
+   * Whether this provider wants to restrict results to just itself.
+   * Other providers won't be invoked, unless this provider doesn't
+   * support the current query.
    * @param {UrlbarQueryContext} queryContext The query context object
-   * @returns {number} The provider's priority for the given query.
+   * @returns {boolean} Whether this provider wants to restrict results.
    * @abstract
    */
-  getPriority(queryContext) {
-    // By default, all providers share the lowest priority.
-    return 0;
+  isRestricting(queryContext) {
+    throw new Error("Trying to access the base class, must be overridden");
   }
 
   /**

browser/components/urlbar/docs/overview.rst

@@ -158,17 +158,15 @@ implementation details may vary deeply among different providers.
       throw new Error("Trying to access the base class, must be overridden");
     }
     /**
-     * Gets the provider's priority.  Priorities are numeric values starting at
-     * zero and increasing in value.  Smaller values are lower priorities, and
-     * larger values are higher priorities.  For a given query, `startQuery` is
-     * called on only the active and highest-priority providers.
+     * Whether this provider wants to restrict results to just itself.
+     * Other providers won't be invoked, unless this provider doesn't
+     * support the current query.
      * @param {UrlbarQueryContext} queryContext The query context object
-     * @returns {number} The provider's priority for the given query.
+     * @returns {boolean} Whether this provider wants to restrict results.
      * @abstract
      */
-    getPriority(queryContext) {
-      // By default, all providers share the lowest priority.
-      return 0;
+    isRestricting(queryContext) {
+      throw new Error("Trying to access the base class, must be overridden");
     }
     /**
      * Starts querying.

browser/components/urlbar/tests/browser/browser_tip_selection.js

@@ -26,8 +26,8 @@ class TipTestProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
-  getPriority(context) {
-    return 1;
+  isRestricting(context) {
+    return true;
   }
   async startQuery(context, addCallback) {
     Assert.ok(true, "Tip provider was invoked");

browser/components/urlbar/tests/browser/browser_updateRows.js

@@ -255,6 +255,9 @@ class TestProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
+  isRestricting(context) {
+    return false;
+  }
   async startQuery(context, addCallback) {
     for (const result of this._results) {
       addCallback(this, result);

browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js

@@ -1071,8 +1071,8 @@ class TipTestProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
-  getPriority(context) {
-    return 1;
+  isRestricting(context) {
+    return true;
   }
   async startQuery(context, addCallback) {
     this._context = context;

browser/components/urlbar/tests/browser/browser_urlbar_resultSpan.js

@@ -254,8 +254,8 @@ class TestProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
-  getPriority(context) {
-    return this._isRestricting ? 1 : 0;
+  isRestricting(context) {
+    return this._isRestricting;
   }
   async startQuery(context, addCallback) {
     Assert.ok(true, "Tip provider was invoked");

browser/components/urlbar/tests/browser/browser_urlbar_suggestedIndex.js

@@ -128,6 +128,9 @@ class TestProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
+  isRestricting(context) {
+    return false;
+  }
   async startQuery(context, addCallback) {
     this._context = context;
     for (const match of this._matches) {

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

@@ -118,9 +118,9 @@ class TestProvider extends UrlbarProvider {
     Assert.ok(context, "context is passed-in");
     return true;
   }
-  getPriority(context) {
+  isRestricting(context) {
     Assert.ok(context, "context is passed-in");
-    return 0;
+    return false;
   }
   async startQuery(context, add) {
     Assert.ok(context, "context is passed-in");

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

@@ -38,6 +38,9 @@ class DelayedProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
+  isRestricting(context) {
+    return false;
+  }
   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_providersManager_filtering.js

@@ -162,6 +162,9 @@ add_task(async function test_filter_isActive() {
       info("Acceptable sources: " + context.sources);
       return context.sources.includes(UrlbarUtils.RESULT_SOURCE.BOOKMARKS);
     }
+    isRestricting(context) {
+      return false;
+    }
     async startQuery(context, add) {
       Assert.ok(false, "Provider should no be invoked");
       for (const match of badMatches) {
@@ -209,6 +212,9 @@ add_task(async function test_filter_queryContext() {
     isActive(context) {
       return true;
     }
+    isRestricting(context) {
+      return false;
+    }
     async startQuery(context, add) {
       Assert.ok(false, "Provider should no be invoked");
     }
@@ -311,6 +317,9 @@ add_task(async function test_nofilter_restrict() {
       Assert.equal(context.sources.length, 1, "Check acceptable sources");
       return true;
     }
+    isRestricting(context) {
+      return false;
+    }
     async startQuery(context, add) {
       Assert.ok(true, "expected provider was invoked");
       for (let match of matches) {
@@ -355,19 +364,13 @@ add_task(async function test_nofilter_restrict() {
   UrlbarProvidersManager.unregisterProvider(provider);
 });
 
-add_task(async function test_filter_priority() {
+add_task(async function test_filter_isRestricting() {
   /**
-   * A test provider.
+   * A test provider that should be invoked and is restricting.
    */
   class TestProvider extends UrlbarProvider {
-    constructor(priority, shouldBeInvoked, namePart = "") {
-      super();
-      this._priority = priority;
-      this._name = `Provider-${priority}` + namePart;
-      this._shouldBeInvoked = shouldBeInvoked;
-    }
     get name() {
-      return this._name;
+      return "GoodProvider";
     }
     get type() {
       return UrlbarUtils.PROVIDER_TYPE.PROFILE;
@@ -375,51 +378,47 @@ add_task(async function test_filter_priority() {
     isActive(context) {
       return true;
     }
-    getPriority(context) {
-      return this._priority;
+    isRestricting(context) {
+      return true;
     }
     async startQuery(context, add) {
-      Assert.ok(this._shouldBeInvoked, `${this.name} was invoked`);
+      Assert.ok(true, "expected provider was invoked");
     }
     cancelQuery(context) {}
     pickResult(result) {}
   }
+  UrlbarProvidersManager.registerProvider(new TestProvider());
 
-  // Test all possible orderings of the providers to make sure the logic that
-  // finds the highest priority providers is correct.
-  let providerPerms = permute([
-    new TestProvider(0, false),
-    new TestProvider(1, false),
-    new TestProvider(2, true, "a"),
-    new TestProvider(2, true, "b"),
-  ]);
-  for (let providers of providerPerms) {
-    for (let provider of providers) {
-      UrlbarProvidersManager.registerProvider(provider);
+  /**
+   * A test provider that should not be invoked because the other one is restricting.
+   */
+  class NoInvokeProvider extends UrlbarProvider {
+    get name() {
+      return "BadProvider";
     }
-    let providerNames = providers.map(p => p.name);
-    let context = createContext(undefined, { providers: providerNames });
-    let controller = UrlbarTestUtils.newMockController();
-    await controller.startQuery(context, controller);
-    for (let name of providerNames) {
-      UrlbarProvidersManager.unregisterProvider({ name });
+    get type() {
+      return UrlbarUtils.PROVIDER_TYPE.PROFILE;
     }
+    isActive(context) {
+      return true;
+    }
+    isRestricting(context) {
+      return false;
+    }
+    async startQuery(context, add) {
+      Assert.ok(false, "Provider should no be invoked");
+    }
+    cancelQuery(context) {}
+    pickResult(result) {}
   }
+  UrlbarProvidersManager.registerProvider(new NoInvokeProvider());
+
+  let context = createContext(undefined, {
+    providers: ["GoodProvider", "BadProvider"],
+  });
+  let controller = UrlbarTestUtils.newMockController();
+
+  await controller.startQuery(context, controller);
+  UrlbarProvidersManager.unregisterProvider({ name: "GoodProvider" });
+  UrlbarProvidersManager.unregisterProvider({ name: "BadProvider" });
 });
-
-function permute(objects) {
-  if (objects.length <= 1) {
-    return [objects];
-  }
-  let perms = [];
-  for (let i = 0; i < objects.length; i++) {
-    let otherObjects = objects.slice();
-    otherObjects.splice(i, 1);
-    let otherPerms = permute(otherObjects);
-    for (let perm of otherPerms) {
-      perm.unshift(objects[i]);
-    }
-    perms = perms.concat(otherPerms);
-  }
-  return perms;
-}

browser/modules/test/browser/browser_UsageTelemetry_urlbar_tip.js

@@ -129,8 +129,8 @@ class TipProvider extends UrlbarProvider {
   isActive(context) {
     return true;
   }
-  getPriority(context) {
-    return 1;
+  isRestricting(context) {
+    return true;
   }
   async startQuery(context, addCallback) {
     context.preselected = true;