Bug 234060

Summary: Make FontCache::invalidateAllFontCaches call invalidate on all worker FontCaches
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: Layout and RenderingAssignee: Cameron McCormack (:heycam) <heycam>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, mattwoodrow, mmaxfield, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 233488    
Attachments:
Description Flags
Patch
darin: review+
Patch none

Description Cameron McCormack (:heycam) 2021-12-08 18:47:48 PST
Make FontCache::invalidateAllFontCaches call invalidate on all worker FontCaches
Comment 1 Cameron McCormack (:heycam) 2021-12-08 18:49:38 PST
Created attachment 446481 [details]
Patch
Comment 2 Darin Adler 2021-12-08 19:00:05 PST
Comment on attachment 446481 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446481&action=review

> Source/WebCore/platform/graphics/FontCache.cpp:397
> +static void callOnAllFontCaches(F function)

Can we use WTF::function instead of making this entire thing a template?

Is "call" really the right way to describe something that posts tasks on the run loop?

> Source/WebCore/platform/graphics/FontCache.cpp:417
> +void FontCache::invalidateAllFontCaches()
> +{
> +    callOnAllFontCaches([](FontCache& fontCache) {
> +        fontCache.invalidate();
> +    });
>  }

For the purposes that callers intend, is it OK that this posts tasks that invalidate the font caches in the future, and doesn’t finish invalidating them before returning? Should we change the name to make that clearer?
Comment 3 Radar WebKit Bug Importer 2021-12-15 18:48:16 PST
<rdar://problem/86553783>
Comment 4 Matt Woodrow 2022-07-24 19:50:08 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 446481 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=446481&action=review
> 
> > Source/WebCore/platform/graphics/FontCache.cpp:397
> > +static void callOnAllFontCaches(F function)
> 
> Can we use WTF::function instead of making this entire thing a template?

WTF::function isn't copyable, so we can't pass it to all of the worker threads. Adding something like a thread safe refcounted wrapper doesn't seem worth it to me.
Comment 5 Matt Woodrow 2022-07-24 19:50:24 PDT
Created attachment 461188 [details]
Patch
Comment 6 EWS 2022-07-24 20:58:19 PDT
Committed 252778@main (073740c9d994): <https://commits.webkit.org/252778@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 461188 [details].