Closed Bug 1277854 Opened 8 years ago Closed 8 years ago

Favicons load really slow in home panels

Categories

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

All
Android
defect

Tracking

(firefox49 fixed, fennec49+)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed
fennec 49+ ---

People

(Reporter: sebastian, Assigned: ahunt)

Details

Attachments

(2 files)

Scrolling through the home panels in Nightly the favicons load really slowly, one at a time. It's almost 1 sec / icon. This seems to be a new regression.
(Backed out changeset 9575e608cabe)

loadUncachedFavicon allows loading favicons from the internet, whereas
getSizedFaviconForPageFromLocal restricts itself to the favicon cache.
(LoadFaviconTask's last parameter is (boolean) onlyFromLocal, which is
set to true in getSizedFaviconForPageFromLocal.

We could probably replace this with an Enum to make the parameter's purpose
more obvious.

Review commit: https://reviewboard.mozilla.org/r/57634/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57634/
Attachment #8759763 - Flags: review?(s.kaspari)
Attachment #8759764 - Flags: review?(s.kaspari)
Regression caused by Bug 1276050.

It's simplest to back it out for now. We could probably clean up favicon loading to make this more obvious, or be more sophisticated and paint the fallback icon initially if we have to download icons from the internet - but that doesn't seem like a good use of time right now.
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Priority: -- → P1
Comment on attachment 8759763 [details]
Bug 1277854 - Back out bug 1276050 to ensure we load only local favicons whe requested

https://reviewboard.mozilla.org/r/57634/#review54452
Attachment #8759763 - Flags: review?(s.kaspari) → review+
Comment on attachment 8759764 [details]
Bug 1277854 - Post: add comment explaining local loading

https://reviewboard.mozilla.org/r/57636/#review54454
Attachment #8759764 - Flags: review?(s.kaspari) → review+
https://hg.mozilla.org/integration/fx-team/rev/367c7cce892f627ad7e1f1ceba0f04feb601522a
Bug 1277854 - Back out bug 1276050 to ensure we load only local favicons when requested r=sebastian

https://hg.mozilla.org/integration/fx-team/rev/2086e849a7e6f30fc56100f04847143d6e575ea2
Bug 1277854 - Post: add comment explaining local loading r=sebastian
https://hg.mozilla.org/mozilla-central/rev/367c7cce892f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
tracking-fennec: ? → 49+
I think this backout may need uplifted to 49.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #9)
> I think this backout may need uplifted to 49.

I *think* this just made it, although I'm not 100% confiden in my mercurial foo:
https://hg.mozilla.org/releases/mozilla-aurora/rev/367c7cce892f
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: