Bug 191368

Summary: Layout Test fast/text/international/khmer-selection.html is crashing
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: Tools / TestsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, ews-watchlist, lforschler, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Windows 10   
Attachments:
Description Flags
Patch
none
Alternative patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Per Arne Vollan 2018-11-07 08:59:55 PST 
The following layout test is failing on Windows:

fast/text/international/khmer-selection.html

Probable cause:

Unknown.

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ftext%2Finternational%2Fkhmer-selection.html
Comment 1 Per Arne Vollan 2019-02-15 17:25:49 PST 
Created attachment 362192 [details]
Patch
Comment 2 Per Arne Vollan 2019-02-15 17:43:07 PST 
rdar://problem/47922356
Comment 3 Myles C. Maxfield 2019-02-18 17:40:00 PST 
Comment on attachment 362192 [details]
Patch

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

> Source/WebCore/platform/graphics/GlyphBuffer.h:161
> +        if (index >= m_offsetsInString->size())
> +            return GlyphBuffer::noOffset;

It looks like this happens any time complex text is underlined. I wish we could migrate Windows to ComplexTextController. I had a patch at https://bugs.webkit.org/show_bug.cgi?id=167954 but it never got finished.

Can we actually implement this feature instead of simply covering up its symptoms? We could make GlyphBuffer::add() not have any default arguments. Maybe I can do this today.
Comment 4 Myles C. Maxfield 2019-02-18 17:59:49 PST 
Created attachment 362361 [details]
Alternative patch
Comment 5 Myles C. Maxfield 2019-02-18 18:00:42 PST 
How about a patch like this? I don't know if it builds because I don't have a Windows machine, but it seems preferable to fix this problem by implementing the feature instead of failing gracefully.
Comment 6 Per Arne Vollan 2019-02-19 10:09:10 PST 
(In reply to Myles C. Maxfield from comment #5)
> How about a patch like this? I don't know if it builds because I don't have
> a Windows machine, but it seems preferable to fix this problem by
> implementing the feature instead of failing gracefully.

Thanks, looks great, Myles! I am not sure why it is not building, probably just a small adjustment needed.
Comment 7 Per Arne Vollan 2019-02-20 11:33:05 PST 
Created attachment 362519 [details]
Patch
Comment 8 Per Arne Vollan 2019-02-20 11:33:40 PST 
(In reply to Per Arne Vollan from comment #7)
> Created attachment 362519 [details]
> Patch

This is Myles' patch.
Comment 9 Per Arne Vollan 2019-02-20 13:25:48 PST 
Created attachment 362533 [details]
Patch
Comment 10 Per Arne Vollan 2019-02-20 15:09:25 PST 
Created attachment 362552 [details]
Patch
Comment 11 Per Arne Vollan 2019-02-20 15:52:34 PST 
Created attachment 362558 [details]
Patch
Comment 12 Per Arne Vollan 2019-02-21 09:04:55 PST 
Created attachment 362613 [details]
Patch
Comment 13 Brent Fulgham 2019-02-21 14:46:17 PST 
Comment on attachment 362613 [details]
Patch

Very nice! r=me.
Comment 14 Per Arne Vollan 2019-02-21 14:55:38 PST 
(In reply to Brent Fulgham from comment #13)
> Comment on attachment 362613 [details]
> Patch
> 
> Very nice! r=me.

Thanks to Myles for creating the patch, and to Brent for reviewing :)
Comment 15 WebKit Commit Bot 2019-02-21 15:12:42 PST 
Comment on attachment 362613 [details]
Patch

Clearing flags on attachment: 362613

Committed r241915: <https://trac.webkit.org/changeset/241915>
Comment 16 WebKit Commit Bot 2019-02-21 15:12:44 PST 
All reviewed patches have been landed.  Closing bug.