Closed
Bug 1133158
Opened 10 years ago
Closed 4 years ago
Trigger removal of items from the reader mode cache when they're deleted from the database
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect, P5)
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: rnewman, Unassigned)
References
Details
Note that this might need to be an async process -- items will be deleted as a result of actions on other devices, and will be applied to the DB by syncing.
We can either:
* Make a note of deleted items, and purge them from the cache some time after Gecko starts.
* Delete items from the cache directly from the Java CP code. This is convenient, wins the space back immediately, but it's potentially more fragile.
Margaret, do you have a strong feeling?
I'm marking this as blocking Bug 1117830, but it's not really a blocker per se.
Comment 1•10 years ago
|
||
I think we already do this...
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ReadingListHelper.java#196
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/HomeFragment.java#367
But I now realize that's just for user-triggered removal. If we're removing things from the DB in the background, yes, we're going to need to do something to clean up the cache.
I think we should try to make this clean up happen at the same time we fetch content for new items. Probably related to whatever solution we'll come up with for bug 1129725 (or maybe we need a new bug to try downloading the content at some time other than when the content observer fires).
Reporter | ||
Updated•10 years ago
|
Component: Data Providers → Reading List
Reporter | ||
Updated•10 years ago
|
Comment 2•10 years ago
|
||
For a first pass here, I think we should just send a "Reader:Removed" message to JS with the URL that was removed, since we already have logic to handle removing this item from the cache.
Do we have a way to know exactly which item was deleted from the ContentObserver we have in ReadingListHelper? Or in addition to querying for a list of items we need to fetch content for, should we also query for a list of items we need to clean up? Perhaps we could just do a query for "IS_DELETED = 1 AND CONTENT_STATUS = STATUS_FETCHED" to find the things we need to clean up.
Flags: needinfo?(rnewman)
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #2)
> For a first pass here, I think we should just send a "Reader:Removed"
> message to JS with the URL that was removed, since we already have logic to
> handle removing this item from the cache.
Sure. Ideally, send a list of URLs -- we're likely to remove multiple items at once.
This approach means that the removal code is doing a split operation -- it's removing from the CP, and it's also removing from the cache.
> Do we have a way to know exactly which item was deleted from the
> ContentObserver we have in ReadingListHelper? Or in addition to querying for
> a list of items we need to fetch content for, should we also query for a
> list of items we need to clean up?
To clarify: this question is about a remote deletion that we'd like to remove from the cache, right?
I don't think querying for IS_DELETED will help, at least not right now -- when a fully synced record is deleted on the server, we delete it from the DB entirely (because it's in sync, so we don't need to keep it around).
So we need to do one or more of four things:
* Have the synchronizer preserve with IS_DELETED if CONTENT_STATUS = FETCHED, so we can have a cleanup task as you describe.
* Have a 'resync' operation, where we make sure we're not caching anything that's not in the DB.
* Have the synchronizer send a message to say that an item has been removed. This relies on Gecko running before the process dies.
* Have the synchronizer directly manipulate the cache from Java code. This is perhaps the simplest operation, but it increases coupling.
Flags: needinfo?(rnewman)
Comment 4•9 years ago
|
||
I'd like to say we don't need to worry about this anymore, but if we're going to continue caching bookmarked reader view items for offline use, we'll want to think about the case where those bookmarks are deleted on other clients.
However, if we're not going to guarantee these things will be available for offline use, some reasonable periodic cache purging should help us avoid taking up too much space.
Comment 5•7 years ago
|
||
Component: Reading List → Awesomescreen
Comment 6•6 years ago
|
||
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195
Needinfo :susheel if you think this bug should be re-triaged.
Priority: -- → P5
Comment 7•4 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•