1145897 - ActivityChooserModel.hasOtherSyncClients gives stale data

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Because of bug 1145896.

We should guard it with FirefoxAccounts.accountsExist.

I think this is only an issue if you have account with synced devices and delete said account (as per the description in bug 1145896).

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: bz://1145897/mcomella

/r/5821 - Bug 1145897 - Check if accounts exist before returning client count in ActivityChooserModel. r=rnewman

Pull down this commit:

hg pull review -r 78e5febff180f95898a5eab7d2388a0ab9752c36

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 8580986 [details]
MozReview Request: bz://1145897/mcomella

https://reviewboard.mozilla.org/r/5819/#review4791

Ship It!

::: mobile/android/base/widget/ActivityChooserModel.java
(Diff revision 1)
> +                !SyncAccounts.syncAccountsExist(mContext))  {

I have a personal style preference for alignment here -- !s in a row.

::: mobile/android/base/widget/ActivityChooserModel.java
(Diff revision 1)
> +        // account with clients was removed so we check if there are any active accounts.

You can phrase this simply as: we can't possibly have any other clients if we don't have an FxA or old Sync account configured.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/5819/#review4839

> I have a personal style preference for alignment here -- !s in a row.

I'd be fine with changing this but the two lines have the same indentation as the line inside the braces (i.e. `return false;`).

> You can phrase this simply as: we can't possibly have any other clients if we don't have an FxA or old Sync account configured.

Done.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/eb81a63a0102

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8580986 [details]
MozReview Request: bz://1145897/mcomella

This should be uplifted with bug 1122302.

Approval Request Comment
[Feature/regressing bug #]: bug 1122302
[User impact if declined]:
  Users who remove sync accounts may still see a list of devices to send to because the DB is using stale data.

[Describe test coverage new/current, TreeHerder]: None
[Risks and why]:
  Low - we use an already tested method to see if accounts exist. In the worst case, we screwed up the logic and the "send to other devices" item will appear when it shouldn't or not appear when it should. 

[String/UUID change made/needed]: None

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9a4c9dc24961

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1145897 - Check if accounts exist before returning client count in ActivityChooserModel. r=rnewman