Bug 159783

Summary: Check whether font is nonnull for GlyphData instead of calling GlyphData::isValid()
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, dbarton, esprehn+autocc, glenn, kondapallykalyan, pvollan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 159702    
Attachments:
Description Flags
Patch none

Description Frédéric Wang (:fredw) 2016-07-14 14:33:16 PDT
See bug 155018 comment 36. Also, probably the reason for bug 159702.

fred@debian:~/webkit/WebKit/Source/WebCore/rendering/mathml$ grep '\.isValid' *
MathOperator.cpp:    return data.isValid() ? data.font->boundsForGlyph(data.glyph) : FloatRect();
MathOperator.cpp:    return data.isValid() ? data.font->widthForGlyph(data.glyph) : 0;
MathOperator.cpp:    return baseGlyph.isValid() && baseGlyph.font == &style.fontCascade().primaryFont();
MathOperator.cpp:    ASSERT(sizeVariant.isValid());
MathOperator.cpp:    ASSERT(m_assembly.extension.isValid());
MathOperator.cpp:    ASSERT(m_assembly.extension.isValid());
MathOperator.cpp:    ASSERT(m_assembly.topOrRight.isValid());
MathOperator.cpp:    ASSERT(m_assembly.bottomOrLeft.isValid());
MathOperator.cpp:    if (m_assembly.middle.isValid()) {
MathOperator.cpp:    ASSERT(m_assembly.bottomOrLeft.isValid());
MathOperator.cpp:    ASSERT(m_assembly.topOrRight.isValid());
MathOperator.cpp:    if (m_assembly.middle.isValid()) {
MathOperator.cpp:        ASSERT(m_variant.isValid());
RenderMathMLOperator.cpp:            float glyphWidth = data.isValid() ? data.font->widthForGlyph(data.glyph) : 0;
RenderMathMLToken.cpp:    if (m_mathVariantGlyph.isValid()) {
RenderMathMLToken.cpp:    if (m_mathVariantGlyph.isValid())
RenderMathMLToken.cpp:    if (!m_mathVariantGlyph.isValid()) {
RenderMathMLToken.cpp:    if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantGlyph.isValid())
RenderMathMLToken.cpp:    if (m_mathVariantGlyph.isValid())
Comment 1 Frédéric Wang (:fredw) 2016-07-14 23:51:51 PDT
Created attachment 283740 [details]
Patch
Comment 2 Brent Fulgham 2016-07-15 09:36:18 PDT
Comment on attachment 283740 [details]
Patch

I wonder if there is other code that really means "font is non-null and also isValid"? r=me
Comment 3 WebKit Commit Bot 2016-07-15 10:04:34 PDT
Comment on attachment 283740 [details]
Patch

Clearing flags on attachment: 283740

Committed r203280: <http://trac.webkit.org/changeset/203280>
Comment 4 WebKit Commit Bot 2016-07-15 10:04:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Frédéric Wang (:fredw) 2016-07-15 10:24:33 PDT
(In reply to comment #2)
> Comment on attachment 283740 [details]
> Patch
> 
> I wonder if there is other code that really means "font is non-null and also
> isValid"? r=me

Oh, I just saw this comment. As defined in GlyphData, isValid means "font || glyph" which sounds weird for me. So just checking 'font' is stronger...