diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index cb631acfefc4..fc563dd98d5e 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1313,9 +1313,6 @@ pref("trailhead.firstrun.branches", "join-dynamic"); // Separate about welcome pref("browser.aboutwelcome.enabled", true); -// Temporary utility to unblock testing on about:welcome experiment variations -pref("browser.aboutwelcome.temp.testExperiment.slug", ""); -pref("browser.aboutwelcome.temp.testExperiment.branch", "control"); // See Console.jsm LOG_LEVELS for all possible values pref("browser.aboutwelcome.log", "warn"); @@ -1325,6 +1322,10 @@ pref("browser.messaging-system.whatsNewPanel.enabled", true); pref("browser.messaging-system.personalized-cfr.scores", "{}"); pref("browser.messaging-system.personalized-cfr.score-threshold", 5000); +// Experiment Manager +pref("messaging-system.log", "warn"); +pref("messaging-system.rsexperimentloader.enabled", true); + // Enable the DOM fullscreen API. pref("full-screen-api.enabled", true); diff --git a/browser/components/newtab/aboutwelcome/AboutWelcomeChild.jsm b/browser/components/newtab/aboutwelcome/AboutWelcomeChild.jsm index 452479ca8ea2..7dc9eb9e654c 100644 --- a/browser/components/newtab/aboutwelcome/AboutWelcomeChild.jsm +++ b/browser/components/newtab/aboutwelcome/AboutWelcomeChild.jsm @@ -11,8 +11,7 @@ const { XPCOMUtils } = ChromeUtils.import( ); XPCOMUtils.defineLazyModuleGetters(this, { - ExperimentAPI: - "resource://activity-stream/aboutwelcome/lib/AboutWelcomeExperimentAPI.jsm", + ExperimentAPI: "resource://messaging-system/experiments/ExperimentAPI.jsm", }); XPCOMUtils.defineLazyGetter(this, "log", () => { @@ -97,11 +96,21 @@ class AboutWelcomeChild extends JSWindowActorChild { * Send initial data to page including experiment information */ AWGetStartupData() { - // TODO: Fetch this from Experiments - const experimentData = ExperimentAPI.getExperiment({ - group: "aboutwelcome", - }); - return Cu.cloneInto(experimentData, this.contentWindow); + return this.wrapPromise( + ExperimentAPI.ready().then(() => { + const experimentData = ExperimentAPI.getExperiment({ + group: "aboutwelcome", + }); + if (experimentData && experimentData.slug) { + log.debug( + `Loading about:welcome with experiment: ${experimentData.slug}` + ); + } else { + log.debug("Loading about:welcome without experiment"); + } + return Cu.cloneInto(experimentData || {}, this.contentWindow); + }) + ); } AWGetFxAMetricsFlowURI() { diff --git a/browser/components/newtab/aboutwelcome/lib/AboutWelcomeExperimentAPI.jsm b/browser/components/newtab/aboutwelcome/lib/AboutWelcomeExperimentAPI.jsm deleted file mode 100644 index fe41ae483dbb..000000000000 --- a/browser/components/newtab/aboutwelcome/lib/AboutWelcomeExperimentAPI.jsm +++ /dev/null @@ -1,212 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -"use strict"; - -const EXPORTED_SYMBOLS = ["ExperimentAPI", "TEST_REFERENCE_RECIPE"]; - -const { XPCOMUtils } = ChromeUtils.import( - "resource://gre/modules/XPCOMUtils.jsm" -); - -XPCOMUtils.defineLazyPreferenceGetter( - this, - "testSlug", - "browser.aboutwelcome.temp.testExperiment.slug", - "" -); - -XPCOMUtils.defineLazyPreferenceGetter( - this, - "testBranch", - "browser.aboutwelcome.temp.testExperiment.branch", - "", - null, - val => val || "control" -); - -// This is used by xpcshell tests -const TEST_REFERENCE_RECIPE = { - slug: "about_welcome_test", - branches: [ - { - slug: "control", - ratio: 1, - value: { - title: "This is the control branch", - }, - }, - { - slug: "variant", - ratio: 1, - value: { - title: "This is the variant branch", - subtitle: "With a subtitle.", - }, - }, - ], -}; - -const PERSONAL_DATA_PROMISE_CARD = { - id: "TRAILHEAD_CARD_12", - content: { - title: { string_id: "onboarding-personal-data-promise-title" }, - text: { string_id: "onboarding-personal-data-promise-text" }, - icon: "pledge", - primary_button: { - label: { string_id: "onboarding-personal-data-promise-button" }, - action: { - type: "OPEN_URL", - data: { - args: "https://www.mozilla.org/firefox/privacy/", - where: "tabshifted", - }, - }, - }, - }, -}; - -const BROWSE_PRIVATELY_CARD = { - content: { - title: { - string_id: "onboarding-browse-privately-title", - }, - text: { - string_id: "onboarding-browse-privately-text", - }, - icon: "private", - primary_button: { - label: { - string_id: "onboarding-browse-privately-button", - }, - action: { - type: "OPEN_PRIVATE_BROWSER_WINDOW", - }, - }, - }, - id: "TRAILHEAD_CARD_4", -}; - -const FX_MONITOR_CARD = { - content: { - title: { - string_id: "onboarding-firefox-monitor-title", - }, - text: { - string_id: "onboarding-firefox-monitor-text2", - }, - icon: "ffmonitor", - primary_button: { - label: { - string_id: "onboarding-firefox-monitor-button", - }, - action: { - type: "OPEN_URL", - data: { - args: "https://monitor.firefox.com/", - where: "tabshifted", - }, - }, - }, - }, - id: "TRAILHEAD_CARD_3", -}; - -const SYNC_CARD = { - content: { - title: { - string_id: "onboarding-data-sync-title", - }, - text: { - string_id: "onboarding-data-sync-text2", - }, - icon: "devices", - primary_button: { - label: { - string_id: "onboarding-data-sync-button2", - }, - action: { - type: "OPEN_URL", - addFlowParams: true, - data: { - args: - "https://accounts.firefox.com/?service=sync&action=email&context=fx_desktop_v3&entrypoint=activity-stream-firstrun&style=trailhead", - where: "tabshifted", - }, - }, - }, - }, - id: "TRAILHEAD_CARD_2", -}; - -const PULL_FACTOR_PRIVACY_RECIPE = { - slug: "aw_pull_factor_privacy", // experiment id - branches: [ - { - slug: "control", - ratio: 1, - value: {}, - }, - { - slug: "variant_1", // branch id - ratio: 1, - value: { - title: "Welcome to Firefox. Fast, safe, private.", - cards: [ - PERSONAL_DATA_PROMISE_CARD, - FX_MONITOR_CARD, - BROWSE_PRIVATELY_CARD, - ], - }, - }, - { - slug: "variant_2", - ratio: 1, - value: { - title: "Welcome to Firefox", - cards: [ - PERSONAL_DATA_PROMISE_CARD, - FX_MONITOR_CARD, - BROWSE_PRIVATELY_CARD, - ], - }, - }, - { - slug: "variant_3", - ratio: 1, - value: { - title: "Welcome to Firefox. Fast, safe, private.", - cards: [SYNC_CARD, FX_MONITOR_CARD, BROWSE_PRIVATELY_CARD], - }, - }, - ], -}; - -const ExperimentAPI = { - _RECIPES: [ - TEST_REFERENCE_RECIPE, - PULL_FACTOR_PRIVACY_RECIPE, - // Add more recipes below - ], - - getExperiment() { - const recipes = this._RECIPES; - const experiment = testSlug && recipes.find(r => r.slug === testSlug); - if (experiment) { - const branch = experiment.branches.find(b => b.slug === testBranch); - return branch ? { slug: experiment.slug, branch } : {}; - } - return {}; - }, - - getValue() { - const recipes = this._RECIPES; - const experiment = testSlug && recipes.find(r => r.slug === testSlug); - if (experiment) { - const branch = experiment.branches.find(b => b.slug === testBranch); - return branch ? branch.value : {}; - } - return {}; - }, -}; diff --git a/browser/components/newtab/test/xpcshell/test_AboutWelcomeExperimentAPI.js b/browser/components/newtab/test/xpcshell/test_AboutWelcomeExperimentAPI.js deleted file mode 100644 index 30f296bfe30e..000000000000 --- a/browser/components/newtab/test/xpcshell/test_AboutWelcomeExperimentAPI.js +++ /dev/null @@ -1,85 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ - */ - -"use strict"; - -const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); -const { ExperimentAPI, TEST_REFERENCE_RECIPE } = ChromeUtils.import( - "resource://activity-stream/aboutwelcome/lib/AboutWelcomeExperimentAPI.jsm" -); - -const SLUG_PREF = "browser.aboutwelcome.temp.testExperiment.slug"; -const BRANCH_PREF = "browser.aboutwelcome.temp.testExperiment.branch"; - -add_task(async function test_getValue() { - registerCleanupFunction(() => { - Services.prefs.clearUserPref(SLUG_PREF); - Services.prefs.clearUserPref(BRANCH_PREF); - }); - - // Return empty array by default - Assert.deepEqual( - ExperimentAPI.getValue(), - {}, - "should return empty data by default" - ); - - // Get control branch for a slug if no branch is set - Services.prefs.setCharPref(SLUG_PREF, "about_welcome_test"); - Assert.deepEqual( - ExperimentAPI.getValue(), - TEST_REFERENCE_RECIPE.branches[0].value, - "should return control branch for a given test experiment if defined in the slug pref" - ); - - // Get recipe for a slug and branch - Services.prefs.setCharPref(SLUG_PREF, "about_welcome_test"); - Services.prefs.setCharPref(BRANCH_PREF, "variant"); - Assert.deepEqual( - ExperimentAPI.getValue(), - TEST_REFERENCE_RECIPE.branches[1].value, - "should return variant branch for a given test experiment if defined in prefs" - ); -}); - -add_task(async function test_getExperiment() { - Services.prefs.clearUserPref(SLUG_PREF); - Services.prefs.clearUserPref(BRANCH_PREF); - - // Return empty array by default - Assert.deepEqual( - ExperimentAPI.getExperiment(), - {}, - "should return empty data by default" - ); - - // Get control branch for a slug if no branch is set - Services.prefs.setCharPref(SLUG_PREF, "about_welcome_test"); - - Assert.deepEqual( - ExperimentAPI.getExperiment().slug, - "about_welcome_test", - "should return test experiment slug" - ); - - Assert.deepEqual( - ExperimentAPI.getExperiment().branch, - TEST_REFERENCE_RECIPE.branches[0], - "should return control branch for a given test experiment if defined in the slug pref" - ); - - // Get recipe for a slug and branch - Services.prefs.setCharPref(SLUG_PREF, "about_welcome_test"); - Services.prefs.setCharPref(BRANCH_PREF, "variant"); - Assert.deepEqual( - ExperimentAPI.getExperiment().branch, - TEST_REFERENCE_RECIPE.branches[1], - "should return variant branch for a given test experiment if defined in prefs" - ); - - registerCleanupFunction(() => { - Services.prefs.clearUserPref(SLUG_PREF); - Services.prefs.clearUserPref(BRANCH_PREF); - }); -}); diff --git a/browser/components/newtab/test/xpcshell/xpcshell.ini b/browser/components/newtab/test/xpcshell/xpcshell.ini index 0390eb04b63c..3ee0d9117c0b 100644 --- a/browser/components/newtab/test/xpcshell/xpcshell.ini +++ b/browser/components/newtab/test/xpcshell/xpcshell.ini @@ -7,4 +7,3 @@ skip-if = toolkit == 'android' [test_ASRouterTargeting_attribution.js] skip-if = toolkit != "cocoa" # osx specific tests [test_AboutWelcomeTelemetry.js] -[test_AboutWelcomeExperimentAPI.js] diff --git a/toolkit/components/messaging-system/experiments/@types/ExperimentManager.d.ts b/toolkit/components/messaging-system/experiments/@types/ExperimentManager.d.ts index 79f225a0a7b0..5e7fa370108a 100644 --- a/toolkit/components/messaging-system/experiments/@types/ExperimentManager.d.ts +++ b/toolkit/components/messaging-system/experiments/@types/ExperimentManager.d.ts @@ -18,4 +18,5 @@ export interface Enrollment { branch: Branch; active: boolean; experimentType: string; + source: string; } diff --git a/toolkit/components/messaging-system/experiments/ExperimentManager.jsm b/toolkit/components/messaging-system/experiments/ExperimentManager.jsm index 478a33de70a7..4474cf911271 100644 --- a/toolkit/components/messaging-system/experiments/ExperimentManager.jsm +++ b/toolkit/components/messaging-system/experiments/ExperimentManager.jsm @@ -20,13 +20,19 @@ XPCOMUtils.defineLazyModuleGetters(this, { ClientEnvironment: "resource://normandy/lib/ClientEnvironment.jsm", ExperimentStore: "resource://messaging-system/experiments/ExperimentStore.jsm", - LogManager: "resource://normandy/lib/LogManager.jsm", NormandyUtils: "resource://normandy/lib/NormandyUtils.jsm", Sampling: "resource://gre/modules/components-utils/Sampling.jsm", TelemetryEvents: "resource://normandy/lib/TelemetryEvents.jsm", TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.jsm", }); +XPCOMUtils.defineLazyGetter(this, "log", () => { + const { Logger } = ChromeUtils.import( + "resource://messaging-system/lib/Logger.jsm" + ); + return new Logger("ExperimentManager"); +}); + // This is included with event telemetry e.g. "enroll" // TODO: Add a new type called "messaging_study" const EVENT_TELEMETRY_STUDY_TYPE = "preference_study"; @@ -43,8 +49,15 @@ class _ExperimentManager { constructor({ id = "experimentmanager", store } = {}) { this.id = id; this.store = store || new ExperimentStore(); - this.slugsSeenInThisSession = new Set(); - this.log = LogManager.getLogger("ExperimentManager"); + this.sessions = new Map(); + + this.filterContext = {}; + Object.defineProperty(this.filterContext, "activeExperiments", { + get: async () => { + await this.store.ready(); + return this.store.getAllActive().map(exp => exp.slug); + }, + }); } /** @@ -62,29 +75,46 @@ class _ExperimentManager { /** * Runs every time a Recipe is updated or seen for the first time. * @param {RecipeArgs} recipe + * @param {string} source */ - async onRecipe(recipe) { + async onRecipe(recipe, source) { const { slug, isEnrollmentPaused } = recipe; - this.slugsSeenInThisSession.add(slug); + if (!source) { + throw new Error("When calling onRecipe, you must specify a source."); + } + + if (!this.sessions.has(source)) { + this.sessions.set(source, new Set()); + } + this.sessions.get(source).add(slug); if (this.store.has(slug)) { this.updateEnrollment(recipe); } else if (isEnrollmentPaused) { - this.log.debug(`Enrollment is paused for "${slug}"`); + log.debug(`Enrollment is paused for "${slug}"`); } else { - await this.enroll(recipe); + await this.enroll(recipe, source); } } - // Runs when the all recipes been processed during an update, including at first run. - onFinalize() { + /** + * Runs when the all recipes been processed during an update, including at first run. + * @param {string} sourceToCheck + */ + onFinalize(sourceToCheck) { + if (!sourceToCheck) { + throw new Error("When calling onFinalize, you must specify a source."); + } const activeExperiments = this.store.getAllActive(); for (const experiment of activeExperiments) { - const { slug } = experiment; - if (!this.slugsSeenInThisSession.has(slug)) { - this.log.debug(`Stopping study for recipe ${slug}`); + const { slug, source } = experiment; + if (sourceToCheck !== source) { + continue; + } + if (!this.sessions.get(source)?.has(slug)) { + log.debug(`Stopping study for recipe ${slug}`); try { this.unenroll(slug, "recipe-not-seen"); } catch (err) { @@ -93,18 +123,22 @@ class _ExperimentManager { } } - this.slugsSeenInThisSession.clear(); + this.sessions.delete(sourceToCheck); } /** * Start a new experiment by enrolling the users * * @param {RecipeArgs} recipe + * @param {string} source * @returns {Promise} The experiment object stored in the data store * @rejects {Error} * @memberof _ExperimentManager */ - async enroll({ slug, branches, experimentType = DEFAULT_EXPERIMENT_TYPE }) { + async enroll( + { slug, branches, experimentType = DEFAULT_EXPERIMENT_TYPE }, + source + ) { if (this.store.has(slug)) { this.sendFailureTelemetry("enrollFailed", slug, "name-conflict"); throw new Error(`An experiment with the slug "${slug}" already exists.`); @@ -114,7 +148,7 @@ class _ExperimentManager { const branch = await this.chooseBranch(slug, branches); if (branch.groups && this.store.hasExperimentForGroups(branch.groups)) { - this.log.debug( + log.debug( `Skipping enrollment for "${slug}" because there is an existing experiment for one of its groups.` ); this.sendFailureTelemetry("enrollFailed", slug, "group-conflict"); @@ -128,13 +162,14 @@ class _ExperimentManager { active: true, enrollmentId, experimentType, + source, }; this.store.addExperiment(experiment); this.setExperimentActive(experiment); this.sendEnrollmentTelemetry(experiment); - this.log.debug(`New experiment started: ${slug}, ${branch.slug}`); + log.debug(`New experiment started: ${slug}, ${branch.slug}`); return experiment; } @@ -150,13 +185,13 @@ class _ExperimentManager { // Don't update experiments that were already unenrolled. if (experiment.active === false) { - this.log.debug(`Enrollment ${recipe.slug} has expired, aborting.`); + log.debug(`Enrollment ${recipe.slug} has expired, aborting.`); return; } // Stay in the same branch, don't re-sample every time. const branch = recipe.branches.find( - branch => branch.slug === experiment.branch + branch => branch.slug === experiment.branch.slug ); if (!branch) { @@ -169,10 +204,9 @@ class _ExperimentManager { * Stop an experiment that is currently active * * @param {string} slug - * @param {object} [options] - * @param {string} [options.reason] + * @param {string} reason */ - unenroll(slug, { reason = "unknown" } = {}) { + unenroll(slug, reason = "unknown") { const experiment = this.store.get(slug); if (!experiment) { this.sendFailureTelemetry("unenrollFailed", slug, "does-not-exist"); @@ -196,7 +230,7 @@ class _ExperimentManager { experiment.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER, }); - this.log.debug(`Experiment unenrolled: ${slug}}`); + log.debug(`Experiment unenrolled: ${slug}}`); } /** diff --git a/toolkit/components/messaging-system/jar.mn b/toolkit/components/messaging-system/jar.mn index fa19e728bd45..a1bb60ab486b 100644 --- a/toolkit/components/messaging-system/jar.mn +++ b/toolkit/components/messaging-system/jar.mn @@ -4,6 +4,5 @@ toolkit.jar: % resource messaging-system %res/messaging-system/ - res/messaging-system/experiments/ExperimentManager.jsm (./experiments/ExperimentManager.jsm) - res/messaging-system/experiments/ExperimentStore.jsm (./experiments/ExperimentStore.jsm) + res/messaging-system/experiments/ (./experiments/*) res/messaging-system/lib/ (./lib/*) diff --git a/toolkit/components/messaging-system/lib/Logger.jsm b/toolkit/components/messaging-system/lib/Logger.jsm new file mode 100644 index 000000000000..2afc3aa526ff --- /dev/null +++ b/toolkit/components/messaging-system/lib/Logger.jsm @@ -0,0 +1,22 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +"use strict"; + +const EXPORTED_SYMBOLS = ["Logger"]; + +const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); +const { ConsoleAPI } = ChromeUtils.import("resource://gre/modules/Console.jsm"); + +const LOGGING_PREF = "messaging-system.log"; + +class Logger extends ConsoleAPI { + constructor(name) { + let consoleOptions = { + prefix: name, + maxLogLevel: Services.prefs.getCharPref(LOGGING_PREF, "warn"), + maxLogLevelPref: LOGGING_PREF, + }; + super(consoleOptions); + } +} diff --git a/toolkit/components/messaging-system/lib/RemoteSettingsExperimentLoader.jsm b/toolkit/components/messaging-system/lib/RemoteSettingsExperimentLoader.jsm new file mode 100644 index 000000000000..628fadfab7d5 --- /dev/null +++ b/toolkit/components/messaging-system/lib/RemoteSettingsExperimentLoader.jsm @@ -0,0 +1,164 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +const EXPORTED_SYMBOLS = [ + "_RemoteSettingsExperimentLoader", + "RemoteSettingsExperimentLoader", +]; + +const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); + +XPCOMUtils.defineLazyModuleGetters(this, { + ASRouterTargeting: "resource://activity-stream/lib/ASRouterTargeting.jsm", + ExperimentManager: + "resource://messaging-system/experiments/ExperimentManager.jsm", + RemoteSettings: "resource://services-settings/remote-settings.js", + CleanupManager: "resource://normandy/lib/CleanupManager.jsm", +}); + +XPCOMUtils.defineLazyGetter(this, "log", () => { + const { Logger } = ChromeUtils.import( + "resource://messaging-system/lib/Logger.jsm" + ); + return new Logger("RSLoader"); +}); + +XPCOMUtils.defineLazyServiceGetter( + this, + "timerManager", + "@mozilla.org/updates/timer-manager;1", + "nsIUpdateTimerManager" +); + +const COLLECTION_ID = "messaging-experiments"; +const ENABLED_PREF = "messaging-system.rsexperimentloader.enabled"; + +const TIMER_NAME = "rs-experiment-loader-timer"; +const TIMER_LAST_UPDATE_PREF = `app.update.lastUpdateTime.${TIMER_NAME}`; +// Use the same update interval as normandy +const RUN_INTERVAL_PREF = "app.normandy.run_interval_seconds"; + +class _RemoteSettingsExperimentLoader { + constructor() { + // Has the timer been set? + this._initialized = false; + // Are we in the middle of updating recipes already? + this._updating = false; + + XPCOMUtils.defineLazyGetter(this, "remoteSettingsClient", () => { + return RemoteSettings(COLLECTION_ID); + }); + + XPCOMUtils.defineLazyPreferenceGetter( + this, + "enabled", + ENABLED_PREF, + false, + this.onEnabledPrefChange.bind(this) + ); + + XPCOMUtils.defineLazyPreferenceGetter( + this, + "intervalInSeconds", + RUN_INTERVAL_PREF, + 21600, + () => this.setTimer() + ); + } + + async init() { + if (this._initialized || !this.enabled) { + return; + } + + this.setTimer(); + CleanupManager.addCleanupHandler(() => this.uninit()); + this._initialized = true; + + await this.updateRecipes(); + } + + uninit() { + if (!this._initialized) { + return; + } + timerManager.unregisterTimer(TIMER_NAME); + this._initialized = false; + } + + async updateRecipes(trigger) { + if (this._updating || !this._initialized) { + return; + } + this._updating = true; + + log.debug("Updating recipes"); + + let recipes; + let loadingError = false; + + try { + recipes = await this.remoteSettingsClient.get(); + } catch (e) { + loadingError = true; + Cu.reportError(e); + } + + if (recipes && !loadingError) { + log.debug("Updating ExperimentManager with new recipes"); + for (const r of recipes) { + if ( + await ASRouterTargeting.isMatch( + r.filter_expression, + ExperimentManager.filterContext, + err => { + log.debug("Targeting failed because of an error"); + Cu.reportError(err); + } + ) + ) { + log.debug(`${r.id} passed filter_expression`); + await ExperimentManager.onRecipe(r.arguments, "rs-loader"); + } else { + log.debug(`${r.id} failed filter_expression`); + } + } + + ExperimentManager.onFinalize("rs-loader"); + } + + if (trigger !== "timer") { + const lastUpdateTime = Math.round(Date.now() / 1000); + Services.prefs.setIntPref(TIMER_LAST_UPDATE_PREF, lastUpdateTime); + } + + this._updating = false; + } + + onEnabledPrefChange(prefName, oldValue, newValue) { + if (this._initialized && !newValue) { + this.uninit(); + } else if (!this._initialized && newValue) { + this.init(); + } + } + + setTimer() { + // When this function is called, updateRecipes is called immediately + // and then every this.intervalInSeconds + timerManager.registerTimer( + TIMER_NAME, + () => this.updateRecipes("timer"), + this.intervalInSeconds + ); + log.debug("Registered update timer"); + } +} + +const RemoteSettingsExperimentLoader = new _RemoteSettingsExperimentLoader(); diff --git a/toolkit/components/messaging-system/moz.build b/toolkit/components/messaging-system/moz.build index bf161a7b7e7e..9a243b5e3aec 100644 --- a/toolkit/components/messaging-system/moz.build +++ b/toolkit/components/messaging-system/moz.build @@ -10,7 +10,6 @@ with Files('**'): XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini'] TESTING_JS_MODULES += [ - 'experiments/ExperimentAPI.jsm', 'test/MSTestUtils.jsm' ] diff --git a/toolkit/components/messaging-system/test/MSTestUtils.jsm b/toolkit/components/messaging-system/test/MSTestUtils.jsm index cb3df2682fa5..f8e8009aa524 100644 --- a/toolkit/components/messaging-system/test/MSTestUtils.jsm +++ b/toolkit/components/messaging-system/test/MSTestUtils.jsm @@ -18,6 +18,10 @@ const { FileTestUtils } = ChromeUtils.import( ); const PATH = FileTestUtils.getTempFile("shared-data-map").path; +const { _RemoteSettingsExperimentLoader } = ChromeUtils.import( + "resource://messaging-system/lib/RemoteSettingsExperimentLoader.jsm" +); + const EXPORTED_SYMBOLS = ["ExperimentFakes"]; const ExperimentFakes = { @@ -30,12 +34,22 @@ const ExperimentFakes = { childStore() { return new ExperimentStore("FakeStore", { isParent: false }); }, + rsLoader() { + const loader = new _RemoteSettingsExperimentLoader(); + // Replace RS client with a fake + Object.defineProperty(loader, "remoteSettingsClient", { + get: () => ({ get: () => Promise.resolve([]) }), + }); + + return loader; + }, experiment(slug, props = {}) { return { slug, active: true, enrollmentId: NormandyUtils.generateUuid(), branch: { slug: "treatment", value: { title: "hello" } }, + source: "test", ...props, }; }, diff --git a/toolkit/components/messaging-system/test/unit/test_ExperimentAPI.js b/toolkit/components/messaging-system/test/unit/test_ExperimentAPI.js index 235cc2be4790..67a608ed2a45 100644 --- a/toolkit/components/messaging-system/test/unit/test_ExperimentAPI.js +++ b/toolkit/components/messaging-system/test/unit/test_ExperimentAPI.js @@ -1,7 +1,7 @@ "use strict"; const { ExperimentAPI } = ChromeUtils.import( - "resource://testing-common/ExperimentAPI.jsm" + "resource://messaging-system/experiments/ExperimentAPI.jsm" ); const { ExperimentFakes } = ChromeUtils.import( "resource://testing-common/MSTestUtils.jsm" diff --git a/toolkit/components/messaging-system/test/unit/test_ExperimentManager_lifecycle.js b/toolkit/components/messaging-system/test/unit/test_ExperimentManager_lifecycle.js index 0edc8473af38..5e06fc856432 100644 --- a/toolkit/components/messaging-system/test/unit/test_ExperimentManager_lifecycle.js +++ b/toolkit/components/messaging-system/test/unit/test_ExperimentManager_lifecycle.js @@ -51,7 +51,7 @@ add_task(async function test_onStartup_setExperimentActive_called() { /** * onRecipe() - * - should add recipe slug to .slugsSeenInThisSession + * - should add recipe slug to .session[source] * - should call .enroll() if the recipe hasn't been seen before; * - should call .update() if the Enrollment already exists in the store; * - should skip enrollment if recipe.isEnrollmentPaused is true @@ -66,12 +66,12 @@ add_task(async function test_onRecipe_track_slug() { await manager.onStartup(); // The first time a recipe has seen; - await manager.onRecipe(fooRecipe); + await manager.onRecipe(fooRecipe, "test"); Assert.equal( - manager.slugsSeenInThisSession.has("foo"), + manager.sessions.get("test").has("foo"), true, - "should add slug to slugsSeenInThisSession" + "should add slug to sessions[test]" ); }); @@ -84,7 +84,7 @@ add_task(async function test_onRecipe_enroll() { const fooRecipe = ExperimentFakes.recipe("foo"); await manager.onStartup(); - await manager.onRecipe(fooRecipe); + await manager.onRecipe(fooRecipe, "test"); Assert.equal( manager.enroll.calledWith(fooRecipe), @@ -107,10 +107,9 @@ add_task(async function test_onRecipe_update() { const fooRecipe = ExperimentFakes.recipe("foo"); await manager.onStartup(); - - await manager.onRecipe(fooRecipe); + await manager.onRecipe(fooRecipe, "test"); // Call again after recipe has already been enrolled - await manager.onRecipe(fooRecipe); + await manager.onRecipe(fooRecipe, "test"); Assert.equal( manager.updateEnrollment.calledWith(fooRecipe), @@ -130,7 +129,7 @@ add_task(async function test_onRecipe_isEnrollmentPaused() { const pausedRecipe = ExperimentFakes.recipe("xyz", { isEnrollmentPaused: true, }); - await manager.onRecipe(pausedRecipe); + await manager.onRecipe(pausedRecipe, "test"); Assert.equal( manager.enroll.calledWith(pausedRecipe), false, @@ -147,7 +146,7 @@ add_task(async function test_onRecipe_isEnrollmentPaused() { isEnrollmentPaused: true, }); await manager.enroll(fooRecipe); - await manager.onRecipe(updatedRecipe); + await manager.onRecipe(updatedRecipe, "test"); Assert.equal( manager.updateEnrollment.calledWith(updatedRecipe), true, @@ -172,11 +171,12 @@ add_task(async function test_onFinalize_unenroll() { manager.store.addExperiment(ExperimentFakes.experiment("foo")); // Simulate adding some other recipes - await manager.onRecipe(ExperimentFakes.recipe("bar")); - await manager.onRecipe(ExperimentFakes.recipe("baz")); + await manager.onStartup(); + await manager.onRecipe(ExperimentFakes.recipe("bar"), "test"); + await manager.onRecipe(ExperimentFakes.recipe("baz"), "test"); // Finalize - manager.onFinalize(); + manager.onFinalize("test"); Assert.equal( manager.unenroll.callCount, @@ -189,8 +189,8 @@ add_task(async function test_onFinalize_unenroll() { "should unenroll a experiment whose recipe wasn't seen in the current session" ); Assert.equal( - manager.slugsSeenInThisSession.size, - 0, - "should clear slugsSeenInThisSession" + manager.sessions.has("test"), + false, + "should clear sessions[test]" ); }); diff --git a/toolkit/components/messaging-system/test/unit/test_ExperimentManager_unenroll.js b/toolkit/components/messaging-system/test/unit/test_ExperimentManager_unenroll.js index 01d7ddd14799..b6118a22f31a 100644 --- a/toolkit/components/messaging-system/test/unit/test_ExperimentManager_unenroll.js +++ b/toolkit/components/messaging-system/test/unit/test_ExperimentManager_unenroll.js @@ -32,7 +32,7 @@ add_task(async function test_set_inactive() { await manager.onStartup(); manager.store.addExperiment(ExperimentFakes.experiment("foo")); - manager.unenroll("foo", { reason: "some-reason" }); + manager.unenroll("foo", "some-reason"); Assert.equal( manager.store.get("foo").active, @@ -49,7 +49,7 @@ add_task(async function test_setExperimentInactive_called() { await manager.onStartup(); manager.store.addExperiment(experiment); - manager.unenroll("foo", { reason: "some-reason" }); + manager.unenroll("foo", "some-reason"); Assert.ok( TelemetryEnvironment.setExperimentInactive.calledWith("foo"), @@ -65,7 +65,7 @@ add_task(async function test_send_unenroll_event() { await manager.onStartup(); manager.store.addExperiment(experiment); - manager.unenroll("foo", { reason: "some-reason" }); + manager.unenroll("foo", "some-reason"); Assert.ok(TelemetryEvents.sendEvent.calledOnce); Assert.deepEqual( diff --git a/toolkit/components/messaging-system/test/unit/test_RemoteSettingsExperimentLoader.js b/toolkit/components/messaging-system/test/unit/test_RemoteSettingsExperimentLoader.js new file mode 100644 index 000000000000..ae2515cc5d80 --- /dev/null +++ b/toolkit/components/messaging-system/test/unit/test_RemoteSettingsExperimentLoader.js @@ -0,0 +1,73 @@ +"use strict"; + +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/MSTestUtils.jsm" +); +const { CleanupManager } = ChromeUtils.import( + "resource://normandy/lib/CleanupManager.jsm" +); + +const ENABLED_PREF = "messaging-system.rsexperimentloader.enabled"; +const RUN_INTERVAL_PREF = "app.normandy.run_interval_seconds"; + +add_task(async function test_lazy_pref_getters() { + const loader = ExperimentFakes.rsLoader(); + sinon.stub(loader, "updateRecipes").resolves(); + + Services.prefs.setIntPref(RUN_INTERVAL_PREF, 123456); + equal( + loader.intervalInSeconds, + 123456, + `should set intervalInSeconds to the value of ${RUN_INTERVAL_PREF}` + ); + + Services.prefs.setBoolPref(ENABLED_PREF, true); + equal( + loader.enabled, + true, + `should set enabled to the value of ${ENABLED_PREF}` + ); + Services.prefs.setBoolPref(ENABLED_PREF, false); + equal(loader.enabled, false); + + Services.prefs.clearUserPref(RUN_INTERVAL_PREF); + Services.prefs.clearUserPref(ENABLED_PREF); +}); + +add_task(async function test_init() { + const loader = ExperimentFakes.rsLoader(); + sinon.stub(loader, "setTimer"); + sinon.stub(loader, "updateRecipes").resolves(); + + Services.prefs.setBoolPref(ENABLED_PREF, false); + await loader.init(); + equal( + loader.setTimer.callCount, + 0, + `should not initialize if ${ENABLED_PREF} pref is false` + ); + + Services.prefs.setBoolPref(ENABLED_PREF, true); + await loader.init(); + ok(loader.setTimer.calledOnce, "should call .setTimer"); + ok(loader.updateRecipes.calledOnce, "should call .updateRecipes"); +}); + +add_task(async function test_init() { + const loader = ExperimentFakes.rsLoader(); + sinon.stub(loader, "setTimer"); + sinon.stub(loader, "updateRecipes").resolves(); + + Services.prefs.setBoolPref(ENABLED_PREF, false); + await loader.init(); + equal( + loader.setTimer.callCount, + 0, + `should not initialize if ${ENABLED_PREF} pref is false` + ); + + Services.prefs.setBoolPref(ENABLED_PREF, true); + await loader.init(); + ok(loader.setTimer.calledOnce, "should call .setTimer"); + ok(loader.updateRecipes.calledOnce, "should call .updateRecipes"); +}); diff --git a/toolkit/components/messaging-system/test/unit/xpcshell.ini b/toolkit/components/messaging-system/test/unit/xpcshell.ini index 1937a7626c9a..d51a077f88b3 100644 --- a/toolkit/components/messaging-system/test/unit/xpcshell.ini +++ b/toolkit/components/messaging-system/test/unit/xpcshell.ini @@ -9,3 +9,4 @@ firefox-appdir = browser [test_ExperimentStore.js] [test_SharedDataMap.js] [test_ExperimentAPI.js] +[test_RemoteSettingsExperimentLoader.js] diff --git a/toolkit/components/normandy/Normandy.jsm b/toolkit/components/normandy/Normandy.jsm index 03139fa90709..dc6f33dee3f7 100644 --- a/toolkit/components/normandy/Normandy.jsm +++ b/toolkit/components/normandy/Normandy.jsm @@ -23,6 +23,8 @@ XPCOMUtils.defineLazyModuleGetters(this, { TelemetryEvents: "resource://normandy/lib/TelemetryEvents.jsm", ExperimentManager: "resource://messaging-system/experiments/ExperimentManager.jsm", + RemoteSettingsExperimentLoader: + "resource://messaging-system/lib/RemoteSettingsExperimentLoader.jsm", }); var EXPORTED_SYMBOLS = ["Normandy"]; @@ -112,6 +114,12 @@ var Normandy = { log.error("Failed to initialize ExperimentManager:", err); } + try { + await RemoteSettingsExperimentLoader.init(); + } catch (err) { + log.error("Failed to initialize RemoteSettingsExperimentLoader:", err); + } + try { await AddonStudies.init(); } catch (err) { diff --git a/toolkit/components/normandy/actions/MessagingExperimentAction.jsm b/toolkit/components/normandy/actions/MessagingExperimentAction.jsm index 67af07904b74..76783ae8f0c0 100644 --- a/toolkit/components/normandy/actions/MessagingExperimentAction.jsm +++ b/toolkit/components/normandy/actions/MessagingExperimentAction.jsm @@ -21,6 +21,7 @@ ChromeUtils.defineModuleGetter( ); const EXPORTED_SYMBOLS = ["MessagingExperimentAction"]; +const RECIPE_SOURCE = "normandy"; class MessagingExperimentAction extends BaseStudyAction { constructor() { @@ -33,11 +34,11 @@ class MessagingExperimentAction extends BaseStudyAction { async _run(recipe) { if (recipe.arguments) { - await this.manager.onRecipe(recipe.arguments); + await this.manager.onRecipe(recipe.arguments, RECIPE_SOURCE); } } async _finalize() { - this.manager.onFinalize(); + this.manager.onFinalize(RECIPE_SOURCE); } } diff --git a/toolkit/components/normandy/test/browser/browser_actions_MessagingExperimentAction.js b/toolkit/components/normandy/test/browser/browser_actions_MessagingExperimentAction.js index d11a1084a785..44d6f77a4ceb 100644 --- a/toolkit/components/normandy/test/browser/browser_actions_MessagingExperimentAction.js +++ b/toolkit/components/normandy/test/browser/browser_actions_MessagingExperimentAction.js @@ -57,8 +57,8 @@ decorate_task( Assert.deepEqual(reportRecipe.args, [[recipe, Uptake.RECIPE_SUCCESS]]); Assert.deepEqual( onRecipeStub.args, - [[recipe.arguments]], - "should call onRecipe with recipe args" + [[recipe.arguments, "normandy"]], + "should call onRecipe with recipe args and 'normandy' source" ); } );