Bug 1186348 - don't initialize the sync identity (and thus don't spin the event loop) as Sync initialized. r=ckarlof

This commit is contained in:
Mark Hammond 2015-09-07 18:11:40 +10:00
parent 509fd5aac4
commit 6c32685ea1
6 changed files with 39 additions and 58 deletions

View File

@ -103,9 +103,11 @@ let gSyncUI = {
// We want to treat "account needs verification" as "needs setup". So
// "reach in" to Weave.Status._authManager to check whether we the signed-in
// user is verified.
// Referencing Weave.Status spins a nested event loop to initialize the
// authManager, so this should always return a value directly.
// This only applies to fxAccounts-based Sync.
// NOTE: We used to have this _authManager hack to avoid a nested
// event-loop from querying Weave.Status.checkSetup() - while that's no
// longer true, we do still have the FxA-specific requirement of checking
// the verified state - so the hack remains. We should consider refactoring
// Sync's "check setup" capabilities to take this into account at some point...
if (Weave.Status._authManager._signedInUser !== undefined) {
// If we have a signed in user already, and that user is not verified,
// revert to the "needs setup" state.

View File

@ -104,12 +104,6 @@ this.BrowserIDManager.prototype = {
// we don't consider the lack of a keybundle as a failure state.
_shouldHaveSyncKeyBundle: false,
get readyToAuthenticate() {
// We are finished initializing when we *should* have a sync key bundle,
// although we might not actually have one due to auth failures etc.
return this._shouldHaveSyncKeyBundle;
},
get needsCustomization() {
try {
return Services.prefs.getBoolPref(PREF_SYNC_SHOW_CUSTOMIZATION);
@ -122,7 +116,19 @@ this.BrowserIDManager.prototype = {
for (let topic of OBSERVER_TOPICS) {
Services.obs.addObserver(this, topic, false);
}
return this.initializeWithCurrentIdentity();
// and a background fetch of account data just so we can set this.account,
// so we have a username available before we've actually done a login.
// XXX - this is actually a hack just for tests and really shouldn't be
// necessary. Also, you'd think it would be safe to allow this.account to
// be set to null when there's no user logged in, but argue with the test
// suite, not with me :)
this._fxaService.getSignedInUser().then(accountData => {
if (accountData) {
this.account = accountData.email;
}
}).catch(err => {
// As above, this is only for tests so it is safe to ignore.
});
},
/**
@ -130,7 +136,7 @@ this.BrowserIDManager.prototype = {
* the user is logged in, or is rejected if the login attempt has failed.
*/
ensureLoggedIn: function() {
if (!this._shouldHaveSyncKeyBundle) {
if (!this._shouldHaveSyncKeyBundle && this.whenReadyToAuthenticate) {
// We are already in the process of logging in.
return this.whenReadyToAuthenticate.promise;
}
@ -160,7 +166,6 @@ this.BrowserIDManager.prototype = {
}
this.resetCredentials();
this._signedInUser = null;
return Promise.resolve();
},
offerSyncOptions: function () {
@ -294,7 +299,8 @@ this.BrowserIDManager.prototype = {
// reauth with the server - in that case we will also get here, but
// should have the same identity.
// initializeWithCurrentIdentity will throw and log if these constraints
// aren't met, so just go ahead and do the init.
// aren't met (indirectly, via _updateSignedInUser()), so just go ahead
// and do the init.
this.initializeWithCurrentIdentity(true);
break;
@ -636,7 +642,6 @@ this.BrowserIDManager.prototype = {
// that there is no authentication dance still under way.
this._shouldHaveSyncKeyBundle = true;
Weave.Status.login = this._authFailureReason;
Services.obs.notifyObservers(null, "weave:ui:login:error", null);
throw err;
});
},

View File

@ -122,7 +122,6 @@ LOGIN_FAILED_NETWORK_ERROR: "error.login.reason.network",
LOGIN_FAILED_SERVER_ERROR: "error.login.reason.server",
LOGIN_FAILED_INVALID_PASSPHRASE: "error.login.reason.recoverykey",
LOGIN_FAILED_LOGIN_REJECTED: "error.login.reason.account",
LOGIN_FAILED_NOT_READY: "error.login.reason.initializing",
// sync failure status codes
METARECORD_DOWNLOAD_FAIL: "error.sync.reason.metarecord_download_fail",

View File

@ -85,18 +85,14 @@ IdentityManager.prototype = {
_syncKeyBundle: null,
/**
* Initialize the identity provider. Returns a promise that is resolved
* when initialization is complete and the provider can be queried for
* its state
* Initialize the identity provider.
*/
initialize: function() {
// Nothing to do for this identity provider.
return Promise.resolve();
},
finalize: function() {
// Nothing to do for this identity provider.
return Promise.resolve();
},
/**
@ -115,14 +111,6 @@ IdentityManager.prototype = {
return Promise.resolve();
},
/**
* Indicates if the identity manager is still initializing
*/
get readyToAuthenticate() {
// We initialize in a fully sync manner, so we are always finished.
return true;
},
get account() {
return Svc.Prefs.get("account", this.username);
},

View File

@ -690,13 +690,6 @@ Sync11Service.prototype = {
},
verifyLogin: function verifyLogin(allow40XRecovery = true) {
// If the identity isn't ready it might not know the username...
if (!this.identity.readyToAuthenticate) {
this._log.info("Not ready to authenticate in verifyLogin.");
this.status.login = LOGIN_FAILED_NOT_READY;
return false;
}
if (!this.identity.username) {
this._log.warn("No username in verifyLogin.");
this.status.login = LOGIN_FAILED_NO_USERNAME;
@ -943,25 +936,22 @@ Sync11Service.prototype = {
return;
}
this.identity.finalize().then(
() => {
// an observer so the FxA migration code can take some action before
// the new identity is created.
Svc.Obs.notify("weave:service:start-over:init-identity");
this.identity.username = "";
this.status.__authManager = null;
this.identity = Status._authManager;
this._clusterManager = this.identity.createClusterManager(this);
Svc.Obs.notify("weave:service:start-over:finish");
}
).then(null,
err => {
this._log.error("startOver failed to re-initialize the identity manager: " + err);
// Still send the observer notification so the current state is
// reflected in the UI.
Svc.Obs.notify("weave:service:start-over:finish");
}
);
try {
this.identity.finalize();
// an observer so the FxA migration code can take some action before
// the new identity is created.
Svc.Obs.notify("weave:service:start-over:init-identity");
this.identity.username = "";
this.status.__authManager = null;
this.identity = Status._authManager;
this._clusterManager = this.identity.createClusterManager(this);
Svc.Obs.notify("weave:service:start-over:finish");
} catch (err) {
this._log.error("startOver failed to re-initialize the identity manager: " + err);
// Still send the observer notification so the current state is
// reflected in the UI.
Svc.Obs.notify("weave:service:start-over:finish");
}
},
persistLogin: function persistLogin() {

View File

@ -30,10 +30,7 @@ this.Status = {
.wrappedJSObject;
let idClass = service.fxAccountsEnabled ? BrowserIDManager : IdentityManager;
this.__authManager = new idClass();
// .initialize returns a promise, so we need to spin until it resolves.
let cb = Async.makeSpinningCallback();
this.__authManager.initialize().then(cb, cb);
cb.wait();
this.__authManager.initialize();
return this.__authManager;
},