Bug 159783 - Check whether font is nonnull for GlyphData instead of calling GlyphData::isValid()
Summary: Check whether font is nonnull for GlyphData instead of calling GlyphData::isV...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on:
Blocks: 159702
  Show dependency treegraph
 
Reported: 2016-07-14 14:33 PDT by Frédéric Wang (:fredw)
Modified: 2016-07-15 10:24 PDT (History)
8 users (show)

See Also:


Attachments
Patch (10.99 KB, patch)
2016-07-14 23:51 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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...