Bug 1501329 - Persist information about canary after resetting client ID r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Fri, 26 Oct 2018 18:01:13 +0000
changeset 443649 72c4825708d7635e82f5a1b171955cd392ef2009
parent 443648 f61505fe89301d998ad015166837ea301f1ec24c
child 443650 db22d59d1d9b72336edfc96e36ce326593bfd5c1
push id109420
push useraciure@mozilla.com
push dateWed, 31 Oct 2018 05:11:56 +0000
treeherdermozilla-inbound@b357da105c49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1501329
milestone65.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 1501329 - Persist information about canary after resetting client ID r=chutten We erroneously reset client IDs on Fennec to a canary client ID. This is now detected and a new valid and random client ID is set. This adds a new boolean attribute "wasCanary" to the `state.json` file generated by ClientID.jsm. Depends on D9544 Differential Revision: https://phabricator.services.mozilla.com/D9903
toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
toolkit/modules/ClientID.jsm
toolkit/modules/tests/xpcshell/test_client_id.js
toolkit/modules/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
@@ -10,20 +10,20 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.import("resource://gre/modules/TelemetrySend.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm", this);
 ChromeUtils.import("resource://gre/modules/Preferences.jsm");
 
 const PING_FORMAT_VERSION = 4;
 const OPTOUT_PING_TYPE = "optout";
 const TEST_PING_TYPE = "test-ping-type";
 
-function sendPing() {
+function sendPing(addEnvironment = false) {
   let options = {
     addClientId: true,
-    addEnvironment: false,
+    addEnvironment,
   };
   return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
 }
 
 add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile();
   // Make sure we don't generate unexpected pings due to pref changes.
@@ -206,26 +206,28 @@ add_task(async function test_clientid_ca
   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");
 });
 
 add_task(async function stopServer() {
   await PingServer.stop();
--- a/toolkit/modules/ClientID.jsm
+++ b/toolkit/modules/ClientID.jsm
@@ -4,19 +4,22 @@
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["ClientID"];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/Log.jsm");
+ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 
 const LOGGER_NAME = "Toolkit.Telemetry";
 const LOGGER_PREFIX = "ClientID::";
+// Must match ID in TelemetryUtils
+const CANARY_CLIENT_ID = "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0";
 
 ChromeUtils.defineModuleGetter(this, "CommonUtils",
                                "resource://services-common/utils.js");
 ChromeUtils.defineModuleGetter(this, "OS",
                                "resource://gre/modules/osfile.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "gDatareportingPath", () => {
   return OS.Path.join(OS.Constants.Path.profileDir, "datareporting");
@@ -52,16 +55,28 @@ var ClientID = Object.freeze({
    *
    * @return {Promise<string>} The stable client ID.
    */
   getClientID() {
     return ClientIDImpl.getClientID();
   },
 
   /**
+   * This returns true if the client ID prior to the last client ID reset was a canary client ID.
+   * Android only. Always returns null on Desktop.
+   */
+  wasCanaryClientID() {
+    if (AppConstants.platform == "android") {
+      return ClientIDImpl.wasCanaryClientID();
+    }
+
+    return null;
+  },
+
+  /**
    * Get the client id synchronously without hitting the disk.
    * This returns:
    *  - the current on-disk client id if it was already loaded
    *  - the client id that we cached into preferences (if any)
    *  - null otherwise
    */
   getCachedClientID() {
     return ClientIDImpl.getCachedClientID();
@@ -102,16 +117,17 @@ var ClientID = Object.freeze({
 });
 
 var ClientIDImpl = {
   _clientID: null,
   _loadClientIdTask: null,
   _saveClientIdTask: null,
   _removeClientIdTask: null,
   _logger: null,
+  _wasCanary: null,
 
   _loadClientID() {
     if (this._loadClientIdTask) {
       return this._loadClientIdTask;
     }
 
     this._loadClientIdTask = this._doLoadClientID();
     let clear = () => this._loadClientIdTask = null;
@@ -125,16 +141,19 @@ var ClientIDImpl = {
    */
   async _doLoadClientID() {
     // If there's a removal in progress, let's wait for it
     await this._removeClientIdTask;
 
     // Try to load the client id from the DRS state file.
     try {
       let state = await CommonUtils.readJSON(gStateFilePath);
+      if (AppConstants.platform == "android" && state && "wasCanary" in state) {
+        this._wasCanary = state.wasCanary;
+      }
       if (state && this.updateClientID(state.clientID)) {
         return this._clientID;
       }
     } catch (e) {
       // fall through to next option
     }
 
     // We dont have an id from the DRS state file yet, generate a new ID.
@@ -152,16 +171,20 @@ var ClientIDImpl = {
 
   /**
    * Save the client ID to the client ID file.
    *
    * @return {Promise} A promise resolved when the client ID is saved to disk.
    */
   async _saveClientID() {
     let obj = { clientID: this._clientID };
+    // We detected a canary client ID when resetting, storing this as a flag
+    if (AppConstants.platform == "android" && this._wasCanary) {
+      obj.wasCanary = true;
+    }
     await OS.File.makeDir(gDatareportingPath);
     await CommonUtils.writeJSON(obj, gStateFilePath);
     this._saveClientIdTask = null;
   },
 
   /**
    * This returns a promise resolving to the the stable client ID we use for
    * data reporting (FHR & Telemetry).
@@ -172,16 +195,24 @@ var ClientIDImpl = {
     if (!this._clientID) {
       return this._loadClientID();
     }
 
     return Promise.resolve(this._clientID);
   },
 
   /**
+   * This returns true if the client ID prior to the last client ID reset was a canary client ID.
+   * Android only. Always returns null on Desktop.
+   */
+  wasCanaryClientID() {
+    return this._wasCanary;
+  },
+
+  /**
    * Get the client id synchronously without hitting the disk.
    * This returns:
    *  - the current on-disk client id if it was already loaded
    *  - the client id that we cached into preferences (if any)
    *  - null otherwise
    */
   getCachedClientID() {
     if (this._clientID) {
@@ -238,24 +269,31 @@ var ClientIDImpl = {
     // Clear the client id from the preference cache.
     Services.prefs.clearUserPref(PREF_CACHED_CLIENTID);
 
     // Remove the client id from disk
     await OS.File.remove(gStateFilePath, {ignoreAbsent: true});
   },
 
   async resetClientID() {
+    let oldClientId = this._clientID;
+
     // Wait for the removal.
     // Asynchronous calls to getClientID will also be blocked on this.
     this._removeClientIdTask = this._doRemoveClientID();
     let clear = () => this._removeClientIdTask = null;
     this._removeClientIdTask.then(clear, clear);
 
     await this._removeClientIdTask;
 
+    // On Android we detect resets after a canary client ID.
+    if (AppConstants.platform == "android" ) {
+      this._wasCanary = oldClientId == CANARY_CLIENT_ID;
+    }
+
     // Generate a new id.
     return this.getClientID();
   },
 
   /**
    * Sets the client id to the given value and updates the value cached in
    * preferences only if the given id is a valid.
    *
--- a/toolkit/modules/tests/xpcshell/test_client_id.js
+++ b/toolkit/modules/tests/xpcshell/test_client_id.js
@@ -2,16 +2,17 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 ChromeUtils.import("resource://gre/modules/ClientID.jsm");
 ChromeUtils.import("resource://services-common/utils.js");
 ChromeUtils.import("resource://gre/modules/osfile.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 
 const PREF_CACHED_CLIENTID = "toolkit.telemetry.cachedClientID";
 
 function run_test() {
   do_get_profile();
   run_next_test();
 }
 
@@ -125,8 +126,32 @@ add_task(async function test_resetParall
   // We should get the same ID twice when requesting it in parallel to a reset.
   let p = ClientID.resetClientID();
   let newClientID = await ClientID.getClientID();
   let otherClientID = await p;
 
   Assert.notEqual(firstClientID, newClientID, "After reset client ID should be different.");
   Assert.equal(newClientID, otherClientID, "Getting the client ID in parallel to a reset should give the same id.");
 });
+
+add_task({
+  skip_if: () => AppConstants.platform != "android",
+}, async function test_FennecCanaryDetect() {
+  const KNOWN_UUID = "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0";
+
+  // We should get a valid UUID after reset
+  let firstClientID = await ClientID.resetClientID();
+  Assert.notEqual(KNOWN_UUID, firstClientID, "Client ID should be random.");
+
+  // Set the canary client ID.
+  await ClientID.setClientID(KNOWN_UUID);
+  Assert.equal(KNOWN_UUID, await ClientID.getClientID(), "Client ID should be known canary.");
+
+  let newClientID = await ClientID.resetClientID();
+  Assert.notEqual(KNOWN_UUID, newClientID, "After reset Client ID should be random.");
+  Assert.notEqual(firstClientID, newClientID, "After reset Client ID should be new.");
+  Assert.ok(ClientID.wasCanaryClientID(), "After reset we should have detected a canary client ID");
+
+  let clientID = await ClientID.resetClientID();
+  Assert.notEqual(KNOWN_UUID, clientID, "After reset Client ID should be random.");
+  Assert.notEqual(newClientID, clientID, "After reset Client ID should be new.");
+  Assert.ok(!ClientID.wasCanaryClientID(), "After reset we should not have detected a canary client ID");
+});
--- a/toolkit/modules/tests/xpcshell/xpcshell.ini
+++ b/toolkit/modules/tests/xpcshell/xpcshell.ini
@@ -7,17 +7,16 @@ support-files =
   chromeappsstore.sqlite
   corrupt.sqlite
   zips/zen.zip
 
 [test_BinarySearch.js]
 skip-if = toolkit == 'android'
 [test_CanonicalJSON.js]
 [test_client_id.js]
-skip-if = toolkit == 'android'
 [test_Color.js]
 [test_CreditCard.js]
 [test_DeferredTask.js]
 skip-if = toolkit == 'android'
 [test_FileUtils.js]
 skip-if = toolkit == 'android'
 [test_FinderIterator.js]
 [test_GMPInstallManager.js]