769745 - Persist Android account settings to disk on each sync

Status: VERIFIED FIXED

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

Description

[Richard Newman [:rnewman]]

(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.

Comment 1

[Nick Alexander :nalexander [he/him]]

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 [e-mail] 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 [e-mail] 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 [e-mail] 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.

Comment 2

[Nick Alexander :nalexander [he/him]]

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.

Comment 3

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3695f3f44753

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3695f3f44753

Comment 5

[Richard Newman [:rnewman]]

Attachment: Patch for uplift.

[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.

Comment 6

[Richard Newman [:rnewman]]

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.

Comment 7

[Richard Newman [:rnewman]]

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.

Comment 8

[Tracy Walker [:tracy]]

Verified this is fixed on 20120709 Nightly (m-c) build following complete STR's in comment 1 (thanks for that Nick!)

Comment 9

[Nick Alexander :nalexander [he/him]]

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.

Comment 10

[Tracy Walker [:tracy]]

Also verified that scenario on 20120709 Nightly (m-c) build. The new account is functional.  The previous account gets clobbered.

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: patch against m-b

Depends on other "things we want to land":

~/Mozilla/mozilla-beta $ hg qser
721760.sync.patch
721760.fennec.patch
735842.patch
769745.patch

Comment 12

[Nick Alexander :nalexander [he/him]]

(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.

Comment 13

[Nick Alexander :nalexander [he/him]]

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.

Comment 14

[Nick Alexander :nalexander [he/him]]

Attachment: patch against m-b

Comment 15

[Nick Alexander :nalexander [he/him]]

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

Comment 16

[Tracy Walker [:tracy]]

Nick, sorry, had some weirdness initially downloading the build.. got it straightened out.  I verify that build also passes outlined test scenarios here.

Comment 17

[Nick Alexander :nalexander [he/him]]

(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!

Comment 18

[Nick Alexander :nalexander [he/him]]

(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 19

[Nick Alexander :nalexander [he/him]]

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.

Comment 20

[Alex Keybl [:akeybl]]

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.

Comment 21

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1de9f7f584b7

Comment 22

[Nick Alexander :nalexander [he/him]]

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

Comment 23

[Nick Alexander :nalexander [he/him]]

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!