ActivityChooserModel.hasOtherSyncClients gives stale data

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

unspecified
Firefox 39
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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).
/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
Attachment #8580986 - Flags: review?(rnewman)
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
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.
Attachment #8580986 - Flags: review?(rnewman) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/eb81a63a0102
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
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
Attachment #8580986 - Flags: approval-mozilla-aurora?
Attachment #8580986 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8580986 - Attachment is obsolete: true
Attachment #8619835 - Flags: review+
You need to log in before you can comment on or make changes to this bug.