[webkit-reviews] review granted: [Bug 219672] [GPU Process] Cache Font objects : [Attachment 415973] Adds Font cache

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 219672: [GPU Process] Cache Font objects
https://bugs.webkit.org/show_bug.cgi?id=219672

Attachment 415973: Adds Font cache

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




--- Comment #11 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 415973
  --> https://bugs.webkit.org/attachment.cgi?id=415973
Adds Font cache

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

The rendering backend and DisplayList changes look reasonable to me.

> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:136
> +    if (unusedFontCount < totalFontCount /
minimumFractionOfUnusedFontCountToTriggerRemoval && unusedFontCount <=
maximumUnusedFontCountToSkipRemoval)

Nit - `minimumFractionOfUnusedFontCountToTriggerRemoval` sounds like it would
be some fraction less than 1 (i.e. 0.25), and then we'd do something like:

unusedFontCount < minimumFractionOfUnusedFontCountToTriggerRemoval *
totalFontCount

> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h:69
> +    HashMap<WebCore::RenderingResourceIdentifier, uint64_t>
m_fontLastRenderingUpdateMap;

Nit - maybe `m_fontIdentifierToLastRenderingUpdateVersionMap`?

> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h:71
> +    uint64_t m_currentRenderingUpdateVersion { 1 };

Can we can add a `using RenderingUpdateVersion = uint64_t;` instead of just
using uint64_t?