Created attachment 446313 [details] Patch

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.

<rdar://problem/86507617>

Created attachment 461036 [details] Patch

Created attachment 461054 [details] Patch

Committed 252671@main (7c81d79e7253): <https://commits.webkit.org/252671@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 461054 [details].