Bug 140653

Summary: [Win] Layout Test mathml/opentype/munderover-layout-resize.html crashes
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: MathMLAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, dbarton, esprehn+autocc, fred.wang, glenn, kondapallykalyan, mmaxfield, mrobinson, sabouhallawa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 159386    
Bug Blocks:    
Attachments:
Description Flags
Patch mmaxfield: review-

Description Brent Fulgham 2015-01-19 17:59:04 PST
The following layout test is flaky on Windows:

mathml/presentation/mo-invisible.html

Probable cause:

The test is crashing in RenderMathMLOperator::advanceForGlyph, where the GlyphData contains a nullptr fontData member.

This seems to be happening for UNICODE character 8290, which corresponds to an invisible multiplication symbol.
Comment 1 Brent Fulgham 2015-01-19 18:07:34 PST
The GlyphData's fontData member can be nullptr in the case where FontGlyphs::glyphDataForSystemFallback cannot find a suitable font for the desired character.

When this happens, we return a default-constructed GlyphData which has a null internal fontData member.

This member is dereference later, causing a crash.
Comment 2 Brent Fulgham 2015-01-19 18:11:43 PST
Actually, the failing test is "mathml/opentype/munderover-layout-resize.html"
Comment 3 Brent Fulgham 2015-01-19 18:13:01 PST
Created attachment 244948 [details]
Patch
Comment 4 Brent Fulgham 2015-01-19 18:13:34 PST
I wonder if this only happens if certain fonts are missing from the test system? DRT should be insuring the right fonts are in place.
Comment 5 Myles C. Maxfield 2015-01-19 18:23:20 PST
Comment on attachment 244948 [details]
Patch

You haven't described why this flakes. It seems to me that consistently crashing and being flakey are substantially different phenomena.
Comment 6 Myles C. Maxfield 2015-01-19 18:23:44 PST
I think we need to do a little more research to figure out what is going on here.
Comment 7 Brent Fulgham 2015-01-19 20:40:51 PST
(In reply to comment #5)
> Comment on attachment 244948 [details]
> Patch
> 
> You haven't described why this flakes. It seems to me that consistently
> crashing and being flakey are substantially different phenomena.

Yes -- this is a repeatable crash, due to a nullptr dereference as I mentioned elsewhere in this bug.

What I'm not sure about is why this particular test can't seem to come up with a valid set of font data. Without any other information, I'm guessing it has to do with the use of the "stretchy.woff" font include. Probably something in the Windows port is not properly implemented.

It looks like the style of the MathML element is empty, and for whatever reason the attempt to access UNICODE character 8290 does not find anything suitable in the fallback logic resulting in null font data.

Once that happens, we crash as soon as we attempt to dereference the font data.
Comment 8 Frédéric Wang (:fredw) 2016-03-14 06:57:08 PDT
@Brent: is it different from bug 140522 (comment 0 mentions 
mathml/presentation/mo-invisible.html and title mathml/opentype/munderover-layout-resize.html)?

Apparently, you're proposed fix already landed: http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp#L268

I suspect stretchy.woff does not have any character for U+8290 so the glyph of a system font (e.g. Cambria Math) should be used when calling style().fontCascade().glyphDataForCharacter(m_textContent, false). Not sure why it does not work on Windows.

BTW, I'd like to mention that the code will change in the current MathML refactoring (bug 152244 and bug 155018) but they are still calling isValid() before using glyph data.
Comment 9 Frédéric Wang (:fredw) 2016-07-03 09:31:25 PDT
http://trac.webkit.org/changeset/202788