695835 - Move favicon download to a separate thread

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Currently we download on the main UI thread. The download takes up too much time.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch. Move favicon download to AsyncTask

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 568438 [details] [diff] [review]
Patch. Move favicon download to AsyncTask

         }
>+        }
>         if (rel.indexOf("icon") != -1) {
>-            mMainHandler.post(new Runnable() { 

Great! Just add a blank line between the class and the "if"

Comment 3

[[:fabrice] Fabrice Desré]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 568438 [details] [diff] [review] [diff] [details] [review]
> Patch. Move favicon download to AsyncTask
> 
>          }
> >+        }
> >         if (rel.indexOf("icon") != -1) {
> >-            mMainHandler.post(new Runnable() { 

rel.indexOf("icon") is not a very good test. It will match for <link rel="rubicon"> for instance.
There's a proper algorithm described here:http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#rel-icon

We can also support apple-touch-icon like we did at http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser-ui.js#1027

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 568438 [details] [diff] [review]
Patch. Move favicon download to AsyncTask

Review of attachment 568438 [details] [diff] [review]:
-----------------------------------------------------------------

::: embedding/android/GeckoApp.java
@@ +517,2 @@
>          if (rel.indexOf("icon") != -1) {
> +            mMainHandler.post(new Runnable() {

why are you posting this to the main thread?

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Incorporated mfinkle and blassey's comment. Fabrice, fixing the actual functionality should prolly go into a seperate bug.

http://hg.mozilla.org/projects/birch/rev/1685afdada81