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)

All
Android
defect

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.
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).
Component: Data Providers → Reading List
Blocks: 1123101
No longer blocks: 1117830
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)
(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)
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.
Component: Reading List → Awesomescreen
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
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
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.