Bug 1335538 - Don't fetch the FxA profile on every UI update in browser-fxaccounts. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Wed, 01 Feb 2017 13:27:37 -0500
changeset 469718 5da7f4b87868281d7bd249c1e730361986cc506e
parent 469123 f3d187bd0733b1182dffc97b5dfe623e18f92a44
child 544277 467cfd8c013bd5f303599bb7bdabf7ac91198bec
push id43810
push userbmo:eoger@fastmail.com
push dateThu, 02 Feb 2017 16:42:17 +0000
reviewersmarkh
bugs1335538
milestone54.0a1
Bug 1335538 - Don't fetch the FxA profile on every UI update in browser-fxaccounts. r?markh MozReview-Commit-ID: F95hmHuWTW6
browser/base/content/browser-fxaccounts.js
browser/base/content/test/general/browser_fxaccounts.js
--- a/browser/base/content/browser-fxaccounts.js
+++ b/browser/base/content/browser-fxaccounts.js
@@ -1,16 +1,17 @@
 /* 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/. */
 
 var gFxAccounts = {
 
   _initialized: false,
   _inCustomizationMode: false,
+  _profileFetched: false,
 
   get weave() {
     delete this.weave;
     return this.weave = Cc["@mozilla.org/weave/service;1"]
                           .getService(Ci.nsISupports)
                           .wrappedJSObject;
   },
 
@@ -135,19 +136,19 @@ var gFxAccounts = {
       Services.obs.removeObserver(this, topic);
     }
 
     this._initialized = false;
   },
 
   observe(subject, topic, data) {
     switch (topic) {
-      case this.FxAccountsCommon.ONPROFILE_IMAGE_CHANGE_NOTIFICATION:
-        this.updateUI();
-        break;
+      case this.FxAccountsCommon.ON_PROFILE_CHANGE_NOTIFICATION:
+        this._profileFetched = false;
+        // Fallthrough intended
       default:
         this.updateUI();
         break;
     }
   },
 
   handleEvent(event) {
     this._inCustomizationMode = event.type == "customizationstarting";
@@ -249,28 +250,29 @@ var gFxAccounts = {
       }
     }
 
     return fxAccounts.getSignedInUser().then(userData => {
       // userData may be null here when the user is not signed-in, but that's expected
       updateWithUserData(userData);
       // unverified users cause us to spew log errors fetching an OAuth token
       // to fetch the profile, so don't even try in that case.
-      if (!userData || !userData.verified || !profileInfoEnabled) {
+      if (!userData || !userData.verified || !profileInfoEnabled || this._profileFetched) {
         return null; // don't even try to grab the profile.
       }
       return fxAccounts.getSignedInUserProfile().catch(err => {
         // Not fetching the profile is sad but the FxA logs will already have noise.
         return null;
       });
     }).then(profile => {
       if (!profile) {
         return;
       }
       updateWithProfile(profile);
+      this._profileFetched = true; // Try to avoid fetching the profile on every UI update
     }).catch(error => {
       // This is most likely in tests, were we quickly log users in and out.
       // The most likely scenario is a user logged out, so reflect that.
       // Bug 995134 calls for better errors so we could retry if we were
       // sure this was the failure reason.
       this.FxAccountsCommon.log.error("Error updating FxA account info", error);
       updateWithUserData(null);
     });
--- a/browser/base/content/test/general/browser_fxaccounts.js
+++ b/browser/base/content/test/general/browser_fxaccounts.js
@@ -111,16 +111,17 @@ add_task(function* test_unverifiedUser()
 });
 */
 
 add_task(function* test_verifiedUserEmptyProfile() {
   // We see 2 updateUI() calls - one for the signedInUser and one after
   // we first fetch the profile. We want them both to fire or we aren't testing
   // the state we think we are testing.
   let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateUI", 2);
+  gFxAccounts._profileFetched = false;
   configureProfileURL({}); // successful but empty profile.
   yield setSignedInUser(true); // this will fire the observer that does the update.
   yield promiseUpdateDone;
 
   // Check the world.
   Assert.ok(isFooterVisible())
   Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com");
   Assert.equal(panelUIStatus.getAttribute("tooltiptext"),
@@ -130,16 +131,17 @@ add_task(function* test_verifiedUserEmpt
   let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences");
   panelUIStatus.click();
   yield promisePreferencesOpened;
   yield signOut();
 });
 
 add_task(function* test_verifiedUserDisplayName() {
   let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateUI", 2);
+  gFxAccounts._profileFetched = false;
   configureProfileURL({ displayName: "Test User Display Name" });
   yield setSignedInUser(true); // this will fire the observer that does the update.
   yield promiseUpdateDone;
 
   Assert.ok(isFooterVisible())
   Assert.equal(panelUILabel.getAttribute("label"), "Test User Display Name");
   Assert.equal(panelUIStatus.getAttribute("tooltiptext"),
                panelUIStatus.getAttribute("signedinTooltiptext"));