1153708 - Saving FxA profile data might clobber loginmanager data

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

The problem is:

* FxA starts up with MP locked, so only partial data is read.
* FxAccountsProfile.jsm fetches the profile data, and calls setUserAccountData() with updated data.
* Now setUserAccountData has been set we've lost the data in the login manager as we haven't previously read it.

To repro, see bug 1146346 comment 20, but when the "Sync Options" pane is open (which triggers the profile code).

See also bug 1150344, which describes how we might fix this.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: Test patch to demonstrate the problem

FTR, this test change demonstrates the underlying problem.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

I opened bug 1157529 for the more general solution, but it's not clear this is going to land in time to fix this bug. Gavin suggested that we probably need to disable the profile saving code or otherwise work around the problem until a better fix is found.

Chris/Ryan/Zaach, can you guys have a chat about this and decide on what you think is the best way forward?

(NI all of you, but really it's just a single NI ;)

Comment 3

[Zachary Carter [:zaach]]

How about we cache profile data in memory only for now? That way it won't hit the server every time the pref pane renders.

Comment 4

[Ryan Kelly [:rfkelly]]

I'll defer completely to Zach's instincts here.  Caching in memory seems like a sensible compromise in the short term.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0004-Bug-1153708-put-the-FxA-profile-fetch-behind-the-sam.patch

As discussed on IRC

Comment 6

[Zachary Carter [:zaach]]

Comment on attachment 8601247 [details] [diff] [review]
0004-Bug-1153708-put-the-FxA-profile-fetch-behind-the-sam.patch

Review of attachment 8601247 [details] [diff] [review]:
-----------------------------------------------------------------

Great! Thanks Mark.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/541073e6d227

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/541073e6d227

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/541073e6d227

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8601247 [details] [diff] [review]
0004-Bug-1153708-put-the-FxA-profile-fetch-behind-the-sam.patch

Approval Request Comment
[Feature/regressing bug #]: 1139657
[User impact if declined]: Master-password users will always be asked to sign in to sync
[Describe test coverage new/current, TreeHerder]: current pass
[Risks and why]: Low - user-facing part of this feature is pref'd off
[String/UUID change made/needed]: None

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8601247 [details] [diff] [review]
0004-Bug-1153708-put-the-FxA-profile-fetch-behind-the-sam.patch

Approved for aurora so that we don't ship 39 with this regression.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Florin, once this lands on 39 can your team verify the fix ? Thanks!

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d485b9425689

Comment 14

[Andrei Vaida [:avaida]]

Assigning this to Catalin for verification.

Comment 15

[Catalin Varga [QA][:VarCat]]

Verified as fixed using:

FF 39.0b6
Build Id: 20150615125213
Mac Os X 10.9.5