Closed Bug 1204536 Opened 9 years ago Closed 9 years ago

Favicons should not be stored on disk while navigating in PBMode

Categories

(Firefox for iOS :: Data Storage, defect)

Other
iOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
fxios-v1.0 --- unaffected
fxios-v1.0.5 --- unaffected
fxios-v1.1 --- affected
fxios 1.1+ ---

People

(Reporter: sleroux, Assigned: sleroux)

References

Details

Attachments

(1 file)

We currently store all favicons we download into our database which would mean there is persistent information stored on disk while the user is navigating in Private Browsing. We need a way to only keep favicons for PBMode in memory and never write them to disk.

:rnewman, I'm somewhat familiar with favicons and the DB but I'm afraid of how deep the rabbit hole goes when it comes the associations we make between favicon/history items/etc. Do you have any ideas on where to start on making favicons memory only? I know SQLite supports in-memory DB only but I don't know how that works when we want some of the DB to write to disk.
Flags: needinfo?(rnewman)
The rabbithole goes quite deep.

If we want to do icons here, I see five options, in increasing order of complexity:


1. Don't write or show favicons anywhere in private browsing mode.


2. Allow "Browser" (should really be "Tab") to manage icons and URL associations in memory. This is like a mini icon manager. This would work because all of the other uses of icons — history lists and top sites — don't apply to private tabs; they'd fall back to the default icon. Those other uses are the principle reason why we have the mappings between URLs, sites, and favicons. If we can live with the temporary and tab-scoped association between URLs and icons, then stuffing that logic into the Tab (or nearby) would work fine. There's a small cost in redundant fetches.


3. Tag icons in the main DB. Once they're used out of private browsing mode, they're PERSISTED. If they're only ever used in private browsing mode, they're TRANSIENT. Periodically kill TRANSIENT icons. This lets data touch disk, but it'll eventually disappear. This keeps our favicon management code largely untouched.


4. Overlay a second favicon access layer on top of what we already have.

We could do that by creating a separate in-memory BrowserDB and throwing it away when the user is done with private browsing. This is architecturally a little more elegant than a bunch of special cases.

We could also do it the obvious way: maintain an in-memory cache, and just look in both places but only write into the memory cache from private browsing and the persistent store from regular browsing.

This is complex but complete.


5. Rewrite our favicon layer to use an abstracted store that's not BrowserDB. This buys other wins, but obviously is a larger change.


I recommend (1) or (2).
Flags: needinfo?(rnewman)
I think there's definitely value in displaying the favicon - especially when Chrome and other apps show the favicon for private tabs. Option 2 sounds like the easiest win for now.
Assignee: nobody → sleroux
Status: NEW → ASSIGNED
Looks like SDWebImageCache has an option you can provide it to only cache the image in memory and not disk. I've added this option on the condition that a tab is private when Favicon.js runs to download a site's favicon. I've also avoided storing the favicon in the profile so we never persist it in sqlite. This works since we only need favicons for the tabs in private mode.
Attachment #8668633 - Flags: review?(rnewman)
Fixed nits and merged
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: