Bug 1581179 - Remove Fennec-specific canary client id fix r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Wed, 18 Sep 2019 09:02:41 +0000
changeset 493770 d2ee35cb9930dd8f19e48d083f8181a8266bfcb5
parent 493769 7afdf6a9c44a567b08f3b51ee8ef2682820349a9
child 493771 1feec83ee6f92a35de0d4b27ebea04e68a0d7ef0
push id36589
push usernerli@mozilla.com
push dateWed, 18 Sep 2019 21:49:27 +0000
treeherdermozilla-central@21aff209f5a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1581179
milestone71.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 1581179 - Remove Fennec-specific canary client id fix r=chutten Differential Revision: https://phabricator.services.mozilla.com/D46027
toolkit/components/telemetry/app/TelemetryController.jsm
toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
toolkit/components/telemetry/tests/unit/xpcshell.ini
--- a/toolkit/components/telemetry/app/TelemetryController.jsm
+++ b/toolkit/components/telemetry/app/TelemetryController.jsm
@@ -757,45 +757,32 @@ var Impl = {
           // TODO: This should probably happen after all the delayed init here.
           this._initialized = true;
           await TelemetryEnvironment.delayedInit();
 
           // Load the ClientID.
           this._clientID = await ClientID.getClientID();
 
           // Fix-up a canary client ID if detected.
-          if (IS_UNIFIED_TELEMETRY) {
-            // On desktop respect the upload preference.
-            const uploadEnabled = Services.prefs.getBoolPref(
-              TelemetryUtils.Preferences.FhrUploadEnabled,
-              false
-            );
-            if (uploadEnabled && this._clientID == Utils.knownClientID) {
-              this._log.trace(
-                "Upload enabled, but got canary client ID. Resetting."
-              );
-              this._clientID = await ClientID.resetClientID();
-            } else if (
-              !uploadEnabled &&
-              this._clientID != Utils.knownClientID
-            ) {
-              this._log.trace(
-                "Upload disabled, but got a valid client ID. Setting canary client ID."
-              );
-              this._clientID = await ClientID.setClientID(
-                TelemetryUtils.knownClientID
-              );
-            }
-          } else if (this._clientID == Utils.knownClientID) {
-            // On Fennec (non-unified Telemetry) we might have set a canary client ID in the past by mistake.
-            // We now always reset to a valid random client ID if this is detected (Bug 1501329).
+          const uploadEnabled = Services.prefs.getBoolPref(
+            TelemetryUtils.Preferences.FhrUploadEnabled,
+            false
+          );
+          if (uploadEnabled && this._clientID == Utils.knownClientID) {
             this._log.trace(
-              "Not unified, but got canary client ID. Resetting."
+              "Upload enabled, but got canary client ID. Resetting."
             );
             this._clientID = await ClientID.resetClientID();
+          } else if (!uploadEnabled && this._clientID != Utils.knownClientID) {
+            this._log.trace(
+              "Upload disabled, but got a valid client ID. Setting canary client ID."
+            );
+            this._clientID = await ClientID.setClientID(
+              TelemetryUtils.knownClientID
+            );
           }
 
           await TelemetrySend.setup(this._testMode);
 
           // Perform TelemetrySession delayed init.
           await TelemetrySession.delayedInit();
           await Services.telemetry.delayedInit();
 
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
@@ -53,23 +53,16 @@ add_task(async function test_setup() {
  * 4. Telemetry upload flag is toggled
  * 5. Instance is started again
  * 6. Detect that upload is enabled and reset client ID
  *
  * This scenario e.g. happens when switching between channels
  * with and without the optout ping reset included.
  */
 add_task(async function test_clientid_reset_after_reenabling() {
-  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
-  if (!isUnified) {
-    // Skipping the test if unified telemetry is off, as no optout ping will
-    // be generated.
-    return;
-  }
-
   await sendPing();
   let ping = await PingServer.promiseNextPing();
   Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
   Assert.ok("clientId" in ping);
 
   let firstClientId = ping.clientId;
   Assert.notEqual(
     TelemetryUtils.knownClientID,
@@ -120,23 +113,16 @@ add_task(async function test_clientid_re
  * 4. Telemetry upload flag is toggled
  * 5. Instance is started again
  * 6. Detect that upload is disabled and sets canary client ID
  *
  * This scenario e.g. happens when switching between channels
  * with and without the optout ping reset included.
  */
 add_task(async function test_clientid_canary_after_disabling() {
-  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
-  if (!isUnified) {
-    // Skipping the test if unified telemetry is off, as no optout ping will
-    // be generated.
-    return;
-  }
-
   await sendPing();
   let ping = await PingServer.promiseNextPing();
   Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
   Assert.ok("clientId" in ping);
 
   let firstClientId = ping.clientId;
   Assert.notEqual(
     TelemetryUtils.knownClientID,
@@ -176,163 +162,11 @@ add_task(async function test_clientid_ca
   let newClientId = await ClientID.getClientID();
   Assert.equal(
     TelemetryUtils.knownClientID,
     newClientId,
     "Client ID should be a canary when upload disabled"
   );
 });
 
-/**
- * On Android Telemetry is not unified.
- * This tests that we reset the client ID if it was previously reset to a canary client ID by accident.
- */
-add_task(async function test_clientid_canary_reset_canary_on_nonunified() {
-  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
-  if (isUnified) {
-    // Skipping the test if unified telemetry is on.
-    return;
-  }
-
-  await sendPing();
-  let ping = await PingServer.promiseNextPing();
-  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
-  Assert.ok("clientId" in ping);
-
-  let firstClientId = ping.clientId;
-  Assert.notEqual(
-    TelemetryUtils.knownClientID,
-    firstClientId,
-    "Client ID should be valid and random"
-  );
-
-  // Force a canary client ID
-  let clientId = await ClientID.setClientID(TelemetryUtils.knownClientID);
-  Assert.equal(TelemetryUtils.knownClientID, clientId);
-
-  // Now shutdown the instance
-  await TelemetryController.testShutdown();
-  await TelemetryStorage.testClearPendingPings();
-
-  // Start the instance
-  await TelemetryController.testReset();
-
-  let newClientId = await ClientID.getClientID();
-  Assert.notEqual(
-    TelemetryUtils.knownClientID,
-    newClientId,
-    "Client ID should be valid and random"
-  );
-  Assert.notEqual(
-    firstClientId,
-    newClientId,
-    "Client ID should be valid and random"
-  );
-});
-
-/**
- * On Android Telemetry is not unified.
- * This tests that we don't touch the client ID if the pref is toggled for some reason.
- */
-add_task(async function test_clientid_canary_nonunified_no_pref_trigger() {
-  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
-  if (isUnified) {
-    // Skipping the test if unified telemetry is on.
-    return;
-  }
-
-  await sendPing();
-  let ping = await PingServer.promiseNextPing();
-  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
-  Assert.ok("clientId" in ping);
-
-  let firstClientId = ping.clientId;
-  Assert.notEqual(
-    TelemetryUtils.knownClientID,
-    firstClientId,
-    "Client ID should be valid and random"
-  );
-
-  // Flip the pref again
-  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, true);
-
-  // Restart the instance
-  await TelemetryController.testShutdown();
-  await TelemetryStorage.testClearPendingPings();
-  await TelemetryController.testReset();
-
-  let newClientId = await ClientID.getClientID();
-  Assert.equal(firstClientId, newClientId, "Client ID should be unmodified");
-
-  // Flip the pref again
-  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
-
-  // Restart the instance
-  await TelemetryController.testShutdown();
-  await TelemetryStorage.testClearPendingPings();
-  await TelemetryController.testReset();
-
-  newClientId = await ClientID.getClientID();
-  Assert.equal(firstClientId, newClientId, "Client ID should be unmodified");
-});
-
-/**
- * On Android Telemetry is not unified.
- * Test that we record the canary flag after detecting a canary client ID and resetting to a new ID.
- */
-add_task(async function test_clientid_canary_nonunified_canary_detected() {
-  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
-  if (isUnified) {
-    // Skipping the test if unified telemetry is on.
-    return;
-  }
-
-  let firstClientId = await ClientID.resetClientID();
-  await TelemetryController.testReset();
-  await sendPing(/* environment */ true);
-  let ping = await PingServer.promiseNextPing();
-  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
-  Assert.equal(
-    firstClientId,
-    ping.clientId,
-    "Client ID should be from the reset"
-  );
-  Assert.ok(!("wasCanary" in ping.environment.profile));
-
-  // Setting canary
-  await ClientID.setClientID(TelemetryUtils.knownClientID);
-
-  // Reset the controller to reset the client ID.
-  await TelemetryController.testReset();
-  await sendPing(/* environment */ true);
-  ping = await PingServer.promiseNextPing();
-  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
-  let clientId = ping.clientId;
-  Assert.notEqual(
-    TelemetryUtils.knownClientID,
-    clientId,
-    "Client ID should have been reset to a valid one."
-  );
-  Assert.notEqual(
-    firstClientId,
-    clientId,
-    "Client ID should be a new one after reset."
-  );
-  Assert.ok(
-    ping.environment.profile.wasCanary,
-    "Previous canary client ID should have been detected after reset."
-  );
-
-  // Reset the controller again.
-  await TelemetryController.testReset();
-  await sendPing(/* environment */ true);
-  ping = await PingServer.promiseNextPing();
-  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
-  Assert.equal(clientId, ping.clientId, "Client ID should be unmodified now.");
-  Assert.ok(
-    ping.environment.profile.wasCanary,
-    "Canary client ID flag should be persisted."
-  );
-});
-
 add_task(async function stopServer() {
   await PingServer.stop();
 });
--- a/toolkit/components/telemetry/tests/unit/xpcshell.ini
+++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini
@@ -42,16 +42,17 @@ skip-if = os == "android"
 tags = addons
 [test_PingAPI.js]
 skip-if = os == "android"
 [test_TelemetryFlagClear.js]
 [test_TelemetryLateWrites.js]
 [test_TelemetryLockCount.js]
 [test_TelemetryController.js]
 [test_TelemetryClientID_reset.js]
+skip-if = os == "android" # Disabled as Android/GeckoView doesn't run TelemetryController
 [test_HealthPing.js]
 skip-if = (verify && (os == 'win')) || (os == 'android' && processor == 'x86_64')
 tags = addons
 [test_TelemetryController_idle.js]
 [test_TelemetryControllerShutdown.js]
 tags = addons
 [test_TelemetryStopwatch.js]
 [test_TelemetryControllerBuildID.js]