1245311 - Investigate simplifying LoadFaviconTask

Status: RESOLVED INVALID

(Firefox for Android Graveyard :: Favicon Handling, defect)

Description

[Andrzej Hunt :ahunt]

LoadFaviconTask is difficult to understand, which has caused issues when implementing apple-touch-icon support (which reuses the Favicon infrastructure for downloading touch-icons). It would be nice to simplify what we're doing here, see the following comment from Bug 1238656 :

(Michael Comella (:mcomella) from comment #12)
> I think it's unfortunate `LoadFaviconTask` is so inherently tied to the
> cache – it both makes this code hard to understand and (obviously) modify to
> not use the cache. I think a better architecture might be:
> 
> `DownloadFaviconTask` – only downloads Favicons
> `LoadFaviconTask extends DownloadFaviconTask` – checks in cache for Favicon
> cache and if it's not there, downloads the Favicon
> `LoadAndCacheFaviconTask extends LoadFaviconTask` – gets the favicon above
> and then caches the result


We need to keep in mind that homescreen shortcut creation should try the following:
1) download apple-touch-icon, avoiding the cache (since the cache scales icons down to ~32px)
2) fallback to downloading the favicon, also avoiding the cache

We could then maybe add a fallback to the normal favicon cache here.

Comment 1

[Andrzej Hunt :ahunt]

Closing this since Sebastian's Favicon rewrite has addressed this.