Closed Bug 769745 Opened 12 years ago Closed 12 years ago

Persist Android account settings to disk on each sync

Categories

(Firefox for Android Graveyard :: Android Sync, defect, P1)

ARM
Android
defect

Tracking

(firefox14+ fixed, firefox15+ fixed)

VERIFIED FIXED
mozilla16
Tracking Status
firefox14 + fixed
firefox15 + fixed

People

(Reporter: rnewman, Assigned: nalexander)

References

()

Details

Attachments

(2 files, 1 obsolete file)

(Temporary?) mitigation for Android account non-persistence. Write out a per-account JSON bundle in our data directory. No concern about privacy/security; Fennec profiles are never on the SD card, and we already write signons.sqlite to this location. Write out every time we decide to do a proper sync. When we get an account removal notification, remove the JSON bundle.
Blocks: 769749
Depends on: 735842
Test procedure: 0. Remove all Firefox installs and Sync Android Accounts. 1. Install Fennec. 2. Start Fennec; verify "Set up Firefox Sync" box appears as expected. 3. Pair Android device to Sync account. 4. Start Fennec; verify "Set up Firefox Sync" box does not appear. 5. Force a sync and verify it worked. Look in the logs for I SyncAdapter(23960) Syncing account named nalexander+test0703@mozilla.com for client named 'Fennec ncalexan on GT-I9100' with client guid Ufj3pz6b4ZV1 (sync account has 0 cl and record the client GUID listed (Ufj3pz6b4ZV1 in this case). 6. From Android > Settings > Applications, move Fennec to SD card. 7. Verify Android Account remains. 8. From Android > Settings > Storage, unmount SD card. 9. Verify Android Account has been removed. 10. From Android > Settings > Storage, mount SD card. 11. Verify Android Account is still missing. 12. Start Fennec; verify "Set up Firefox Sync" box does not appear. 13. Verify Android Account has been re-instated. 14. Force a sync and verify that it works (via logs). Look for I SyncAdapter(24781) Syncing account named nalexander+test0703@mozilla.com for client named 'Fennec ncalexan on GT-I9100' with client guid Ufj3pz6b4ZV1 (sync account has 0 cl in the logs, and verify that the client GUID listed has not changed. Verify that the old sync settings (esp. timestamps) have remained; you should not see any 51329:W FxSync(23960) ServerSyncStage :: Remote engine syncID different from local engine syncID: resetting local engine and assuming remote engine syncID. 15. From Android > Accounts and sync, Remove the Sync Account. 16. Start Fennec; verify "Set up Firefox Sync" box appears as expected. 17. Return to Android > Accounts and sync and verify that Android Account has *not* been re-created. 18. Pair Android device to Sync account again, and verify that a complete first sync happens (meaning any existing preferences have been wiped); you should see log messages like I SyncAdapter(24781) Syncing account named nalexander+test0703@mozilla.com for client named 'Fennec ncalexan on GT-I9100' with client guid Ufj3pz6b4ZV1 (sync account has 0 cl 19. Verify Fennec > Sync takes you to Account settings page if a Sync account exists, and offers to set things up if an account doesn't exist.
Known issue: if you disable syncing the Account, then do an SD card unmount/mount cycle, then start Fennec to re-create the Account, the disabled sync setting is NOT preserved. All the code is in place to handle this, but since we never get called to sync again, we never get a chance to write the Account pickle with the disabled flag to disk. We could work around this yet further by periodically writing pickles from Fennec.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
[Approval Request Comment] Bug caused by (feature/regressing bug #): Because Android. Tracked in Bug 768102. User impact if declined: Sync accounts will disappear if Firefox is installed on SD card when assorted actions (mounting over USB, ejecting SD card) occur. Testing completed (on m-c, etc.): Workaround just landed in today's Nightly. Want QA, but also want to reach 14.0.1. Risk to taking this patch (and alternatives if risky): Workaround involves writing Sync credentials to disk (not a privacy concern; we already write your whole Firefox profile, and don't write profile to SD card for this reason) and reading them whenever we attempt to access the account. This might impact non-main-thread startup time, or more so. Otherwise, I don't consider this particularly risky. The alternative is to forbid allowing the installation of Firefox to SD card, which we might end up having to do (in order to run background services, or just to be more robust), but we'd like to try this as a stopgap. String or UUID changes made by this patch: None.
Attachment #640130 - Flags: review+
Attachment #640130 - Flags: approval-mozilla-release?
Attachment #640130 - Flags: approval-mozilla-beta?
Attachment #640130 - Flags: approval-mozilla-aurora?
Comment on attachment 640130 [details] [diff] [review] Patch for uplift. Snag: the patch in Bug 769745 depends on refactorings that have not yet landed on mozilla-release, so I can't prepare a final patch for uplift. Neither does the patch apply cleanly on mozilla-beta. We've been doing a lot of work on account stuff in the past weeks, and this is the consequence. I don't feel like I have time or brain tonight to rewrite it, and I'm unavailable in the morning. If Nick is up early (or if Chenxia is feeling like attacking someone else's patch in the next nine hours, and earning brownie points), let's see if we can get a pair of patches for mozilla-release that we can throw at QA before we cut-off for build. Otherwise, we can target beta and any subsequent point release, or mobile drivers can decide if this is important enough to hold 14.0.1.
Attachment #640130 - Flags: approval-mozilla-release?
Attachment #640130 - Flags: approval-mozilla-beta?
Comment on attachment 640130 [details] [diff] [review] Patch for uplift. Nick is working on rebasing this for beta, which is apparently the source for 14.0.1.
Attachment #640130 - Flags: approval-mozilla-beta?
Verified this is fixed on 20120709 Nightly (m-c) build following complete STR's in comment 1 (thanks for that Nick!)
Status: RESOLVED → VERIFIED
Additional QA scenario: 0. pair Android device 1. sync Android (which writes json to disk) 2. unmount SD card (which removes Android Account object) 3. mount SD card 4. do not run Fennec, but verify you can add a *different* Sync account via Settings > Accounts & sync > Add account... 5. verify Fennec can start, verify Syncing the new account is working, verify we haven't clobbered the new account, etc.
Also verified that scenario on 20120709 Nightly (m-c) build. The new account is functional. The previous account gets clobbered.
Attached patch patch against m-b (obsolete) — Splinter Review
Depends on other "things we want to land": ~/Mozilla/mozilla-beta $ hg qser 721760.sync.patch 721760.fennec.patch 735842.patch 769745.patch
(In reply to Nick Alexander :nalexander from comment #11) > Created attachment 640293 [details] [diff] [review] > patch against m-b Preliminary device testing right now; will post .apk for QA if this looks good on device.
Oh dear, Fennec dies after a few seconds: I Gecko(13452) WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /Users/ncalexan/Mozilla/mozilla-beta/xpcom/base/nsTraceRefcntImpl.cpp, line 173 F libc(13452) Fatal signal 11 (SIGSEGV) at 0x5a5a5a62 (code=1) E JavaBinder(13452) Unknown binder error code. 0xfffffff7 W InputDispatcher(1988) channel '420ffbd0 org.mozilla.fennec_ncalexan/org.mozilla.fennec_ncalexan.App (server)' ~ Consumer closed input channel or an error occurred. events=0x8 E InputDispatcher(1988) channel '420ffbd0 org.mozilla.fennec_ncalexan/org.mozilla.fennec_ncalexan.App (server)' ~ Channel is unrecoverably broken and will be disposed! W InputDispatcher(1988) Attempted to unregister already unregistered input channel '420ffbd0 org.mozilla.fennec_ncalexan/org.mozilla.fennec_ncalexan.App (server)' E JavaBinder(13452) Unknown binder error code. 0xfffffff7 I WindowManager(1988) WINDOW DIED Window{420ffbd0 org.mozilla.fennec_ncalexan/org.mozilla.fennec_ncalexan.App paused=true} W WindowManager(1988) Force-removing child win cwin from container win E JavaBinder(13452) Unknown binder error code. 0xfffffff7 E ActivityThread(13452) Activity org.mozilla.fennec_ncalexan.App has leaked IntentReceiver org.mozilla.gecko.GeckoConnectivityReceiver@415a4b68 that was originally registered here. Are you missing a call to unregisterReceiver()? E ActivityThread(13452) android.app.IntentReceiverLeaked: Activity org.mozilla.fennec_ncalexan.App has leaked IntentReceiver org.mozilla.gecko.GeckoConnectivityReceiver@415a4b68 that was originally registered here. Are you missing a call to unregisterReceiver()? E ActivityThread(13452) at android.app.LoadedApk$ReceiverDispatcher.<init>(LoadedApk.java:763) E ActivityThread(13452) at android.app.LoadedApk.getReceiverDispatcher(LoadedApk.java:567) E ActivityThread(13452) at android.app.ContextImpl.registerReceiverInternal(ContextImpl.java:1106) E ActivityThread(13452) at android.app.ContextImpl.registerReceiver(ContextImpl.java:1093) E ActivityThread(13452) at android.app.ContextImpl.registerReceiver(ContextImpl.java:1087) E ActivityThread(13452) at android.content.ContextWrapper.registerReceiver(ContextWrapper.java:341) E ActivityThread(13452) at org.mozilla.gecko.GeckoConnectivityReceiver.registerFor(GeckoConnectivityReceiver.java:89) E ActivityThread(13452) at org.mozilla.gecko.GeckoApp.initialize(GeckoApp.java:1825) E ActivityThread(13452) at org.mozilla.gecko.GeckoApp.onWindowFocusChanged(GeckoApp.java:2084) E ActivityThread(13452) at com.android.internal.policy.impl.PhoneWindow$DecorView.onWindowFocusChanged(PhoneWindow.java:2530) E ActivityThread(13452) at android.view.View.dispatchWindowFocusChanged(View.java:5875) E ActivityThread(13452) at android.view.ViewGroup.dispatchWindowFocusChanged(ViewGroup.java:851) E ActivityThread(13452) at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2707) E ActivityThread(13452) at android.os.Handler.dispatchMessage(Handler.java:99) E ActivityThread(13452) at android.os.Looper.loop(Looper.java:137) E ActivityThread(13452) at android.app.ActivityThread.main(ActivityThread.java:4507) E ActivityThread(13452) at java.lang.reflect.Method.invokeNative(Native Method) E ActivityThread(13452) at java.lang.reflect.Method.invoke(Method.java:511) E ActivityThread(13452) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:790) E ActivityThread(13452) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:557) E ActivityThread(13452) at dalvik.system.NativeStart.main(Native Method) I SurfaceFlinger(1831) id=779 Removed idx=1 Map Size=3 I SurfaceFlinger(1831) id=779 Removed idx=-2 Map Size=3 I WindowManager(1988) WIN DEATH: win I ActivityManager(1988) Process org.mozilla.fennec_ncalexan (pid 13452) has died.
Attachment #640293 - Attachment is obsolete: true
Okay, m-b seems to be working well. Patch updated and my APK (org.mozilla.fennec_ncalexan) available at http://people.mozilla.com/~nalexander/fennec-beta-sd-card.apk
Nick, sorry, had some weirdness initially downloading the build.. got it straightened out. I verify that build also passes outlined test scenarios here.
(In reply to Tracy Walker [:tracy] from comment #16) > Nick, sorry, had some weirdness initially downloading the build.. got it > straightened out. I verify that build also passes outlined test scenarios > here. Huzzah!
(In reply to Nick Alexander :nalexander from comment #13) > Oh dear, Fennec dies after a few seconds: This was resolved by clobber building m-b.
Comment on attachment 640326 [details] [diff] [review] patch against m-b Copied and modified from rnewman's request: [Approval Request Comment] Bug caused by (feature/regressing bug #): Because Android. Tracked in Bug 768102. User impact if declined: Sync accounts will disappear if Firefox is installed on SD card when assorted actions (mounting over USB, ejecting SD card) occur. Testing completed (on m-c, etc.): Workaround just landed in today's Nightly. QAed by tracy against Nightly and a custom build against m-b (available on this ticket). Risk to taking this patch (and alternatives if risky): Workaround involves writing Sync credentials to disk (not a privacy concern; we already write your whole Firefox profile, and don't write profile to SD card for this reason) and reading them whenever we attempt to access the account. This might impact non-main-thread startup time, or more so. Otherwise, I don't consider this particularly risky. The alternative is to forbid allowing the installation of Firefox to SD card, which we might end up having to do (in order to run background services, or just to be more robust), but we'd like to try this as a stopgap. String or UUID changes made by this patch: None.
Attachment #640326 - Flags: approval-mozilla-beta?
Comment on attachment 640130 [details] [diff] [review] Patch for uplift. [Triage Comment] Nick believes that this fix is very low risk, and we have seen a lot of comments where users keep losing their sync profiles on Android. Let's land on branches.
Attachment #640130 - Flags: approval-mozilla-beta?
Attachment #640130 - Flags: approval-mozilla-beta+
Attachment #640130 - Flags: approval-mozilla-aurora?
Attachment #640130 - Flags: approval-mozilla-aurora+
For completeness: m-a is good to. My APK (org.mozilla.fennec_ncalexan) is available at http://people.mozilla.com/~nalexander/fennec-aurora-sd-card.apk Multiple landings ftw!
Attachment #640326 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Product: Mozilla Services → Android Background Services
Product: Android Background Services → Firefox for Android
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: