[webkit-reviews] review granted: [Bug 233986] Move FontDatabases to FontCache : [Attachment 446313] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 10 14:55:08 PST 2021

Myles C. Maxfield <mmaxfield at apple.com> has granted Cameron McCormack (:heycam)
<heycam at apple.com>'s request for review:
Bug 233986: Move FontDatabases to FontCache

Attachment 446313: Patch


--- Comment #2 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 446313
  --> https://bugs.webkit.org/attachment.cgi?id=446313

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
> +	   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


> Source/WebCore/platform/graphics/FontCache.h:186

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);


> 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 =

I kind of think the name of this function is too generic. Web browsers have
lots of databases inside them.

More information about the webkit-reviews mailing list