Bug 1765907 - PBM experiment message should override default promo message r=mviar

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

browser/actors/AboutPrivateBrowsingChild.jsm

@@ -32,6 +32,13 @@ class AboutPrivateBrowsingChild extends RemotePageChild {
     Cu.exportFunction(this.PrivateBrowsingRecordClick.bind(this), window, {
       defineAs: "PrivateBrowsingRecordClick",
     });
+    Cu.exportFunction(
+      this.PrivateBrowsingShouldHideDefault.bind(this),
+      window,
+      {
+        defineAs: "PrivateBrowsingShouldHideDefault",
+      }
+    );
     Cu.exportFunction(
       this.PrivateBrowsingExposureTelemetry.bind(this),
       window,
@@ -47,6 +54,12 @@ class AboutPrivateBrowsingChild extends RemotePageChild {
     if (experiment) {
       Services.telemetry.recordEvent("aboutprivatebrowsing", "click", source);
     }
+    return experiment;
+  }
+
+  PrivateBrowsingShouldHideDefault() {
+    const config = NimbusFeatures.pbNewtab.getAllVariables() || {};
+    return config?.content?.hideDefault;
   }
 
   PrivateBrowsingExposureTelemetry() {

browser/components/newtab/lib/ASRouter.jsm

@@ -1621,14 +1621,26 @@ class _ASRouter {
     return this.loadMessagesFromAllProviders();
   }
 
-  async sendPBNewTabMessage({ tabId }) {
+  async sendPBNewTabMessage({ tabId, hideDefault }) {
     let message = null;
 
     await this.loadMessagesFromAllProviders();
 
+    // If message has hideDefault property set to true
+    // remove from state all pb_newtab messages with type default
+    if (hideDefault) {
+      await this.setState(state => ({
+        messages: state.messages.filter(
+          m => !(m.template === "pb_newtab" && m.type === "default")
+        ),
+      }));
+    }
+
     const telemetryObject = { tabId };
     TelemetryStopwatch.start("MS_MESSAGE_REQUEST_TIME_MS", telemetryObject);
-    message = await this.handleMessageRequest({ template: "pb_newtab" });
+    message = await this.handleMessageRequest({
+      template: "pb_newtab",
+    });
     TelemetryStopwatch.finish("MS_MESSAGE_REQUEST_TIME_MS", telemetryObject);
 
     // Format urls if any are defined

browser/components/newtab/lib/OnboardingMessageProvider.jsm

@@ -97,6 +97,7 @@ const ONBOARDING_MESSAGES = () => [
   },
   {
     id: "PB_NEWTAB_FOCUS_PROMO",
+    type: "default",
     template: "pb_newtab",
     groups: ["pbNewtab"],
     content: {
@@ -123,7 +124,6 @@ const ONBOARDING_MESSAGES = () => [
               id: "FOCUS_PROMO",
               template: "multistage",
               modal: "tab",
-              metrics: "block",
               backdrop: "transparent",
               screens: [
                 {
@@ -206,6 +206,7 @@ const ONBOARDING_MESSAGES = () => [
   },
   {
     id: "PB_NEWTAB_KLAR_PROMO",
+    type: "default",
     template: "pb_newtab",
     groups: ["pbNewtab"],
     content: {
@@ -233,7 +234,6 @@ const ONBOARDING_MESSAGES = () => [
               id: "KLAR_PROMO",
               template: "multistage",
               modal: "tab",
-              metrics: "block",
               backdrop: "transparent",
               screens: [
                 {

browser/components/privatebrowsing/content/aboutPrivateBrowsing.js

@@ -119,7 +119,14 @@ async function renderPromo({
   } else if (promoButton?.action?.type === "SHOW_SPOTLIGHT") {
     linkEl.addEventListener("click", async event => {
       event.preventDefault();
-      window.PrivateBrowsingRecordClick("promo_link");
+      // Record promo click telemetry and set metrics as allow for spotlight
+      // modal opened on promo click if user is enrolled in an experiment
+      let isExperiment = window.PrivateBrowsingRecordClick("promo_link");
+      const promoButtonData = promoButton?.action?.data;
+      if (promoButtonData?.content) {
+        promoButtonData.content.metrics = isExperiment ? "allow" : "block";
+      }
+
       await RPMSendQuery("SpecialMessageActionDispatch", promoButton.action);
     });
   } else {
@@ -238,14 +245,17 @@ async function handlePromoOnPreload(message) {
 async function setupFeatureConfig() {
   let config = null;
   let message = null;
+
   try {
     config = window.PrivateBrowsingFeatureConfig();
   } catch (e) {}
+
   if (!Object.keys(config).length) {
+    let hideDefault = window.PrivateBrowsingShouldHideDefault();
     try {
       let response = await window.ASRouterMessage({
         type: "PBNEWTAB_MESSAGE_REQUEST",
-        data: {},
+        data: { hideDefault: !!hideDefault },
       });
       message = response?.message;
       config = message?.content;

browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about_nimbus_dismiss.js

@@ -10,6 +10,7 @@ add_task(async function test_experiment_messaging_system_dismiss() {
     id: `PB_NEWTAB_MESSAGING_SYSTEM_${Math.random()}`,
     template: "pb_newtab",
     content: {
+      hideDefault: true,
       promoEnabled: true,
       infoEnabled: true,
       infoBody: "fluent:about-private-browsing-info-title",
@@ -57,3 +58,64 @@ add_task(async function test_experiment_messaging_system_dismiss() {
   await BrowserTestUtils.closeWindow(win2);
   await doExperimentCleanup();
 });
+
+add_task(async function test_experiment_messaging_show_default_on_dismiss() {
+  registerCleanupFunction(() => {
+    ASRouter.resetMessageState();
+  });
+  let doExperimentCleanup = await setupMSExperimentWithMessage({
+    id: `PB_NEWTAB_MESSAGING_SYSTEM_${Math.random()}`,
+    template: "pb_newtab",
+    content: {
+      hideDefault: false,
+      promoEnabled: true,
+      infoEnabled: true,
+      infoBody: "fluent:about-private-browsing-info-title",
+      promoLinkText: "fluent:about-private-browsing-prominent-cta",
+      infoLinkUrl: "http://foo.example.com",
+      promoLinkUrl: "http://bar.example.com",
+    },
+    // Priority ensures this message is picked over the one in
+    // OnboardingMessageProvider
+    priority: 5,
+    targeting: "true",
+  });
+
+  let { win: win1, tab: tab1 } = await openTabAndWaitForRender();
+
+  await SpecialPowers.spawn(tab1, [], async function() {
+    ok(
+      content.document.querySelector(".promo"),
+      "should render the promo experiment message"
+    );
+
+    content.document.querySelector("#dismiss-btn").click();
+    info("button clicked");
+  });
+
+  let telemetryEvent = await waitForTelemetryEvent("aboutprivatebrowsing");
+
+  ok(
+    telemetryEvent[2] == "click" && telemetryEvent[3] == "dismiss_button",
+    "recorded the dismiss button click"
+  );
+
+  let { win: win2, tab: tab2 } = await openTabAndWaitForRender();
+
+  await SpecialPowers.spawn(tab2, [], async function() {
+    const promoHeader = content.document.getElementById("promo-header");
+    ok(
+      content.document.querySelector(".promo"),
+      "should render the default promo message after dismissing experiment promo"
+    );
+    is(
+      promoHeader.textContent,
+      "Next-level privacy on mobile",
+      "Correct default values are shown"
+    );
+  });
+
+  await BrowserTestUtils.closeWindow(win1);
+  await BrowserTestUtils.closeWindow(win2);
+  await doExperimentCleanup();
+});

browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about_nimbus_impressions.js

@@ -10,11 +10,17 @@
  */
 
 add_task(async function test_experiment_messaging_system_impressions() {
+  registerCleanupFunction(() => {
+    ASRouter.resetMessageState();
+  });
   const LOCALE = Services.locale.appLocaleAsBCP47;
+  let experimentId = `pb_newtab_${Math.random()}`;
+
   let doExperimentCleanup = await setupMSExperimentWithMessage({
-    id: `PB_NEWTAB_MESSAGING_SYSTEM_${Math.random()}`,
+    id: experimentId,
     template: "pb_newtab",
     content: {
+      hideDefault: true,
       promoEnabled: true,
       infoEnabled: true,
       infoBody: "fluent:about-private-browsing-info-title",
@@ -43,20 +49,19 @@ add_task(async function test_experiment_messaging_system_impressions() {
     );
   });
 
-  await waitForTelemetryEvent("normandy");
+  let event = await waitForTelemetryEvent("normandy", experimentId);
-  TelemetryTestUtils.assertEvents(
+
-    [
+  ok(
-      {
+    event[1] == "normandy" &&
-        method: "expose",
+      event[2] == "expose" &&
-        extra: {
+      event[3] == "nimbus_experiment" &&
-          featureId: "pbNewtab",
+      event[4].includes(experimentId) &&
-        },
+      event[5].featureId == "pbNewtab",
-      },
+    "recorded telemetry for expose"
-    ],
-    { category: "normandy" },
-    { process: "content" }
   );
 
+  Services.telemetry.clearEvents();
+
   let { win: win2, tab: tab2 } = await openTabAndWaitForRender();
 
   await SpecialPowers.spawn(tab2, [LOCALE], async function(locale) {
@@ -67,20 +72,19 @@ add_task(async function test_experiment_messaging_system_impressions() {
     );
   });
 
-  await waitForTelemetryEvent("normandy");
+  let event2 = await waitForTelemetryEvent("normandy", experimentId);
-  TelemetryTestUtils.assertEvents(
+
-    [
+  ok(
-      {
+    event2[1] == "normandy" &&
-        method: "expose",
+      event2[2] == "expose" &&
-        extra: {
+      event2[3] == "nimbus_experiment" &&
-          featureId: "pbNewtab",
+      event2[4].includes(experimentId) &&
-        },
+      event2[5].featureId == "pbNewtab",
-      },
+    "recorded telemetry for expose"
-    ],
-    { category: "normandy" },
-    { process: "content" }
   );
 
+  Services.telemetry.clearEvents();
+
   let { win: win3, tab: tab3 } = await openTabAndWaitForRender();
 
   await SpecialPowers.spawn(tab3, [], async function() {
@@ -90,10 +94,15 @@ add_task(async function test_experiment_messaging_system_impressions() {
       "should no longer render the experiment message after 2 impressions"
     );
   });
-  TelemetryTestUtils.assertNumberOfEvents(0, {
+
-    category: "normandy",
+  let event3 = Services.telemetry.snapshotEvents(
-    method: "expose",
+    Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
-  });
+    false
+  ).content;
+
+  Assert.equal(!!event3, false, "Should not have promo expose");
+
+  Services.telemetry.clearEvents();
 
   await BrowserTestUtils.closeWindow(win1);
   await BrowserTestUtils.closeWindow(win2);

browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about_nimbus_messaging.js

@@ -15,6 +15,7 @@ add_task(async function test_experiment_messaging_system() {
     id: "PB_NEWTAB_MESSAGING_SYSTEM",
     template: "pb_newtab",
     content: {
+      hideDefault: true,
       promoEnabled: true,
       infoEnabled: true,
       infoBody: "fluent:about-private-browsing-info-title",
@@ -28,22 +29,6 @@ add_task(async function test_experiment_messaging_system() {
     targeting: "true",
   });
 
-  await TestUtils.waitForCondition(() => {
-    Services.telemetry.clearEvents();
-    let events = Services.telemetry.snapshotEvents(
-      Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
-      true
-    ).content;
-    info("waiting for events to clear, but:");
-    info(JSON.stringify(events));
-    return !events || !events.length;
-  }, "Waiting for telemetry events to get cleared");
-  Services.telemetry.snapshotEvents(
-    Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
-    true
-  );
-  Services.telemetry.clearEvents();
-
   let { win, tab } = await openTabAndWaitForRender();
 
   await SpecialPowers.spawn(tab, [LOCALE], async function(locale) {
@@ -80,23 +65,6 @@ add_task(async function test_experiment_messaging_system() {
     );
   });
 
-  await waitForTelemetryEvent("normandy");
-
-  TelemetryTestUtils.assertEvents(
-    [
-      {
-        method: "expose",
-        extra: {
-          featureId: "pbNewtab",
-        },
-      },
-    ],
-    { category: "normandy" },
-    { process: "content" }
-  );
-
-  Services.telemetry.clearEvents();
-
   await BrowserTestUtils.closeWindow(win);
   await doExperimentCleanup();
 });

browser/components/privatebrowsing/test/browser/head.js

@@ -112,23 +112,30 @@ function _initTest() {
   );
 }
 
-function waitForTelemetryEvent(category) {
+function waitForTelemetryEvent(category, value) {
   info("waiting for telemetry event");
   return TestUtils.waitForCondition(() => {
     let events = Services.telemetry.snapshotEvents(
       Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
       false
     ).content;
+
     if (!events) {
       return null;
     }
     events = events.filter(e => e[1] == category);
     info(JSON.stringify(events));
+
+    // Check for experimentId passed as value
+    // if exists return events only for specific experimentId
+    if (value) {
+      events = events.filter(e => e[4].includes(value));
+    }
     if (events.length) {
       return events[0];
     }
     return null;
-  }, "waiting for telemetry event");
+  }, "wait and retrieve telemetry event");
 }
 
 async function setupMSExperimentWithMessage(message) {
@@ -144,12 +151,6 @@ async function setupMSExperimentWithMessage(message) {
   });
   await SpecialPowers.pushPrefEnv({
     set: [
-      // Disable onboarding, so we don't have default stuff showing up on the
-      // private browsing surface.
-      [
-        "browser.newtabpage.activity-stream.asrouter.providers.onboarding",
-        '{"id":"onboarding","type":"local","localProvider":"OnboardingMessageProvider","enabled":false,"exclude":[]}',
-      ],
       [
         "browser.newtabpage.activity-stream.asrouter.providers.messaging-experiments",
         '{"id":"messaging-experiments","enabled":true,"type":"remote-experiments","messageGroups":["pbNewtab"],"updateCycleInMs":0}',

toolkit/components/nimbus/test/NimbusTestUtils.jsm

@@ -222,25 +222,29 @@ const ExperimentFakes = {
     { manager = ExperimentManager } = {}
   ) {
     await manager.store.ready();
-    let recipe = this.recipe(
+    // Use id passed in featureConfig value to compute experimentId
-      `${featureConfig.featureId}-experiment-${Math.random()}`,
+    // This help filter telemetry events (such as expose) in race conditions when telemetry
-      {
+    // from multiple experiments with same featureId co-exist in snapshot
-        bucketConfig: {
+    let experimentId = `${featureConfig.featureId}${
-          namespace: "mstest-utils",
+      featureConfig?.value?.id ? "-" + featureConfig?.value?.id : ""
-          randomizationUnit: "normandy_id",
+    }-experiment-${Math.random()}`;
-          start: 0,
+
-          count: 1000,
+    let recipe = this.recipe(experimentId, {
-          total: 1000,
+      bucketConfig: {
+        namespace: "mstest-utils",
+        randomizationUnit: "normandy_id",
+        start: 0,
+        count: 1000,
+        total: 1000,
+      },
+      branches: [
+        {
+          slug: "control",
+          ratio: 1,
+          features: [featureConfig],
         },
-        branches: [
+      ],
-          {
+    });
-            slug: "control",
-            ratio: 1,
-            features: [featureConfig],
-          },
-        ],
-      }
-    );
     let {
       enrollmentPromise,
       doExperimentCleanup,