Bug 1569812 - Quantumbar: Allow extensions to specify search engines by name, URL, or alias. r=mak

`UrlbarResult` objects for search results have a `payload.engine` property, the name of the engine, but extensions won't always know the name Firefox uses internally or even any name at all. For example, the top sites API returns search results with aliases but not names. We should let extensions specify engine URLs and aliases, and we can use them to look up the engine name.

The UrlbarResult.jsm change is unrelated but fixes a minor annoyance I found while working on the top-sites extension. My background script returns results with `isKeywordOffer: true`, but that causes an error in `UrlbarResult.payloadAndSimpleHighlights` because it converts string values to arrays and then expects non-string values to be arrays. Instead, it should be converting non-array values to arrays.

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

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

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

@@ -36,6 +36,7 @@ add_task(async function startup() {
   // engine from over the network.
   let engine = await Services.search.addEngineWithDetails("Test engine", {
     template: "http://example.com/?s=%S",
+    alias: "@testengine",
   });
   Services.search.defaultEngine = engine;
 
@@ -298,6 +299,186 @@ add_task(async function test_onProviderResultsRequested() {
   await ext.unload();
 });
 
+// Extensions can specify search engines using engine names, aliases, and URLs.
+add_task(async function test_onProviderResultsRequested_searchEngines() {
+  let ext = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["urlbar"],
+    },
+    isPrivileged: true,
+    incognitoOverride: "spanning",
+    background() {
+      browser.urlbar.onBehaviorRequested.addListener(query => {
+        return "restricting";
+      }, "test");
+      browser.urlbar.onResultsRequested.addListener(query => {
+        return [
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              engine: "Test engine",
+              suggestion: "engine specified",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              keyword: "@testengine",
+              suggestion: "keyword specified",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              url: "http://example.com/?s",
+              suggestion: "url specified",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              engine: "Test engine",
+              keyword: "@testengine",
+              url: "http://example.com/?s",
+              suggestion: "engine, keyword, and url specified",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              keyword: "@testengine",
+              url: "http://example.com/?s",
+              suggestion: "keyword and url specified",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              suggestion: "no engine",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              engine: "bogus",
+              suggestion: "no matching engine",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              keyword: "@bogus",
+              suggestion: "no matching keyword",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              url: "http://bogus-no-search-engine.com/",
+              suggestion: "no matching url",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              url: "bogus",
+              suggestion: "invalid url",
+            },
+          },
+          {
+            type: "search",
+            source: "search",
+            payload: {
+              url: "foo:bar",
+              suggestion: "url with no hostname",
+            },
+          },
+        ];
+      }, "test");
+    },
+  });
+  await ext.startup();
+
+  // Run a query.
+  let context = new UrlbarQueryContext({
+    allowAutofill: false,
+    isPrivate: false,
+    maxResults: 10,
+    searchString: "test",
+  });
+  let controller = new UrlbarController({
+    browserWindow: {
+      location: {
+        href: AppConstants.BROWSER_CHROME_URL,
+      },
+    },
+  });
+  await controller.startQuery(context);
+
+  // Check the results.  The first several are valid and should include "Test
+  // engine" as the engine.  The others don't specify an engine and are
+  // therefore invalid, so they should be ignored.
+  let expectedResults = [
+    {
+      type: UrlbarUtils.RESULT_TYPE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engine: "Test engine",
+      title: "engine specified",
+      heuristic: false,
+    },
+    {
+      type: UrlbarUtils.RESULT_TYPE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engine: "Test engine",
+      title: "keyword specified",
+      heuristic: false,
+    },
+    {
+      type: UrlbarUtils.RESULT_TYPE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engine: "Test engine",
+      title: "url specified",
+      heuristic: false,
+    },
+    {
+      type: UrlbarUtils.RESULT_TYPE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engine: "Test engine",
+      title: "engine, keyword, and url specified",
+      heuristic: false,
+    },
+    {
+      type: UrlbarUtils.RESULT_TYPE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engine: "Test engine",
+      title: "keyword and url specified",
+      heuristic: false,
+    },
+  ];
+
+  let actualResults = context.results.map(r => ({
+    type: r.type,
+    source: r.source,
+    engine: r.payload.engine || null,
+    title: r.title,
+    heuristic: r.heuristic,
+  }));
+
+  Assert.deepEqual(actualResults, expectedResults);
+
+  await ext.unload();
+});
+
 // Adds two providers, one active and one inactive.  Only the active provider
 // should be asked to return results.
 add_task(async function test_activeAndInactiveProviders() {

browser/components/urlbar/UrlbarProviderExtension.jsm

@@ -15,6 +15,9 @@ const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 XPCOMUtils.defineLazyModuleGetters(this, {
+  PlacesSearchAutocompleteProvider:
+    "resource://gre/modules/PlacesSearchAutocompleteProvider.jsm",
+  Services: "resource://gre/modules/Services.jsm",
   SkippableTimer: "resource:///modules/UrlbarUtils.jsm",
   UrlbarProvider: "resource:///modules/UrlbarUtils.jsm",
   UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm",
@@ -189,13 +192,12 @@ class UrlbarProviderExtension extends UrlbarProvider {
     let extResults = await this._notifyListener("resultsRequested", context);
     if (extResults) {
       for (let extResult of extResults) {
-        let result;
-        try {
-          result = this._makeUrlbarResult(context, extResult);
-        } catch (err) {
-          continue;
+        let result = await this._makeUrlbarResult(context, extResult).catch(
+          Cu.reportError
+        );
+        if (result) {
+          addCallback(this, result);
         }
-        addCallback(this, result);
       }
     }
   }
@@ -261,7 +263,41 @@ class UrlbarProviderExtension extends UrlbarProvider {
    * @returns {UrlbarResult}
    *   The UrlbarResult object.
    */
-  _makeUrlbarResult(context, extResult) {
+  async _makeUrlbarResult(context, extResult) {
+    // If the result is a search result, make sure its payload has a valid
+    // `engine` property, which is the name of an engine, and which we use later
+    // on to look up the nsISearchEngine.  We allow the extension to specify the
+    // engine by its name, alias, or domain.  Prefer aliases over domains since
+    // one domain can have many engines.
+    if (extResult.type == "search") {
+      let engine;
+      if (extResult.payload.engine) {
+        // Validate the engine name by looking it up.
+        engine = Services.search.getEngineByName(extResult.payload.engine);
+      } else if (extResult.payload.keyword) {
+        // Look up the engine by its alias.
+        engine = await PlacesSearchAutocompleteProvider.engineForAlias(
+          extResult.payload.keyword
+        );
+      } else if (extResult.payload.url) {
+        // Look up the engine by its domain.
+        let host;
+        try {
+          host = new URL(extResult.payload.url).hostname;
+        } catch (err) {}
+        if (host) {
+          engine = await PlacesSearchAutocompleteProvider.engineForDomainPrefix(
+            host
+          );
+        }
+      }
+      if (!engine) {
+        // No engine found.
+        throw new Error("Invalid or missing engine specified by extension");
+      }
+      extResult.payload.engine = engine.name;
+    }
+
     return new UrlbarResult(
       UrlbarProviderExtension.RESULT_TYPES[extResult.type],
       UrlbarProviderExtension.SOURCE_TYPES[extResult.source],

browser/components/urlbar/UrlbarResult.jsm

@@ -168,9 +168,9 @@ class UrlbarResult {
    * @returns {array} An array [payload, payloadHighlights].
    */
   static payloadAndSimpleHighlights(tokens, payloadInfo) {
-    // Convert string values in payloadInfo to [value, false] arrays.
+    // Convert scalar values in payloadInfo to [value] arrays.
     for (let [name, info] of Object.entries(payloadInfo)) {
-      if (typeof info == "string") {
+      if (!Array.isArray(info)) {
         payloadInfo[name] = [info];
       }
     }