Move favicon download to a separate thread

RESOLVED FIXED

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mfinkle, Assigned: gcp)

Tracking

unspecified
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

(Whiteboard: [QA?])

Attachments

(1 attachment)

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

Updated

6 years ago
Assignee: nobody → gpascutto
Priority: -- → P1
(Assignee)

Comment 1

6 years ago
Created attachment 568438 [details] [diff] [review]
Patch. Move favicon download to AsyncTask
Attachment #568438 - Flags: review?(mark.finkle)
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
OS: Linux → Android
Hardware: x86 → ARM
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"
Attachment #568438 - Flags: review?(mark.finkle) → review+
(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 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?
(Assignee)

Comment 5

6 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Whiteboard: [QA?]
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.