FaviconHelper::ObtainCachedIconFile does main thread IO and causes hangs
Categories
(Core :: Widget: Win32, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox129 | --- | fixed |
People
(Reporter: johannh, Assigned: mconley, NeedInfo)
References
(Blocks 1 open bug)
Details
(Keywords: perf:responsiveness, Whiteboard: [bhr:ObtainCachedIconFile])
Attachments
(3 files)
FaviconHelper::ObtainCachedIconFile has been showing up in BHR for causing hangs due to its main thread IO: https://searchfox.org/mozilla-central/rev/fc3d974254660b34638b2af9d5431618b191b233/widget/windows/WinUtils.cpp#1528 Now, the file.exists call in there is probably completely unnecessary, but I'm not sure if it's possible to get rid of the GetLastModified time.
|
||
Updated•6 years ago
|
|
||
Updated•5 years ago
|
|
||
Comment 1•4 years ago
•
|
||
This is a specific subset of the main thread I/O done by jumplists (bug 1529276).
|
||
Updated•2 years ago
|
|
Assignee | |
Updated•9 months ago
|
|
||
Updated•7 months ago
|
|
|
Assignee | |
Comment 2•7 months ago
|
||
I think what we can do here is that we can introduce a new method, in FaviconHelper - FaviconHelper::ObtainCachedIconFileAsync. We can then transition the new Jump List backend and this usage here (which already ignores the synchronous return value) to that new method.
The new method can then do the existence testing / stat'ing on the IO thread rather than the main thread, and call into a runnable with either the path to the cached favicon or an nsresult reporting what has gone wrong.
And then we can update nsIJumpListBuilder.obtainAndCacheFavicon to return a Promise instead and use our new backend.
|
|
Assignee | |
Updated•4 months ago
|
|
|
Assignee | |
Comment 3•4 months ago
|
||
|
||
Updated•2 months ago
|
|
|
Assignee | |
Comment 4•2 months ago
|
||
Depends on D203761
|
|
Assignee | |
Comment 5•2 months ago
|
||
Depends on D209152
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78b1a5e7ec1e Add an async version of FaviconHelper::ObtainCachedIconFile. r=rkraesig,win-reviewers https://hg.mozilla.org/integration/autoland/rev/ce0ff7d06ed4 Update WindowsJumpList.sys.mjs to use the async favicon caching utility. r=rkraesig https://hg.mozilla.org/integration/autoland/rev/4d04db1983e3 Add tests for nsIJumpListBuilder.obtainAndCacheFaviconAsync. r=mossop https://hg.mozilla.org/integration/autoland/rev/22bf63387696 apply code formatting via Lando
|
||
Comment 7•14 days ago
|
||
Backed out for causing xpcshell failures on test_JumpListBuilder_obtainAndCacheFaviconAsync.js
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/947f25cfac26 Add an async version of FaviconHelper::ObtainCachedIconFile. r=rkraesig,win-reviewers https://hg.mozilla.org/integration/autoland/rev/38b6ce46cca2 Update WindowsJumpList.sys.mjs to use the async favicon caching utility. r=rkraesig https://hg.mozilla.org/integration/autoland/rev/e9c1549806bf Add tests for nsIJumpListBuilder.obtainAndCacheFaviconAsync. r=mossop https://hg.mozilla.org/integration/autoland/rev/c1a66ab56f9d apply code formatting via Lando
|
||
Comment 9•7 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/947f25cfac26
https://hg.mozilla.org/mozilla-central/rev/38b6ce46cca2
https://hg.mozilla.org/mozilla-central/rev/e9c1549806bf
https://hg.mozilla.org/mozilla-central/rev/c1a66ab56f9d
Description
•