Backed out changeset 0bbd19cd784a (bug 1190279) for browser_fxaccounts.js failures. FIREFOX_BETA_41_BASE
authorRyan VanderMeulen <ryanvm@gmail.com>
Mon, 10 Aug 2015 12:35:36 -0400
changeset 281859 a019592053c4d93fbbafab8d0bd709529e3746de
parent 281858 7a647fc4d0ba97572d72e308bb016d0b9d2ab5ff
child 281860 d561dc208e61b2f2b4e82ab61710e14f56da4ddb
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1190279
milestone41.0a2
backs out0bbd19cd784a51359c03200acb21dc2d508ced9a
Backed out changeset 0bbd19cd784a (bug 1190279) for browser_fxaccounts.js failures.
browser/base/content/aboutaccounts/aboutaccounts.js
browser/base/content/browser-fxaccounts.js
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_fxaccounts.js
browser/base/content/test/general/fxa_profile_handler.sjs
browser/components/preferences/in-content/sync.js
--- a/browser/base/content/aboutaccounts/aboutaccounts.js
+++ b/browser/base/content/aboutaccounts/aboutaccounts.js
@@ -337,18 +337,16 @@ function init() {
             show("stage", "intro");
             // load the remote frame in the background
             wrapper.init(fxAccounts.getAccountsSignUpURI(), urlParams);
           }
         });
       }
       break;
     }
-  }).catch(err => {
-    error("Failed to get the signed in user: " + err);
   });
 }
 
 // Causes the "top-level" element with |id| to be shown - all other top-level
 // elements are hidden.  Optionally, ensures that only 1 "second-level" element
 // inside the top-level one is shown.
 function show(id, childId) {
   // top-level items are either <div> or <iframe>
--- a/browser/base/content/browser-fxaccounts.js
+++ b/browser/base/content/browser-fxaccounts.js
@@ -28,17 +28,16 @@ let gFxAccounts = {
     delete this.topics;
     return this.topics = [
       "weave:service:ready",
       "weave:service:sync:start",
       "weave:service:login:error",
       "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",
       this.FxAccountsCommon.ON_PROFILE_CHANGE_NOTIFICATION,
     ];
   },
 
   get panelUIFooter() {
@@ -225,37 +224,36 @@ let gFxAccounts = {
     this.showDoorhanger("sync-error-panel");
   },
 
   updateUI: function () {
     this.updateAppMenuItem();
     this.updateMigrationNotification();
   },
 
-  // Note that updateAppMenuItem() returns a Promise that's only used by tests.
   updateAppMenuItem: function () {
     if (this._migrationInfo) {
       this.updateAppMenuItemForMigration();
-      return Promise.resolve();
+      return;
     }
 
     let profileInfoEnabled = false;
     try {
       profileInfoEnabled = Services.prefs.getBoolPref("identity.fxaccounts.profile_image.enabled");
     } catch (e) { }
 
     // Bail out if FxA is disabled.
     if (!this.weave.fxAccountsEnabled) {
       // When migration transitions from needs-verification to the null state,
       // fxAccountsEnabled is false because migration has not yet finished.  In
       // that case, hide the button.  We'll get another notification with a null
       // state once migration is complete.
       this.panelUIFooter.hidden = true;
       this.panelUIFooter.removeAttribute("fxastatus");
-      return Promise.resolve();
+      return;
     }
 
     this.panelUIFooter.hidden = false;
 
     // Make sure the button is disabled in customization mode.
     if (this._inCustomizationMode) {
       this.panelUIStatus.setAttribute("disabled", "true");
       this.panelUILabel.setAttribute("disabled", "true");
@@ -313,39 +311,33 @@ let gFxAccounts = {
             this.panelUIFooter.setAttribute("fxaprofileimage", "set");
             this.panelUIAvatar.style.listStyleImage = "url('" + profile.avatar + "')";
           };
           img.src = profile.avatar;
         }
       }
     }
 
-    return fxAccounts.getSignedInUser().then(userData => {
+    // Calling getSignedInUserProfile() without a user logged in causes log
+    // noise that looks like an actual error...
+    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) {
-        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;
-      });
+      return userData ? fxAccounts.getSignedInUserProfile() : null;
     }).then(profile => {
       if (!profile) {
         return;
       }
       updateWithProfile(profile);
     }).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);
+      this.FxAccountsCommon.log.error("Error updating FxA profile", error);
       updateWithUserData(null);
     });
   },
 
   updateAppMenuItemForMigration: Task.async(function* () {
     let status = null;
     let label = null;
     switch (this._migrationInfo.state) {
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -296,18 +296,16 @@ skip-if = e10s
 [browser_drag.js]
 skip-if = true # browser_drag.js is disabled, as it needs to be updated for the new behavior from bug 320638.
 [browser_favicon_change.js]
 [browser_favicon_change_not_in_document.js]
 skip-if = e10s
 [browser_findbarClose.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_gestureSupport.js]
 skip-if = e10s # Bug 863514 - no gesture support.
 [browser_getshortcutoruri.js]
 [browser_hide_removing.js]
 [browser_homeDrop.js]
deleted file mode 100644
--- a/browser/base/content/test/general/browser_fxaccounts.js
+++ /dev/null
@@ -1,258 +0,0 @@
-/* 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;
-});
deleted file mode 100644
--- a/browser/base/content/test/general/fxa_profile_handler.sjs
+++ /dev/null
@@ -1,34 +0,0 @@
-/* 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);
-  }
-}
--- a/browser/components/preferences/in-content/sync.js
+++ b/browser/components/preferences/in-content/sync.js
@@ -91,17 +91,16 @@ let gSyncPane = {
 
   _init: function () {
     let topics = ["weave:service:login:error",
                   "weave:service:login:finish",
                   "weave:service:start-over:finish",
                   "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";
 
     // Add the observers now and remove them on unload
     //XXXzpao This should use Services.obs.* but Weave's Obs does nice handling
     //        of `this`. Fix in a followup. (bug 583347)
     topics.forEach(function (topic) {