1026068 - Convert favicons in Tb to use Places storage/apis

Status: NEW

(Thunderbird :: Mail Window Front End, defect)

Description

[Merike Sell (:merike)]

I don't have the exact STR here but I'll try to describe the best I can. I mostly use unread view for folder list. Whenever some folder is added to the list there's very high probability that the icons of feed folders flicker, that is the icon disappears and reappears. I've checked network activity and it seems that Thunderbird is fetching / for favicon when this happens. This happens several times a day because most of my feeds are set to update every few hours.

Most often I see this when I press F5 to update news.mozilla.org account which is the last one in my folder list. When there are new news items there folder list scrolls back to the top. When I then scroll down feed icons below the fold are refreshed.

Shouldn't favicons be cached? For how long? If they already should, how do I find out why this not happening? I don't get any errors/warnings when this happens.

Comment 1

[alta88]

You should have a recent Tb33 to have the followup patches (which just catch failures better).

The favicon url, or blank if invalid/failed, is cached in the folderpane tree rowmap's folderItem object.  It's done only once at startup as treerows are painted (a row must be scrolled into view to paint) and is never explicitly expired in a session.  The standard nsITreeView method getImageSrc paints the image to the tree.

If the rowmap is destroyed, that destroys the favicon cache (and everything else in rowmap) and of course the favicons will be re-queried.  It's destroyed if the foldpane view changes only, afaict.  If you simply have the Unread or other view, then neither F5 nor auto biff should have any effect (ie worksforme).  Nor does creating a new folder in Unread rebuild the rowmap.

I've never seen flickering; most favicons paint quickly the first time and failed/error icons are permanently resolved to default in no more than 8 seconds and not revisited.  Perhaps if you have no or a small browser cache size, gecko may be expiring images, but this is at an entirely different cache level.  Feed folderpane favicons do not use the places database/infrastructure for storing favicons.

Favicons can be disabled generically by setting browser.chrome.favicons or browser.chrome.site_icons to false.

Comment 2

[Merike Sell (:merike)]

(In reply to alta88 from comment #1)
> You should have a recent Tb33 to have the followup patches (which just catch
> failures better).
Still no exact STR but I was able to repro with a new profile on 33.0a1 (2014-06-17). It doesn't seem to affect all feeds with favicons. https://wiki.mozilla.org/index.php?title=Special:RecentChanges&feed=atom doesn't blank favicon and fetch / but http://rss.gmane.org/messages/excerpts/gmane.comp.mozilla.devel.platform does. Will try to get STR.

> The favicon url, or blank if invalid/failed, is cached in the folderpane
> tree rowmap's folderItem object.  It's done only once at startup as treerows
> are painted (a row must be scrolled into view to paint) and is never
> explicitly expired in a session.  The standard nsITreeView method
> getImageSrc paints the image to the tree.
I don't do it but does that mean people opening/closing their email client multiple times a day fetch all / for their feeds multiple times per day?

> If the rowmap is destroyed, that destroys the favicon cache (and everything
> else in rowmap) and of course the favicons will be re-queried.  It's
> destroyed if the foldpane view changes only, afaict.  If you simply have the
> Unread or other view, then neither F5 nor auto biff should have any effect
> (ie worksforme).  Nor does creating a new folder in Unread rebuild the
> rowmap.
Throwing all favicons away when folderpane view is changed sounds rather wasteful. Querying / can take much longer than just getting the feed. It would be nice to avoid that if possible.

> I've never seen flickering; most favicons paint quickly the first time and
> failed/error icons are permanently resolved to default in no more than 8
> seconds and not revisited.  Perhaps if you have no or a small browser cache
> size, gecko may be expiring images, but this is at an entirely different
> cache level.
The default cache size for new profile seems to be 350MB so I don't think running out of cache is causing this for me.

> Favicons can be disabled generically by setting browser.chrome.favicons or
> browser.chrome.site_icons to false.
That's good to know. I'm a bit undecided currently whether I like favicons in my email client or not.

Comment 3

[alta88]

1. A cross restart permanent local cache of favicons requires places history to be enabled in Tb (history is disabled by default).  This would then require a lot of UI, as in Fx, to ensure user control over such url history, for privacy etc.  I doubt it would be approved (unless content tabs became real browsers) nor would I request it just for favicons, nor would I use places favicons without such user control/UI.  So permanent favicon storage for Tb is unlikely to happen.

So favicons will be loaded once per session.  It certainly is much less network traffic than getting a feed even once.

2. 'Flickering' means rapid change in shape/opacity.  Correct me if not, but what you are really seeing is a delay in painting of *some* icons, on initial scroll into view of the row(s).  For valid favicons found at the well known [homepage/feed <link>]/favicon.ico location, it's instant on a reasonable network.  Otherwise, further attempts are made, async, to get the homepage and parse it for link rel containing 'icon'.  So this cannot be helped, given #1 above.  Once a folder is resolved, a favicon doesn't disappear for that view, for the rest of the session.

The gmane feed <link> is one that requires parsing to get the icon.  Besides being an obsolete rss1.0 rdf feed.

As favicons 'pop' into display, it may appear like there's activity depending on each url's favicon policy and the folder's positon in the tree.  I sort of like it, but to each their own.  You have the pref..

3. Yes, the resolution of favicons could be stored elsewhere rather than the rowmap, so teardowns don't involve re-queries.  If I complete Bug 257037 there will be infrastructure in place to do this easily.

If you want to change this bug to #3 that's ok, but the rest are going to be wontfix.

Comment 4

[Merike Sell (:merike)]

(In reply to alta88 from comment #3)
> 1. A cross restart permanent local cache of favicons requires places history
> to be enabled in Tb (history is disabled by default).  This would then
> require a lot of UI, as in Fx, to ensure user control over such url history,
> for privacy etc.  I doubt it would be approved (unless content tabs became
> real browsers) nor would I request it just for favicons, nor would I use
> places favicons without such user control/UI.  So permanent favicon storage
> for Tb is unlikely to happen.
Ok, that sounds indeed way too much effort to implement. And since I really appreciate your contributions to feed handling I wouldn't want you to spend too much time for little gain.

> So favicons will be loaded once per session.  It certainly is much less
> network traffic than getting a feed even once.
I don't understand you here. By "certainly" do you mean always? I don't think you want to say that / of the domain is never bigger and slower to load than feed url? My set of feeds might not be representative at all but after looking at traffic when changing folderpane view I can say that there's about a third or so feeds where it actually goes to get / for favicon not just the favicon itself.

> 2. 'Flickering' means rapid change in shape/opacity.  Correct me if not, but
> what you are really seeing is a delay in painting of *some* icons, on
> initial scroll into view of the row(s).
Right, that's me using terms I didn't check the exact meaning for. What I meant to express was that many favicons appear and disappear repeatedly but not rapidly.

> For valid favicons found at the
> well known [homepage/feed <link>]/favicon.ico location, it's instant on a
> reasonable network.  Otherwise, further attempts are made, async, to get the
> homepage and parse it for link rel containing 'icon'.  So this cannot be
> helped, given #1 above.  Once a folder is resolved, a favicon doesn't
> disappear for that view, for the rest of the session.
That last part is where my experience differs and where it bothers me most. The rest of my questions where mostly curiosity. I start Thunderbird in Unread mode and never change that. By session you do mean the timeframe from starting Thunderbird until either shutting it down or restarting it, correct? If I F5 news account folderpane scrolls to the top and feed icons below the fold get reloaded as soon as I scroll down again. If I don't use F5 then after some time, I suspect on one of the automatic checks for new items, feed icons below the fold still get thrown away. This is all without changing the view for folderpane. I don't have exact measures how often that happens, but I noticed it once today during 2-hour timeframe.

> 3. Yes, the resolution of favicons could be stored elsewhere rather than the
> rowmap, so teardowns don't involve re-queries.  If I complete Bug 257037
> there will be infrastructure in place to do this easily.
> 
> If you want to change this bug to #3 that's ok, but the rest are going to be
> wontfix.
Based on what you've described it looks like my folderpane rowmap gets teardowns every hour or two for some reason. Changing the bug to #3 would be ok too but to me it makes more sense to try to figure out why it happens more often than once per session / view change if that shouldn't be the case. If you're ok with that it could be set to unconfirmed until I can figure out STR with new profile.

Comment 5

[alta88]

(In reply to Merike (:merike) from comment #4)
> (In reply to alta88 from comment #3)
> > 1. A cross restart permanent local cache of favicons requires places history
> > to be enabled in Tb (history is disabled by default).  This would then
> > require a lot of UI, as in Fx, to ensure user control over such url history,
> > for privacy etc.  I doubt it would be approved (unless content tabs became
> > real browsers) nor would I request it just for favicons, nor would I use
> > places favicons without such user control/UI.  So permanent favicon storage
> > for Tb is unlikely to happen.
> Ok, that sounds indeed way too much effort to implement. And since I really
> appreciate your contributions to feed handling I wouldn't want you to spend
> too much time for little gain.
> 

Thanks.

> > So favicons will be loaded once per session.  It certainly is much less
> > network traffic than getting a feed even once.
> I don't understand you here. By "certainly" do you mean always? I don't
> think you want to say that / of the domain is never bigger and slower to
> load than feed url? My set of feeds might not be representative at all but
> after looking at traffic when changing folderpane view I can say that
> there's about a third or so feeds where it actually goes to get / for
> favicon not just the favicon itself.
> 

Well, I'd guess 2/3s get the 16x16 image bits only and the rest need parsing, and it's possible/likely a homepage might be bigger than a feed file. ;)

> > 2. 'Flickering' means rapid change in shape/opacity.  Correct me if not, but
> > what you are really seeing is a delay in painting of *some* icons, on
> > initial scroll into view of the row(s).
> Right, that's me using terms I didn't check the exact meaning for. What I
> meant to express was that many favicons appear and disappear repeatedly but
> not rapidly.
> 
> > For valid favicons found at the
> > well known [homepage/feed <link>]/favicon.ico location, it's instant on a
> > reasonable network.  Otherwise, further attempts are made, async, to get the
> > homepage and parse it for link rel containing 'icon'.  So this cannot be
> > helped, given #1 above.  Once a folder is resolved, a favicon doesn't
> > disappear for that view, for the rest of the session.
> That last part is where my experience differs and where it bothers me most.
> The rest of my questions where mostly curiosity. I start Thunderbird in
> Unread mode and never change that. By session you do mean the timeframe from
> starting Thunderbird until either shutting it down or restarting it,
> correct? If I F5 news account folderpane scrolls to the top and feed icons
> below the fold get reloaded as soon as I scroll down again. If I don't use
> F5 then after some time, I suspect on one of the automatic checks for new
> items, feed icons below the fold still get thrown away. This is all without
> changing the view for folderpane. I don't have exact measures how often that
> happens, but I noticed it once today during 2-hour timeframe.
> 

So the first thing to know is that a favicon is not ever requested *until* the row scrolls into view the first time.  In Unread Folders, I can F5 on various folders/accounts and existing favicons never go away.  On startup, make sure to first scroll top to bottom and get all favicons resolved to prove this.

> > 3. Yes, the resolution of favicons could be stored elsewhere rather than the
> > rowmap, so teardowns don't involve re-queries.  If I complete Bug 257037
> > there will be infrastructure in place to do this easily.
> > 
> > If you want to change this bug to #3 that's ok, but the rest are going to be
> > wontfix.
> Based on what you've described it looks like my folderpane rowmap gets
> teardowns every hour or two for some reason. Changing the bug to #3 would be
> ok too but to me it makes more sense to try to figure out why it happens
> more often than once per session / view change if that shouldn't be the
> case. If you're ok with that it could be set to unconfirmed until I can
> figure out STR with new profile.

I don't think there should be any reason for a teardown unless you change the folder view explicitly, so if true, this would need str.

Comment 6

[Magnus Melin [:mkmelin]]

(In reply to alta88 from comment #3)
> 1. A cross restart permanent local cache of favicons requires places history
> to be enabled in Tb (history is disabled by default).  This would then

We're enabling history it in bug 920118. (More UI in bug 1020339.)

Comment 7

[alta88]

(In reply to Magnus Melin from comment #6)
> (In reply to alta88 from comment #3)
> > 1. A cross restart permanent local cache of favicons requires places history
> > to be enabled in Tb (history is disabled by default).  This would then
> 
> We're enabling history it in bug 920118. (More UI in bug 1020339.)

Oh well that changes everything..  As long as we have a UI to clear history, I'd much rather use the working places apis than the crappy api that was necessary for non permanent storage.

Comment 8

[Merike Sell (:merike)]

Does conversion to places api render my original problem of disappearing and reappearing favicons completely irrelevant? If so, are there any other downsides to frequent teardowns of folderpane data? Such that I should file a new bug about that?

Because I took the time to investigate and this teardown happens every time news account checks for new messages. But only if I have subscribed to mozilla.mozillians and mozilla.dev.usability groups. Not sure why these two groups cause it but they tend to appear in unread view even when there are no unread items in them... I've got other mozilla groups subscribed that don't trigger this behaviour.

Comment 9

[alta88]

Are you saying if a newsgroup (nntp) account updates (auto or manual F5), in Unread folders view (tree or compact) then upon update (with new messages or no), certain subscriptions will cause feed folder icons to redisplay, indicating a teardown?

I can' reproduce that and it sounds very odd; I assume you've done this on a clean profile etc etc.  If not then try safe mode and maybe repair the newsgroup folders (it will be like a new sub though and ask for redownload and lose the .rc file).

The fact that some favicons resolve slowly has apparently unmasked a teardown issue, and going to places will mask it again.  Unless the view is changed, there should be no reason to rebuild the rowmap.

(A poor nsITreeView implementation might rebuild all rows rather than do the work of figuring out insertions/removals, but I think folderpane is pretty good.)

Comment 10

[Merike Sell (:merike)]

(In reply to alta88 from comment #9)
> Are you saying if a newsgroup (nntp) account updates (auto or manual F5), in
> Unread folders view (tree or compact) then upon update (with new messages or
> no), certain subscriptions will cause feed folder icons to redisplay,
> indicating a teardown?

In some cases, yes. I started from my daily profile by making a copy (and changing file paths I found pointing to the old folder) and then making it smaller by removing accounts and custom settings and so on while keeping the bug. In the end I got it small enough that I was able to take a new profile and make it similar enough that it would reproduce the problem.

In that new profile I had one feed account with three subscriptions and a news.mozilla.org account. I subscribed to these two groups, marked all items read (some combination of Shift+C or Ctrl+A followed by M or similar that I haven't identified) and shut down Thunderbird.

At that point newsrc-news.mozilla.org file contained:

mozilla.mozillians: 1,1177-1510,1512-1578,1580-1676
mozilla.dev.usability: 1-2704

When I then ran Thunderbird again in unread view then it reproduced for me every time news account updated. I was trying different contents of that file earlier and not all of them triggered this issue. Marking news folders read doesn't always result in this content. I don't quite understand what it depends on. Also, having this content might not be all that is needed. I could zip one of the profiles that triggers it if you want to take a look. I don't know enough to tell whether this content itself is in some way erroneous or if folderpane is to be blamed for not handling it properly. All in all it's a very confusing and irritating bug :)

Comment 11

[alta88]

Aha, in fact folderpane, upon detecting a new message in a folder, rebuilds the view rather than finding the insertion point in the rowmap array, when the previously unlisted folder needs to be added to the Unread list.  Reproducible by adding mozillians but not selecting it to get all downloads then going to Unread view; on biff it will add the folder (but not indicate any unread).

(It also looks like basic things such as biffState and hasNewMessages aren't being set properly on nntp folders after all this time).

> All in all it's a very confusing and irritating bug :)

For that view on every biff, absolutely.  You could, in order of decreasing lol:
1)always read your feeds and eat your vegetables
2)doc says, well then don't use that view
3)unpref favicons
4)contact all your feeds and get the ones who can't put a favicon in the usual place to do so

</lol>

Implementing places will be the fix, if the blocking bug doesn't go anywhere I'll cache the icons in non destructive per session memory..

Comment 13

[WADA:World Anti-bad-Duping Agency]

FYI.
Firefox team is trying to move favicons blobs out of places.sqlite. 
> Bug 977177 - move favicons blobs out of places.sqlite to their own database

Comment 14

[alta88]

Bug 1308727 has implemented session persistence for favicons, to survive folderpane rowMap teardowns.