Merge b26da19 into 970b789

mozilla · Aug 1, 2017 · 6514e4b · 6514e4b
2 parents 970b789 + b26da19
commit 6514e4b
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 61 deletions.
diff --git a/system-addon/lib/ActivityStream.jsm b/system-addon/lib/ActivityStream.jsm
@@ -147,7 +147,7 @@ const FEEDS_DATA = [
     name: "snippets",
     factory: () => new SnippetsFeed(),
     title: "Gets snippets data",
-    value: false
+    value: true
   },
   {
     name: "systemtick",

diff --git a/system-addon/lib/SnippetsFeed.jsm b/system-addon/lib/SnippetsFeed.jsm
@@ -14,8 +14,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "ProfileAge",
 
 // Url to fetch snippets, in the urlFormatter service format.
 const SNIPPETS_URL_PREF = "browser.aboutHomeSnippets.updateUrl";
-const TELEMETRY_PREF = "datareporting.healthreport.uploadEnabled";
 const ONBOARDING_FINISHED_PREF = "browser.onboarding.notification.finished";
+const FXA_USERNAME_PREF = "services.sync.username";
 
 // Should be bumped up if the snippets content format changes.
 const STARTPAGE_VERSION = 5;
@@ -48,22 +48,29 @@ this.SnippetsFeed = class SnippetsFeed {
       version: STARTPAGE_VERSION,
       profileCreatedWeeksAgo: profileInfo.createdWeeksAgo,
       profileResetWeeksAgo: profileInfo.resetWeeksAgo,
-      telemetryEnabled: Services.prefs.getBoolPref(TELEMETRY_PREF),
-      onboardingFinished: Services.prefs.getBoolPref(ONBOARDING_FINISHED_PREF)
+      telemetryEnabled: Services.telemetry.canRecordBase,
+      onboardingFinished: Services.prefs.getBoolPref(ONBOARDING_FINISHED_PREF),
+      fxaccount: Services.prefs.prefHasUserValue(FXA_USERNAME_PREF)
     };
 
     this.store.dispatch(ac.BroadcastToContent({type: at.SNIPPETS_DATA, data}));
   }
+  _refreshCanRecordBase() {
+    // TODO: There is currently no way to listen for changes to this value, so
+    // we are just refreshing it on every new tab instead. A bug is filed
+    // here to fix this: https://bugzilla.mozilla.org/show_bug.cgi?id=1386318
+    this.store.dispatch({type: at.SNIPPETS_DATA, data: {telemetryEnabled: Services.telemetry.canRecordBase}});
+  }
   async init() {
     await this._refresh();
     Services.prefs.addObserver(ONBOARDING_FINISHED_PREF, this._refresh);
     Services.prefs.addObserver(SNIPPETS_URL_PREF, this._refresh);
-    Services.prefs.addObserver(TELEMETRY_PREF, this._refresh);
+    Services.prefs.addObserver(FXA_USERNAME_PREF, this._refresh);
   }
   uninit() {
     Services.prefs.removeObserver(ONBOARDING_FINISHED_PREF, this._refresh);
     Services.prefs.removeObserver(SNIPPETS_URL_PREF, this._refresh);
-    Services.prefs.removeObserver(TELEMETRY_PREF, this._refresh);
+    Services.prefs.removeObserver(FXA_USERNAME_PREF, this._refresh);
     this.store.dispatch({type: at.SNIPPETS_RESET});
   }
   onAction(action) {
@@ -74,6 +81,9 @@ this.SnippetsFeed = class SnippetsFeed {
       case at.FEED_INIT:
         if (action.data === "feeds.snippets") { this.init(); }
         break;
+      case at.NEW_TAB_INIT:
+        this._refreshCanRecordBase();
+        break;
     }
   }
 };

diff --git a/system-addon/lib/TelemetrySender.jsm b/system-addon/lib/TelemetrySender.jsm
@@ -6,6 +6,7 @@ const {interfaces: Ci, utils: Cu} = Components;
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.importGlobalProperties(["fetch"]);
+Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "console",
   "resource://gre/modules/Console.jsm");
@@ -19,8 +20,6 @@ const ENDPOINT_PREF = `${PREF_BRANCH}telemetry.ping.endpoint`;
 const TELEMETRY_PREF = `${PREF_BRANCH}telemetry`;
 const LOGGING_PREF = `${PREF_BRANCH}telemetry.log`;
 
-const FHR_UPLOAD_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
-
 /**
  * Observe various notifications and send them to a telemetry endpoint.
  *
@@ -44,10 +43,6 @@ function TelemetrySender(args) {
   this._onTelemetryPrefChange = this._onTelemetryPrefChange.bind(this);
   this._prefs.observe(TELEMETRY_PREF, this._onTelemetryPrefChange);
 
-  this._fhrEnabled = this._prefs.get(FHR_UPLOAD_ENABLED_PREF);
-  this._onFhrPrefChange = this._onFhrPrefChange.bind(this);
-  this._prefs.observe(FHR_UPLOAD_ENABLED_PREF, this._onFhrPrefChange);
-
   this.logging = this._prefs.get(LOGGING_PREF);
   this._onLoggingPrefChange = this._onLoggingPrefChange.bind(this);
   this._prefs.observe(LOGGING_PREF, this._onLoggingPrefChange);
@@ -57,7 +52,9 @@ function TelemetrySender(args) {
 
 TelemetrySender.prototype = {
   get enabled() {
-    return this._enabled && this._fhrEnabled;
+    // Note: Services.telemetry.canRecordBase is the general indicator for
+    // opt-out Firefox Telemetry
+    return this._enabled && Services.telemetry.canRecordBase;
   },
 
   _onLoggingPrefChange(prefVal) {
@@ -68,10 +65,6 @@ TelemetrySender.prototype = {
     this._enabled = prefVal;
   },
 
-  _onFhrPrefChange(prefVal) {
-    this._fhrEnabled = prefVal;
-  },
-
   sendPing(data) {
     if (this.logging) {
       // performance related pings cause a lot of logging, so we mute them
@@ -95,7 +88,6 @@ TelemetrySender.prototype = {
     try {
       this._prefs.ignore(TELEMETRY_PREF, this._onTelemetryPrefChange);
       this._prefs.ignore(LOGGING_PREF, this._onLoggingPrefChange);
-      this._prefs.ignore(FHR_UPLOAD_ENABLED_PREF, this._onFhrPrefChange);
     } catch (e) {
       Cu.reportError(e);
     }
@@ -105,7 +97,6 @@ TelemetrySender.prototype = {
 this.TelemetrySender = TelemetrySender;
 this.TelemetrySenderConstants = {
   ENDPOINT_PREF,
-  FHR_UPLOAD_ENABLED_PREF,
   TELEMETRY_PREF,
   LOGGING_PREF
 };

diff --git a/system-addon/test/unit/lib/SnippetsFeed.test.js b/system-addon/test/unit/lib/SnippetsFeed.test.js
@@ -1,13 +1,16 @@
 const {SnippetsFeed} = require("lib/SnippetsFeed.jsm");
 const {actionTypes: at} = require("common/Actions.jsm");
 const {GlobalOverrider} = require("test/unit/utils");
+const {createStore, combineReducers} = require("redux");
+const {reducers} = require("common/Reducers.jsm");
 
 const WEEK_IN_MS = 7 * 24 * 60 * 60 * 1000;
 
 let overrider = new GlobalOverrider();
 
 describe("SnippetsFeed", () => {
   let sandbox;
+  let store;
   let clock;
   beforeEach(() => {
     clock = sinon.useFakeTimers();
@@ -20,6 +23,8 @@ describe("SnippetsFeed", () => {
       }
     });
     sandbox = sinon.sandbox.create();
+    store = createStore(combineReducers(reducers));
+    sinon.spy(store, "dispatch");
   });
   afterEach(() => {
     clock.restore();
@@ -30,50 +35,59 @@ describe("SnippetsFeed", () => {
     const url = "foo.com/%STARTPAGE_VERSION%";
     sandbox.stub(global.Services.prefs, "getStringPref").returns(url);
     sandbox.stub(global.Services.prefs, "getBoolPref")
-      .withArgs("datareporting.healthreport.uploadEnabled")
-      .returns(true)
       .withArgs("browser.onboarding.notification.finished")
       .returns(false);
+    sandbox.stub(global.Services.prefs, "prefHasUserValue")
+      .withArgs("services.sync.username")
+      .returns(true);
+    sandbox.stub(global.Services.telemetry, "canRecordBase").value(false);
 
     const feed = new SnippetsFeed();
-    feed.store = {dispatch: sandbox.stub()};
-
+    feed.store = store;
     clock.tick(WEEK_IN_MS * 2);
 
     await feed.init();
 
-    assert.calledOnce(feed.store.dispatch);
+    const state = store.getState().Snippets;
+
+    assert.propertyVal(state, "snippetsURL", "foo.com/5");
+    assert.propertyVal(state, "version", 5);
+    assert.propertyVal(state, "profileCreatedWeeksAgo", 2);
+    assert.propertyVal(state, "profileResetWeeksAgo", 1);
+    assert.propertyVal(state, "telemetryEnabled", false);
+    assert.propertyVal(state, "onboardingFinished", false);
+    assert.propertyVal(state, "fxaccount", true);
+  });
+  it("should update telemetryEnabled on each new tab", () => {
+    sandbox.stub(global.Services.telemetry, "canRecordBase").value(false);
+    const feed = new SnippetsFeed();
+    feed.store = store;
+
+    feed.onAction({type: at.NEW_TAB_INIT});
 
-    const action = feed.store.dispatch.firstCall.args[0];
-    assert.propertyVal(action, "type", at.SNIPPETS_DATA);
-    assert.isObject(action.data);
-    assert.propertyVal(action.data, "snippetsURL", "foo.com/5");
-    assert.propertyVal(action.data, "version", 5);
-    assert.propertyVal(action.data, "profileCreatedWeeksAgo", 2);
-    assert.propertyVal(action.data, "profileResetWeeksAgo", 1);
-    assert.propertyVal(action.data, "telemetryEnabled", true);
-    assert.propertyVal(action.data, "onboardingFinished", false);
+    const state = store.getState().Snippets;
+    assert.propertyVal(state, "telemetryEnabled", false);
   });
   it("should call .init on an INIT aciton", () => {
     const feed = new SnippetsFeed();
     sandbox.stub(feed, "init");
-    feed.store = {dispatch: sandbox.stub()};
+    feed.store = store;
 
     feed.onAction({type: at.INIT});
     assert.calledOnce(feed.init);
   });
   it("should call .init when a FEED_INIT happens for feeds.snippets", () => {
     const feed = new SnippetsFeed();
     sandbox.stub(feed, "init");
-    feed.store = {dispatch: sandbox.stub()};
+    feed.store = store;
 
     feed.onAction({type: at.FEED_INIT, data: "feeds.snippets"});
 
     assert.calledOnce(feed.init);
   });
   it("should dispatch a SNIPPETS_RESET on uninit", () => {
     const feed = new SnippetsFeed();
-    feed.store = {dispatch: sandbox.stub()};
+    feed.store = store;
 
     feed.uninit();
 

diff --git a/system-addon/test/unit/lib/TelemetrySender.test.js b/system-addon/test/unit/lib/TelemetrySender.test.js
@@ -3,7 +3,7 @@
 
 const {GlobalOverrider, FakePrefs} = require("test/unit/utils");
 const {TelemetrySender, TelemetrySenderConstants} = require("lib/TelemetrySender.jsm");
-const {ENDPOINT_PREF, FHR_UPLOAD_ENABLED_PREF, TELEMETRY_PREF, LOGGING_PREF} =
+const {ENDPOINT_PREF, TELEMETRY_PREF, LOGGING_PREF} =
   TelemetrySenderConstants;
 
 /**
@@ -52,23 +52,23 @@ describe("TelemetrySender", () => {
 
   describe("#enabled", () => {
     let testParams = [
-      {enabledPref: true, fhrPref: true, result: true},
-      {enabledPref: false, fhrPref: true, result: false},
-      {enabledPref: true, fhrPref: false, result: false},
-      {enabledPref: false, fhrPref: false, result: false}
+      {enabledPref: true, canRecordBase: true, result: true},
+      {enabledPref: false, canRecordBase: true, result: false},
+      {enabledPref: true, canRecordBase: false, result: false},
+      {enabledPref: false, canRecordBase: false, result: false}
     ];
 
     function testEnabled(p) {
       FakePrefs.prototype.prefs[TELEMETRY_PREF] = p.enabledPref;
-      FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = p.fhrPref;
+      sandbox.stub(global.Services.telemetry, "canRecordBase").value(p.canRecordBase);
 
       tSender = new TelemetrySender(tsArgs);
 
       assert.equal(tSender.enabled, p.result);
     }
 
     for (let p of testParams) {
-      it(`should return ${p.result} if the fhrPref is ${p.fhrPref} and telemetry.enabled is ${p.enabledPref}`, () => {
+      it(`should return ${p.result} if the Services.telemetry.canRecordBase is ${p.canRecordBase} and telemetry.enabled is ${p.enabledPref}`, () => {
         testEnabled(p);
       });
     }
@@ -77,7 +77,7 @@ describe("TelemetrySender", () => {
       beforeEach(() => {
         FakePrefs.prototype.prefs = {};
         FakePrefs.prototype.prefs[TELEMETRY_PREF] = true;
-        FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = true;
+        sandbox.stub(global.Services.telemetry, "canRecordBase").value(true);
         tSender = new TelemetrySender(tsArgs);
         assert.propertyVal(tSender, "enabled", true);
       });
@@ -92,7 +92,7 @@ describe("TelemetrySender", () => {
     describe("telemetry.enabled pref changes from false to true", () => {
       beforeEach(() => {
         FakePrefs.prototype.prefs = {};
-        FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = true;
+        sandbox.stub(global.Services.telemetry, "canRecordBase").value(true);
         FakePrefs.prototype.prefs[TELEMETRY_PREF] = false;
         tSender = new TelemetrySender(tsArgs);
 
@@ -106,34 +106,34 @@ describe("TelemetrySender", () => {
       });
     });
 
-    describe("FHR enabled pref changes from true to false", () => {
+    describe("canRecordBase changes from true to false", () => {
       beforeEach(() => {
         FakePrefs.prototype.prefs = {};
         FakePrefs.prototype.prefs[TELEMETRY_PREF] = true;
-        FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = true;
+        sandbox.stub(global.Services.telemetry, "canRecordBase").value(true);
         tSender = new TelemetrySender(tsArgs);
         assert.propertyVal(tSender, "enabled", true);
       });
 
       it("should set the enabled property to false", () => {
-        fakePrefs.set(FHR_UPLOAD_ENABLED_PREF, false);
+        sandbox.stub(global.Services.telemetry, "canRecordBase").value(false);
 
         assert.propertyVal(tSender, "enabled", false);
       });
     });
 
-    describe("FHR enabled pref changes from false to true", () => {
+    describe("canRecordBase changes from false to true", () => {
       beforeEach(() => {
         FakePrefs.prototype.prefs = {};
-        FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = false;
+        sandbox.stub(global.Services.telemetry, "canRecordBase").value(false);
         FakePrefs.prototype.prefs[TELEMETRY_PREF] = true;
         tSender = new TelemetrySender(tsArgs);
 
         assert.propertyVal(tSender, "enabled", false);
       });
 
       it("should set the enabled property to true", () => {
-        fakePrefs.set(FHR_UPLOAD_ENABLED_PREF, true);
+        sandbox.stub(global.Services.telemetry, "canRecordBase").value(true);
 
         assert.propertyVal(tSender, "enabled", true);
       });
@@ -143,7 +143,7 @@ describe("TelemetrySender", () => {
   describe("#sendPing()", () => {
     beforeEach(() => {
       FakePrefs.prototype.prefs = {};
-      FakePrefs.prototype.prefs[FHR_UPLOAD_ENABLED_PREF] = true;
+      sandbox.stub(global.Services.telemetry, "canRecordBase").value(true);
       FakePrefs.prototype.prefs[TELEMETRY_PREF] = true;
       FakePrefs.prototype.prefs[ENDPOINT_PREF] = fakeEndpointUrl;
       tSender = new TelemetrySender(tsArgs);
@@ -208,15 +208,6 @@ describe("TelemetrySender", () => {
       assert.notProperty(fakePrefs.observers, TELEMETRY_PREF);
     });
 
-    it("should remove the fhrpref listener", () => {
-      tSender = new TelemetrySender(tsArgs);
-      assert.property(fakePrefs.observers, FHR_UPLOAD_ENABLED_PREF);
-
-      tSender.uninit();
-
-      assert.notProperty(fakePrefs.observers, FHR_UPLOAD_ENABLED_PREF);
-    });
-
     it("should remove the telemetry log listener", () => {
       tSender = new TelemetrySender(tsArgs);
       assert.property(fakePrefs.observers, LOGGING_PREF);

diff --git a/system-addon/test/unit/unit-entry.js b/system-addon/test/unit/unit-entry.js
@@ -28,6 +28,10 @@ overrider.set({
   fetch() {},
   Preferences: FakePrefs,
   Services: {
+    telemetry: {
+      canRecordBase: true,
+      canRecordExtended: true
+    },
     locale: {
       getAppLocalesAsLangTags() {},
       getRequestedLocale() {},