Bug 1809640 - Move add-on search engine checks from the search service to AddonSearchEngine. r=search-reviewers,daleharvey

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

toolkit/components/search/AddonSearchEngine.sys.mjs

@@ -15,9 +15,17 @@ ChromeUtils.defineESModuleGetters(lazy, {
 });
 
 XPCOMUtils.defineLazyModuleGetters(lazy, {
+  AddonManager: "resource://gre/modules/AddonManager.jsm",
   ExtensionParent: "resource://gre/modules/ExtensionParent.jsm",
 });
 
+XPCOMUtils.defineLazyGetter(lazy, "logConsole", () => {
+  return console.createInstance({
+    prefix: "AddonSearchEngine",
+    maxLogLevel: lazy.SearchUtils.loggingEnabled ? "Debug" : "Warn",
+  });
+});
+
 /**
  * AddonSearchEngine represents a search engine defined by an add-on.
  */
@@ -139,6 +147,69 @@ export class AddonSearchEngine extends SearchEngine {
     return super.toJSON();
   }
 
+  /**
+   * Checks to see if this engine's settings are in sync with what the add-on
+   * manager has, and reports the results to telemetry.
+   */
+  async checkAndReportIfSettingsValid() {
+    let addon = await lazy.AddonManager.getAddonByID(this._extensionID);
+
+    if (!addon) {
+      lazy.logConsole.debug(
+        `Add-on ${this._extensionID} for search engine ${this.name} is not installed!`
+      );
+      Services.telemetry.keyedScalarSet(
+        "browser.searchinit.engine_invalid_webextension",
+        this._extensionID,
+        1
+      );
+    } else if (!addon.isActive) {
+      lazy.logConsole.debug(
+        `Add-on ${this._extensionID} for search engine ${this.name} is not active!`
+      );
+      Services.telemetry.keyedScalarSet(
+        "browser.searchinit.engine_invalid_webextension",
+        this._extensionID,
+        2
+      );
+    } else {
+      let policy = await AddonSearchEngine.getExtensionPolicy(
+        this._extensionID
+      );
+      let providerSettings =
+        policy.extension.manifest?.chrome_settings_overrides?.search_provider;
+
+      if (!providerSettings) {
+        lazy.logConsole.debug(
+          `Add-on ${this._extensionID} for search engine ${this.name} no longer has an engine defined`
+        );
+        Services.telemetry.keyedScalarSet(
+          "browser.searchinit.engine_invalid_webextension",
+          this._extensionID,
+          4
+        );
+      } else if (this.name != providerSettings.name) {
+        lazy.logConsole.debug(
+          `Add-on ${this._extensionID} for search engine ${this.name} has a different name!`
+        );
+        Services.telemetry.keyedScalarSet(
+          "browser.searchinit.engine_invalid_webextension",
+          this._extensionID,
+          5
+        );
+      } else if (!this.checkSearchUrlMatchesManifest(providerSettings)) {
+        lazy.logConsole.debug(
+          `Add-on ${this._extensionID} for search engine ${this.name} has out-of-date manifest!`
+        );
+        Services.telemetry.keyedScalarSet(
+          "browser.searchinit.engine_invalid_webextension",
+          this._extensionID,
+          6
+        );
+      }
+    }
+  }
+
   /**
    * Initializes the engine based on the manifest and other values.
    *
@@ -210,4 +281,25 @@ export class AddonSearchEngine extends SearchEngine {
       configuration
     );
   }
+
+  /**
+   * Gets the WebExtensionPolicy for an add-on.
+   *
+   * @param {string} id
+   *   The WebExtension id.
+   * @returns {WebExtensionPolicy}
+   */
+  static async getExtensionPolicy(id) {
+    let policy = WebExtensionPolicy.getByID(id);
+    if (!policy) {
+      let idPrefix = id.split("@")[0];
+      let path = `resource://search-extensions/${idPrefix}/`;
+      await lazy.AddonManager.installBuiltinAddon(path);
+      policy = WebExtensionPolicy.getByID(id);
+    }
+    // On startup the extension may have not finished parsing the
+    // manifest, wait for that here.
+    await policy.readyPromise;
+    return policy;
+  }
 }

toolkit/components/search/SearchService.sys.mjs

@@ -1781,7 +1781,9 @@ export class SearchService {
           continue;
         }
 
-        policy = await this.#getExtensionPolicy(engine._extensionID);
+        policy = await lazy.AddonSearchEngine.getExtensionPolicy(
+          engine._extensionID
+        );
         locale =
           replacementEngines[0].webExtension.locale ||
           lazy.SearchUtils.DEFAULT_TAG;
@@ -1808,7 +1810,9 @@ export class SearchService {
       } else {
         // This is an existing engine that we should update (we don't know if
         // the configuration for this engine has changed or not).
-        policy = await this.#getExtensionPolicy(engine._extensionID);
+        policy = await lazy.AddonSearchEngine.getExtensionPolicy(
+          engine._extensionID
+        );
         locale = engine._locale;
         manifest = await this.#getManifestForLocale(policy.extension, locale);
       }
@@ -2466,68 +2470,8 @@ export class SearchService {
     lazy.logConsole.debug("Running check on WebExtension engines");
 
     for (let engine of this._engines.values()) {
-      if (
-        engine.isAppProvided ||
-        !engine._extensionID ||
-        engine._extensionID == "set-via-policy" ||
-        engine._extensionID == "set-via-user"
-      ) {
-        continue;
-      }
-
-      let addon = await lazy.AddonManager.getAddonByID(engine._extensionID);
-
-      if (!addon) {
-        lazy.logConsole.debug(
-          `Add-on ${engine._extensionID} for search engine ${engine.name} is not installed!`
-        );
-        Services.telemetry.keyedScalarSet(
-          "browser.searchinit.engine_invalid_webextension",
-          engine._extensionID,
-          1
-        );
-      } else if (!addon.isActive) {
-        lazy.logConsole.debug(
-          `Add-on ${engine._extensionID} for search engine ${engine.name} is not active!`
-        );
-        Services.telemetry.keyedScalarSet(
-          "browser.searchinit.engine_invalid_webextension",
-          engine._extensionID,
-          2
-        );
-      } else {
-        let policy = await this.#getExtensionPolicy(engine._extensionID);
-        let providerSettings =
-          policy.extension.manifest?.chrome_settings_overrides?.search_provider;
-
-        if (!providerSettings) {
-          lazy.logConsole.debug(
-            `Add-on ${engine._extensionID} for search engine ${engine.name} no longer has an engine defined`
-          );
-          Services.telemetry.keyedScalarSet(
-            "browser.searchinit.engine_invalid_webextension",
-            engine._extensionID,
-            4
-          );
-        } else if (engine.name != providerSettings.name) {
-          lazy.logConsole.debug(
-            `Add-on ${engine._extensionID} for search engine ${engine.name} has a different name!`
-          );
-          Services.telemetry.keyedScalarSet(
-            "browser.searchinit.engine_invalid_webextension",
-            engine._extensionID,
-            5
-          );
-        } else if (!engine.checkSearchUrlMatchesManifest(providerSettings)) {
-          lazy.logConsole.debug(
-            `Add-on ${engine._extensionID} for search engine ${engine.name} has out-of-date manifest!`
-          );
-          Services.telemetry.keyedScalarSet(
-            "browser.searchinit.engine_invalid_webextension",
-            engine._extensionID,
-            6
-          );
-        }
+      if (engine instanceof lazy.AddonSearchEngine && !engine.isAppProvided) {
+        await engine.checkAndReportIfSettingsValid();
       }
     }
     lazy.logConsole.debug("WebExtension engine check complete");
@@ -3278,27 +3222,6 @@ export class SearchService {
     }
   }
 
-  /**
-   * Gets the WebExtensionPolicy for an add-on.
-   *
-   * @param {string} id
-   *   The WebExtension id.
-   * @returns {WebExtensionPolicy}
-   */
-  async #getExtensionPolicy(id) {
-    let policy = WebExtensionPolicy.getByID(id);
-    if (!policy) {
-      let idPrefix = id.split("@")[0];
-      let path = `resource://search-extensions/${idPrefix}/`;
-      await lazy.AddonManager.installBuiltinAddon(path);
-      policy = WebExtensionPolicy.getByID(id);
-    }
-    // On startup the extension may have not finished parsing the
-    // manifest, wait for that here.
-    await policy.readyPromise;
-    return policy;
-  }
-
   #nimbusSearchUpdatedFun = null;
 
   async #nimbusSearchUpdated() {
@@ -3536,7 +3459,9 @@ export class SearchService {
    */
   async _makeEngineFromConfig(config) {
     lazy.logConsole.debug("_makeEngineFromConfig:", config);
-    let policy = await this.#getExtensionPolicy(config.webExtension.id);
+    let policy = await lazy.AddonSearchEngine.getExtensionPolicy(
+      config.webExtension.id
+    );
     let locale =
       "locale" in config.webExtension
         ? config.webExtension.locale