Bug 1171253 - enable FxA profile image in Sync preferences pane. r=zaach
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 18 Jun 2015 19:28:11 +1000
changeset 249528 7e03a233b801345e49ffe80a318a2638a227bc33
parent 249527 e798232c7a8ace7900a4a85053f452843c1dbe52
child 249529 502391d38d886650ea71072c3d5ab9c50df8f5d6
push id28929
push userryanvm@gmail.com
push dateThu, 18 Jun 2015 19:51:41 +0000
treeherdermozilla-central@d56a1257088e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszaach
bugs1171253
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 1171253 - enable FxA profile image in Sync preferences pane. r=zaach
browser/app/profile/firefox.js
services/fxaccounts/FxAccounts.jsm
services/fxaccounts/FxAccountsProfile.jsm
services/fxaccounts/FxAccountsProfileClient.jsm
services/fxaccounts/tests/xpcshell/test_accounts.js
services/fxaccounts/tests/xpcshell/test_profile.js
services/fxaccounts/tests/xpcshell/test_profile_client.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1824,16 +1824,19 @@ pref("identity.fxaccounts.remote.webchan
 pref("identity.fxaccounts.settings.uri", "https://accounts.firefox.com/settings");
 
 // The remote URL of the FxA Profile Server
 pref("identity.fxaccounts.remote.profile.uri", "https://profile.accounts.firefox.com/v1");
 
 // The remote URL of the FxA OAuth Server
 pref("identity.fxaccounts.remote.oauth.uri", "https://oauth.accounts.firefox.com/v1");
 
+// Whether we display profile images in the UI or not.
+pref("identity.fxaccounts.profile_image.enabled", true);
+
 // Migrate any existing Firefox Account data from the default profile to the
 // Developer Edition profile.
 #ifdef MOZ_DEV_EDITION
 pref("identity.fxaccounts.migrateToDevEdition", true);
 #else
 pref("identity.fxaccounts.migrateToDevEdition", false);
 #endif
 
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -106,22 +106,16 @@ AccountState.prototype = {
       this.whenKeysReadyDeferred = null;
     }
 
     this.cert = null;
     this.keyPair = null;
     this.signedInUser = null;
     this.uid = null;
     this.fxaInternal = null;
-    this.initProfilePromise = null;
-
-    if (this.profile) {
-      this.profile.tearDown();
-      this.profile = null;
-    }
   },
 
   // Clobber all cached data and write that empty data to storage.
   signOut() {
     this.cert = null;
     this.keyPair = null;
     this.signedInUser = null;
     this.oauthTokens = {};
@@ -289,51 +283,16 @@ AccountState.prototype = {
       };
       log.debug("got keyPair");
       delete this.cert;
       d.resolve(this.keyPair.keyPair);
     });
     return d.promise.then(result => this.resolve(result));
   },
 
-  // Get the account's profile image URL from the profile server
-  getProfile: function () {
-    return this.initProfile()
-      .then(() => this.profile.getProfile());
-  },
-
-  // Instantiate a FxAccountsProfile with a fresh OAuth token if needed
-  initProfile: function () {
-
-    let profileServerUrl = Services.urlFormatter.formatURLPref("identity.fxaccounts.remote.profile.uri");
-
-    let oAuthOptions = {
-      scope: "profile"
-    };
-
-    if (this.initProfilePromise) {
-      return this.initProfilePromise;
-    }
-
-    this.initProfilePromise = this.fxaInternal.getOAuthToken(oAuthOptions)
-      .then(token => {
-        this.profile = new FxAccountsProfile(this, {
-          profileServerUrl: profileServerUrl,
-          token: token
-        });
-        this.initProfilePromise = null;
-      })
-      .then(null, err => {
-        this.initProfilePromise = null;
-        throw err;
-      });
-
-    return this.initProfilePromise;
-  },
-
   resolve: function(result) {
     if (!this.isCurrent) {
       log.info("An accountState promise was resolved, but was actually rejected" +
                " due to a different user being signed in. Originally resolved" +
                " with: " + result);
       return Promise.reject(new Error("A different user signed in"));
     }
     return Promise.resolve(result);
@@ -589,16 +548,29 @@ FxAccountsInternal.prototype = {
 
   get fxAccountsClient() {
     if (!this._fxAccountsClient) {
       this._fxAccountsClient = new FxAccountsClient();
     }
     return this._fxAccountsClient;
   },
 
+  // The profile object used to fetch the actual user profile.
+  _profile: null,
+  get profile() {
+    if (!this._profile) {
+      let profileServerUrl = Services.urlFormatter.formatURLPref("identity.fxaccounts.remote.profile.uri");
+      this._profile = new FxAccountsProfile({
+        fxa: this,
+        profileServerUrl: profileServerUrl,
+      });
+    }
+    return this._profile;
+  },
+
   /**
    * Return the current time in milliseconds as an integer.  Allows tests to
    * manipulate the date to simulate certificate expiration.
    */
   now: function() {
     return this.fxAccountsClient.now();
   },
 
@@ -844,16 +816,20 @@ FxAccountsInternal.prototype = {
   },
 
   /**
    * This function should be called in conjunction with a server-side
    * signOut via FxAccountsClient.
    */
   _signOutLocal: function signOutLocal() {
     let currentAccountState = this.currentAccountState;
+    if (this._profile) {
+      this._profile.tearDown();
+      this._profile = null;
+    }
     return currentAccountState.signOut().then(() => {
       this.abortExistingFlow(); // this resets this.currentAccountState.
     });
   },
 
   _signOutServer: function signOutServer(sessionToken) {
     return this.fxAccountsClient.signOut(sessionToken);
   },
@@ -1425,27 +1401,27 @@ FxAccountsInternal.prototype = {
    *          INVALID_PARAMETER
    *          NO_ACCOUNT
    *          UNVERIFIED_ACCOUNT
    *          NETWORK_ERROR
    *          AUTH_ERROR
    *          UNKNOWN_ERROR
    */
   getSignedInUserProfile: function () {
-    let accountState = this.currentAccountState;
-    return accountState.getProfile()
-      .then((profileData) => {
+    let currentState = this.currentAccountState;
+    return this.profile.getProfile().then(
+      profileData => {
         let profile = JSON.parse(JSON.stringify(profileData));
-        return accountState.resolve(profile);
+        return currentState.resolve(profile);
       },
-      (error) => {
+      error => {
         log.error("Could not retrieve profile data", error);
-        return accountState.reject(error);
-      })
-      .then(null, err => Promise.reject(this._errorToErrorClass(err)));
+        return currentState.reject(error);
+      }
+    ).catch(err => Promise.reject(this._errorToErrorClass(err)));
   },
 };
 
 /**
  * JSONStorage constructor that creates instances that may set/get
  * to a specified file, in a directory that will be created if it
  * doesn't exist.
  *
--- a/services/fxaccounts/FxAccountsProfile.jsm
+++ b/services/fxaccounts/FxAccountsProfile.jsm
@@ -12,19 +12,19 @@
  * the user's profile in open browser tabs, and cacheing/invalidating profile data.
  */
 
 this.EXPORTED_SYMBOLS = ["FxAccountsProfile"];
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
+Cu.import("resource://gre/modules/FxAccounts.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "FxAccountsProfileClient",
   "resource://gre/modules/FxAccountsProfileClient.jsm");
 
 // Based off of deepEqual from Assert.jsm
 function deepEqual(actual, expected) {
   if (actual === expected) {
     return true;
@@ -66,61 +66,61 @@ function objEquiv(a, b) {
   }
   return true;
 }
 
 function hasChanged(oldData, newData) {
   return !deepEqual(oldData, newData);
 }
 
-this.FxAccountsProfile = function (accountState, options = {}) {
-  this.currentAccountState = accountState;
+this.FxAccountsProfile = function (options = {}) {
+  this._cachedProfile = null;
+  this.fxa = options.fxa || fxAccounts;
   this.client = options.profileClient || new FxAccountsProfileClient({
+    fxa: this.fxa,
     serverURL: options.profileServerUrl,
-    token: options.token
   });
 
   // for testing
   if (options.channel) {
     this.channel = options.channel;
   }
 }
 
 this.FxAccountsProfile.prototype = {
 
   tearDown: function () {
-    this.currentAccountState = null;
+    this.fxa = null;
     this.client = null;
+    this._cachedProfile = null;
   },
 
   _getCachedProfile: function () {
-    let currentState = this.currentAccountState;
-    return currentState.getUserAccountData()
-      .then(cachedData => cachedData.profile);
+    // The cached profile will end up back in the generic accountData
+    // once bug 1157529 is fixed.
+    return Promise.resolve(this._cachedProfile);
   },
 
   _notifyProfileChange: function (uid) {
     Services.obs.notifyObservers(null, ON_PROFILE_CHANGE_NOTIFICATION, uid);
   },
 
   // Cache fetched data if it is different from what's in the cache.
   // Send out a notification if it has changed so that UI can update.
   _cacheProfile: function (profileData) {
-    let currentState = this.currentAccountState;
-    if (!currentState) {
-      return;
+    if (!hasChanged(this._cachedProfile, profileData)) {
+      log.debug("fetched profile matches cached copy");
+      return Promise.resolve(null); // indicates no change (but only tests care)
     }
-    return currentState.getUserAccountData()
-      .then(data => {
-        if (!hasChanged(data.profile, profileData)) {
-          return;
-        }
-        data.profile = profileData;
-        return currentState.setUserAccountData(data)
-          .then(() => this._notifyProfileChange(data.uid));
+    this._cachedProfile = profileData;
+    return this.fxa.getSignedInUser()
+      .then(userData => {
+        log.debug("notifying profile changed for user ${uid}", userData);
+        this._notifyProfileChange(userData.uid);
+        return profileData;
       });
   },
 
   _fetchAndCacheProfile: function () {
     return this.client.fetchProfile()
       .then(profile => {
         return this._cacheProfile(profile).then(() => profile);
       });
@@ -128,17 +128,21 @@ this.FxAccountsProfile.prototype = {
 
   // Returns cached data right away if available, then fetches the latest profile
   // data in the background. After data is fetched a notification will be sent
   // out if the profile has changed.
   getProfile: function () {
     return this._getCachedProfile()
       .then(cachedProfile => {
         if (cachedProfile) {
-          this._fetchAndCacheProfile();
+          // Note that _fetchAndCacheProfile isn't returned, so continues
+          // in the background.
+          this._fetchAndCacheProfile().catch(err => {
+            log.error("Background refresh of profile failed", err);
+          });
           return cachedProfile;
         }
         return this._fetchAndCacheProfile();
       })
       .then(profile => {
         return profile;
       });
   },
--- a/services/fxaccounts/FxAccountsProfileClient.jsm
+++ b/services/fxaccounts/FxAccountsProfileClient.jsm
@@ -1,89 +1,139 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
  * A client to fetch profile information for a Firefox Account.
  */
+ "use strict;"
 
 this.EXPORTED_SYMBOLS = ["FxAccountsProfileClient", "FxAccountsProfileClientError"];
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
+Cu.import("resource://gre/modules/FxAccounts.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://services-common/rest.js");
 
 Cu.importGlobalProperties(["URL"]);
 
 /**
  * Create a new FxAccountsProfileClient to be able to fetch Firefox Account profile information.
  *
  * @param {Object} options Options
  *   @param {String} options.serverURL
  *   The URL of the profile server to query.
  *   Example: https://profile.accounts.firefox.com/v1
  *   @param {String} options.token
  *   The bearer token to access the profile server
  * @constructor
  */
 this.FxAccountsProfileClient = function(options) {
-  if (!options || !options.serverURL || !options.token) {
-    throw new Error("Missing 'serverURL' or 'token' configuration option");
+  if (!options || !options.serverURL) {
+    throw new Error("Missing 'serverURL' configuration option");
   }
 
+  this.fxa = options.fxa || fxAccounts;
+  // This is a work-around for loop that manages its own oauth tokens.
+  // * If |token| is in options we use it and don't attempt any token refresh
+  //  on 401. This is for loop.
+  // * If |token| doesn't exist we will fetch our own token. This is for the
+  //   normal FxAccounts methods for obtaining the profile.
+  // We should nuke all |this.token| support once loop moves closer to FxAccounts.
+  this.token = options.token;
+
   try {
     this.serverURL = new URL(options.serverURL);
   } catch (e) {
     throw new Error("Invalid 'serverURL'");
   }
-  this.token = options.token;
+  this.oauthOptions = {
+    scope: "profile",
+  };
   log.debug("FxAccountsProfileClient: Initialized");
 };
 
 this.FxAccountsProfileClient.prototype = {
   /**
    * {nsIURI}
    * The server to fetch profile information from.
    */
   serverURL: null,
 
   /**
-   * {String}
-   * Profile server bearer OAuth token.
-   */
-  token: null,
-
-  /**
    * Interface for making remote requests.
    */
   _Request: RESTRequest,
 
   /**
-   * Remote request helper
+   * Remote request helper which abstracts authentication away.
    *
    * @param {String} path
    *        Profile server path, i.e "/profile".
    * @param {String} [method]
    *        Type of request, i.e "GET".
    * @return Promise
    *         Resolves: {Object} Successful response from the Profile server.
    *         Rejects: {FxAccountsProfileClientError} Profile client error.
    * @private
    */
-  _createRequest: function(path, method = "GET") {
+  _createRequest: Task.async(function* (path, method = "GET") {
+    let token = this.token;
+    if (!token) {
+      // tokens are cached, so getting them each request is cheap.
+      token = yield this.fxa.getOAuthToken(this.oauthOptions);
+    }
+    try {
+      return (yield this._rawRequest(path, method, token));
+    } catch (ex if ex instanceof FxAccountsProfileClientError && ex.code == 401) {
+      // If this object was instantiated with a token then we don't refresh it.
+      if (this.token) {
+        throw ex;
+      }
+      // it's an auth error - assume our token expired and retry.
+      log.info("Fetching the profile returned a 401 - revoking our token and retrying");
+      yield this.fxa.removeCachedOAuthToken({token});
+      token = yield this.fxa.getOAuthToken(this.oauthOptions);
+      // and try with the new token - if that also fails then we fail after
+      // revoking the token.
+      try {
+        return (yield this._rawRequest(path, method, token));
+      } catch (ex if ex instanceof FxAccountsProfileClientError && ex.code == 401) {
+        log.info("Retry fetching the profile still returned a 401 - revoking our token and failing");
+        yield this.fxa.removeCachedOAuthToken({token});
+        throw ex;
+      }
+    }
+  }),
+
+  /**
+   * Remote "raw" request helper - doesn't handle auth errors and tokens.
+   *
+   * @param {String} path
+   *        Profile server path, i.e "/profile".
+   * @param {String} method
+   *        Type of request, i.e "GET".
+   * @param {String} token
+   * @return Promise
+   *         Resolves: {Object} Successful response from the Profile server.
+   *         Rejects: {FxAccountsProfileClientError} Profile client error.
+   * @private
+   */
+  _rawRequest: function(path, method, token) {
     return new Promise((resolve, reject) => {
       let profileDataUrl = this.serverURL + path;
       let request = new this._Request(profileDataUrl);
       method = method.toUpperCase();
 
-      request.setHeader("Authorization", "Bearer " + this.token);
+      request.setHeader("Authorization", "Bearer " + token);
       request.setHeader("Accept", "application/json");
 
       request.onComplete = function (error) {
         if (error) {
           return reject(new FxAccountsProfileClientError({
             error: ERROR_NETWORK,
             errno: ERRNO_NETWORK,
             message: error.toString(),
@@ -101,17 +151,22 @@ this.FxAccountsProfileClient.prototype =
             message: request.response.body,
           }));
         }
 
         // "response.success" means status code is 200
         if (request.response.success) {
           return resolve(body);
         } else {
-          return reject(new FxAccountsProfileClientError(body));
+          return reject(new FxAccountsProfileClientError({
+            error: body.error || ERROR_UNKNOWN,
+            errno: body.errno || ERRNO_UNKNOWN_ERROR,
+            code: request.response.status,
+            message: body.message || body,
+          }));
         }
       };
 
       if (method === "GET") {
         request.get();
       } else {
         // method not supported
         return reject(new FxAccountsProfileClientError({
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -955,114 +955,75 @@ add_test(function test_getOAuthToken_unk
     fxa.getOAuthToken({ scope: "profile", client: client })
       .then(null, err => {
          do_check_eq(err.message, "UNKNOWN_ERROR");
          run_next_test();
       });
   });
 });
 
-add_test(function test_accountState_initProfile() {
-  let fxa = new MockFxAccounts();
-  let alice = getTestUser("alice");
-  alice.verified = true;
-
-  fxa.internal.getOAuthToken = function (opts) {
-    return Promise.resolve("token");
-  };
-
-  fxa.setSignedInUser(alice).then(() => {
-    let accountState = fxa.internal.currentAccountState;
-
-    accountState.initProfile(options)
-      .then(result => {
-         do_check_true(!!accountState.profile);
-         run_next_test();
-      });
-  });
-
-});
-
-add_test(function test_accountState_getProfile() {
-  let fxa = new MockFxAccounts();
+add_test(function test_getSignedInUserProfile() {
   let alice = getTestUser("alice");
   alice.verified = true;
 
   let mockProfile = {
     getProfile: function () {
       return Promise.resolve({ avatar: "image" });
     }
   };
+  let fxa = new FxAccounts({
+    _profile: mockProfile,
+  });
 
   fxa.setSignedInUser(alice).then(() => {
-    let accountState = fxa.internal.currentAccountState;
-    accountState.profile = mockProfile;
-    accountState.initProfilePromise = new Promise((resolve, reject) => resolve(mockProfile));
-
-    accountState.getProfile()
+    fxa.getSignedInUserProfile()
       .then(result => {
          do_check_true(!!result);
          do_check_eq(result.avatar, "image");
          run_next_test();
       });
   });
-
-});
-
-add_test(function test_getSignedInUserProfile_ok() {
-  let fxa = new MockFxAccounts();
-  let alice = getTestUser("alice");
-  alice.verified = true;
-
-  fxa.setSignedInUser(alice).then(() => {
-    let accountState = fxa.internal.currentAccountState;
-    accountState.getProfile = function () {
-      return Promise.resolve({ avatar: "image" });
-    };
-
-    fxa.getSignedInUserProfile()
-      .then(result => {
-         do_check_eq(result.avatar, "image");
-         run_next_test();
-      });
-  });
-
 });
 
 add_test(function test_getSignedInUserProfile_error_uses_account_data() {
   let fxa = new MockFxAccounts();
   let alice = getTestUser("alice");
   alice.verified = true;
 
   fxa.internal.getSignedInUser = function () {
     return Promise.resolve({ email: "foo@bar.com" });
   };
 
+  let teardownCalled = false;
   fxa.setSignedInUser(alice).then(() => {
-    let accountState = fxa.internal.currentAccountState;
-    accountState.getProfile = function () {
-      return Promise.reject("boom");
+    fxa.internal._profile = {
+      getProfile: function () {
+        return Promise.reject("boom");
+      },
+      tearDown: function() {
+        teardownCalled = true;
+      }
     };
 
     fxa.getSignedInUserProfile()
       .catch(error => {
-         do_check_eq(error.message, "UNKNOWN_ERROR");
-         fxa.signOut().then(run_next_test);
+        do_check_eq(error.message, "UNKNOWN_ERROR");
+        fxa.signOut().then(() => {
+          do_check_true(teardownCalled);
+          run_next_test();
+        });
       });
   });
-
 });
 
 add_test(function test_getSignedInUserProfile_unverified_account() {
   let fxa = new MockFxAccounts();
   let alice = getTestUser("alice");
 
   fxa.setSignedInUser(alice).then(() => {
-    let accountState = fxa.internal.currentAccountState;
-
     fxa.getSignedInUserProfile()
       .catch(error => {
          do_check_eq(error.message, "UNVERIFIED_ACCOUNT");
          fxa.signOut().then(run_next_test);
       });
   });
 
 });
--- a/services/fxaccounts/tests/xpcshell/test_profile.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile.js
@@ -6,22 +6,16 @@
 Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
 Cu.import("resource://gre/modules/FxAccountsProfileClient.jsm");
 Cu.import("resource://gre/modules/FxAccountsProfile.jsm");
 
 const URL_STRING = "https://example.com";
 Services.prefs.setCharPref("identity.fxaccounts.settings.uri", "https://example.com/settings");
 
-const PROFILE_CLIENT_OPTIONS = {
-  token: "123ABC",
-  serverURL: "http://127.0.0.1:1111/v1",
-  profileServerUrl: "http://127.0.0.1:1111/v1"
-};
-
 const STATUS_SUCCESS = 200;
 
 /**
  * Mock request responder
  * @param {String} response
  *        Mocked raw response from the server
  * @returns {Function}
  */
@@ -53,152 +47,166 @@ let mockResponseError = function (error)
       setHeader: function () {},
       head: function () {
         this.onComplete(error);
       }
     };
   };
 };
 
-let mockClient = function () {
-  let client = new FxAccountsProfileClient(PROFILE_CLIENT_OPTIONS);
-  return client;
+let mockClient = function (fxa) {
+  let options = {
+    serverURL: "http://127.0.0.1:1111/v1",
+    fxa: fxa,
+  }
+  return new FxAccountsProfileClient(options);
 };
 
 const ACCOUNT_DATA = {
   uid: "abc123"
 };
 
-function AccountData () {
+function FxaMock() {
 }
-AccountData.prototype = {
-  getUserAccountData: function () {
+FxaMock.prototype = {
+  currentAccountState: {
+    profile: null,
+    get isCurrent() true,
+  },
+
+  getSignedInUser: function () {
     return Promise.resolve(ACCOUNT_DATA);
   }
 };
 
-let mockAccountData = function () {
-  return new AccountData();
+let mockFxa = function() {
+  return new FxaMock();
 };
 
+function CreateFxAccountsProfile(fxa = null, client = null) {
+  if (!fxa) {
+    fxa = mockFxa();
+  }
+  let options = {
+    fxa: fxa,
+    profileServerUrl: "http://127.0.0.1:1111/v1"
+  }
+  if (client) {
+    options.profileClient = client;
+  }
+  return new FxAccountsProfile(options);
+}
+
 add_test(function getCachedProfile() {
-  let accountData = mockAccountData();
-  accountData.getUserAccountData = function () {
-    return Promise.resolve({
-      profile: { avatar: "myurl" }
-    });
-  };
-  let profile = new FxAccountsProfile(accountData, PROFILE_CLIENT_OPTIONS);
+  let profile = CreateFxAccountsProfile();
+  // a little pointless until bug 1157529 is fixed...
+  profile._cachedProfile = { avatar: "myurl" };
 
   return profile._getCachedProfile()
     .then(function (cached) {
       do_check_eq(cached.avatar, "myurl");
       run_next_test();
     });
 });
 
 add_test(function cacheProfile_change() {
-  let accountData = mockAccountData();
+  let fxa = mockFxa();
+/* Saving profile data disabled - bug 1157529
   let setUserAccountDataCalled = false;
-  accountData.setUserAccountData = function (data) {
+  fxa.setUserAccountData = function (data) {
     setUserAccountDataCalled = true;
     do_check_eq(data.profile.avatar, "myurl");
     return Promise.resolve();
   };
-  let profile = new FxAccountsProfile(accountData, PROFILE_CLIENT_OPTIONS);
+*/
+  let profile = CreateFxAccountsProfile(fxa);
 
   makeObserver(ON_PROFILE_CHANGE_NOTIFICATION, function (subject, topic, data) {
     do_check_eq(data, ACCOUNT_DATA.uid);
-    do_check_true(setUserAccountDataCalled);
+//    do_check_true(setUserAccountDataCalled); - bug 1157529
     run_next_test();
   });
 
   return profile._cacheProfile({ avatar: "myurl" });
 });
 
 add_test(function cacheProfile_no_change() {
-  let accountData = mockAccountData();
-  accountData.getUserAccountData = function () {
-    return Promise.resolve({
-      profile: { avatar: "myurl" }
-    });
-  };
-  accountData.setUserAccountData = function (data) {
+  let fxa = mockFxa();
+  let profile = CreateFxAccountsProfile(fxa)
+  profile._cachedProfile = { avatar: "myurl" };
+// XXX - saving is disabled (but we can leave that in for now as we are
+// just checking it is *not* called)
+  fxa.setSignedInUser = function (data) {
     throw new Error("should not update account data");
   };
-  let profile = new FxAccountsProfile(accountData, PROFILE_CLIENT_OPTIONS);
 
   return profile._cacheProfile({ avatar: "myurl" })
     .then((result) => {
       do_check_false(!!result);
       run_next_test();
     });
 });
 
 add_test(function fetchAndCacheProfile_ok() {
-  let client = mockClient();
+  let client = mockClient(mockFxa());
   client.fetchProfile = function () {
     return Promise.resolve({ avatar: "myimg"});
   };
-  let profile = new FxAccountsProfile(mockAccountData(), {
-    profileClient: client
-  });
+  let profile = CreateFxAccountsProfile(null, client);
 
   profile._cacheProfile = function (toCache) {
     do_check_eq(toCache.avatar, "myimg");
     return Promise.resolve();
   };
 
   return profile._fetchAndCacheProfile()
     .then(result => {
       do_check_eq(result.avatar, "myimg");
       run_next_test();
     });
 });
 
 add_test(function tearDown_ok() {
-  let profile = new FxAccountsProfile(mockAccountData(), PROFILE_CLIENT_OPTIONS);
+  let profile = CreateFxAccountsProfile();
 
   do_check_true(!!profile.client);
-  do_check_true(!!profile.currentAccountState);
+  do_check_true(!!profile.fxa);
 
   profile.tearDown();
-  do_check_null(profile.currentAccountState);
+  do_check_null(profile.fxa);
   do_check_null(profile.client);
 
   run_next_test();
 });
 
 add_test(function getProfile_ok() {
   let cachedUrl = "myurl";
-  let accountData = mockAccountData();
   let didFetch = false;
 
-  let profile = new FxAccountsProfile(accountData, PROFILE_CLIENT_OPTIONS);
+  let profile = CreateFxAccountsProfile();
   profile._getCachedProfile = function () {
     return Promise.resolve({ avatar: cachedUrl });
   };
 
   profile._fetchAndCacheProfile = function () {
     didFetch = true;
+    return Promise.resolve();
   };
 
   return profile.getProfile()
     .then(result => {
       do_check_eq(result.avatar, cachedUrl);
       do_check_true(didFetch);
       run_next_test();
     });
 });
 
 add_test(function getProfile_no_cache() {
   let fetchedUrl = "newUrl";
-  let accountData = mockAccountData();
-
-  let profile = new FxAccountsProfile(accountData, PROFILE_CLIENT_OPTIONS);
+  let profile = CreateFxAccountsProfile();
   profile._getCachedProfile = function () {
     return Promise.resolve();
   };
 
   profile._fetchAndCacheProfile = function () {
     return Promise.resolve({ avatar: fetchedUrl });
   };
 
@@ -207,33 +215,33 @@ add_test(function getProfile_no_cache() 
       do_check_eq(result.avatar, fetchedUrl);
       run_next_test();
     });
 });
 
 add_test(function getProfile_has_cached_fetch_deleted() {
   let cachedUrl = "myurl";
 
-  let client = mockClient();
+  let fxa = mockFxa();
+  let client = mockClient(fxa);
   client.fetchProfile = function () {
     return Promise.resolve({ avatar: null });
   };
 
-  let accountData = mockAccountData();
-  accountData.getUserAccountData = function () {
-    return Promise.resolve({ profile: { avatar: cachedUrl } });
-  };
-  accountData.setUserAccountData = function (data) {
-    do_check_null(data.profile.avatar);
-    run_next_test();
-    return Promise.resolve();
-  };
+  let profile = CreateFxAccountsProfile(fxa, client);
+  profile._cachedProfile = { avatar: cachedUrl };
 
-  let profile = new FxAccountsProfile(accountData, {
-    profileClient: client
+// instead of checking this in a mocked "save" function, just check after the
+// observer
+  makeObserver(ON_PROFILE_CHANGE_NOTIFICATION, function (subject, topic, data) {
+    profile.getProfile()
+      .then(profileData => {
+        do_check_null(profileData.avatar);
+        run_next_test();
+      });
   });
 
   return profile.getProfile()
     .then(result => {
       do_check_eq(result.avatar, "myurl");
     });
 });
 
--- a/services/fxaccounts/tests/xpcshell/test_profile_client.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile_client.js
@@ -1,21 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
 Cu.import("resource://gre/modules/FxAccountsProfileClient.jsm");
 
-const PROFILE_OPTIONS = {
-  token: "123ABC",
-  serverURL: "http://127.0.0.1:1111/v1",
-};
-
 const STATUS_SUCCESS = 200;
 
 /**
  * Mock request responder
  * @param {String} response
  *        Mocked raw response from the server
  * @returns {Function}
  */
@@ -30,16 +25,31 @@ let mockResponse = function (response) {
         this.onComplete();
       }
     };
   };
 
   return Request;
 };
 
+// A simple mock FxA that hands out tokens without checking them and doesn't
+// expect tokens to be revoked. We have specific token tests further down that
+// has more checks here.
+let mockFxa = {
+  getOAuthToken(options) {
+    do_check_eq(options.scope, "profile");
+    return "token";
+  }
+}
+
+const PROFILE_OPTIONS = {
+  serverURL: "http://127.0.0.1:1111/v1",
+  fxa: mockFxa,
+};
+
 /**
  * Mock request error responder
  * @param {Error} error
  *        Error object
  * @returns {Function}
  */
 let mockResponseError = function (error) {
   return function () {
@@ -93,39 +103,178 @@ add_test(function parseErrorResponse () 
         run_next_test();
       }
     );
 });
 
 add_test(function serverErrorResponse () {
   let client = new FxAccountsProfileClient(PROFILE_OPTIONS);
   let response = {
-    status: 401,
-    body: "{ \"code\": 401, \"errno\": 100, \"error\": \"Bad Request\", \"message\": \"Unauthorized\", \"reason\": \"Bearer token not provided\" }",
+    status: 500,
+    body: "{ \"code\": 500, \"errno\": 100, \"error\": \"Bad Request\", \"message\": \"Something went wrong\", \"reason\": \"Because the internet\" }",
   };
 
   client._Request = new mockResponse(response);
   client.fetchProfile()
     .then(
     null,
     function (e) {
       do_check_eq(e.name, "FxAccountsProfileClientError");
+      do_check_eq(e.code, 500);
+      do_check_eq(e.errno, 100);
+      do_check_eq(e.error, "Bad Request");
+      do_check_eq(e.message, "Something went wrong");
+      run_next_test();
+    }
+  );
+});
+
+// Test that we get a token, then if we get a 401 we revoke it, get a new one
+// and retry.
+add_test(function server401ResponseThenSuccess () {
+  // The last token we handed out.
+  let lastToken = -1;
+  // The number of times our removeCachedOAuthToken function was called.
+  let numTokensRemoved = 0;
+
+  let mockFxa = {
+    getOAuthToken(options) {
+      do_check_eq(options.scope, "profile");
+      return "" + ++lastToken; // tokens are strings.
+    },
+    removeCachedOAuthToken(options) {
+      // This test never has more than 1 token alive at once, so the token
+      // being revoked must always be the last token we handed out.
+      do_check_eq(parseInt(options.token), lastToken);
+      ++numTokensRemoved;
+    }
+  }
+  let profileOptions = {
+    serverURL: "http://127.0.0.1:1111/v1",
+    fxa: mockFxa,
+  };
+  let client = new FxAccountsProfileClient(profileOptions);
+
+  // 2 responses - first one implying the token has expired, second works.
+  let responses = [
+    {
+      status: 401,
+      body: "{ \"code\": 401, \"errno\": 100, \"error\": \"Token expired\", \"message\": \"That token is too old\", \"reason\": \"Because security\" }",
+    },
+    {
+      success: true,
+      status: STATUS_SUCCESS,
+      body: "{\"avatar\":\"http://example.com/image.jpg\",\"id\":\"0d5c1a89b8c54580b8e3e8adadae864a\"}",
+    },
+  ];
+
+  let numRequests = 0;
+  let numAuthHeaders = 0;
+  // Like mockResponse but we want access to headers etc.
+  client._Request = function(requestUri) {
+    return {
+      setHeader: function (name, value) {
+        if (name == "Authorization") {
+          numAuthHeaders++;
+          do_check_eq(value, "Bearer " + lastToken);
+        }
+      },
+      get: function () {
+        this.response = responses[numRequests];
+        ++numRequests;
+        this.onComplete();
+      }
+    };
+  }
+
+  client.fetchProfile()
+    .then(result => {
+      do_check_eq(result.avatar, "http://example.com/image.jpg");
+      do_check_eq(result.id, "0d5c1a89b8c54580b8e3e8adadae864a");
+      // should have been exactly 2 requests and exactly 2 auth headers.
+      do_check_eq(numRequests, 2);
+      do_check_eq(numAuthHeaders, 2);
+      // and we should have seen one token revoked.
+      do_check_eq(numTokensRemoved, 1);
+
+      run_next_test();
+    }
+  );
+});
+
+// Test that we get a token, then if we get a 401 we revoke it, get a new one
+// and retry - but we *still* get a 401 on the retry, so the caller sees that.
+add_test(function server401ResponsePersists () {
+  // The last token we handed out.
+  let lastToken = -1;
+  // The number of times our removeCachedOAuthToken function was called.
+  let numTokensRemoved = 0;
+
+  let mockFxa = {
+    getOAuthToken(options) {
+      do_check_eq(options.scope, "profile");
+      return "" + ++lastToken; // tokens are strings.
+    },
+    removeCachedOAuthToken(options) {
+      // This test never has more than 1 token alive at once, so the token
+      // being revoked must always be the last token we handed out.
+      do_check_eq(parseInt(options.token), lastToken);
+      ++numTokensRemoved;
+    }
+  }
+  let profileOptions = {
+    serverURL: "http://127.0.0.1:1111/v1",
+    fxa: mockFxa,
+  };
+  let client = new FxAccountsProfileClient(profileOptions);
+
+  let response = {
+      status: 401,
+      body: "{ \"code\": 401, \"errno\": 100, \"error\": \"It's not your token, it's you!\", \"message\": \"I don't like you\", \"reason\": \"Because security\" }",
+  };
+
+  let numRequests = 0;
+  let numAuthHeaders = 0;
+  client._Request = function(requestUri) {
+    return {
+      setHeader: function (name, value) {
+        if (name == "Authorization") {
+          numAuthHeaders++;
+          do_check_eq(value, "Bearer " + lastToken);
+        }
+      },
+      get: function () {
+        this.response = response;
+        ++numRequests;
+        this.onComplete();
+      }
+    };
+  }
+
+  client.fetchProfile().then(
+    null,
+    function (e) {
+      do_check_eq(e.name, "FxAccountsProfileClientError");
       do_check_eq(e.code, 401);
       do_check_eq(e.errno, 100);
-      do_check_eq(e.error, "Bad Request");
-      do_check_eq(e.message, "Unauthorized");
+      do_check_eq(e.error, "It's not your token, it's you!");
+      // should have been exactly 2 requests and exactly 2 auth headers.
+      do_check_eq(numRequests, 2);
+      do_check_eq(numAuthHeaders, 2);
+      // and we should have seen both tokens revoked.
+      do_check_eq(numTokensRemoved, 2);
       run_next_test();
     }
   );
 });
 
 add_test(function networkErrorResponse () {
   let client = new FxAccountsProfileClient({
-    token: "123ABC",
-    serverURL: "http://"
+    serverURL: "http://",
+    fxa: mockFxa,
   });
   client.fetchProfile()
     .then(
       null,
       function (e) {
         do_check_eq(e.name, "FxAccountsProfileClientError");
         do_check_eq(e.code, null);
         do_check_eq(e.errno, ERRNO_NETWORK);
@@ -186,28 +335,22 @@ add_test(function fetchProfileImage_succ
         do_check_eq(result.id, "0d5c1a89b8c54580b8e3e8adadae864a");
         run_next_test();
       }
     );
 });
 
 add_test(function constructorTests() {
   validationHelper(undefined,
-    "Error: Missing 'serverURL' or 'token' configuration option");
+    "Error: Missing 'serverURL' configuration option");
 
   validationHelper({},
-    "Error: Missing 'serverURL' or 'token' configuration option");
-
-  validationHelper({ serverURL: "http://example.com" },
-    "Error: Missing 'serverURL' or 'token' configuration option");
+    "Error: Missing 'serverURL' configuration option");
 
-  validationHelper({ token: "123ABC" },
-    "Error: Missing 'serverURL' or 'token' configuration option");
-
-  validationHelper({ token: "123ABC", serverURL: "badUrl" },
+  validationHelper({ serverURL: "badUrl" },
     "Error: Invalid 'serverURL'");
 
   run_next_test();
 });
 
 add_test(function errorTests() {
   let error1 = new FxAccountsProfileClientError();
   do_check_eq(error1.name, "FxAccountsProfileClientError");
@@ -250,15 +393,19 @@ function run_test() {
  *
  * @param {Object} options
  *        FxAccountsProfileClient constructor options
  * @param {String} expected
  *        Expected error message
  * @returns {*}
  */
 function validationHelper(options, expected) {
+  // add fxa to options - that missing isn't what we are testing here.
+  if (options) {
+    options.fxa = mockFxa;
+  }
   try {
     new FxAccountsProfileClient(options);
   } catch (e) {
     return do_check_eq(e.toString(), expected);
   }
   throw new Error("Validation helper error");
 }