Bug 1692228 - Send exposure for New Tab Feature r=andreio

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

browser/actors/AboutNewTabChild.jsm

@@ -18,6 +18,11 @@ const { PrivateBrowsingUtils } = ChromeUtils.import(
   "resource://gre/modules/PrivateBrowsingUtils.jsm"
 );
 
+XPCOMUtils.defineLazyModuleGetters(this, {
+  ExperimentFeature:
+    "resource://messaging-system/experiments/ExperimentAPI.jsm",
+});
+
 XPCOMUtils.defineLazyPreferenceGetter(
   this,
   "ACTIVITY_STREAM_DEBUG",
@@ -26,12 +31,15 @@ XPCOMUtils.defineLazyPreferenceGetter(
 );
 
 XPCOMUtils.defineLazyGetter(this, "awExperimentFeature", () => {
-  const { ExperimentFeature } = ChromeUtils.import(
-    "resource://messaging-system/experiments/ExperimentAPI.jsm"
-  );
   return new ExperimentFeature("aboutwelcome");
 });
 
+// Note: newtab feature info is currently being loaded in PrefsFeed.jsm,
+// But we're recording exposure events here.
+XPCOMUtils.defineLazyGetter(this, "newtabExperimentFeature", () => {
+  return new ExperimentFeature("newtab");
+});
+
 class AboutNewTabChild extends JSWindowActorChild {
   handleEvent(event) {
     if (event.type == "DOMContentLoaded") {
@@ -83,6 +91,11 @@ class AboutNewTabChild extends JSWindowActorChild {
             PrivateBrowsingUtils.permanentPrivateBrowsing))
       ) {
         this.sendAsyncMessage("DefaultBrowserNotification");
+
+        // Send an exposure event to record when we have an experiment active
+        newtabExperimentFeature
+          .ready()
+          .then(() => newtabExperimentFeature.recordExposureEvent());
       }
     }
   }

browser/components/newtab/lib/PrefsFeed.jsm

@@ -83,10 +83,7 @@ this.PrefsFeed = class PrefsFeed {
    * Handler for when experiment data updates.
    */
   onExperimentUpdated(event, reason) {
-    const value =
-      aboutNewTabFeature.getValue({
-        sendExposurePing: false,
-      }) || {};
+    const value = aboutNewTabFeature.getValue() || {};
     this.store.dispatch(
       ac.BroadcastToContent({
         type: at.PREF_CHANGED,
@@ -164,10 +161,7 @@ this.PrefsFeed = class PrefsFeed {
     });
 
     // Add experiment values and default values
-    values.featureConfig =
-      aboutNewTabFeature.getValue({
-        sendExposurePing: false,
-      }) || {};
+    values.featureConfig = aboutNewTabFeature.getValue() || {};
 
     this._setBoolPref(values, "newNewtabExperience.enabled", false);
     this._setBoolPref(values, "customizationMenu.enabled", false);

toolkit/components/messaging-system/experiments/ExperimentAPI.jsm

@@ -89,13 +89,13 @@ const ExperimentAPI = {
 
   /**
    * Returns an experiment, including all its metadata
-   * Sends exposure ping
+   * Sends exposure event
    *
    * @param {{slug?: string, featureId?: string}} options slug = An experiment identifier
    * or feature = a stable identifier for a type of experiment
    * @returns {{slug: string, active: bool}} A matching experiment if one is found.
    */
-  getExperiment({ slug, featureId, sendExposurePing } = {}) {
+  getExperiment({ slug, featureId, sendExposureEvent } = {}) {
     if (!slug && !featureId) {
       throw new Error(
         "getExperiment(options) must include a slug or a feature."
@@ -115,7 +115,7 @@ const ExperimentAPI = {
       return {
         slug: experimentData.slug,
         active: experimentData.active,
-        branch: this.activateBranch({ featureId, sendExposurePing }),
+        branch: this.activateBranch({ featureId, sendExposureEvent }),
       };
     }
 
@@ -124,7 +124,7 @@ const ExperimentAPI = {
 
   /**
    * Return experiment slug its status and the enrolled branch slug
-   * Does NOT send exposure ping because you only have access to the slugs
+   * Does NOT send exposure event because you only have access to the slugs
    */
   getExperimentMetaData({ slug, featureId }) {
     if (!slug && !featureId) {
@@ -156,10 +156,10 @@ const ExperimentAPI = {
 
   /**
    * Return FeatureConfig from first active experiment where it can be found
-   * @param {{slug: string, featureId: string, sendExposurePing: bool}}
+   * @param {{slug: string, featureId: string, sendExposureEvent: bool}}
    * @returns {Branch | null}
    */
-  activateBranch({ slug, featureId, sendExposurePing }) {
+  activateBranch({ slug, featureId, sendExposureEvent }) {
     let experiment = null;
     try {
       if (slug) {
@@ -175,7 +175,7 @@ const ExperimentAPI = {
       return null;
     }
 
-    if (sendExposurePing) {
+    if (sendExposureEvent) {
       this.recordExposureEvent({
         experimentSlug: experiment.slug,
         branchSlug: experiment.branch.slug,
@@ -345,16 +345,20 @@ class ExperimentFeature {
     });
   }
 
+  ready() {
+    return ExperimentAPI.ready();
+  }
+
   /**
    * Lookup feature in active experiments and return enabled.
    * By default, this will send an exposure event.
-   * @param {{sendExposurePing: boolean, defaultValue?: any}} options
+   * @param {{sendExposureEvent: boolean, defaultValue?: any}} options
    * @returns {obj} The feature value
    */
-  isEnabled({ sendExposurePing, defaultValue = null } = {}) {
+  isEnabled({ sendExposureEvent, defaultValue = null } = {}) {
     const branch = ExperimentAPI.activateBranch({
       featureId: this.featureId,
-      sendExposurePing,
+      sendExposureEvent,
     });
 
     // First, try to return an experiment value if it exists.
@@ -374,13 +378,13 @@ class ExperimentFeature {
   /**
    * Lookup feature in active experiments and return value.
    * By default, this will send an exposure event.
-   * @param {{sendExposurePing: boolean, defaultValue?: any}} options
+   * @param {{sendExposureEvent: boolean, defaultValue?: any}} options
    * @returns {obj} The feature value
    */
-  getValue({ sendExposurePing, defaultValue = null } = {}) {
+  getValue({ sendExposureEvent, defaultValue = null } = {}) {
     const branch = ExperimentAPI.activateBranch({
       featureId: this.featureId,
-      sendExposurePing,
+      sendExposureEvent,
     });
     if (branch?.feature?.value) {
       return branch.feature.value;
@@ -389,6 +393,13 @@ class ExperimentFeature {
     return this.defaultPrefValues.value || defaultValue;
   }
 
+  recordExposureEvent() {
+    ExperimentAPI.activateBranch({
+      featureId: this.featureId,
+      sendExposureEvent: true,
+    });
+  }
+
   onUpdate(callback) {
     ExperimentAPI._store._onFeatureUpdate(this.featureId, callback);
   }

toolkit/components/messaging-system/test/unit/test_ExperimentAPI.js

@@ -158,7 +158,7 @@ add_task(async function test_getExperiment_feature() {
 
   Assert.ok(exposureStub.notCalled, "Not called by default");
 
-  ExperimentAPI.getExperiment({ featureId: "cfr", sendExposurePing: true });
+  ExperimentAPI.getExperiment({ featureId: "cfr", sendExposureEvent: true });
 
   Assert.ok(exposureStub.calledOnce, "Called explicitly.");
 
@@ -409,7 +409,7 @@ add_task(async function test_activateBranch_activationEvent() {
     "Exposure is not sent by default by activateBranch"
   );
 
-  ExperimentAPI.activateBranch({ featureId: "green", sendExposurePing: true });
+  ExperimentAPI.activateBranch({ featureId: "green", sendExposureEvent: true });
 
   Assert.equal(stub.callCount, 1, "Called by doing activateBranch");
   Assert.deepEqual(
@@ -469,7 +469,7 @@ add_task(async function test_activateBranch_noActivationEvent() {
   // Call activateBranch to trigger an activation event
   ExperimentAPI.activateBranch({ featureId: "green" });
 
-  Assert.equal(stub.callCount, 0, "Not called: sendExposurePing is false");
+  Assert.equal(stub.callCount, 0, "Not called: sendExposureEvent is false");
   sandbox.restore();
 });
 

toolkit/components/messaging-system/test/unit/test_ExperimentAPI_ExperimentFeature.js

@@ -64,6 +64,39 @@ add_task(async function test_feature_manifest_is_valid() {
   });
 });
 
+/**
+ * # ExperimentFeature.getValue
+ */
+add_task(async function test_ExperimentFeature_ready() {
+  const { sandbox, manager } = await setupForExperimentFeature();
+
+  const featureInstance = new ExperimentFeature("foo", FAKE_FEATURE_MANIFEST);
+
+  const expected = ExperimentFakes.experiment("anexperiment", {
+    branch: {
+      slug: "treatment",
+      feature: {
+        featureId: "foo",
+        enabled: true,
+        value: { whoa: true },
+      },
+    },
+  });
+
+  manager.store.addExperiment(expected);
+
+  await featureInstance.ready();
+
+  Assert.deepEqual(
+    featureInstance.getValue(),
+    { whoa: true },
+    "should return getValue after waiting on ready"
+  );
+
+  Services.prefs.clearUserPref("testprefbranch.value");
+  sandbox.restore();
+});
+
 /**
  * # ExperimentFeature.getValue
  */
@@ -196,7 +229,7 @@ add_task(
 
     Assert.ok(exposureSpy.notCalled, "should emit exposure by default event");
 
-    featureInstance.isEnabled({ sendExposurePing: true });
+    featureInstance.isEnabled({ sendExposureEvent: true });
 
     Assert.ok(exposureSpy.calledOnce, "should emit exposure event");
 
@@ -225,12 +258,50 @@ add_task(async function test_ExperimentFeature_isEnabled_no_exposure() {
 
   const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent");
 
-  const actual = featureInstance.isEnabled({ sendExposurePing: false });
+  const actual = featureInstance.isEnabled({ sendExposureEvent: false });
 
   Assert.deepEqual(actual, false, "should return feature as disabled");
   Assert.ok(
     exposureSpy.notCalled,
-    "should not emit an exposure event when options = { sendExposurePing: false}"
+    "should not emit an exposure event when options = { sendExposureEvent: false}"
+  );
+
+  sandbox.restore();
+});
+
+add_task(async function test_record_exposure_event() {
+  const { sandbox, manager } = await setupForExperimentFeature();
+
+  const featureInstance = new ExperimentFeature("foo", FAKE_FEATURE_MANIFEST);
+  const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent");
+  sandbox.stub(ExperimentAPI, "_store").get(() => manager.store);
+
+  featureInstance.recordExposureEvent();
+
+  Assert.ok(
+    exposureSpy.notCalled,
+    "should not emit an exposure event when no experiment is active"
+  );
+
+  manager.store.addExperiment(
+    ExperimentFakes.experiment("blah", {
+      featureIds: ["foo"],
+      branch: {
+        slug: "treatment",
+        feature: {
+          featureId: "foo",
+          enabled: false,
+          value: null,
+        },
+      },
+    })
+  );
+
+  featureInstance.recordExposureEvent();
+
+  Assert.ok(
+    exposureSpy.calledOnce,
+    "should emit an exposure event when there is an experiment"
   );
 
   sandbox.restore();