Bug 1190279 - fix UI for unverified FxA users in both hamburger menu and Sync prefs pane. r=oeger

browser/base/content/aboutaccounts/aboutaccounts.js

@@ -358,6 +358,8 @@ function init() {
       }
       break;
     }
+  }).catch(err => {
+    error("Failed to get the signed in user: " + err);
   });
 }
 

browser/base/content/browser-fxaccounts.js

@@ -33,6 +33,7 @@ let gFxAccounts = {
       "weave:service:setup-complete",
       "weave:ui:login:error",
       "fxa-migration:state-changed",
+      this.FxAccountsCommon.ONLOGIN_NOTIFICATION,
       this.FxAccountsCommon.ONVERIFIED_NOTIFICATION,
       this.FxAccountsCommon.ONLOGOUT_NOTIFICATION,
       "weave:notification:removed",
@@ -222,10 +223,11 @@ let gFxAccounts = {
     this.updateMigrationNotification();
   },
 
+  // Note that updateAppMenuItem() returns a Promise that's only used by tests.
   updateAppMenuItem: function () {
     if (this._migrationInfo) {
       this.updateAppMenuItemForMigration();
-      return;
+      return Promise.resolve();
     }
 
     let profileInfoEnabled = false;
@@ -241,7 +243,7 @@ let gFxAccounts = {
       // state once migration is complete.
       this.panelUIFooter.hidden = true;
       this.panelUIFooter.removeAttribute("fxastatus");
-      return;
+      return Promise.resolve();
     }
 
     this.panelUIFooter.hidden = false;
@@ -311,12 +313,18 @@ let gFxAccounts = {
       }
     }
 
-    // Calling getSignedInUserProfile() without a user logged in causes log
-    // noise that looks like an actual error...
-    fxAccounts.getSignedInUser().then(userData => {
+    return fxAccounts.getSignedInUser().then(userData => {
       // userData may be null here when the user is not signed-in, but that's expected
       updateWithUserData(userData);
-      return userData ? fxAccounts.getSignedInUserProfile() : null;
+      // 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) {
+        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;
@@ -327,7 +335,7 @@ let gFxAccounts = {
       // 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 profile", error);
+      this.FxAccountsCommon.log.error("Error updating FxA account info", error);
       updateWithUserData(null);
     });
   },

browser/base/content/test/general/browser.ini

@@ -303,6 +303,8 @@ skip-if = true # browser_drag.js is disabled, as it needs to be updated for the
 [browser_focusonkeydown.js]
 [browser_fullscreen-window-open.js]
 skip-if = buildapp == 'mulet' || e10s || os == "linux" # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly. Linux: Intermittent failures - bug 941575.
+[browser_fxaccounts.js]
+support-files = fxa_profile_handler.sjs
 [browser_fxa_migrate.js]
 [browser_fxa_oauth.js]
 [browser_fxa_web_channel.js]

browser/base/content/test/general/browser_fxaccounts.js

@@ -0,0 +1,258 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+let {Log} = Cu.import("resource://gre/modules/Log.jsm", {});
+let {Task} = Cu.import("resource://gre/modules/Task.jsm", {});
+let {fxAccounts} = Cu.import("resource://gre/modules/FxAccounts.jsm", {});
+let FxAccountsCommon = {};
+Cu.import("resource://gre/modules/FxAccountsCommon.js", FxAccountsCommon);
+
+const TEST_ROOT = "http://example.com/browser/browser/base/content/test/general/";
+
+// instrument gFxAccounts to send observer notifications when it's done
+// what it does.
+(function() {
+  let unstubs = {}; // The original functions we stub out.
+
+  // The stub functions.
+  let stubs = {
+    updateAppMenuItem: function() {
+      return unstubs['updateAppMenuItem'].call(gFxAccounts).then(() => {
+        Services.obs.notifyObservers(null, "test:browser_fxaccounts:updateAppMenuItem", null);
+      });
+    },
+    // Opening preferences is trickier than it should be as leaks are reported
+    // due to the promises it fires off at load time  and there's no clear way to
+    // know when they are done.
+    // So just ensure openPreferences is called rather than whether it opens.
+    openPreferences: function() {
+      Services.obs.notifyObservers(null, "test:browser_fxaccounts:openPreferences", null);
+    }
+  };
+
+  for (let name in stubs) {
+    unstubs[name] = gFxAccounts[name];
+    gFxAccounts[name] = stubs[name];
+  }
+  // and undo our damage at the end.
+  registerCleanupFunction(() => {
+    for (let name in unstubs) {
+      gFxAccounts[name] = unstubs[name];
+    }
+    stubs = unstubs = null;
+  });
+})();
+
+// Other setup/cleanup
+let newTab;
+
+Services.prefs.setCharPref("identity.fxaccounts.remote.signup.uri",
+                           TEST_ROOT + "accounts_testRemoteCommands.html");
+
+registerCleanupFunction(() => {
+  Services.prefs.clearUserPref("identity.fxaccounts.remote.signup.uri");
+  Services.prefs.clearUserPref("identity.fxaccounts.remote.profile.uri");
+  gBrowser.removeTab(newTab);
+});
+
+add_task(function* initialize() {
+  // Set a new tab with something other than about:blank, so it doesn't get reused.
+  // We must wait for it to load or the promiseTabOpen() call in the next test
+  // gets confused.
+  newTab = gBrowser.selectedTab = gBrowser.addTab("about:mozilla", {animate: false});
+  yield promiseTabLoaded(newTab);
+});
+
+// The elements we care about.
+let panelUILabel = document.getElementById("PanelUI-fxa-label");
+let panelUIStatus = document.getElementById("PanelUI-fxa-status");
+let panelUIFooter = document.getElementById("PanelUI-footer-fxa");
+
+// The tests
+add_task(function* test_nouser() {
+  let user = yield fxAccounts.getSignedInUser();
+  Assert.strictEqual(user, null, "start with no user signed in");
+  let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem");
+  Services.obs.notifyObservers(null, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, null);
+  yield promiseUpdateDone;
+
+  // Check the world - the FxA footer area is visible as it is offering a signin.
+  Assert.ok(isFooterVisible())
+
+  Assert.equal(panelUILabel.getAttribute("label"), panelUIStatus.getAttribute("defaultlabel"));
+  Assert.ok(!panelUIStatus.hasAttribute("tooltiptext"), "no tooltip when signed out");
+  Assert.ok(!panelUIFooter.hasAttribute("fxastatus"), "no fxsstatus when signed out");
+  Assert.ok(!panelUIFooter.hasAttribute("fxaprofileimage"), "no fxaprofileimage when signed out");
+
+  let promiseOpen = promiseTabOpen("about:accounts?entryPoint=menupanel");
+  panelUIStatus.click();
+  yield promiseOpen;
+});
+
+/*
+XXX - Bug 1191162 - need a better hawk mock story or this will leak in debug builds.
+
+add_task(function* test_unverifiedUser() {
+  let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem");
+  yield setSignedInUser(false); // 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"),
+               panelUIStatus.getAttribute("signedinTooltiptext"));
+  Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin");
+  let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences");
+  panelUIStatus.click();
+  yield promisePreferencesOpened
+  yield signOut();
+});
+*/
+
+add_task(function* test_verifiedUserEmptyProfile() {
+  // We see 2 updateAppMenuItem() 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:updateAppMenuItem", 2);
+  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"),
+               panelUIStatus.getAttribute("signedinTooltiptext"));
+  Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin");
+
+  let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences");
+  panelUIStatus.click();
+  yield promisePreferencesOpened;
+  yield signOut();
+});
+
+add_task(function* test_verifiedUserDisplayName() {
+  let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2);
+  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"));
+  Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin");
+  yield signOut();
+});
+
+add_task(function* test_verifiedUserProfileFailure() {
+  // profile failure means only one observer fires.
+  let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 1);
+  configureProfileURL(null, 500);
+  yield setSignedInUser(true); // this will fire the observer that does the update.
+  yield promiseUpdateDone;
+
+  Assert.ok(isFooterVisible())
+  Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com");
+  Assert.equal(panelUIStatus.getAttribute("tooltiptext"),
+               panelUIStatus.getAttribute("signedinTooltiptext"));
+  Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin");
+  yield signOut();
+});
+
+// Helpers.
+function isFooterVisible() {
+  let style = window.getComputedStyle(panelUIFooter);
+  return style.getPropertyValue("display") == "flex";
+}
+
+function configureProfileURL(profile, responseStatus = 200) {
+  let responseBody = profile ? JSON.stringify(profile) : "";
+  let url = TEST_ROOT + "fxa_profile_handler.sjs?" +
+            "responseStatus=" + responseStatus +
+            "responseBody=" + responseBody +
+            // This is a bit cheeky - the FxA code will just append "/profile"
+            // to the preference value. We arrange for this to be seen by our
+            //.sjs as part of the query string.
+            "&path=";
+
+  Services.prefs.setCharPref("identity.fxaccounts.remote.profile.uri", url);
+}
+
+function promiseObserver(topic, count = 1) {
+  return new Promise(resolve => {
+    let obs = (subject, topic, data) => {
+      if (--count == 0) {
+        Services.obs.removeObserver(obs, topic);
+        resolve(subject);
+      }
+    }
+    Services.obs.addObserver(obs, topic, false);
+  });
+}
+
+// Stolen from browser_aboutHome.js
+function promiseWaitForEvent(node, type, capturing) {
+  return new Promise((resolve) => {
+    node.addEventListener(type, function listener(event) {
+      node.removeEventListener(type, listener, capturing);
+      resolve(event);
+    }, capturing);
+  });
+}
+
+let promiseTabOpen = Task.async(function*(urlBase) {
+  info("Waiting for tab to open...");
+  let event = yield promiseWaitForEvent(gBrowser.tabContainer, "TabOpen", true);
+  let tab = event.target;
+  yield promiseTabLoadEvent(tab);
+  ok(tab.linkedBrowser.currentURI.spec.startsWith(urlBase),
+     "Got " + tab.linkedBrowser.currentURI.spec + ", expecting " + urlBase);
+  let whenUnloaded = promiseTabUnloaded(tab);
+  gBrowser.removeTab(tab);
+  yield whenUnloaded;
+});
+
+function promiseTabUnloaded(tab)
+{
+  return new Promise(resolve => {
+    info("Wait for tab to unload");
+    function handle(event) {
+      tab.linkedBrowser.removeEventListener("unload", handle, true);
+      info("Got unload event");
+      resolve(event);
+    }
+    tab.linkedBrowser.addEventListener("unload", handle, true, true);
+  });
+}
+
+// FxAccounts helpers.
+function setSignedInUser(verified) {
+  let data = {
+    email: "foo@example.com",
+    uid: "1234@lcip.org",
+    assertion: "foobar",
+    sessionToken: "dead",
+    kA: "beef",
+    kB: "cafe",
+    verified: verified,
+
+    oauthTokens: {
+      // a token for the profile server.
+      profile: "key value",
+    }
+  }
+  return fxAccounts.setSignedInUser(data);
+}
+
+let signOut = Task.async(function* () {
+  // This test needs to make sure that any updates for the logout have
+  // completed before starting the next test, or we see the observer
+  // notifications get out of sync.
+  let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem");
+  // we always want a "localOnly" signout here...
+  yield fxAccounts.signOut(true);
+  yield promiseUpdateDone;
+});

browser/base/content/test/general/fxa_profile_handler.sjs

@@ -0,0 +1,34 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This is basically an echo server!
+// We just grab responseStatus and responseBody query params!
+
+function reallyHandleRequest(request, response) {
+  var query = "?" + request.queryString;
+
+  var responseStatus = 200;
+  var match = /responseStatus=([^&]*)/.exec(query);
+  if (match) {
+    responseStatus = parseInt(match[1]);
+  }
+
+  var responseBody = "";
+  match = /responseBody=([^&]*)/.exec(query);
+  if (match) {
+    responseBody = decodeURIComponent(match[1]);
+  }
+
+  response.setStatusLine("1.0", responseStatus, "OK");
+  response.write(responseBody);
+}
+
+function handleRequest(request, response)
+{
+  try {
+    reallyHandleRequest(request, response);
+  } catch (e) {
+    response.setStatusLine("1.0", 500, "NotOK");
+    response.write("Error handling request: " + e);
+  }
+}

browser/components/preferences/in-content/sync.js

@@ -119,6 +119,7 @@ let gSyncPane = {
                   "weave:service:setup-complete",
                   "weave:service:logout:finish",
                   FxAccountsCommon.ONVERIFIED_NOTIFICATION,
+                  FxAccountsCommon.ONLOGIN_NOTIFICATION,
                   FxAccountsCommon.ON_PROFILE_CHANGE_NOTIFICATION,
                   ];
     let migrateTopic = "fxa-migration:state-changed";