[webkit-reviews] review granted: [Bug 173962] REGRESSION(r216944): Font loads can cause Chinese characters to draw as .notdef : [Attachment 314756] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 173962: REGRESSION(r216944): Font loads can cause Chinese characters to
draw as .notdef
https://bugs.webkit.org/show_bug.cgi?id=173962

Attachment 314756: Patch

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




--- Comment #17 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 314756
  --> https://bugs.webkit.org/attachment.cgi?id=314756
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:289
> +    if (!m_derivedFontData)
> +	   m_derivedFontData = std::make_unique<DerivedFonts>();

This is repeated 3 times. Maybe make an ensureDerivedFontData() function?

> Source/WebCore/platform/graphics/Font.h:137
> +    const Font& invisibleFont() const;

This is a little mysterious. Is this the font you use if you don't want the
characters to display?

> Source/WebCore/platform/graphics/Font.h:284
> +	   RefPtr<Font> invisible;

I would have said "Font" after the name. "invisibleFont". Same for the
preceding.

> Source/WebCore/platform/graphics/FontCascadeFonts.cpp:333
> +

remove this blank line.