Bug 1435838 - Show a notice on about:studies if Studies are disabled r?Gijs r?glasserc draft
authorMike Cooper <mcooper@mozilla.com>
Thu, 08 Feb 2018 14:49:05 -0800
changeset 752798 edb65e66b07f
parent 752727 c5120bcaf7bd
push id98395
push userbmo:mcooper@mozilla.com
push dateFri, 09 Feb 2018 00:26:00 +0000
reviewersGijs, glasserc
bugs1435838
milestone60.0a1
Bug 1435838 - Show a notice on about:studies if Studies are disabled r?Gijs r?glasserc MozReview-Commit-ID: K6XCegnhu4b
browser/extensions/shield-recipe-client/content/AboutPages.jsm
browser/extensions/shield-recipe-client/content/about-studies/common.js
browser/extensions/shield-recipe-client/content/about-studies/shield-studies.js
browser/extensions/shield-recipe-client/content/shield-content-frame.js
browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm
browser/extensions/shield-recipe-client/test/browser/browser_about_studies.js
--- a/browser/extensions/shield-recipe-client/content/AboutPages.jsm
+++ b/browser/extensions/shield-recipe-client/content/AboutPages.jsm
@@ -8,16 +8,19 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 ChromeUtils.defineModuleGetter(
   this, "CleanupManager", "resource://shield-recipe-client/lib/CleanupManager.jsm",
 );
 ChromeUtils.defineModuleGetter(
   this, "AddonStudies", "resource://shield-recipe-client/lib/AddonStudies.jsm",
 );
+ChromeUtils.defineModuleGetter(
+  this, "RecipeRunner", "resource://shield-recipe-client/lib/RecipeRunner.jsm",
+);
 
 this.EXPORTED_SYMBOLS = ["AboutPages"];
 
 const SHIELD_LEARN_MORE_URL_PREF = "extensions.shield-recipe-client.shieldLearnMoreUrl";
 
 // Due to bug 1051238 frame scripts are cached forever, so we can't update them
 // as a restartless add-on. The Math.random() is the work around for this.
 const PROCESS_SCRIPT = (
@@ -207,12 +210,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/browser/extensions/shield-recipe-client/content/about-studies/common.js
+++ b/browser/extensions/shield-recipe-client/content/about-studies/common.js
@@ -117,18 +117,19 @@ class RemoteValue {
     }
   }
 }
 
 /**
  * Collection of RemoteValue instances used within the page.
  */
 const remoteValues = {
-  StudyList: new RemoteValue("StudyList", []),
-  ShieldLearnMoreHref: new RemoteValue("ShieldLearnMoreHref", ""),
+  studyList: new RemoteValue("StudyList"),
+  shieldLearnMoreHref: new RemoteValue("ShieldLearnMoreHref"),
+  studiesEnabled: new RemoteValue("StudiesEnabled"),
 };
 
 /**
  * Dispatches a page event to the privileged frame script for this tab.
  * @param {String} action
  * @param {Object} data
  */
 function sendPageEvent(action, data) {
--- a/browser/extensions/shield-recipe-client/content/about-studies/shield-studies.js
+++ b/browser/extensions/shield-recipe-client/content/about-studies/shield-studies.js
@@ -1,44 +1,20 @@
 /* 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";
 /* global classnames FxButton InfoBox PropTypes r React remoteValues sendPageEvent */
 
 window.ShieldStudies = class ShieldStudies extends React.Component {
-  constructor(props) {
-    super(props);
-    this.state = {
-      learnMoreHref: null,
-    };
-  }
-
-  componentDidMount() {
-    remoteValues.ShieldLearnMoreHref.subscribe(this);
-  }
-
-  componentWillUnmount() {
-    remoteValues.ShieldLearnMoreHref.unsubscribe(this);
-  }
-
-  receiveRemoteValue(name, value) {
-    this.setState({
-      learnMoreHref: value,
-    });
-  }
 
   render() {
     return (
       r("div", {},
-        r(InfoBox, {},
-          r("span", {}, "What's this? Firefox may install and run studies from time to time."),
-          r("a", {id: "shield-studies-learn-more", href: this.state.learnMoreHref}, "Learn more"),
-          r(UpdatePreferencesButton, {}, "Update Preferences"),
-        ),
+        r(WhatsThisBox),
         r(StudyList),
       )
     );
   }
 };
 
 class UpdatePreferencesButton extends React.Component {
   constructor(props) {
@@ -60,48 +36,67 @@ class UpdatePreferencesButton extends Re
     );
   }
 }
 
 class StudyList extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      studies: [],
+      studies: null,
     };
   }
 
   componentDidMount() {
-    remoteValues.StudyList.subscribe(this);
+    remoteValues.studyList.subscribe(this);
   }
 
   componentWillUnmount() {
-    remoteValues.StudyList.unsubscribe(this);
+    remoteValues.studyList.unsubscribe(this);
   }
 
   receiveRemoteValue(name, value) {
-    const studies = value.slice();
+    if (value) {
+      const studies = value.slice();
 
-    // Sort by active status, then by start date descending.
-    studies.sort((a, b) => {
-      if (a.active !== b.active) {
-        return a.active ? -1 : 1;
-      }
-      return b.studyStartDate - a.studyStartDate;
-    });
+      // Sort by active status, then by start date descending.
+      studies.sort((a, b) => {
+        if (a.active !== b.active) {
+          return a.active ? -1 : 1;
+        }
+        return b.studyStartDate - a.studyStartDate;
+      });
 
-    this.setState({studies});
+      this.setState({studies});
+    } else {
+      this.setState({studies: value});
+    }
   }
 
   render() {
+    const { studies } = this.state;
+
+    if (studies === null) {
+      // loading
+      return null;
+    }
+
+    let info = null;
+    if (studies.length === 0) {
+      info = r("p", {className: "study-list-info"}, "You have not participated in any studies.");
+    }
+
     return (
-      r("ul", {className: "study-list"},
-        this.state.studies.map(study => (
-          r(StudyListItem, {key: study.name, study})
-        ))
+      r("div", {},
+        info,
+        r("ul", {className: "study-list"},
+          this.state.studies.map(study => (
+            r(StudyListItem, {key: study.name, study})
+          ))
+        ),
       )
     );
   }
 }
 
 class StudyListItem extends React.Component {
   constructor(props) {
     super(props);
@@ -141,8 +136,65 @@ class StudyListItem extends React.Compon
 StudyListItem.propTypes = {
   study: PropTypes.shape({
     recipeId: PropTypes.number.isRequired,
     name: PropTypes.string.isRequired,
     active: PropTypes.boolean,
     description: PropTypes.string.isRequired,
   }).isRequired,
 };
+
+class WhatsThisBox extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      learnMoreHref: null,
+      studiesEnabled: null,
+    };
+  }
+
+  componentDidMount() {
+    remoteValues.shieldLearnMoreHref.subscribe(this);
+    remoteValues.studiesEnabled.subscribe(this);
+  }
+
+  componentWillUnmount() {
+    remoteValues.shieldLearnMoreHref.unsubscribe(this);
+    remoteValues.studiesEnabled.unsubscribe(this);
+  }
+
+  receiveRemoteValue(name, value) {
+    switch (name) {
+      case "ShieldLearnMoreHref": {
+        this.setState({ learnMoreHref: value });
+        break;
+      }
+      case "StudiesEnabled": {
+        this.setState({ studiesEnabled: value });
+        break;
+      }
+      default: {
+        console.error(`Unknown remote value ${name}`);
+      }
+    }
+  }
+
+  render() {
+    const { learnMoreHref, studiesEnabled } = this.state;
+
+    let message = null;
+
+    // studiesEnabled can be null, in which case do nothing
+    if (studiesEnabled === false) {
+      message = r("span", {}, "This is a list of studies that you have participated in. No new studies will run.");
+    } else if (studiesEnabled === true) {
+      message = r("span", {}, "What's this? Firefox may install and run studies from time to time.");
+    }
+
+    return (
+      r(InfoBox, {},
+        message,
+        r("a", {id: "shield-studies-learn-more", href: learnMoreHref}, "Learn more"),
+        r(UpdatePreferencesButton, {}, "Update Preferences"),
+       )
+    );
+  }
+}
--- a/browser/extensions/shield-recipe-client/content/shield-content-frame.js
+++ b/browser/extensions/shield-recipe-client/content/shield-content-frame.js
@@ -50,16 +50,22 @@ class ShieldFrameListener {
         break;
       // Actions that can be performed in the content process
       case "GetRemoteValue:ShieldLearnMoreHref":
         this.triggerPageCallback(
           "ReceiveRemoteValue:ShieldLearnMoreHref",
           frameGlobal.AboutPages.aboutStudies.getShieldLearnMoreHref()
         );
         break;
+      case "GetRemoteValue:StudiesEnabled":
+        this.triggerPageCallback(
+          "ReceiveRemoteValue:StudiesEnabled",
+          frameGlobal.AboutPages.aboutStudies.getStudiesEnabled()
+        );
+        break;
       case "NavigateToDataPreferences":
         sendAsyncMessage("Shield:OpenDataPreferences");
         break;
     }
   }
 
   /**
    * Check that the current webpage's origin is about:studies.
--- a/browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm
+++ b/browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm
@@ -80,20 +80,20 @@ this.RecipeRunner = {
     if (this.enabled) {
       return;
     }
     this.registerTimer();
     this.enabled = true;
   },
 
   disable() {
-    if (!this.enabled) {
-      return;
+    if (this.enabled) {
+      this.unregisterTimer();
     }
-    this.unregisterTimer();
+    // this.enabled may be null, so always set it to false
     this.enabled = false;
   },
 
   /** Watch for prefs to change, and call this.observer when they do */
   watchPrefs() {
     for (const pref of PREFS_TO_WATCH) {
       Services.prefs.addObserver(pref, this);
     }
@@ -151,21 +151,22 @@ this.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://")) {
-      log.warn(`Disabling shield because ${API_URL_PREF} is not an HTTPS url: ${apiUrl}.`);
+      log.warn(`Disabling Shield because ${API_URL_PREF} is not an HTTPS url: ${apiUrl}.`);
       this.disable();
       return;
     }
 
+    log.debug(`Enabling Shield`);
     this.enable();
   },
 
   registerTimer() {
     this.updateRunInterval();
     CleanupManager.addCleanupHandler(() => timerManager.unregisterTimer(TIMER_NAME));
   },
 
--- a/browser/extensions/shield-recipe-client/test/browser/browser_about_studies.js
+++ b/browser/extensions/shield-recipe-client/test/browser/browser_about_studies.js
@@ -1,11 +1,12 @@
 "use strict";
 
 ChromeUtils.import("resource://shield-recipe-client/lib/AddonStudies.jsm", this);
+ChromeUtils.import("resource://shield-recipe-client/lib/RecipeRunner.jsm", this);
 ChromeUtils.import("resource://shield-recipe-client-content/AboutPages.jsm", this);
 
 function withAboutStudies(testFunc) {
   return async (...args) => (
     BrowserTestUtils.withNewTab("about:studies", async browser => (
       testFunc(...args, browser)
     ))
   );
@@ -150,8 +151,49 @@ decorate_task(
 
     const updatedStudy1 = await AddonStudies.get(study1.recipeId);
     ok(
       !updatedStudy1.active,
       "Clicking the remove button marks the study as inactive in storage."
     );
   }
 );
+
+decorate_task(
+  AddonStudies.withStudies([]),
+  withAboutStudies,
+  async function testStudyListing(studies, browser) {
+    await ContentTask.spawn(browser, null, async () => {
+      const doc = content.document;
+      await ContentTaskUtils.waitForCondition(() => doc.querySelectorAll(".study-list").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",
+      );
+    });
+  }
+);
+
+decorate_task(
+  withAboutStudies,
+  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(
+          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",
+        );
+      });
+    } finally {
+      // reset RecipeRunner.enabled
+      RecipeRunner.checkPrefs();
+    }
+  }
+);