Bug 233986 - Move FontDatabases to FontCache
Summary: Move FontDatabases to FontCache
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Cameron McCormack (:heycam)
URL:
Keywords: InRadar
Depends on:
Blocks: 233488
  Show dependency treegraph
 
Reported: 2021-12-07 23:03 PST by Cameron McCormack (:heycam)
Modified: 2021-12-14 23:04 PST (History)
5 users (show)

See Also:


Attachments
Patch (32.67 KB, patch)
2021-12-07 23:08 PST, Cameron McCormack (:heycam)
mmaxfield: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron McCormack (:heycam) 2021-12-07 23:03:12 PST
Move FontDatabases to FontCache
Comment 1 Cameron McCormack (:heycam) 2021-12-07 23:08:25 PST
Created attachment 446313 [details]
Patch
Comment 2 Myles C. Maxfield 2021-12-10 14:55:08 PST
Comment on attachment 446313 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        This patch moves the FontDatabase instances to live on the FontCache,
> +        of which we already have one per thread.

Yes I think this is the right thing to do.

> Source/WebCore/ChangeLog:17
> +        We also move the FontDatabase definition to a separate file, to reduce

"We"?

> Source/WebCore/platform/graphics/FontCache.h:186
> +#if PLATFORM(COCOA)

I think we generally don't guard declarations (as opposed to definitions) if they're not called.

> Source/WebCore/platform/graphics/FontCache.h:213
> +    FontDatabase m_databaseAllowingUserInstalledFonts { AllowUserInstalledFonts::Yes };
> +    FontDatabase m_databaseDisallowingUserInstalledFonts { AllowUserInstalledFonts::No };

🆒

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1054
> +    auto& fontDatabase = database(allowUserInstalledFonts);

thanks

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1145
>          FontCache::invalidateAllFontCaches();

The implementation of this will execute the above two lines?

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1253
> +    auto& fontDatabase = database(fontDescription.shouldAllowUserInstalledFonts());

I kind of think the name of this function is too generic. Web browsers have lots of databases inside them.
Comment 3 Radar WebKit Bug Importer 2021-12-14 23:04:16 PST
<rdar://problem/86507617>