Bug 1139743 (part 2) - make the 'current account data' less racey. r=ckarlof
authorMark Hammond <mhammond@skippinet.com.au>
Fri, 03 Apr 2015 12:47:00 +1100
changeset 267449 46415f2b0f35a59b1ed688056eb6b689eba63cfe
parent 267448 d674c20be9f62d64bbd586e51d941759c6c371e9
child 267450 a450bb0b9d62e4cfc74aaeb507792b9288d1ed44
push id4830
push userjlund@mozilla.com
push dateMon, 29 Jun 2015 20:18:48 +0000
treeherdermozilla-beta@4c2175bb0420 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckarlof
bugs1139743
milestone40.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 1139743 (part 2) - make the 'current account data' less racey. r=ckarlof
services/fxaccounts/FxAccounts.jsm
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -76,16 +76,17 @@ let AccountState = function(fxaInternal)
 
 AccountState.prototype = {
   cert: null,
   keyPair: null,
   signedInUser: null,
   whenVerifiedDeferred: null,
   whenKeysReadyDeferred: null,
   profile: null,
+  promiseInitialAccountData: null,
 
   get isCurrent() this.fxaInternal && this.fxaInternal.currentAccountState === this,
 
   abort: function() {
     if (this.whenVerifiedDeferred) {
       this.whenVerifiedDeferred.reject(
         new Error("Verification aborted; Another user signing in"));
       this.whenVerifiedDeferred = null;
@@ -105,55 +106,67 @@ AccountState.prototype = {
 
     if (this.profile) {
       this.profile.tearDown();
       this.profile = null;
     }
   },
 
   getUserAccountData: function() {
-    // Skip disk if user is cached.
+    if (!this.isCurrent) {
+      return this.reject(new Error("Another user has signed in"));
+    }
+    if (this.promiseInitialAccountData) {
+      // We are still reading the data for the first and only time.
+      return this.promiseInitialAccountData;
+    }
+    // We've previously read it successfully (and possibly updated it since)
     if (this.signedInUser) {
       return this.resolve(this.signedInUser.accountData);
     }
 
-    return this.fxaInternal.signedInUserStorage.get().then(
+    // First and only read.
+    return this.promiseInitialAccountData = this.fxaInternal.signedInUserStorage.get().then(
       user => {
         if (logPII) {
           // don't stringify unless it will be written. We should replace this
           // check with param substitutions added in bug 966674
           log.debug("getUserAccountData -> " + JSON.stringify(user));
         }
         if (user && user.version == this.version) {
           log.debug("setting signed in user");
           this.signedInUser = user;
         }
+        this.promiseInitialAccountData = null;
         return this.resolve(user ? user.accountData : null);
       },
       err => {
+        this.promiseInitialAccountData = null;
         if (err instanceof OS.File.Error && err.becauseNoSuchFile) {
           // File hasn't been created yet.  That will be done
           // on the first call to getSignedInUser
           return this.resolve(null);
         }
         return this.reject(err);
       }
     );
   },
 
   setUserAccountData: function(accountData) {
-    return this.fxaInternal.signedInUserStorage.get().then(record => {
-      if (!this.isCurrent) {
-        return this.reject(new Error("Another user has signed in"));
-      }
-      record.accountData = accountData;
-      this.signedInUser = record;
-      return this.fxaInternal.signedInUserStorage.set(record)
+    if (!this.isCurrent) {
+      return this.reject(new Error("Another user has signed in"));
+    }
+    if (this.promiseInitialAccountData) {
+      throw new Error("Can't set account data before it's been read.");
+    }
+    // Set our signedInUser before we start the write, so any updates to the
+    // data while the write completes are still captured.
+    this.signedInUser = {version: DATA_FORMAT_VERSION, accountData: accountData};
+    return this.fxaInternal.signedInUserStorage.set(this.signedInUser)
         .then(() => this.resolve(accountData));
-    });
   },
 
 
   getCertificate: function(data, keyPair, mustBeValidUntil) {
     if (logPII) {
       // don't stringify unless it will be written. We should replace this
       // check with param substitutions added in bug 966674
       log.debug("getCertificate" + JSON.stringify(this.signedInUser));