Bug 1166128 - Don't rely on datareporting.healthreport.uploadEnabled to control telemetry submission on Android. r=vladan
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Fri, 22 May 2015 00:10:29 +0700
changeset 275837 5f9055c07cdddaa9b677f81ca804a4e41d34ac98
parent 275836 1442cb35801fa0f7eb906b4a2943b3e7e9b3fed7
child 275838 4975d5fbad0b962698ab9e4e87ff0da29f40422c
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvladan
bugs1166128
milestone41.0a1
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
Bug 1166128 - Don't rely on datareporting.healthreport.uploadEnabled to control telemetry submission on Android. r=vladan
toolkit/components/telemetry/TelemetryController.jsm
toolkit/components/telemetry/tests/unit/test_TelemetryController.js
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -1178,22 +1178,33 @@ let Impl = {
   get clientID() {
     return this._clientID;
   },
 
   /**
    * Check if pings can be sent to the server. If FHR is not allowed to upload,
    * pings are not sent to the server (Telemetry is a sub-feature of FHR).
    * If unified telemetry is off, don't send pings if Telemetry is disabled.
+   *
    * @return {Boolean} True if pings can be send to the servers, false otherwise.
    */
   _canSend: function() {
-    return (Telemetry.isOfficialTelemetry || this._testMode) &&
-           Preferences.get(PREF_FHR_UPLOAD_ENABLED, false) &&
-           (IS_UNIFIED_TELEMETRY || Preferences.get(PREF_ENABLED));
+    // We only send pings from official builds, but allow overriding this for tests.
+    if (!Telemetry.isOfficialTelemetry && !this._testMode) {
+      return false;
+    }
+
+    // With unified Telemetry, the FHR upload setting controls whether we can send pings.
+    // The Telemetry pref enables sending extended data sets instead.
+    if (IS_UNIFIED_TELEMETRY) {
+      return Preferences.get(PREF_FHR_UPLOAD_ENABLED, false);
+    }
+
+    // Without unified Telemetry, the Telemetry enabled pref controls ping sending.
+    return Preferences.get(PREF_ENABLED, false);
   },
 
   /**
    * Get an object describing the current state of this module for AsyncShutdown diagnostics.
    */
   _getState: function() {
     return {
       initialized: this._initialized,
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
@@ -26,16 +26,17 @@ const PLATFORM_VERSION = "1.9.2";
 const APP_VERSION = "1";
 const APP_NAME = "XPCShell";
 
 const PREF_BRANCH = "toolkit.telemetry.";
 const PREF_ENABLED = PREF_BRANCH + "enabled";
 const PREF_ARCHIVE_ENABLED = PREF_BRANCH + "archive.enabled";
 const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
 const PREF_FHR_SERVICE_ENABLED = "datareporting.healthreport.service.enabled";
+const PREF_UNIFIED = PREF_BRANCH + "unified";
 
 const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry);
 
 let gHttpServer = new HttpServer();
 let gServerStarted = false;
 let gRequestIterator = null;
 let gClientID = null;
 
@@ -217,50 +218,55 @@ add_task(function* test_pingHasEnvironme
 
 add_task(function* test_archivePings() {
   const ARCHIVE_PATH =
     OS.Path.join(OS.Constants.Path.profileDir, "datareporting", "archived");
 
   let now = new Date(2009, 10, 18, 12, 0, 0);
   fakeNow(now);
 
-  // Disable FHR upload so that pings don't get sent.
-  Preferences.set(PREF_FHR_UPLOAD_ENABLED, false);
+  // Disable ping upload so that pings don't get sent.
+  // With unified telemetry the FHR upload pref controls this,
+  // with non-unified telemetry the Telemetry enabled pref.
+  const isUnified = Preferences.get(PREF_UNIFIED, false);
+  const uploadPref = isUnified ? PREF_FHR_UPLOAD_ENABLED : PREF_ENABLED;
+  Preferences.set(uploadPref, false);
 
   // Register a new Ping Handler that asserts if a ping is received, then send a ping.
   registerPingHandler(() => Assert.ok(false, "Telemetry must not send pings if not allowed to."));
   let pingId = yield sendPing(true, true);
 
   // Check that the ping was archived, even with upload disabled.
   let ping = yield TelemetryArchive.promiseArchivedPingById(pingId);
-  Assert.equal(ping.id, pingId, "TelemetryController must archive pings if FHR is enabled.");
+  Assert.equal(ping.id, pingId, "TelemetryController should still archive pings.");
 
   // Check that pings don't get archived if not allowed to.
   now = new Date(2010, 10, 18, 12, 0, 0);
   fakeNow(now);
   Preferences.set(PREF_ARCHIVE_ENABLED, false);
   pingId = yield sendPing(true, true);
   let promise = TelemetryArchive.promiseArchivedPingById(pingId);
   Assert.ok((yield promiseRejects(promise)),
-            "TelemetryController must not archive pings if the archive pref is disabled.");
+    "TelemetryController should not archive pings if the archive pref is disabled.");
 
   // Enable archiving and the upload so that pings get sent and archived again.
-  Preferences.set(PREF_FHR_UPLOAD_ENABLED, true);
+  Preferences.set(uploadPref, true);
   Preferences.set(PREF_ARCHIVE_ENABLED, true);
 
   now = new Date(2014, 06, 18, 22, 0, 0);
   fakeNow(now);
   // Restore the non asserting ping handler. This is done by the Request() constructor.
   gRequestIterator = Iterator(new Request());
   pingId = yield sendPing(true, true);
 
   // Check that we archive pings when successfully sending them.
   yield gRequestIterator.next();
   ping = yield TelemetryArchive.promiseArchivedPingById(pingId);
-  Assert.equal(ping.id, pingId, "TelemetryController must archive pings if FHR is enabled.");
+  Assert.equal(ping.id, pingId,
+    "TelemetryController should still archive pings if ping upload is enabled.");
 });
 
 // Test that we fuzz the submission time around midnight properly
 // to avoid overloading the telemetry servers.
 add_task(function* test_midnightPingSendFuzzing() {
   const fuzzingDelay = 60 * 60 * 1000;
   fakeMidnightPingFuzzingDelay(fuzzingDelay);
   let now = new Date(2030, 5, 1, 11, 00, 0);