1380215 - Remote device disconnection doesn't fully reset sync if sync is yet to initialize

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P3)

Description

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

tl;dr - disconnecting a desktop device via "device manager" should behave the same as if the user clicked "Disconnect" on that device.

STR:
* From one profile, disconnect some other desktop profile.
* Start the disconnected profile. Notice there is a notification shown telling you the device has been disconnected, and about:prefs and the hamburger menu all correctly reflect the reset.
* However, visit about:config and look for services.sync preferences.

Expected:
* All preferences should be at their default values, just like when you manually click the "disconnect" button.

Actual:
* All prefs have the old values, including the old GUIDs, the timestamps from the last sync etc.

In theory, this should be as simple as having sync listen for ON_DEVICE_DISCONNECTED_NOTIFICATION, but in practice it seems likely that sync isn't going to have setup the listeners in time for this.

Comment 1

[Edouard Oger [:eoger]]

Does startOver() reset those prefs?
Here's one of the listeners of ONLOGOUT_NOTIFICATION (which is fired by FxAccounts#signOut): http://searchfox.org/mozilla-central/source/services/sync/modules/browserid_identity.js#295

Comment 2

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

(In reply to Edouard Oger [:eoger] from comment #1)
> Does startOver() reset those prefs?

Yep - that's the call we need to make.

> Here's one of the listeners of ONLOGOUT_NOTIFICATION (which is fired by
> FxAccounts#signOut):
> http://searchfox.org/mozilla-central/source/services/sync/modules/
> browserid_identity.js#295

Hrm, right, thanks - so I suspect the problem is simply that browserid_identity hasn't been loaded when this push comes in. IOW, I suspect things will work correctly if the profile is already running when it is disconnected, but doesn't when the push comes in those first 10 seconds of the profile starting.

Comment 3

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

FWIW, I believe the same problem exists here if we detect account deletion at startup - either via push, or a 401 attempting to fetch the profile, after which we check if the account exists - if sync hasn't been initialized the wrong thing happens.

Comment 4

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

Attachment: Bug 1380215 - FxA now ensures Sync is loaded before sending observer notifications.

Review commit: https://reviewboard.mozilla.org/r/205296/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/205296/

Comment 5

[Edouard Oger [:eoger]]

Comment on attachment 8934372 [details]
Bug 1380215 - FxA now ensures Sync is loaded before sending observer notifications.

https://reviewboard.mozilla.org/r/205296/#review211082

Thanks!

::: services/fxaccounts/FxAccounts.jsm:88
(Diff revision 1)
>  
> +// A poor-man's "registry" of promise-returning functions to call before we
> +// send observer notifications. Primarily used so parts of Firefox which are
> +// yet to load for performance reasons can be force-loaded and thus not miss
> +// the notification.
> +const OBSERVER_PRELOADS = [

As we discussed on IRC, I'm not sure we will add anything else to that array for some time, but I don't care that much if you prefer to keep this.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1400ddefec6c
FxA now ensures Sync is loaded before sending observer notifications. r=eoger

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/1400ddefec6c