Bug 140653 - [Win] Layout Test mathml/opentype/munderover-layout-resize.html crashes
Summary: [Win] Layout Test mathml/opentype/munderover-layout-resize.html crashes
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Brent Fulgham
Depends on: 159386
  Show dependency treegraph
Reported: 2015-01-19 17:59 PST by Brent Fulgham
Modified: 2016-07-03 13:05 PDT (History)
10 users (show)

See Also:

Patch (1.74 KB, patch)
2015-01-19 18:13 PST, Brent Fulgham
mmaxfield: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-01-19 17:59:04 PST
The following layout test is flaky on Windows:


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]
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]

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