Prevent animated SVGs being stored in Places
Categories
(Toolkit :: Places, defect, P3)
Tracking
()
People
(Reporter: mail.fabulous.cupcake, Unassigned)
References
(Depends on 1 open bug)
Details
(Keywords: perf, Whiteboard: [sng])
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:136.0) Gecko/20100101 Firefox/136.0
Steps to reproduce:
Bookmark a website which has animated SVG as favicon
In my case, after quite lengthy debugging, I found that I bookmarked
https://uchu.style/
Actual results:
Idle CPU usage (viewed from about:processes) increases from 1% to 50% to 190%
Expected results:
Not increase like crazy and remain 1%
|
Reporter | |
Comment 1•10 months ago
|
||
This took me 2 hours to find! It super obscure since about:processes did not break down the top Firefox entry
I tried resetting so many things, almost went insane when I found out it's caused by one specific bookmark item.
Considering how easy it is for any normal user to add such bookmark and how obscure it is, I think it's very important for firefox to stop animating favicons in the bookmarks bar, or limit the CPU usage — increasing idle CPU usage to 50-190% because you added one random bookmark is crazy
|
|
Reporter | |
Comment 2•10 months ago
|
||
Note on Steps to Reproduce:
The bookmark favicon must be visually visible for the issue to occur
This means the bookmark must be added to bookmarks bar, and the bookmarks bar is configured to Always Shown or Shown in new tab.
|
||
Comment 3•10 months ago
|
||
The Bugbug bot thinks this bug should belong to the 'Core::Graphics: WebRender' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
|
||
Comment 4•10 months ago
|
||
I can reproduce the issue.
Profiler
https://share.firefox.dev/41DRcor
https://share.firefox.dev/3DAaJyc
|
|
||
Comment 5•9 months ago
|
||
The severity field is not set for this bug.
:gw, could you have a look please?
For more information, please visit BugBot documentation.
|
||
Comment 6•9 months ago
|
||
Likely an issue with how (often) Gecko is sending content to WR in this case, rather than WR itself, I think.
|
|
||
Comment 7•9 months ago
|
||
The severity field is not set for this bug.
:bas.schouten, could you have a look please?
For more information, please visit BugBot documentation.
|
||
Comment 8•8 months ago
|
||
The Performance Impact Calculator has determined this bug's performance impact to be low. If you'd like to request re-triage, you can reset the Performance Impact flag to "?" or needinfo the triage sheriff.
Platforms: Windows
Websites affected: Rare
Resource impact: Severe
[x] Able to reproduce locally
[x] Bug affects multiple sites
|
|
||
Comment 9•8 months ago
|
||
Tentatively moving this.
|
||
Comment 10•8 months ago
|
||
Hi FabulousCupcake,
Did you notice the animated favicon was in your New Tab page when you saw this CPU increase? From the looks of the profile, the animation is coming from the privileged about content process, which makes me think it's probably listed in Top Sites.
It's an SVG animated icon, and from some testing, I can see that it's animating in all surfaces, including the Library and bookmarks toolbar.
I'm going to put this in Toolkit :: Places, because I think this is something we might want to deal with in the FaviconService... we might want to find a way of suppressing animated favicons through that service.
|
|
Reporter | |
Comment 11•8 months ago
|
||
Hey Mike 👋
I have my new tab page set to blank, and I see in about:performance that the cpu usage is increased (both when I initially noticed, and just now, rechecked).
As for the profile, I don't remember uploading any, so I'm assuming it's alice0775's profile from comment #4 and I am not sure how hey had their new tab page configured
|
||
Comment 12•8 months ago
|
||
(In reply to FabulousCupcake from comment #0)
Bookmark a website which has animated SVG as favicon
In my case, after quite lengthy debugging, I found that I bookmarked
https://uchu.style/
[...]
Idle CPU usage (viewed from about:processes) increases from 1% to 50% to 190%
Given the above, this is likely a version of bug 1821864, just manifesting in a slightly different way from what's described in that bug (thanks Alice0775 for the see-also linkage). I'll mark this as a dependency of that bug -- after (or as) we fix that bug for favicons in the tab-strip, we'd want to be sure it's also fixed for the "bookmark site" flow that's described in this bug here.
(In reply to Mike Conley (:mconley) (:⚙️) from comment #10)
we might want to find a way of suppressing animated favicons through that service.
See also bug 111373, and some additional observations/thoughts on disabling/pausing favicon animations in bug 1821864 comment 7 through bug 1821864 comment 9.
|
|
||
Updated•8 months ago
|
|
||
Comment 13•8 months ago
|
||
After talking with Marco in our bug scrub, we are transforming this bug to instead be about prevent animated SVGs from being stored in the Places favicon cache.
Short term, we could just opt not to store any animated SVGs and use a fallback from the site.
But long term, it would be nice if we could just take one frame from an animated SVG. Daniel, do you know if that would be possible?
|
||
Updated•8 months ago
|
|
|
||
Comment 14•8 months ago
|
||
(In reply to James Teow [:jteow] from comment #13)
But long term, it would be nice if we could just take one frame from an animated SVG. Daniel, do you know if that would be possible?
Yeah, there are a couple of options for that:
(1) We could use imgIContainer.idl's getFrameAtSize() API which accepts an option FRAME_FIRST and a requested resolution-size (e.g. the display-pixel-size of the favicon rendering area). That's probably what we want here to use here.
(We handle FRAME_FIRST in the SVG-as-an-image code by seeking to t=0 in the animation timeline, here.)
https://searchfox.org/mozilla-central/rev/126697140e711e04a9d95edae537541c3bde89cc/image/imgIContainer.idl#320-328
Perhaps the favicon service would want to call that function, and then convert the surface into a PNG data-URI or something along those lines, and use that here?
Or, maybe better:
(2) It looks like we have a FrozenImage class which is a form of imgIContainer and exists purely to serve as a wrapper for some other imgIContainer and disable animation. I haven't worked with this type before, but it looks like it might be exactly what we want here.
|
|
||
Comment 15•8 months ago
|
||
In case FrozenImage does end up being what we want - it looks like we use that in just one spot, here:
https://searchfox.org/mozilla-central/rev/126697140e711e04a9d95edae537541c3bde89cc/image/imgRequestProxy.cpp#1094-1095
That code might be relevant for cribbing/sharing, and/or for figuring out what the right utility functions might be best to call as part of getting a FrozenImage set up.
Description
•