Layout Test fast/text/international/khmer-selection.html is crashing

https://bugs.webkit.org/show_bug.cgi?id=191368

Reviewed by Brent Fulgham.

Source/WebCore:

GlyphBuffer's offset array wasn't getting filled by UniscribeController.
Our underlining code requires this array.

Uniscribe gives us a character -> glyph mapping, so we just have to compute
the inverse and give it to the GlyphBuffer.

This patch is written by Myles C. Maxfield.

Test: fast/text/international/khmer-selection.html.

* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::add):
* platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
* platform/graphics/win/UniscribeController.cpp:
(WebCore::UniscribeController::advance):
(WebCore::UniscribeController::itemizeShapeAndPlace):
(WebCore::UniscribeController::shapeAndPlaceItem):
* platform/graphics/win/UniscribeController.h:

LayoutTests:

* platform/win/TestExpectations:


Canonical link: https://commits.webkit.org/209282@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241915 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,12 @@
+2019-02-21  Per Arne Vollan  <pvollan@apple.com>
+
+        Layout Test fast/text/international/khmer-selection.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=191368
+
+        Reviewed by Brent Fulgham.
+
+        * platform/win/TestExpectations:
+
 2019-02-21  Dean Jackson  <dino@apple.com>
 
         Rotation animations sometimes use the wrong origin (affects apple.com)

LayoutTests/platform/win/TestExpectations

@@ -2505,7 +2505,7 @@ fast/frames/frame-navigation.html [ Skip ]
 
 fast/text/atsui-pointtooffset-calls-cg.html [ Pass Failure ]
 fast/text/international/hindi-whitespace.html [ Failure ]
-fast/text/international/khmer-selection.html [ Failure Crash ]
+fast/text/international/khmer-selection.html [ Failure ]
 webkit.org/b/140231 fast/text/international/plane2.html [ Failure ]
 webkit.org/b/35973 fast/multicol/hit-test-above-or-below.html [ Failure ]
 webkit.org/b/49769 fast/dom/HTMLProgressElement/progress-element-with-child-crash.html [ Failure ]
@@ -4221,10 +4221,6 @@ webkit.org/b/191366 fast/block/basic/child-block-level-box-with-height-percent.h
 webkit.org/b/191366 fast/block/basic/height-percentage-simple.html [ Failure ]
 webkit.org/b/191366 fast/block/basic/quirk-mode-percent-height.html [ Failure ]
 
-webkit.org/b/191368 fast/text/stroking-decorations.html [ Crash ]
-webkit.org/b/191368 imported/blink/fast/text/international/complex-text-trailing-space.html [ Crash ]
-webkit.org/b/191368 imported/blink/fast/text/sub-pixel/complex-text-preferred-width.html [ Crash ]
-
 webkit.org/b/191584 animations/animation-direction-normal.html [ Failure ]
 webkit.org/b/191584 animations/animation-direction-reverse.html [ Failure ]
 webkit.org/b/191584 animations/dynamic-stylesheet-loading.html [ Failure ]

Source/WebCore/ChangeLog

@@ -1,3 +1,30 @@
+2019-02-21  Per Arne Vollan  <pvollan@apple.com>
+
+        Layout Test fast/text/international/khmer-selection.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=191368
+
+        Reviewed by Brent Fulgham.
+
+        GlyphBuffer's offset array wasn't getting filled by UniscribeController.
+        Our underlining code requires this array.
+
+        Uniscribe gives us a character -> glyph mapping, so we just have to compute
+        the inverse and give it to the GlyphBuffer.
+
+        This patch is written by Myles C. Maxfield.
+
+        Test: fast/text/international/khmer-selection.html.
+
+        * platform/graphics/GlyphBuffer.h:
+        (WebCore::GlyphBuffer::add):
+        * platform/graphics/displaylists/DisplayListItems.cpp:
+        (WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
+        * platform/graphics/win/UniscribeController.cpp:
+        (WebCore::UniscribeController::advance):
+        (WebCore::UniscribeController::itemizeShapeAndPlace):
+        (WebCore::UniscribeController::shapeAndPlaceItem):
+        * platform/graphics/win/UniscribeController.h:
+
 2019-02-21  Andy Estes  <aestes@apple.com>
 
         contentfiltering tests leak documents

Source/WebCore/platform/graphics/GlyphBuffer.h

@@ -117,7 +117,7 @@ class GlyphBuffer {
         add(glyph, font, advance, offsetInString);
     }
 
-    void add(Glyph glyph, const Font* font, GlyphBufferAdvance advance, unsigned offsetInString = noOffset)
+    void add(Glyph glyph, const Font* font, GlyphBufferAdvance advance, unsigned offsetInString)
     {
         m_font.append(font);
         m_glyphs.append(glyph);

Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp

@@ -364,7 +364,7 @@ inline GlyphBuffer DrawGlyphs::generateGlyphBuffer() const
 {
     GlyphBuffer result;
     for (size_t i = 0; i < m_glyphs.size(); ++i) {
-        result.add(m_glyphs[i], &m_font.get(), m_advances[i]);
+        result.add(m_glyphs[i], &m_font.get(), m_advances[i], GlyphBuffer::noOffset);
     }
     return result;
 }

Source/WebCore/platform/graphics/win/UniscribeController.cpp

@@ -171,7 +171,7 @@ void UniscribeController::advance(unsigned offset, GlyphBuffer* glyphBuffer)
             int itemStart = m_run.rtl() ? index + 1 : indexOfFontTransition;
             int itemLength = m_run.rtl() ? indexOfFontTransition - index : index - indexOfFontTransition;
             m_currentCharacter = baseCharacter + itemStart;
-            itemizeShapeAndPlace((isSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemLength, fontData, glyphBuffer);
+            itemizeShapeAndPlace((isSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemStart, itemLength, fontData, glyphBuffer);
             indexOfFontTransition = index;
         }
     }
@@ -183,13 +183,13 @@ void UniscribeController::advance(unsigned offset, GlyphBuffer* glyphBuffer)
 
         int itemStart = m_run.rtl() ? 0 : indexOfFontTransition;
         m_currentCharacter = baseCharacter + itemStart;
-        itemizeShapeAndPlace((nextIsSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemLength, nextFontData, glyphBuffer);
+        itemizeShapeAndPlace((nextIsSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemStart, itemLength, nextFontData, glyphBuffer);
     }
 
     m_currentCharacter = baseCharacter + length;
 }
 
-void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned length, const Font* fontData, GlyphBuffer* glyphBuffer)
+void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned stringOffset, unsigned length, const Font* fontData, GlyphBuffer* glyphBuffer)
 {
     // ScriptItemize (in Windows XP versions prior to SP2) can overflow by 1.  This is why there is an extra empty item
     // hanging out at the end of the array
@@ -208,12 +208,12 @@ void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned length,
 
     if (m_run.rtl()) {
         for (int i = m_items.size() - 2; i >= 0; i--) {
-            if (!shapeAndPlaceItem(cp, i, fontData, glyphBuffer))
+            if (!shapeAndPlaceItem(cp, stringOffset, i, fontData, glyphBuffer))
                 return;
         }
     } else {
         for (unsigned i = 0; i < m_items.size() - 1; i++) {
-            if (!shapeAndPlaceItem(cp, i, fontData, glyphBuffer))
+            if (!shapeAndPlaceItem(cp, stringOffset, i, fontData, glyphBuffer))
                 return;
         }
     }
@@ -231,7 +231,7 @@ void UniscribeController::resetControlAndState()
     m_state.fOverrideDirection = m_run.directionalOverride();
 }
 
-bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned i, const Font* fontData, GlyphBuffer* glyphBuffer)
+bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned stringOffset, unsigned i, const Font* fontData, GlyphBuffer* glyphBuffer)
 {
     // Determine the string for this item.
     const UChar* str = cp + m_items[i].iCharPos;
@@ -253,6 +253,14 @@ bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned i, const F
     if (!shape(str, len, item, fontData, glyphs, clusters, visualAttributes))
         return true;
 
+    Vector<Optional<unsigned>> stringOffsets(glyphs.size());
+    for (unsigned i = 0; i < len; ++i) {
+        if (stringOffsets[clusters[i]])
+            stringOffsets[clusters[i]] = std::min(*stringOffsets[clusters[i]], i);
+        else
+            stringOffsets[clusters[i]] = i;
+    }
+
     // We now have a collection of glyphs.
     Vector<GOFFSET> offsets;
     Vector<int> advances;
@@ -367,7 +375,7 @@ bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned i, const F
             else
                 glyphBuffer->expandLastAdvance(origin);
             GlyphBufferAdvance glyphAdvance(-origin.width() + advance, -origin.height());
-            glyphBuffer->add(glyph, fontData, glyphAdvance);
+            glyphBuffer->add(glyph, fontData, glyphAdvance, stringOffsets[k].valueOr(0) + stringOffset);
         }
 
         FloatRect glyphBounds = fontData->boundsForGlyph(glyph);

Source/WebCore/platform/graphics/win/UniscribeController.h

@@ -54,8 +54,8 @@ class UniscribeController {
 private:    
     void resetControlAndState();
 
-    void itemizeShapeAndPlace(const UChar*, unsigned length, const Font*, GlyphBuffer*);
-    bool shapeAndPlaceItem(const UChar*, unsigned index, const Font*, GlyphBuffer*);
+    void itemizeShapeAndPlace(const UChar*, unsigned stringOffset, unsigned length, const Font*, GlyphBuffer*);
+    bool shapeAndPlaceItem(const UChar*, unsigned stringOffset, unsigned index, const Font*, GlyphBuffer*);
     bool shape(const UChar* str, int len, SCRIPT_ITEM, const Font*,
                Vector<WORD>& glyphs, Vector<WORD>& clusters,
                Vector<SCRIPT_VISATTR>& visualAttributes);