[webkit-reviews] review granted: [Bug 137550] Remove second-to-last client of WebFontCache : [Attachment 239521] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 137550: Remove second-to-last client of WebFontCache
https://bugs.webkit.org/show_bug.cgi?id=137550

Attachment 239521: Patch
https://bugs.webkit.org/attachment.cgi?id=239521&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=239521&action=review


> Source/WebKit/mac/WebView/WebHTMLView.mm:4897
>	   // Find the font the same way the rendering code would later if it
encountered this CSS.

This whole “find font” sequence, in the end, is just used to get a font name. I
think we should just refactor into a helper that returns an NSString rather
than having this tricky dance here in the middle of a function that’s already
doing so many other things.

> Source/WebKit/mac/WebView/WebHTMLView.mm:4902
> +	   FontCachePurgePreventer purgePreventer;

Why is this needed? This seems like a really tricky class to use right if this
is really needed for a simple case like this. Can purging really happen in the
tiny window before we call platformData().font() and then fontName?