Bug 1191912 - Part 1 - Move client id caching to ClientID.jsm. r=rvitillo,a=ritu
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Tue, 11 Aug 2015 12:17:29 +0200
changeset 288729 463fe048a77808672970ccbde58faa7670afab30
parent 288728 f4e43d7abccdd486098a2f577fd753c2d517116c
child 288730 86a5a7e54c8f7fddbf3b0f00de37ba94d43f2e99
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrvitillo, ritu
bugs1191912
milestone42.0a2
Bug 1191912 - Part 1 - Move client id caching to ClientID.jsm. r=rvitillo,a=ritu
toolkit/components/telemetry/TelemetryController.jsm
toolkit/modules/ClientID.jsm
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -673,32 +673,31 @@ let Impl = {
     }
 
     this._attachObservers();
 
     // For very short session durations, we may never load the client
     // id from disk.
     // We try to cache it in prefs to avoid this, even though this may
     // lead to some stale client ids.
-    this._clientID = Preferences.get(PREF_CACHED_CLIENTID, null);
+    this._clientID = ClientID.getCachedClientID();
 
     // Delay full telemetry initialization to give the browser time to
     // run various late initializers. Otherwise our gathered memory
     // footprint and other numbers would be too optimistic.
     this._delayedInitTaskDeferred = Promise.defer();
     this._delayedInitTask = new DeferredTask(function* () {
       try {
         // TODO: This should probably happen after all the delayed init here.
         this._initialized = true;
 
         yield TelemetrySend.setup(this._testMode);
 
-        // Load the ClientID and update the cache.
+        // Load the ClientID.
         this._clientID = yield ClientID.getClientID();
-        Preferences.set(PREF_CACHED_CLIENTID, this._clientID);
 
         // Purge the pings archive by removing outdated pings. We don't wait for this
         // task to complete, but TelemetryStorage blocks on it during shutdown.
         TelemetryStorage.runCleanPingArchiveTask();
 
         Telemetry.asyncFetchTelemetryData(function () {});
         this._delayedInitTaskDeferred.resolve();
       } catch (e) {
--- a/toolkit/modules/ClientID.jsm
+++ b/toolkit/modules/ClientID.jsm
@@ -6,40 +6,54 @@
 
 this.EXPORTED_SYMBOLS = ["ClientID"];
 
 const {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Preferences.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils",
                                   "resource://services-common/utils.js");
 
 XPCOMUtils.defineLazyGetter(this, "gDatareportingPath", () => {
   return OS.Path.join(OS.Constants.Path.profileDir, "datareporting");
 });
 
 XPCOMUtils.defineLazyGetter(this, "gStateFilePath", () => {
   return OS.Path.join(gDatareportingPath, "state.json");
 });
 
+const PREF_CACHED_CLIENTID = "toolkit.telemetry.cachedClientID";
+
 this.ClientID = Object.freeze({
   /**
    * This returns a promise resolving to the the stable client ID we use for
    * data reporting (FHR & Telemetry). Previously exising FHR client IDs are
    * migrated to this.
    *
    * @return {Promise<string>} The stable client ID.
    */
   getClientID: function() {
     return ClientIDImpl.getClientID();
   },
 
+/**
+   * 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: function() {
+    return ClientIDImpl.getCachedClientID();
+  },
+
   /**
    * Only used for testing. Invalidates the client ID so that it gets read
    * again from file.
    */
   _reset: function() {
     return ClientIDImpl._reset();
   },
 });
@@ -66,37 +80,40 @@ let ClientIDImpl = {
     // service, then to a common shared module.
     // Consequently, we try to import an existing FHR ID, so we can keep using it.
 
     // Try to load the client id from the DRS state file first.
     try {
       let state = yield CommonUtils.readJSON(gStateFilePath);
       if (state && 'clientID' in state && typeof(state.clientID) == 'string') {
         this._clientID = state.clientID;
+        Preferences.set(PREF_CACHED_CLIENTID, this._clientID);
         return this._clientID;
       }
     } catch (e) {
       // fall through to next option
     }
 
     // If we dont have DRS state yet, try to import from the FHR state.
     try {
       let fhrStatePath = OS.Path.join(OS.Constants.Path.profileDir, "healthreport", "state.json");
       let state = yield CommonUtils.readJSON(fhrStatePath);
       if (state && 'clientID' in state && typeof(state.clientID) == 'string') {
         this._clientID = state.clientID;
+        Preferences.set(PREF_CACHED_CLIENTID, this._clientID);
         this._saveClientID();
         return this._clientID;
       }
     } catch (e) {
       // fall through to next option
     }
 
     // We dont have an id from FHR yet, generate a new ID.
     this._clientID = CommonUtils.generateUUID();
+    Preferences.set(PREF_CACHED_CLIENTID, this._clientID);
     this._saveClientIdTask = this._saveClientID();
 
     // Wait on persisting the id. Otherwise failure to save the ID would result in
     // the client creating and subsequently sending multiple IDs to the server.
     // This would appear as multiple clients submitting similar data, which would
     // result in orphaning.
     yield this._saveClientIdTask;
 
@@ -125,16 +142,33 @@ let ClientIDImpl = {
   getClientID: function() {
     if (!this._clientID) {
       return this._loadClientID();
     }
 
     return Promise.resolve(this._clientID);
   },
 
+  /**
+   * 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: function() {
+    if (this._clientID) {
+      // Already loaded the client id from disk.
+      return this._clientID;
+    }
+
+    // Not yet loaded, return the cached client id if we have one.
+    return Preferences.get(PREF_CACHED_CLIENTID, null);
+  },
+
   /*
    * Resets the provider. This is for testing only.
    */
   _reset: Task.async(function* () {
     yield this._loadClientIdTask;
     yield this._saveClientIdTask;
     this._clientID = null;
   }),