Makes sure that debug-only layout test failures are not to the ZERO WIDTH SPACE mapping to SPACE glyph complaints (http://b/1317563), fixes a layout test (fast/text/zero-width-characters.html), and provides an updated patch for WebKit.org bug 20237 (https://bugs.webkit.org/show_bug.cgi?id=20237).

This change brings handling of the ZWS and CJK character widths down to the level of SimpleFontData by creating special (sub-classed) SimpleFontData objects that are used in GlyphData. These instances are created when the glyph cache is being filled (GlyphPage::fill). More better things are possible, but at the moment I thought it might be good to just get the basics right.

Also, a couple of the layout tests are brought back to pre-font-metric-hacks removal versions.


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@1557 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 132 insertions, 57 deletions

diff --git a/webkit/pending/Font.cpp b/webkit/pending/Font.cpp
index 3f9b6b9..ef3091d 100644
--- a/webkit/pending/Font.cpp
+++ b/webkit/pending/Font.cpp
@@ -163,7 +163,7 @@ void WidthIterator::advance(int offset, GlyphBuffer* glyphBuffer)
             float tabWidth = m_font->tabWidth();
             width = tabWidth - fmodf(m_run.xPos() + runWidthSoFar, tabWidth);
         } else {
-            width = fontData->widthForGlyph(c, glyph);
+            width = fontData->widthForGlyph(glyph);
             // We special case spaces in two ways when applying word rounding.
             // First, we round spaces to an adjusted width in all fonts.
             // Second, in fixed-pitch fonts we ensure that all characters that
@@ -809,4 +809,17 @@ FontSelector* Font::fontSelector() const
     return m_fontList ? m_fontList->fontSelector() : 0;
 }
 
+// static
+bool Font::isCJKCodePoint(UChar32 c)
+{
+    // AC00..D7AF; Hangul Syllables
+    if ((0xAC00 <= c) && (c <= 0xD7AF))
+        return true;
+
+    // CJK ideographs
+    UErrorCode errorCode;
+    return uscript_getScript(c, &errorCode) == USCRIPT_HAN &&
+        U_SUCCESS(errorCode);
+}
+
 }
diff --git a/webkit/pending/SimpleFontData.cpp b/webkit/pending/SimpleFontData.cpp
index fa0e2b1..54abade 100644
--- a/webkit/pending/SimpleFontData.cpp
+++ b/webkit/pending/SimpleFontData.cpp
@@ -50,7 +50,8 @@ SimpleFontData::SimpleFontData(const FontPlatformData& f, bool customFont, bool
     , m_isCustomFont(customFont)
     , m_isLoading(loading)
     , m_smallCapsFontData(0)
-    , m_cjkGlyphWidth(cGlyphWidthUnknown)
+    , m_zeroWidthFontData(new ZeroWidthFontData())
+    , m_cjkWidthFontData(new CJKWidthFontData())
 {
 #if ENABLE(SVG_FONTS) && !PLATFORM(QT)
     if (SVGFontFaceElement* svgFontFaceElement = svgFontData ? svgFontData->svgFontFaceElement() : 0) {
@@ -77,6 +78,8 @@ SimpleFontData::SimpleFontData(const FontPlatformData& f, bool customFont, bool
         determinePitch();
         m_missingGlyphData.fontData = this;
         m_missingGlyphData.glyph = 0;
+        m_zeroWidthFontData->init(this);
+        m_cjkWidthFontData->init(this);
         return;
     }
 #endif
@@ -92,6 +95,8 @@ SimpleFontData::SimpleFontData(const FontPlatformData& f, bool customFont, bool
         determinePitch();
         m_missingGlyphData.fontData = this;
         m_missingGlyphData.glyph = 0;
+        m_zeroWidthFontData->init(this);
+        m_cjkWidthFontData->init(this);
         return;
     }
 
@@ -100,11 +105,18 @@ SimpleFontData::SimpleFontData(const FontPlatformData& f, bool customFont, bool
     // every character and the space are the same width.  Otherwise we round.
     static const UChar32 space_char = ' ';
     m_spaceGlyph = glyphPageZero->glyphDataForCharacter(space_char).glyph;
-    float width = widthForGlyph(space_char, m_spaceGlyph);
+    float width = widthForGlyph(m_spaceGlyph);
     m_spaceWidth = width;
     determinePitch();
     m_adjustedSpaceWidth = m_treatAsFixedPitch ? ceilf(width) : roundf(width);
 
+    // TODO(dglazkov): Investigate and implement across platforms, if needed
+#if PLATFORM(WIN)
+    // ZERO WIDTH SPACES are explicitly mapped to share the glyph
+    // with SPACE (with width adjusted to 0) during GlyphPage::fill
+    // This is currently only implemented for Windows port. The FontData
+    // remapping may very well be needed for other platforms.
+#else
     // Force the glyph for ZERO WIDTH SPACE to have zero width, unless it is shared with SPACE.
     // Helvetica is an example of a non-zero width ZERO WIDTH SPACE glyph.
     // See <http://bugs.webkit.org/show_bug.cgi?id=13178>
@@ -117,9 +129,23 @@ SimpleFontData::SimpleFontData(const FontPlatformData& f, bool customFont, bool
         else
             LOG_ERROR("Font maps SPACE and ZERO WIDTH SPACE to the same glyph. Glyph width not overridden.");
     }
+#endif
 
     m_missingGlyphData.fontData = this;
     m_missingGlyphData.glyph = 0;
+    m_zeroWidthFontData->init(this);
+    m_cjkWidthFontData->init(this);
+}
+
+SimpleFontData::SimpleFontData()
+    : m_treatAsFixedPitch(false)
+#if ENABLE(SVG_FONTS)
+    , m_svgFontData(0)
+#endif
+    , m_isCustomFont(0)
+    , m_isLoading(0)
+    , m_smallCapsFontData(0)
+{
 }
 
 SimpleFontData::~SimpleFontData()
@@ -133,48 +159,15 @@ SimpleFontData::~SimpleFontData()
     // it will be deleted then, so we don't need to do anything here.
 }
 
-// Use the character corresponding the glyph to determine if the glyph
-// is a fixed width CJK glyph. This will allow us to save on storage in
-// GlyphWidthMap for CJK glyph entries having the same width value.
-float SimpleFontData::widthForGlyph(UChar32 c, Glyph glyph) const
+float SimpleFontData::widthForGlyph(Glyph glyph) const
 {
-    bool is_CJK = IsCJKCodePoint(c);
-    float width = is_CJK ? m_cjkGlyphWidth : m_glyphToWidthMap.widthForGlyph(glyph);
-
-#ifndef NDEBUG
-    // Test our optimization that assuming all CGK glyphs have the same width
-    if (is_CJK) {
-        const float actual_width = platformWidthForGlyph(glyph);
-        ASSERT((cGlyphWidthUnknown == width) || (actual_width == width));
-    }
-#endif
-
-    // Some characters should be zero width and we want to ignore whatever
-    // crazy stuff the font may have (or not defined). If the font doesn't
-    // define it, we don't want to measure the width of the "invalid character"
-    // box, for example.
-    //
-    // Note that we have to exempt control characters, which
-    // treatAsZeroWidthSpace would normally return true for. This is primarily
-    // for \n since it will be rendered as a regular space in HTML.
-    //
-    // TODO(brettw): we should have Font::treatAsZeroWidthSpace return true for
-    // zero width spaces (U+200B) just like Font::treatAsSpace will return true
-    // for spaces. Then the additional OR is not necessary.
-    if (c > ' ' && (Font::treatAsZeroWidthSpace(c) || c == 0x200b))
-      return 0.0f;
-
+    float width = m_glyphToWidthMap.widthForGlyph(glyph);
     if (width != cGlyphWidthUnknown)
         return width;
 
     width = platformWidthForGlyph(glyph);
+    m_glyphToWidthMap.setWidthForGlyph(glyph, width);
 
-    if (is_CJK) {
-        m_cjkGlyphWidth = width;
-    } else {
-        m_glyphToWidthMap.setWidthForGlyph(glyph, width);
-    }
-    
     return width;
 }
 
@@ -188,21 +181,57 @@ bool SimpleFontData::isSegmented() const
     return false;
 }
 
-bool SimpleFontData::IsCJKCodePoint(UChar32 c) const
+const SimpleFontData* SimpleFontData::zeroWidthFontData() const
 {
-    // 3400..4DBF; CJK Unified Ideographs Extension A
-    // 4DC0..4DFF; Yijing Hexagram Symbols
-    // 4E00..9FFF; CJK Unified Ideographs
-    if ((0x3400 <= c) && (c <= 0x9FFF))
-        return true;
-    // AC00..D7AF; Hangul Syllables
-    if ((0xAC00 <= c) && (c <= 0xD7AF))
-        return true;
-    // F900..FAFF; CJK Compatibility Ideographs
-    if ((0xF900 <= c) && (c <= 0xFAFF))
-        return true;
+    return m_zeroWidthFontData.get();
+}
 
-    return false;
+const SimpleFontData* SimpleFontData::cjkWidthFontData() const
+{
+    return m_cjkWidthFontData.get();
+}
+
+void ZeroWidthFontData::init(SimpleFontData* fontData)
+{
+    m_font = fontData->m_font;
+    m_smallCapsFontData = fontData->m_smallCapsFontData;
+    m_ascent = fontData->m_ascent;
+    m_descent = fontData->m_descent;
+    m_lineSpacing = fontData->m_lineSpacing;
+    m_lineGap = fontData->m_lineGap;
+    m_maxCharWidth = 0;
+    m_avgCharWidth = 0;
+    m_xHeight = fontData->m_xHeight;
+    m_unitsPerEm = fontData->m_unitsPerEm;
+    m_spaceWidth = 0;
+    m_spaceGlyph = 0;
+    m_adjustedSpaceWidth = fontData->m_adjustedSpaceWidth;
+#if PLATFORM(WIN)
+    m_scriptCache = 0;
+    m_scriptFontProperties = 0;
+#endif
+}
+
+CJKWidthFontData::CJKWidthFontData()
+    : m_cjkGlyphWidth(cGlyphWidthUnknown)
+{
+}
+
+float CJKWidthFontData::widthForGlyph(Glyph glyph) const
+{
+    if (m_cjkGlyphWidth != cGlyphWidthUnknown)
+        return m_cjkGlyphWidth;
+
+    float width = platformWidthForGlyph(glyph);
+    m_cjkGlyphWidth = width;
+
+#ifndef NDEBUG
+    // Test our optimization that assuming all CGK glyphs have the same width
+    const float actual_width = platformWidthForGlyph(glyph);
+    ASSERT((cGlyphWidthUnknown == width) || (actual_width == width));
+#endif
+
+    return width;
 }
 
 } // namespace WebCore
diff --git a/webkit/pending/SimpleFontData.h b/webkit/pending/SimpleFontData.h
index c1a84aa..1ebdeaa 100644
--- a/webkit/pending/SimpleFontData.h
+++ b/webkit/pending/SimpleFontData.h
@@ -44,6 +44,8 @@ class FontPlatformData;
 class SharedBuffer;
 class SVGFontData;
 class WidthMap;
+class ZeroWidthFontData;
+class CJKWidthFontData;
 
 enum Pitch { UnknownPitch, FixedPitch, VariablePitch };
 
@@ -52,6 +54,10 @@ public:
     SimpleFontData(const FontPlatformData&, bool customFont = false, bool loading = false, SVGFontData* data = 0);
     virtual ~SimpleFontData();
 
+protected:
+    // sub-class constructor
+    SimpleFontData();
+
 public:
     const FontPlatformData& platformData() const { return m_font; }
     SimpleFontData* smallCapsFontData(const FontDescription& fontDescription) const;
@@ -67,7 +73,7 @@ public:
     float xHeight() const { return m_xHeight; }
     unsigned unitsPerEm() const { return m_unitsPerEm; }
 
-    float widthForGlyph(UChar32, Glyph) const;
+    virtual float widthForGlyph(Glyph) const;
     float platformWidthForGlyph(Glyph) const;
 
     virtual const SimpleFontData* fontDataForCharacter(UChar32) const;
@@ -117,12 +123,14 @@ public:
     wxFont getWxFont() const { return m_font.font(); }
 #endif
 
+    const SimpleFontData* zeroWidthFontData() const;
+    const SimpleFontData* cjkWidthFontData() const;
+
 private:
     void platformInit();
     void platformDestroy();
     
     void commonInit();
-    bool IsCJKCodePoint(UChar32) const;
 
 public:
     int m_ascent;
@@ -155,9 +163,6 @@ public:
 
     mutable SimpleFontData* m_smallCapsFontData;
 
-    // Optimization for CJK glyphs
-    mutable float m_cjkGlyphWidth;
-
 #if PLATFORM(CG)
     float m_syntheticBoldOffset;
 #endif
@@ -176,6 +181,34 @@ public:
     mutable SCRIPT_CACHE m_scriptCache;
     mutable SCRIPT_FONTPROPERTIES* m_scriptFontProperties;
 #endif
+
+private:
+    OwnPtr<ZeroWidthFontData> m_zeroWidthFontData;
+    OwnPtr<CJKWidthFontData> m_cjkWidthFontData;
+};
+
+// SimpleFontData sub-classes:
+
+// Has a single zero-width glyph, used for ZWS and 
+// UCHAR_DEFAULT_IGNORABLE_CODE_POINT characters
+class ZeroWidthFontData : public SimpleFontData {
+public:
+    void init(SimpleFontData*);
+    virtual float widthForGlyph(Glyph) const { return 0.0f; }
+};
+
+// Monospaced, single glyph and width, used for CJK characters
+// The assumption made here can break for some high-quality CJK fonts with
+// proportional CJK glyphs.
+class CJKWidthFontData : public ZeroWidthFontData {
+public:
+    CJKWidthFontData();
+
+    virtual float widthForGlyph(Glyph) const;
+
+private:
+    // Optimization for CJK glyphs
+    mutable float m_cjkGlyphWidth;
 };
 
 } // namespace WebCore