From 1d73a580fe1b2c3ebed8009d5fae8d0eae09208c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Ziad=C3=A9?= Date: Tue, 29 Apr 2025 14:04:48 +0000 Subject: [PATCH] Bug 1960813 - relax engineId allowed chars so we can use webext ids r=rpl,ngrato Differential Revision: https://phabricator.services.mozilla.com/D245687 --- .../extensions/parent/ext-trial-ml.js | 14 ++---- .../ml/actors/MLEngineParent.sys.mjs | 3 +- .../ml/content/EngineProcess.sys.mjs | 4 +- toolkit/components/ml/content/Utils.sys.mjs | 35 ++++++++++++++ .../ml/tests/browser/browser_ml_utils.js | 46 +++++++++++++++++++ 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/toolkit/components/extensions/parent/ext-trial-ml.js b/toolkit/components/extensions/parent/ext-trial-ml.js index 3cef3d49e234..e5325f1aaca6 100644 --- a/toolkit/components/extensions/parent/ext-trial-ml.js +++ b/toolkit/components/extensions/parent/ext-trial-ml.js @@ -4,14 +4,11 @@ "use strict"; -var { ExtensionCommon } = ChromeUtils.importESModule( - "resource://gre/modules/ExtensionCommon.sys.mjs" -); - ChromeUtils.defineESModuleGetters(this, { createEngine: "chrome://global/content/ml/EngineProcess.sys.mjs", PipelineOptions: "chrome://global/content/ml/EngineProcess.sys.mjs", ModelHub: "chrome://global/content/ml/ModelHub.sys.mjs", + addonIdToEngineId: "chrome://global/content/ml/Utils.sys.mjs", }); const PREF_EXTENSIONS_ML_ENABLED = "extensions.ml.enabled"; @@ -91,15 +88,10 @@ class TrialML extends ExtensionAPI { } /** - * Converts an extension id to a pipeline id + * Converts an extension id to a pipeline id by prefixing it. */ static getPipelineId(extensionId) { - // makeWidgetId() replaces all illegal characters with "_" - // so an id like {XXX-XXX-XXX} becomes _XXX_XXX_XXX_ - // which is then converted to ML-ENGINE-XXX-XXX-XXX - return `ML-ENGINE-${ExtensionCommon.makeWidgetId(extensionId) - .replace(/^_|_$/g, "") - .replace(/_/g, "-")}`; + return addonIdToEngineId(extensionId); } /** diff --git a/toolkit/components/ml/actors/MLEngineParent.sys.mjs b/toolkit/components/ml/actors/MLEngineParent.sys.mjs index 41d8f718f017..1edfdbbfe74d 100644 --- a/toolkit/components/ml/actors/MLEngineParent.sys.mjs +++ b/toolkit/components/ml/actors/MLEngineParent.sys.mjs @@ -30,6 +30,7 @@ ChromeUtils.defineESModuleGetters(lazy, { ModelHub: "chrome://global/content/ml/ModelHub.sys.mjs", getInferenceProcessInfo: "chrome://global/content/ml/Utils.sys.mjs", Progress: "chrome://global/content/ml/Utils.sys.mjs", + isAddonEngineId: "chrome://global/content/ml/Utils.sys.mjs", }); XPCOMUtils.defineLazyServiceGetter( @@ -819,7 +820,7 @@ class MLEngine { * @returns {string} */ getGleanLabel() { - if (this.engineId.startsWith("ML-ENGINE-")) { + if (lazy.isAddonEngineId(this.engineId)) { return "webextension"; } return this.engineId; diff --git a/toolkit/components/ml/content/EngineProcess.sys.mjs b/toolkit/components/ml/content/EngineProcess.sys.mjs index 1969c578f100..946b2ab66a30 100644 --- a/toolkit/components/ml/content/EngineProcess.sys.mjs +++ b/toolkit/components/ml/content/EngineProcess.sys.mjs @@ -629,7 +629,7 @@ export class PipelineOptions { * * Throws an exception if the name or ID is invalid. * - * @param {string} field - The name of the field being validated (e.g., taskName, engineId). + * @param {string} field - The name of the field being validated. * @param {string} value - The value of the field to validate. * @throws {PipelineOptionsValidationError} Throws a validation error if the ID is invalid. * @private @@ -786,7 +786,7 @@ export class PipelineOptions { } } // Validating values. - if (["taskName", "engineId"].includes(key)) { + if (key === "taskName") { this.#validateTaskName(key, options[key]); } diff --git a/toolkit/components/ml/content/Utils.sys.mjs b/toolkit/components/ml/content/Utils.sys.mjs index 583190b4bbcb..572cf893f895 100644 --- a/toolkit/components/ml/content/Utils.sys.mjs +++ b/toolkit/components/ml/content/Utils.sys.mjs @@ -996,3 +996,38 @@ export class RemoteSettingsManager { return records[0]; } } + +const ADDON_PREFIX = "ML-ENGINE-"; + +/** + * Check if an engine id is for an addon + * + * @param {string} engineId - The engine id to check + * @returns {boolean} True if the engine id is for an addon + */ +export function isAddonEngineId(engineId) { + return engineId.startsWith(ADDON_PREFIX); +} + +/** + * Converts an addon id to an engine id + * + * @param {string} addonId - The addon id to convert + * @returns {string} The engine id + */ +export function addonIdToEngineId(addonId) { + return `${ADDON_PREFIX}${addonId}`; +} + +/** + * Converts an engine Id into an addon id + * + * @param {string} engineId - The engine id to convert + * @returns {string|null} The addon id. null if the engine id is invalid + */ +export function engineIdToAddonId(engineId) { + if (!engineId.startsWith(ADDON_PREFIX)) { + return null; + } + return engineId.substring(ADDON_PREFIX.length); +} diff --git a/toolkit/components/ml/tests/browser/browser_ml_utils.js b/toolkit/components/ml/tests/browser/browser_ml_utils.js index b298df347bb0..a551dce9eb95 100644 --- a/toolkit/components/ml/tests/browser/browser_ml_utils.js +++ b/toolkit/components/ml/tests/browser/browser_ml_utils.js @@ -12,6 +12,9 @@ const { RejectionType, BlockListManager, RemoteSettingsManager, + isAddonEngineId, + addonIdToEngineId, + engineIdToAddonId, } = ChromeUtils.importESModule("chrome://global/content/ml/Utils.sys.mjs"); const { OPFS } = ChromeUtils.importESModule( @@ -1107,3 +1110,46 @@ add_task(async function testBlockListManagerWithRS() { RemoteSettingsManager.removeMocks(); }); + +add_task(function test_addon_engine_id_utilities() { + const prefix = "ML-ENGINE-"; + + // Valid addon ID + const addonId = "custom-addon"; + const engineId = addonIdToEngineId(addonId); + + Assert.equal( + engineId, + `${prefix}${addonId}`, + "addonIdToEngineId should add the correct prefix" + ); + Assert.ok( + isAddonEngineId(engineId), + "isAddonEngineId should detect prefixed engine ID" + ); + Assert.equal( + engineIdToAddonId(engineId), + addonId, + "engineIdToAddonId should return original addon ID" + ); + + // Invalid engine ID + const invalidEngineId = "ENGINE-custom-addon"; + Assert.ok( + !isAddonEngineId(invalidEngineId), + "isAddonEngineId should reject non-prefixed engine ID" + ); + Assert.equal( + engineIdToAddonId(invalidEngineId), + null, + "engineIdToAddonId should return null for invalid ID" + ); + + // Edge case: empty string + Assert.ok(!isAddonEngineId(""), "isAddonEngineId should reject empty string"); + Assert.equal( + engineIdToAddonId(""), + null, + "engineIdToAddonId should return null for empty string" + ); +});