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

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied 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 373046: Patch

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




--- Comment #2 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 373046
  --> https://bugs.webkit.org/attachment.cgi?id=373046
Patch

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

> Source/WebCore/page/ProcessWarming.cpp:82
> +    static NeverDestroyed<Vector<String>> families =
std::initializer_list<String> {
> +	   "-apple-system"_s,
> +	   "-webkit-standard"_s,
> +	   "sans-serif"_s,
> +	   "system-ui"_s,
> +	   "Arial"_s,
> +	   "Avenir"_s,
> +	   "Helvetica"_s,
> +	   "Helvetica Neue"_s,
> +	   "Hiragino Sans GB"_s,
> +	   "Lucida Grande"_s,
> +	   "PingFang"_s,
> +	   "SF Pro Text"_s,
> +	   "SF Pro Icons"_s,
> +	   "STHeiti"_s,
> +	   "Segoe UI"_s,
> +	   "Times"_s,
> +	   "Times New Roman"_s
> +    };

There is substantial power and memory cost in warming fonts. The code here runs
for every web process and should only warm things that are very likely needed.
I don't think this list fits the criteria. With language specific fonts, it
looks more like a benchmark hack.

I'm not opposed to globally prewarming more fonts but each entry needs to be
justified based on real world data, not artificial benchmark scenarios.