If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Removing Sync account does not remove synced tabs

VERIFIED FIXED in Firefox 34

Status

()

Firefox for Android
Android Sync
P2
normal
VERIFIED FIXED
5 years ago
4 days ago

People

(Reporter: Demetrius Schwab, Assigned: vivek, Mentored)

Tracking

({papercut})

unspecified
Firefox 34
All
Android
papercut
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=java][good second bug])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Due to my Nexus S still showing up under my synced tabs even though it wasn't there for months I decided to delete my sync account and recreate a new one. After deleting it and setting it up again on my laptop I go over to my Nexus 7. I have to manually delete sync, and sign up again with my laptop. The only problem is that it still lists the devices that I haven't owned for months and nothing I attempt removes them.

Updated

5 years ago
Component: General → Android Sync
Product: Firefox for Android → Mozilla Services
Version: Firefox 21 → unspecified
Hmm, there are a few things I can think of that might be involved:

1) Deleting sync might mean that remote records in the tabs DB on each device are never purged.  Cleaning DBs on sync removal and cleaning the tabs DB periodically from Fennec would address this.

2) We might have a clients record TTL issue.  Old devices are supposed to drop out of the list of clients after 1 or 3 weeks (Android or desktop, respectively).  We could verify that clients records are, in fact, being purged.

Thanks for the report.  I will try to look at 1) in the near future; QA could you test 2)?
Harshit, are you interested in addressing this?  There are a bunch of ways you could do this, but I think there is already an "Android Accounts changed" observer somewhere in Fennec.  If that observer finds that the last Sync account is deleted, it could try to delete all remote tabs in the database.
Whiteboard: [mentor=nalexander][lang=java][good first bug]
So, at

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AboutHomeContent.java#143

we register a callback that updates the layout of about:home when sync accounts are updated.  We should make sure this removes the Sync portion of about:home (I think it does) and then we should also try to be clever about cleaning the tabs and clients DB.

I suggest writing some code that carefully cleans the tabs and clients DB, then trying to tie it in to this callback.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services

Comment 4

5 years ago
Hello.  I was wondering if my partner and I could work on this issue for a class project.  Please let me know if this bug is still open.  Thank you so much.
(In reply to Tsai from comment #4)
> Hello.  I was wondering if my partner and I could work on this issue for a
> class project.  Please let me know if this bug is still open.  Thank you so
> much.

Definitely still open!  Read through the comments and see where you get; you can leave questions on the ticket and I'll reply as quickly as I can.  Thanks for helping.
QA Contact: lala.pashayan

Updated

5 years ago
Assignee: nobody → lala.pashayan
Hi Lala, any progress here?
Keywords: papercut
Priority: -- → P2
Hardware: ARM → All
Duplicate of this bug: 977918
Vivek: this might be a good next bug for you; involved, but you're providing great patches on other tickets...
Flags: needinfo?(vivekb.balakrishnan)
Whiteboard: [mentor=nalexander][lang=java][good first bug] → [mentor=nalexander][lang=java][good second bug]
(Assignee)

Comment 9

4 years ago
I'm interested in working on this.
Flags: needinfo?(vivekb.balakrishnan)
Feel free to work on this and to attach a patch and we'll happily review it for feedback. There seems to be several interested parties working on this so don't mind the already assigned member.
AaronMT: I invited Vivek to work on this because he's been doing great work on some other tickets.  Clearing the assigned fields since Lala hasn't commented in many months.
Assignee: lala.pashayan → nobody
QA Contact: lala.pashayan
There are a few parts to this.

1. Add a function that deletes the entire clients database (corresponds to remote Sync devices) and the non-local tabs (the tabs from remote devices) from the tabs database.  Non-local tabs have client_guid == null; see http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/BrowserContract.java#298.

See http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/FennecTabsRepository.java#272 for how to delete things from the databases (but don't try to use that wipe function directly, since it's tied up in other machinery.)

2. Add static member functions to both Fx- and Sync- AccountDeletedService to invoke your database cleaning functions, and add them to the list of things invoked:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/receivers/FxAccountDeletedService.java#57
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/receivers/SyncAccountDeletedService.java#76

3. Test with a new Sync account.  (Testing with old sync is tricky right now :)
Mentor: nalexander@mozilla.com
Whiteboard: [mentor=nalexander][lang=java][good second bug] → [lang=java][good second bug]
Created attachment 8446221 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/467
Attachment #8446221 - Flags: review?(nalexander)
Assignee: nobody → vivekb.balakrishnan
Comment on attachment 8446221 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/467

Really nice work, vivek.  Awesome to see an *ooooold* ticket get some love.  I've left a ton of tiny comments on the Github PR, and a few larger remarks.  Flag me for re-review when it's ready.

Thanks!  Great job!
Attachment #8446221 - Flags: review?(nalexander) → feedback+
Flags: needinfo?(vivekb.balakrishnan)
(Assignee)

Comment 15

3 years ago
Hi Nick,
I've updated the pull request with feedback comments.
Flags: needinfo?(vivekb.balakrishnan)
(Assignee)

Updated

3 years ago
Flags: needinfo?(nalexander)
(In reply to vivek from comment #15)
> Hi Nick,
> I've updated the pull request with feedback comments.

Sorry, I don't have time to get to this before I leave on vacation tomorrow.  I'll be back mid-August.  In the meantime, perhaps rnewman can look at this.
Flags: needinfo?(nalexander) → needinfo?(rnewman)
Attachment #8446221 - Flags: feedback?(rnewman)
Flags: needinfo?(rnewman)
Comment on attachment 8446221 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/467

Looks good; just nits.
Attachment #8446221 - Flags: feedback?(rnewman) → feedback+
(Assignee)

Comment 18

3 years ago
nits corrected and pull request updated
Flags: needinfo?(rnewman)
Status: NEW → ASSIGNED
Flags: needinfo?(rnewman)
Summary: Removing sync does not remove synced tabs. → Removing Sync account does not remove synced tabs
Attachment #8446221 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/7614df67484e
https://hg.mozilla.org/mozilla-central/rev/7614df67484e
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
rnewman landed this, but didn't include a file (for reasons unknown):

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

Updated

3 years ago
Status: RESOLVED → VERIFIED

Updated

4 days ago
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.