Bug 1498940 - Reflect study opt-out in about:studies when Normandy is generally enabled. r=Gijs
☠☠ backed out by d08c28ed42f1 ☠ ☠
authorMichael Cooper <mcooper@mozilla.com>
Thu, 25 Oct 2018 17:20:25 +0000
changeset 491359 f0ed99b29aafb5fd7f613a000634b53927af4259
parent 491358 ae8cdf156f9a61151a9bbcec47b9c6f5a4a05f33
child 491360 ec90e4f6f2a2ff225bc8e49975deb5d0cc31bce7
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersGijs
bugs1498940
milestone65.0a1
Bug 1498940 - Reflect study opt-out in about:studies when Normandy is generally enabled. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D9738
toolkit/components/normandy/content/AboutPages.jsm
toolkit/components/normandy/lib/RecipeRunner.jsm
toolkit/components/normandy/test/browser/browser_about_studies.js
--- a/toolkit/components/normandy/content/AboutPages.jsm
+++ b/toolkit/components/normandy/content/AboutPages.jsm
@@ -10,16 +10,17 @@ ChromeUtils.defineModuleGetter(this, "Ad
 ChromeUtils.defineModuleGetter(this, "AddonStudyAction", "resource://normandy/actions/AddonStudyAction.jsm");
 ChromeUtils.defineModuleGetter(this, "CleanupManager", "resource://normandy/lib/CleanupManager.jsm");
 ChromeUtils.defineModuleGetter(this, "PreferenceExperiments", "resource://normandy/lib/PreferenceExperiments.jsm");
 ChromeUtils.defineModuleGetter(this, "RecipeRunner", "resource://normandy/lib/RecipeRunner.jsm");
 
 var EXPORTED_SYMBOLS = ["AboutPages"];
 
 const SHIELD_LEARN_MORE_URL_PREF = "app.normandy.shieldLearnMoreUrl";
+XPCOMUtils.defineLazyPreferenceGetter(this, "gOptOutStudiesEnabled", "app.shield.optoutstudies.enabled");
 
 
 /**
  * Class for managing an about: page that Normandy provides. Adapted from
  * browser/components/pocket/content/AboutPocket.jsm.
  *
  * @implements nsIFactory
  * @implements nsIAboutModule
@@ -187,20 +188,19 @@ XPCOMUtils.defineLazyGetter(this.AboutPa
      * content processes safely.
      *
      * @param {<browser>} target
      *   XUL <browser> element for the tab containing the about:studies page
      *   that requested a study list.
      */
     sendStudiesEnabled(target) {
       RecipeRunner.checkPrefs();
+      const studiesEnabled = RecipeRunner.enabled && gOptOutStudiesEnabled;
       try {
-        target.messageManager.sendAsyncMessage("Shield:ReceiveStudiesEnabled", {
-          studiesEnabled: RecipeRunner.enabled,
-        });
+        target.messageManager.sendAsyncMessage("Shield:ReceiveStudiesEnabled", { studiesEnabled });
       } catch (err) {
         // The child process might be gone, so no need to throw here.
         Cu.reportError(err);
       }
     },
 
     /**
      * Disable an active add-on study and remove its add-on.
@@ -232,17 +232,12 @@ XPCOMUtils.defineLazyGetter(this.AboutPa
     openDataPreferences() {
       const browserWindow = Services.wm.getMostRecentWindow("navigator:browser");
       browserWindow.openPreferences("privacy-reports", {origin: "aboutStudies"});
     },
 
     getShieldLearnMoreHref() {
       return Services.urlFormatter.formatURLPref(SHIELD_LEARN_MORE_URL_PREF);
     },
-
-    getStudiesEnabled() {
-      RecipeRunner.checkPrefs();
-      return RecipeRunner.enabled;
-    },
   });
 
   return aboutStudies;
 });
--- a/toolkit/components/normandy/lib/RecipeRunner.jsm
+++ b/toolkit/components/normandy/lib/RecipeRunner.jsm
@@ -153,17 +153,22 @@ var RecipeRunner = {
 
     if (!Services.policies.isAllowed("Shield")) {
       log.debug("Disabling Shield because it's blocked by policy.");
       this.disable();
       return;
     }
 
     const apiUrl = Services.prefs.getCharPref(API_URL_PREF);
-    if (!apiUrl || !apiUrl.startsWith("https://")) {
+    if (!apiUrl) {
+      log.warn(`Disabling Shield because ${API_URL_PREF} is not set.`);
+      this.disable();
+      return;
+    }
+    if (!apiUrl.startsWith("https://")) {
       log.warn(`Disabling Shield because ${API_URL_PREF} is not an HTTPS url: ${apiUrl}.`);
       this.disable();
       return;
     }
 
     log.debug(`Enabling Shield`);
     this.enable();
   },
--- a/toolkit/components/normandy/test/browser/browser_about_studies.js
+++ b/toolkit/components/normandy/test/browser/browser_about_studies.js
@@ -230,17 +230,17 @@ decorate_task(
     );
   }
 );
 
 // Test that a message is shown when no studies have been run
 decorate_task(
   AddonStudies.withStudies([]),
   withAboutStudies,
-  async function testStudyListing(studies, browser) {
+  async function testStudyListingNoStudies(studies, browser) {
     await ContentTask.spawn(browser, null, async () => {
       const doc = content.document;
       await ContentTaskUtils.waitForCondition(() => doc.querySelectorAll(".study-list-info").length);
       const studyRows = doc.querySelectorAll(".study-list .study");
       is(studyRows.length, 0, "There should be no studies");
       is(
         doc.querySelector(".study-list-info").textContent,
         "You have not participated in any studies.",
@@ -248,17 +248,32 @@ decorate_task(
       );
     });
   }
 );
 
 // Test that the message shown when studies are disabled and studies exist
 decorate_task(
   withAboutStudies,
-  async function testStudyListing(browser) {
+  AddonStudies.withStudies([
+    addonStudyFactory({
+      name: "A Fake Add-on Study",
+      active: false,
+      description: "A fake description",
+      studyStartDate: new Date(2018, 0, 4),
+    }),
+  ]),
+  PreferenceExperiments.withMockExperiments([
+    preferenceStudyFactory({
+      name: "B Fake Preference Study",
+      lastSeen: new Date(2018, 0, 5),
+      expired: true,
+    }),
+  ]),
+  async function testStudyListingDisabled(browser, addonStudies, preferenceStudies) {
     try {
       RecipeRunner.disable();
 
       await ContentTask.spawn(browser, null, async () => {
         const doc = content.document;
         await ContentTaskUtils.waitForCondition(() => doc.querySelector(".info-box-content > span"));
 
         is(
@@ -269,8 +284,39 @@ decorate_task(
       });
     } finally {
       // reset RecipeRunner.enabled
       RecipeRunner.checkPrefs();
     }
   }
 );
 
+// Test for bug 1498940 - detects studies disabled when only study opt-out is set
+decorate_task(
+  withPrefEnv({
+    set: [
+      ["datareporting.healthreport.uploadEnabled", true],
+      ["app.normandy.api_url", "https://example.com"],
+      ["app.shield.optoutstudies.enabled", false],
+    ],
+  }),
+  withAboutStudies,
+  AddonStudies.withStudies([]),
+  PreferenceExperiments.withMockExperiments([]),
+  async function testStudyListingStudiesOptOut(browser) {
+    RecipeRunner.checkPrefs();
+    ok(
+      RecipeRunner.enabled,
+      "RecipeRunner should be enabled as a Precondition",
+    );
+
+    await ContentTask.spawn(browser, null, async () => {
+      const doc = content.document;
+      await ContentTaskUtils.waitForCondition(() => doc.querySelector(".info-box-content > span").textContent);
+
+      is(
+        doc.querySelector(".info-box-content > span").textContent,
+        "This is a list of studies that you have participated in. No new studies will run.",
+        "A message is shown when studies are disabled",
+      );
+    });
+  }
+);