Closed Bug 834536 Opened 11 years ago Closed 11 years ago

Store favicon URLs in favicon cache rather than page URLs

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 27

People

(Reporter: bnicholson, Assigned: ckitching)

References

Details

The LRU favicon cache stores favicons using the page's URL, but this is bad since many pages can use the same favicon (thus resulting in redundant entries in the cache). We should instead store them by favicon URL.
Blocks: 785945
At Margaret's request - wanted for patches in Bug 888326. This problem will end up being solved over there.
Assignee: nobody → ckitching
Blocks: 888326
Status: NEW → ASSIGNED
(In reply to Chris Kitching [:ckitching] from comment #1)
> At Margaret's request - wanted for patches in Bug 888326. This problem will
> end up being solved over there.

Oh, I didn't realize that we weren't already doing this. If it turns out to be even somewhat complicated, we should keep that work in this bug, rather than continue to increase the scope of bug 888326.
(In reply to :Margaret Leibovic from comment #2)
> (In reply to Chris Kitching [:ckitching] from comment #1)
> > At Margaret's request - wanted for patches in Bug 888326. This problem will
> > end up being solved over there.
> 
> Oh, I didn't realize that we weren't already doing this. If it turns out to
> be even somewhat complicated, we should keep that work in this bug, rather
> than continue to increase the scope of bug 888326.

See what you think of the patches over there - what is used as the cache key is sort of fundamental to a caching system, so writing it one way and then flipping it to the other seems likely to be the maximally complicated way of doing things. Still, I'll split the patches if you like - the scope of the other bug is quite considerable.
No longer blocks: 888326
Depends on: FaviconRevamp
Fixed by Bug 914296.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.