STR: * delete the chrome://FxAccounts saved login. * restart the browser. * wait for you to be prompted to re-sign in to sync * re-sign in * note that a full sync starts (you probably need to check the logs to confirm this) I'm fairly sure the problem is https://dxr.mozilla.org/mozilla-central/rev/9b8bf5feb0b52aa4b03aa5fa3d4f0727b2974663/services/sync/modules/browserid_identity.js#271, which causes us to hit https://dxr.mozilla.org/mozilla-central/rev/9b8bf5feb0b52aa4b03aa5fa3d4f0727b2974663/services/sync/modules/stages/enginesync.js#106 - which means this has existed forever. However, I think it's still somewhat bad as a full sync is more likely to be interrupted and do bad things. I think the solution is at https://dxr.mozilla.org/mozilla-central/rev/9b8bf5feb0b52aa4b03aa5fa3d4f0727b2974663/services/sync/modules/browserid_identity.js#320 - instead of always passing |true|, we should see if Sync is already configured. However, part of that block we want to skip sends a "weave:service:setup-complete" notification, which displays a popup that Sync is now syncing - which we probably *do* want - but I think that can be solved by having that observe() method have a .then() which sends the notification. OTOH though, it's not clear why we need to set that pref even for the really-first-sync case - the client *should* already be reset (as signing out of sync resets it) Richard, can you think of a good reason to write that pref?

I have very hazy memories, which I'll do my best to dig up and capture here for posterity. The point of setting that persistent firstSync thing is twofold: 1. Because we take/took some actions after the first sync (e.g., uploading a correct meta/global), and so we don't want to fail to complete the first sync (e.g., user quits the browser because it's taking too long) and move right along. I'm not sure if we still do so, but it would be a really good idea to check! 2. Because there are three different things a user might want to do on a first sync: wipe local and take remote; wipe remote and take local; merge the two. We need to remember those things across restarts if the process isn't complete. We no longer expose UI to do this, but I think we still want to (albeit it's a low priority). I agree that simply losing credentials is not sufficient motivation to do a full re-sync: Sync will later detect any related problems via meta/global. That might be best stated as: "re-signing in" is not the same thing as signing in the first time. I think your remedy of passing false is correct for the re-signing-in case (but test, of course!). I would still write the pref for first-time users.

1321740

Whoops, that is not what I meant to paste... Anyway, what I meant to say is that it's unclear to me if this is all we need to do, or if we need to ensure that the username is unchanged. I think that case should be handled later (after fetching meta/global?), so I tried to avoid checking since it would complicate the code some.

Comment on attachment 8835101 [details] Bug 1321740 - Avoid a full sync after signing in due to fxa reauthentication. https://reviewboard.mozilla.org/r/110804/#review112278 See comment 0 - I think we do want the "weave:service:setup-complete" notification and the trigger to sync now.

(In reply to Thom Chiovoloni [:tcsc] from comment #4) > Anyway, what I meant to say is that it's unclear to me if this is all we > need to do, or if we need to ensure that the username is unchanged. I think that's OK - we expect logout in that scenario. > I think > that case should be handled later (after fetching meta/global?), so I tried > to avoid checking since it would complicate the code some. Yeah, agreed - thanks!

(In reply to Mark Hammond [:markh] from comment #0) > ... However, part of that block we want to skip > sends a "weave:service:setup-complete" notification, which displays a popup > that Sync is now syncing - which we probably *do* want... Worth noting this is actually triggered by fxaccounts:onverified (https://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#250-251), and so never fires as a result of this event (with or without my changes). But starting the sync explicitly and letting other observers know seems like a good idea to me. Let me know if you want me to e.g. send a new event that will also trigger the doorhanger (triggering fxaccounts:onverified from outside of fxa sounds gross to me, but a new event seems fine). This is more of a UX question, though.

Comment on attachment 8835101 [details] Bug 1321740 - Avoid a full sync after signing in due to fxa reauthentication. https://reviewboard.mozilla.org/r/110804/#review112628 whenReadyToAuthenticate is smelly and should die, but not in this bug :) Thanks!

(In reply to Thom Chiovoloni [:tcsc] from comment #8) > Worth noting this is actually triggered by fxaccounts:onverified > (https://dxr.mozilla.org/mozilla-central/source/browser/components/ > nsBrowserGlue.js#250-251), and so never fires as a result of this event > (with or without my changes). hrm - I could have sworn I saw that notification, but if not, then there's no need to change that behaviour here.

Pushed by tchiovoloni@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/174570c9ca6b Avoid a full sync after signing in due to fxa reauthentication. r=markh

I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=77718349&repo=autoland https://hg.mozilla.org/integration/autoland/rev/06f837d52004

Sorry about that and the delay, should be fixed now https://treeherder.mozilla.org/#/jobs?repo=try&revision=a37e6c30087f3f4f1e4ccced376f579d5066ed82

Pushed by tchiovoloni@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ea643137f3e Avoid a full sync after signing in due to fxa reauthentication. r=markh