Bug 1685748 - Migrate legacy extension search engines to use supported extension definitions. r=mak

These should generally have been updated previously, but the search service expected a same-name match. We have seen instances where these were different so we should migrate them.

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

browser/components/BrowserGlue.jsm

@@ -2762,7 +2762,7 @@ BrowserGlue.prototype = {
 
       () => OsEnvironment.reportAllowedAppSources(),
 
-      () => Services.search.checkWebExtensionEngines(),
+      () => Services.search.runBackgroundChecks(),
 
       () => BrowserUsageTelemetry.reportInstallationTelemetry(),
     ];

toolkit/components/search/SearchService.jsm

@@ -1312,12 +1312,63 @@ SearchService.prototype = {
     return this._initialized;
   },
 
+  /**
+   * Runs background checks for the search service. This is called from
+   * BrowserGlue and may be run once per session if the user is idle for
+   * long enough.
+   */
+  async runBackgroundChecks() {
+    await this.init();
+    await this._migrateLegacyEngines();
+    await this._checkWebExtensionEngines();
+  },
+
+  /**
+   * Migrates legacy add-ons which used the OpenSearch definitions to
+   * WebExtensions, if an equivalent WebExtension is installed.
+   *
+   * Run during the background checks.
+   */
+  async _migrateLegacyEngines() {
+    logConsole.debug("Running migrate legacy engines");
+
+    const matchRegExp = /extensions\/(.*?)\.xpi!/i;
+    for (let engine of this._engines.values()) {
+      if (
+        !engine.isAppProvided &&
+        !engine._extensionID &&
+        engine._loadPath.includes("[profile]/extensions/")
+      ) {
+        let match = engine._loadPath.match(matchRegExp);
+        if (match?.[1]) {
+          // There's a chance here that the WebExtension might not be
+          // installed any longer, even though the engine is. We'll deal
+          // with that in `checkWebExtensionEngines`.
+          let engines = await this.getEnginesByExtensionID(match[1]);
+          if (engines.length) {
+            logConsole.debug(
+              `Migrating ${engine.name} to WebExtension install`
+            );
+
+            if (this.defaultEngine == engine) {
+              this.defaultEngine = engines[0];
+            }
+            await this.removeEngine(engine);
+          }
+        }
+      }
+    }
+
+    logConsole.debug("Migrate legacy engines complete");
+  },
+
   /**
    * Checks if Search Engines associated with WebExtensions are valid and
    * up-to-date, and reports them via telemetry if not.
+   *
+   * Run during the background checks.
    */
-  async checkWebExtensionEngines() {
-    await this.init();
+  async _checkWebExtensionEngines() {
     logConsole.debug("Running check on WebExtension engines");
 
     for (let engine of this._engines.values()) {

toolkit/components/search/nsISearchService.idl

@@ -255,9 +255,9 @@ interface nsISearchService : nsISupports
   readonly attribute bool isInitialized;
 
   /**
-   * Checks if WebExtension Search Engines are valid and up-to-date.
+   * Runs background checks; Designed to be run on idle.
    */
-  Promise checkWebExtensionEngines();
+  Promise runBackgroundChecks();
 
   /**
    * Resets the default engine to its original value.

toolkit/components/search/tests/SearchTestUtils.jsm

@@ -183,7 +183,9 @@ var SearchTestUtils = Object.freeze({
 
     let extension = gTestScope.ExtensionTestUtils.loadExtension(extensionInfo);
     await extension.startup();
-    await AddonTestUtils.waitForSearchProviderStartup(extension);
+    if (!options.skipWaitForSearchEngine) {
+      await AddonTestUtils.waitForSearchProviderStartup(extension);
+    }
     return extension;
   },
 

toolkit/components/search/tests/xpcshell/data/search-migration.json

@@ -0,0 +1,68 @@
+{
+  "version": 1,
+  "buildID": "20121106",
+  "locale": "en-US",
+  "metaData": {},
+  "engines": [
+    {
+      "_name": "engine1",
+      "_metaData": {
+        "alias": "testAlias"
+      },
+      "_isAppProvided": true
+    },
+    {
+      "_name": "engine2",
+      "_metaData": {
+        "alias": null,
+        "hidden": true
+      },
+      "_isAppProvided": true
+    },
+    {
+      "_name": "simple",
+      "_loadPath": "jar:[profile]/extensions/simple@tests.mozilla.org.xpi!/simple.xml",
+      "_shortName": "simple",
+      "description": "A migration test engine",
+      "__searchForm": "http://www.example.com/",
+      "_metaData": {},
+      "_urls": [
+        {
+          "template": "http://www.example.com/search",
+          "rels": [],
+          "resultDomain": "google.com",
+          "params": [
+            {
+              "name": "q",
+              "value": "{searchTerms}"
+            }
+          ]
+        }
+      ],
+      "queryCharset": "UTF-8"
+    },
+    {
+      "_name": "simple search",
+      "_loadPath": "[other]addEngineWithDetails:simple@tests.mozilla.org",
+      "_shortName": "simple search",
+      "description": "A migration test engine",
+      "__searchForm": "http://www.example.com/",
+      "_metaData": {},
+      "_urls": [
+        {
+          "template": "http://www.example.com/search",
+          "rels": [],
+          "resultDomain": "google.com",
+          "params": [
+            {
+              "name": "q",
+              "value": "{searchTerms}"
+            }
+          ]
+        }
+      ],
+      "queryCharset": "UTF-8",
+      "_extensionID": "simple@tests.mozilla.org"
+    }
+  ]
+}

toolkit/components/search/tests/xpcshell/test_webextensions_migrate_to.js

@@ -0,0 +1,61 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Test migrating legacy add-on engines in background.
+ */
+
+"use strict";
+
+SearchTestUtils.initXPCShellAddonManager(this);
+
+const kExtensionID = "simple@tests.mozilla.org";
+let extension;
+
+add_task(async function setup() {
+  useHttpServer("opensearch");
+  await AddonTestUtils.promiseStartupManager();
+  await SearchTestUtils.useTestEngines("data1");
+
+  let data = await readJSONFile(do_get_file("data/search-migration.json"));
+
+  await promiseSaveSettingsData(data);
+
+  // Add the add-on so add-on manager has a valid item.
+  extension = await SearchTestUtils.installSearchExtension({
+    id: "simple",
+    name: "simple search",
+    search_url: "https://example.com/",
+    skipWaitForSearchEngine: true,
+  });
+});
+
+add_task(async function test_migrateLegacyEngineDifferentName() {
+  await Services.search.init();
+
+  let engine = Services.search.getEngineByName("simple");
+  Assert.ok(engine, "Should have the legacy add-on engine.");
+
+  // Set this engine as default, the new engine should become the default
+  // after migration.
+  await Services.search.setDefault(engine);
+
+  engine = Services.search.getEngineByName("simple search");
+  Assert.ok(engine, "Should have the WebExtension engine.");
+
+  await Services.search.runBackgroundChecks();
+
+  engine = Services.search.getEngineByName("simple");
+  Assert.ok(!engine, "Should have removed the legacy add-on engine");
+
+  engine = Services.search.getEngineByName("simple search");
+  Assert.ok(engine, "Should have kept the WebExtension engine.");
+
+  Assert.equal(
+    (await Services.search.getDefault()).name,
+    engine.name,
+    "Should have switched to the WebExtension engine as default."
+  );
+
+  await extension.unload();
+});

toolkit/components/search/tests/xpcshell/test_webextensions_valid.js

@@ -52,7 +52,7 @@ add_task(async function test_valid_extensions_do_nothing() {
     "Should have installed the engine"
   );
 
-  await Services.search.checkWebExtensionEngines();
+  await Services.search.runBackgroundChecks();
 
   let scalars = TelemetryTestUtils.getProcessScalars("parent", true, true);
 
@@ -71,7 +71,7 @@ add_task(async function test_different_url() {
     search_url_get_params: "?q={searchTerms}",
   });
 
-  await Services.search.checkWebExtensionEngines();
+  await Services.search.runBackgroundChecks();
 
   TelemetryTestUtils.assertKeyedScalar(
     TelemetryTestUtils.getProcessScalars("parent", true, true),
@@ -89,7 +89,7 @@ add_task(async function test_disabled_extension() {
   // stubbed removeEngine.
   await extension.addon.disable();
 
-  await Services.search.checkWebExtensionEngines();
+  await Services.search.runBackgroundChecks();
 
   TelemetryTestUtils.assertKeyedScalar(
     TelemetryTestUtils.getProcessScalars("parent", true, true),
@@ -111,7 +111,7 @@ add_task(async function test_missing_extension() {
   // stubbed removeEngine.
   await extension.unload();
 
-  await Services.search.checkWebExtensionEngines();
+  await Services.search.runBackgroundChecks();
 
   TelemetryTestUtils.assertKeyedScalar(
     TelemetryTestUtils.getProcessScalars("parent", true, true),
@@ -128,7 +128,7 @@ add_task(async function test_user_engine() {
 
   await Services.search.addUserEngine("test", "https://example.com/", "fake");
 
-  await Services.search.checkWebExtensionEngines();
+  await Services.search.runBackgroundChecks();
   let scalars = TelemetryTestUtils.getProcessScalars("parent", true, true);
 
   Assert.deepEqual(
@@ -151,7 +151,7 @@ add_task(async function test_policy_engine() {
     },
   });
 
-  await Services.search.checkWebExtensionEngines();
+  await Services.search.runBackgroundChecks();
   let scalars = TelemetryTestUtils.getProcessScalars("parent", true, true);
 
   Assert.deepEqual(

toolkit/components/search/tests/xpcshell/xpcshell.ini

@@ -150,6 +150,8 @@ support-files = data/search_ignorelist.json
 [test_validate_manifests.js]
 [test_webextensions_builtin_upgrade.js]
 [test_webextensions_install.js]
+[test_webextensions_migrate_to.js]
+support-files = data/search-migration.json
 [test_webextensions_normandy_upgrade.js]
 [test_webextensions_startup_remove.js]
 [test_webextensions_upgrade.js]