[webkit-reviews] review granted: [Bug 199283] Prewarm font cache with more fonts : [Attachment 374141] Patch

[bugzilla-daemon at webkit.org]

Myles C. Maxfield <mmaxfield at apple.com> has granted Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 199283: Prewarm font cache with more fonts
https://bugs.webkit.org/show_bug.cgi?id=199283

Attachment 374141: Patch

https://bugs.webkit.org/attachment.cgi?id=374141&action=review




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

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

> Source/WebCore/page/ProcessWarming.cpp:-77
> -    // Cache system UI font fallbacks. Almost every web process needs these.
> -    // Initializing one size is sufficient to warm CoreText caches.

Really? Most web content doesn't use system-ui. Where do the uses come from?

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1644
> +    prewarmInfo.fontNamesRequiringSystemFallback =
fontFamiliesForPrewarming();

I don't think this is right. We only take the "system fallback" path for the
system font. Arial, Helvetica, Helvetica Neue, Times, and Times New Roman would
never hit that path. OTOH, it could be that running these CoreText functions
ahead-of-time is still helpful regarding the codepath those fonts ::do:: go
through. Do we still get the benefit if we move this list to the "seenFamilies"
member?

"SF Pro Text" shouldn't match any fonts. Are we sure this one is really
valuable?