Backed out 2 changesets (bug 1501877, bug 1498940) for failing browser_about_studies.js CLOSED TREE
authorCiure Andrei <aciure@mozilla.com>
Thu, 25 Oct 2018 21:24:45 +0300
changeset 443022 d08c28ed42f1bef0b77d38064759b797f377a998
parent 443021 821f982805ab2fbc2e77e0673eedf3d2a163fe45
child 443023 9397caac80907d5f9a1e79fbcc0774f742efd6fa
push id71755
push useraciure@mozilla.com
push dateThu, 25 Oct 2018 18:25:11 +0000
treeherderautoland@d08c28ed42f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1501877, 1498940
milestone65.0a1
backs outf0ed99b29aafb5fd7f613a000634b53927af4259
ae8cdf156f9a61151a9bbcec47b9c6f5a4a05f33
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 2 changesets (bug 1501877, bug 1498940) for failing browser_about_studies.js CLOSED TREE Backed out changeset f0ed99b29aaf (bug 1498940) Backed out changeset ae8cdf156f9a (bug 1501877)
toolkit/components/normandy/content/AboutPages.jsm
toolkit/components/normandy/content/about-studies/about-studies.js
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,17 +10,16 @@ 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
@@ -188,19 +187,20 @@ 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 });
+        target.messageManager.sendAsyncMessage("Shield:ReceiveStudiesEnabled", {
+          studiesEnabled: RecipeRunner.enabled,
+        });
       } 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,12 +232,17 @@ 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/content/about-studies/about-studies.js
+++ b/toolkit/components/normandy/content/about-studies/about-studies.js
@@ -145,25 +145,25 @@ class StudyList extends React.Component 
     }
 
     activeStudies.sort((a, b) => b.sortDate - a.sortDate);
     inactiveStudies.sort((a, b) => b.sortDate - a.sortDate);
 
     return (
       r("div", {},
         r("h2", {}, translations.activeStudiesList),
-        r("ul", { className: "study-list active-study-list" },
+        r("ul", { className: "study-list" },
           activeStudies.map(study => (
             study.type === "addon"
             ? r(AddonStudyListItem, { key: study.name, study, translations })
             : r(PreferenceStudyListItem, { key: study.name, study, translations })
           )),
         ),
         r("h2", {}, translations.completedStudiesList),
-        r("ul", { className: "study-list inactive-study-list" },
+        r("ul", { className: "study-list" },
           inactiveStudies.map(study => (
             study.type === "addon"
             ? r(AddonStudyListItem, { key: study.name, study, translations })
             : r(PreferenceStudyListItem, { key: study.name, study, translations })
           )),
         ),
       )
     );
--- a/toolkit/components/normandy/lib/RecipeRunner.jsm
+++ b/toolkit/components/normandy/lib/RecipeRunner.jsm
@@ -153,22 +153,17 @@ 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) {
-      log.warn(`Disabling Shield because ${API_URL_PREF} is not set.`);
-      this.disable();
-      return;
-    }
-    if (!apiUrl.startsWith("https://")) {
+    if (!apiUrl || !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
@@ -66,17 +66,16 @@ decorate_task(
       "about:preferences#privacy",
       "Clicking Update Preferences opens the privacy section of the new about:preferences.",
     );
 
     BrowserTestUtils.removeTab(tab);
   }
 );
 
-// Test that the study listing shows studies in the proper order and grouping
 decorate_task(
   AddonStudies.withStudies([
     addonStudyFactory({
       name: "A Fake Add-on Study",
       active: true,
       description: "A fake description",
       studyStartDate: new Date(2018, 0, 4),
     }),
@@ -195,29 +194,29 @@ decorate_task(
       );
       ok(
         !inactivePrefStudy.querySelector(".remove-button"),
         "Inactive studies do not show a remove button"
       );
 
       activeAddonStudy.querySelector(".remove-button").click();
       await ContentTaskUtils.waitForCondition(() => (
-        getStudyRow(doc, addonStudies[0].name).matches(".study.disabled")
+        getStudyRow(doc, addonStudies[0].name).matches(".study--disabled")
       ));
       ok(
-        getStudyRow(doc, addonStudies[0].name).matches(".study.disabled"),
+        getStudyRow(doc, addonStudies[0].name).matches(".study--disabled"),
         "Clicking the remove button updates the UI to show that the study has been disabled."
       );
 
       activePrefStudy.querySelector(".remove-button").click();
       await ContentTaskUtils.waitForCondition(() => (
-        getStudyRow(doc, prefStudies[0].name).matches(".study.disabled")
+        getStudyRow(doc, prefStudies[0].name).matches(".study--disabled")
       ));
       ok(
-        getStudyRow(doc, prefStudies[0].name).matches(".study.disabled"),
+        getStudyRow(doc, prefStudies[0].name).matches(".study--disabled"),
         "Clicking the remove button updates the UI to show that the study has been disabled."
       );
     });
 
     const updatedAddonStudy = await AddonStudies.get(addonStudies[0].recipeId);
     ok(
       !updatedAddonStudy.active,
       "Clicking the remove button marks addon studies as inactive in storage."
@@ -226,54 +225,37 @@ decorate_task(
     const updatedPrefStudy = await PreferenceExperiments.get(prefStudies[0].name);
     ok(
       updatedPrefStudy.expired,
       "Clicking the remove button marks preference studies as expired in storage."
     );
   }
 );
 
-// Test that a message is shown when no studies have been run
 decorate_task(
   AddonStudies.withStudies([]),
   withAboutStudies,
-  async function testStudyListingNoStudies(studies, browser) {
+  async function testStudyListing(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.",
         "A message is shown when no studies exist",
       );
     });
   }
 );
 
-// Test that the message shown when studies are disabled and studies exist
 decorate_task(
   withAboutStudies,
-  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) {
+  async function testStudyListing(browser) {
     try {
       RecipeRunner.disable();
 
       await ContentTask.spawn(browser, null, async () => {
         const doc = content.document;
         await ContentTaskUtils.waitForCondition(() => doc.querySelector(".info-box-content > span"));
 
         is(
@@ -282,41 +264,9 @@ decorate_task(
           "A message is shown when studies are disabled",
         );
       });
     } 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",
-      );
-    });
-  }
-);
+).only();