Add heuristic to LayoutTextControl::hasValidAvgCharWidth()

third_party/WebKit/Source/core/layout/LayoutTextControl.cpp

Index: third_party/WebKit/Source/core/layout/LayoutTextControl.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTextControl.cpp b/third_party/WebKit/Source/core/layout/LayoutTextControl.cpp
index 0b87e69471d1ea3a3d4236788629a02bb3089c0b..e8ee104847271c40bf0e0a1bf8b00699bf61c968 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTextControl.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTextControl.cpp
@@ -189,8 +189,15 @@ static const char* const fontFamiliesWithInvalidCharWidth[] = {
 // from the width of a '0'. This only seems to apply to a fixed number of Mac fonts,
 // but, in order to get similar rendering across platforms, we do this check for
 // all platforms.
-bool LayoutTextControl::hasValidAvgCharWidth(const AtomicString& family)
+bool LayoutTextControl::hasValidAvgCharWidth(const SimpleFontData* font, const AtomicString& family)

[kochi, 2016/07/13 09:36:12]

Can't this be "const SimpleFontData& font"?
(that saves line 196 DCHECK)

[kojii, 2016/07/13 11:20:35]

Making "*" to "&" doesn't assure non-null, so it changes to "DCHECK(&font)",
which looks strange, no?

[tkent, 2016/07/13 23:14:35]

In this case,
 * making the argument type |const SimpleFontData&|,
 * remove DCHECK(font) here, and
 * add DCHECK(primaryFont) in getAvgCharWidth()

would be reasonable.

[tkent, 2016/07/14 00:07:09]

eae@ just added a runtime null check. https://codereview.chromium.org/2149683002

 {
+    // Some fonts match avgCharWidth to CJK full-width characters.
+    // Heuristic check to avoid such fonts.
+    DCHECK(font);
+    const FontMetrics& metrics = font->getFontMetrics();
+    if (metrics.hasZeroWidth() && font->avgCharWidth() > metrics.zeroWidth() * 1.7)
+        return false;
+
     static HashSet<AtomicString>* fontFamiliesWithInvalidCharWidthMap = nullptr;
 
     if (family.isEmpty())
@@ -209,10 +216,9 @@ bool LayoutTextControl::hasValidAvgCharWidth(const AtomicString& family)
 float LayoutTextControl::getAvgCharWidth(const AtomicString& family) const
 {
     const Font& font = style()->font();
-    if (hasValidAvgCharWidth(family)) {
-        ASSERT(font.primaryFont());
-        return roundf(font.primaryFont()->avgCharWidth());
-    }
+    const SimpleFontData* primaryFont = font.primaryFont();
+    if (hasValidAvgCharWidth(primaryFont, family))
+        return roundf(primaryFont->avgCharWidth());
 
     const UChar ch = '0';
     const String str = String(&ch, 1);