Bug 956605 - have about:accounts do the right thing based on user logged in states. r=ckarlof

browser/base/content/aboutaccounts/aboutaccounts.js

@@ -9,9 +9,17 @@ const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/FxAccounts.jsm");
 
+let fxAccountsCommon = {};
+Cu.import("resource://gre/modules/FxAccountsCommon.js", fxAccountsCommon);
+
 const PREF_LAST_FXA_USER = "identity.fxaccounts.lastSignedInUserHash";
 const PREF_SYNC_SHOW_CUSTOMIZATION = "services.sync.ui.showCustomizationDialog";
 
+const OBSERVER_TOPICS = [
+  fxAccountsCommon.ONVERIFIED_NOTIFICATION,
+  fxAccountsCommon.ONLOGOUT_NOTIFICATION,
+];
+
 function log(msg) {
   //dump("FXA: " + msg + "\n");
 };
@@ -150,6 +158,11 @@ let wrapper = {
     xps.whenLoaded().then(() => {
       return fxAccounts.setSignedInUser(accountData);
     }).then(() => {
+      // If the user data is verified, we want it to immediately look like
+      // they are signed in without waiting for messages to bounce around.
+      if (accountData.verified) {
+        showManage();
+      }
       this.injectData("message", { status: "login" });
       // until we sort out a better UX, just leave the jelly page in place.
       // If the account email is not yet verified, it will tell the user to
@@ -249,23 +262,36 @@ function openPrefs() {
 }
 
 function init() {
-  if (window.location.href.contains("action=signin")) {
-    show("remote");
-    wrapper.init(fxAccounts.getAccountsSignInURI());
-  } else if (window.location.href.contains("action=signup")) {
-    show("remote");
-    wrapper.init();
-  } else if (window.location.href.contains("action=reauth")) {
-    fxAccounts.promiseAccountsForceSigninURI().then(url => {
-      show("remote");
-      wrapper.init(url);
-    });
-  } else {
-    // Check if we have a local account
-    fxAccounts.getSignedInUser().then(user => {
+  fxAccounts.getSignedInUser().then(user => {
+    if (window.location.href.contains("action=signin")) {
       if (user) {
-        show("stage");
-        show("manage");
+        // asking to sign-in when already signed in just shows manage.
+        showManage();
+      } else {
+        show("remote");
+        wrapper.init(fxAccounts.getAccountsSignInURI());
+      }
+    } else if (window.location.href.contains("action=signup")) {
+      if (user) {
+        // asking to sign-up when already signed in just shows manage.
+        showManage();
+      } else {
+        show("remote");
+        wrapper.init();
+      }
+    } else if (window.location.href.contains("action=reauth")) {
+      // ideally we would only show this when we know the user is in a
+      // "must reauthenticate" state - but we don't.
+      // As the email address will be included in the URL returned from
+      // promiseAccountsForceSigninURI, just always show it.
+      fxAccounts.promiseAccountsForceSigninURI().then(url => {
+        show("remote");
+        wrapper.init(url);
+      });
+    } else {
+      // No action specified
+      if (user) {
+        showManage();
         let sb = Services.strings.createBundle("chrome://browser/locale/syncSetup.properties");
         document.title = sb.GetStringFromName("manage.pageTitle");
       } else {
@@ -274,8 +300,8 @@ function init() {
         // load the remote frame in the background
         wrapper.init();
       }
-    });
-  }
+    }
+  });
 }
 
 function show(id) {
@@ -285,7 +311,38 @@ function hide(id) {
   document.getElementById(id).style.display = 'none';
 }
 
+function showManage() {
+  show("stage");
+  show("manage");
+  hide("remote");
+  hide("intro");
+}
+
 document.addEventListener("DOMContentLoaded", function onload() {
   document.removeEventListener("DOMContentLoaded", onload, true);
   init();
 }, true);
+
+function initObservers() {
+  function observe(subject, topic, data) {
+    log("about:accounts observed " + topic);
+    if (topic == fxAccountsCommon.ONLOGOUT_NOTIFICATION) {
+      // All about:account windows get changed to action=signin on logout.
+      window.location = "about:accounts?action=signin";
+      return;
+    }
+    // must be onverified - just about:accounts is loaded.
+    window.location = "about:accounts";
+  }
+
+  for (let topic of OBSERVER_TOPICS) {
+    Services.obs.addObserver(observe, topic, false);
+  }
+  window.addEventListener("unload", function(event) {
+    log("about:accounts unloading")
+    for (let topic of OBSERVER_TOPICS) {
+      Services.obs.removeObserver(observe, topic);
+    }
+  });
+}
+initObservers();

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

@@ -28,7 +28,10 @@ registerCleanupFunction(function() {
 let gTests = [
 {
   desc: "Test the remote commands",
-  teardown: () => gBrowser.removeCurrentTab(),
+  teardown: function* () {
+    gBrowser.removeCurrentTab();
+    yield fxAccounts.signOut();
+  },
   run: function* ()
   {
     setPref("identity.fxaccounts.remote.uri",
@@ -57,22 +60,57 @@ let gTests = [
       ok(false, "Failed to get all commands");
       deferred.reject();
     }
-    return deferred.promise.then(() => fxAccounts.signOut());
+    yield deferred.promise;
   }
 },
 {
-  desc: "Test action=signin",
+  desc: "Test action=signin - no user logged in",
   teardown: () => gBrowser.removeCurrentTab(),
   run: function* ()
   {
+    // When this loads with no user logged-in, we expect the "normal" URL
     const expected_url = "https://example.com/?is_sign_in";
     setPref("identity.fxaccounts.remote.signin.uri", expected_url);
     let [tab, url] = yield promiseNewTabWithIframeLoadEvent("about:accounts?action=signin");
     is(url, expected_url, "action=signin got the expected URL");
+    // we expect the remote iframe to be shown.
+    yield checkVisibilities(tab, {
+      stage: false, // parent of 'manage' and 'intro'
+      manage: false,
+      intro: false, // this is  "get started"
+      remote: true
+    });
   }
 },
 {
-  desc: "Test action=signup",
+  desc: "Test action=signin - user logged in",
+  teardown: function* () {
+    gBrowser.removeCurrentTab();
+    yield signOut();
+  },
+  run: function* ()
+  {
+    // When this loads with a user logged-in, we expect the normal URL to
+    // have been ignored and the "manage" page to be shown.
+    const expected_url = "https://example.com/?is_sign_in";
+    setPref("identity.fxaccounts.remote.signin.uri", expected_url);
+    yield setSignedInUser();
+    let tab = yield promiseNewTabLoadEvent("about:accounts?action=signin");
+    // about:accounts initializes after fetching the current user from Fxa -
+    // so we also request it - by the time we get it we know it should have
+    // done its thing.
+    yield fxAccounts.getSignedInUser();
+    // we expect "manage" to be shown.
+    yield checkVisibilities(tab, {
+      stage: true, // parent of 'manage' and 'intro'
+      manage: true,
+      intro: false, // this is  "get started"
+      remote: false
+    });
+  }
+},
+{
+  desc: "Test action=signup - no user logged in",
   teardown: () => gBrowser.removeCurrentTab(),
   run: function* ()
   {
@@ -80,13 +118,39 @@ let gTests = [
     setPref("identity.fxaccounts.remote.uri", expected_url);
     let [tab, url] = yield promiseNewTabWithIframeLoadEvent("about:accounts?action=signup");
     is(url, expected_url, "action=signup got the expected URL");
+    // we expect the remote iframe to be shown.
+    yield checkVisibilities(tab, {
+      stage: false, // parent of 'manage' and 'intro'
+      manage: false,
+      intro: false, // this is  "get started"
+      remote: true
+    });
+  },
+},
+{
+  desc: "Test action=signup - user logged in",
+  teardown: () => gBrowser.removeCurrentTab(),
+  run: function* ()
+  {
+    const expected_url = "https://example.com/?is_sign_up";
+    setPref("identity.fxaccounts.remote.uri", expected_url);
+    yield setSignedInUser();
+    let tab = yield promiseNewTabLoadEvent("about:accounts?action=signup");
+    yield fxAccounts.getSignedInUser();
+    // we expect "manage" to be shown.
+    yield checkVisibilities(tab, {
+      stage: true, // parent of 'manage' and 'intro'
+      manage: true,
+      intro: false, // this is  "get started"
+      remote: false
+    });
   },
 },
 {
   desc: "Test action=reauth",
   teardown: function* () {
     gBrowser.removeCurrentTab();
-    yield fxAccounts.signOut();
+    yield signOut();
   },
   run: function* ()
   {
@@ -102,14 +166,29 @@ let gTests = [
       verified: true
     };
 
-    yield fxAccounts.setSignedInUser(userData);
+    yield setSignedInUser();
     let [tab, url] = yield promiseNewTabWithIframeLoadEvent("about:accounts?action=reauth");
     // The current user will be appended to the url
     let expected = expected_url + "&email=foo%40example.com";
     is(url, expected, "action=reauth got the expected URL");
   },
 },
-
+{
+  desc: "Test observers about:accounts",
+  teardown: function() {
+    gBrowser.removeCurrentTab();
+  },
+  run: function* () {
+    setPref("identity.fxaccounts.remote.uri", "https://example.com/");
+    yield setSignedInUser();
+    let tab = yield promiseNewTabLoadEvent("about:accounts");
+    // sign the user out - the tab should have action=signin
+    yield signOut();
+    // wait for the new load.
+    yield promiseOneMessage(tab, "test:document:load");
+    is(tab.linkedBrowser.contentDocument.location.href, "about:accounts?action=signin");
+  }
+},
 ]; // gTests
 
 function test()
@@ -130,9 +209,18 @@ function test()
   });
 }
 
+function promiseOneMessage(tab, messageName) {
+  let mm = tab.linkedBrowser.messageManager;
+  let deferred = Promise.defer();
+  mm.addMessageListener(messageName, function onmessage(message) {
+    mm.removeMessageListener(messageName, onmessage);
+    deferred.resolve(message);
+  });
+  return deferred.promise;
+}
+
 function promiseNewTabLoadEvent(aUrl)
 {
-  let deferred = Promise.defer();
   let tab = gBrowser.selectedTab = gBrowser.addTab(aUrl);
   let browser = tab.linkedBrowser;
   let mm = browser.messageManager;
@@ -140,11 +228,9 @@ function promiseNewTabLoadEvent(aUrl)
   // give it an e10s-friendly content script to help with our tests.
   mm.loadFrameScript(CHROME_BASE + "content_aboutAccounts.js", true);
   // and wait for it to tell us about the load.
-  mm.addMessageListener("test:document:load", function onLoad() {
-    mm.removeMessageListener("test:document:load", onLoad);
-    deferred.resolve(tab);
-  });
-  return deferred.promise;
+  return promiseOneMessage(tab, "test:document:load").then(
+    () => tab
+  );
 }
 
 // Returns a promise which is resolved with the iframe's URL after a new
@@ -164,3 +250,39 @@ function promiseNewTabWithIframeLoadEvent(aUrl) {
   });
   return deferred.promise;
 }
+
+function checkVisibilities(tab, data) {
+  let ids = Object.keys(data);
+  let mm = tab.linkedBrowser.messageManager;
+  let deferred = Promise.defer();
+  mm.addMessageListener("test:check-visibilities-response", function onResponse(message) {
+    mm.removeMessageListener("test:check-visibilities-response", onResponse);
+    for (let id of ids) {
+      is(message.data[id], data[id], "Element '" + id + "' has correct visibility");
+    }
+    deferred.resolve();
+  });
+  mm.sendAsyncMessage("test:check-visibilities", {ids: ids});
+  return deferred.promise;
+}
+
+// watch out - these will fire observers which if you aren't careful, may
+// interfere with the tests.
+function setSignedInUser(data) {
+  if (!data) {
+    data = {
+      email: "foo@example.com",
+      uid: "1234@lcip.org",
+      assertion: "foobar",
+      sessionToken: "dead",
+      kA: "beef",
+      kB: "cafe",
+      verified: true
+    }
+  }
+ return fxAccounts.setSignedInUser(data);
+}
+
+function signOut() {
+  return fxAccounts.signOut();
+}

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

@@ -5,9 +5,16 @@
 // This file is loaded as a "content script" for browser_aboutAccounts tests
 "use strict";
 
-addEventListener("DOMContentLoaded", function domContentLoaded(event) {
-  removeEventListener("DOMContentLoaded", domContentLoaded);
+addEventListener("load", function load(event) {
+  if (event.target != content.document) {
+    return;
+  }
+//  content.document.removeEventListener("load", load, true);
   sendAsyncMessage("test:document:load");
+}, true);
+
+addEventListener("DOMContentLoaded", function domContentLoaded(event) {
+  removeEventListener("DOMContentLoaded", domContentLoaded, true);
   let iframe = content.document.getElementById("remote");
   iframe.addEventListener("load", function iframeLoaded(event) {
     if (iframe.contentWindow.location.href == "about:blank" ||
@@ -17,4 +24,25 @@ addEventListener("DOMContentLoaded", function domContentLoaded(event) {
     iframe.removeEventListener("load", iframeLoaded, true);
     sendAsyncMessage("test:iframe:load", {url: iframe.getAttribute("src")});
   }, true);
+}, true);
+
+// Return the visibility state of a list of ids.
+addMessageListener("test:check-visibilities", function (message) {
+  let result = {};
+  for (let id of message.data.ids) {
+    let elt = content.document.getElementById(id);
+    if (elt) {
+      let displayStyle = content.window.getComputedStyle(elt).display;
+      if (displayStyle == 'none') {
+        result[id] = false;
+      } else if (displayStyle == 'block') {
+        result[id] = true;
+      } else {
+        result[id] = "strange: " + displayStyle; // tests should fail!
+      }
+    } else {
+      result[id] = "doesn't exist: " + id;
+    }
+  }
+  sendAsyncMessage("test:check-visibilities-response", result);
 });