Changeset 203280 in webkit


Ignore:
Timestamp:
Jul 15, 2016 10:03:45 AM (8 years ago)
Author:
commit-queue@webkit.org
Message:

Check whether font is nonnull for GlyphData instead of calling GlyphData::isValid()
https://bugs.webkit.org/show_bug.cgi?id=159783

Patch by Frederic Wang <fwang@igalia.com> on 2016-07-15
Reviewed by Brent Fulgham.

GlyphData::isValid() returns true for GlyphData with null 'font' pointer when the 'glyph'
index is nonzero. This behavior is not expected by the MathML code and we have had crashes
in our test suite in the past on Windows (e.g. bug 140653). We thus replace the call to
GlyphData::isValid() with a stronger verification: Whether the 'font' pointer is nonzero.

No new tests, this only makes null pointer checks stronger.

  • rendering/mathml/MathOperator.cpp:

(WebCore::boundsForGlyph):
(WebCore::advanceWidthForGlyph):
(WebCore::MathOperator::getBaseGlyph):
(WebCore::MathOperator::setSizeVariant):
(WebCore::MathOperator::fillWithVerticalExtensionGlyph):
(WebCore::MathOperator::fillWithHorizontalExtensionGlyph):
(WebCore::MathOperator::paintVerticalGlyphAssembly):
(WebCore::MathOperator::paintHorizontalGlyphAssembly):
(WebCore::MathOperator::paint):

  • rendering/mathml/RenderMathMLOperator.cpp:

(WebCore::RenderMathMLOperator::computePreferredLogicalWidths):

  • rendering/mathml/RenderMathMLToken.cpp:

(WebCore::RenderMathMLToken::computePreferredLogicalWidths):
(WebCore::RenderMathMLToken::firstLineBaseline):
(WebCore::RenderMathMLToken::layoutBlock):
(WebCore::RenderMathMLToken::paint):
(WebCore::RenderMathMLToken::paintChildren):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r203279 r203280  
     12016-07-15  Frederic Wang  <fwang@igalia.com>
     2
     3        Check whether font is nonnull for GlyphData instead of calling GlyphData::isValid()
     4        https://bugs.webkit.org/show_bug.cgi?id=159783
     5
     6        Reviewed by Brent Fulgham.
     7
     8        GlyphData::isValid() returns true for GlyphData with null 'font' pointer when the 'glyph'
     9        index is nonzero. This behavior is not expected by the MathML code and we have had crashes
     10        in our test suite in the past on Windows (e.g. bug 140653). We thus replace the call to
     11        GlyphData::isValid() with a stronger verification: Whether the 'font' pointer is nonzero.
     12
     13        No new tests, this only makes null pointer checks stronger.
     14
     15        * rendering/mathml/MathOperator.cpp:
     16        (WebCore::boundsForGlyph):
     17        (WebCore::advanceWidthForGlyph):
     18        (WebCore::MathOperator::getBaseGlyph):
     19        (WebCore::MathOperator::setSizeVariant):
     20        (WebCore::MathOperator::fillWithVerticalExtensionGlyph):
     21        (WebCore::MathOperator::fillWithHorizontalExtensionGlyph):
     22        (WebCore::MathOperator::paintVerticalGlyphAssembly):
     23        (WebCore::MathOperator::paintHorizontalGlyphAssembly):
     24        (WebCore::MathOperator::paint):
     25        * rendering/mathml/RenderMathMLOperator.cpp:
     26        (WebCore::RenderMathMLOperator::computePreferredLogicalWidths):
     27        * rendering/mathml/RenderMathMLToken.cpp:
     28        (WebCore::RenderMathMLToken::computePreferredLogicalWidths):
     29        (WebCore::RenderMathMLToken::firstLineBaseline):
     30        (WebCore::RenderMathMLToken::layoutBlock):
     31        (WebCore::RenderMathMLToken::paint):
     32        (WebCore::RenderMathMLToken::paintChildren):
     33
    1342016-07-15  Frederic Wang  <fwang@igalia.com>
    235
  • trunk/Source/WebCore/rendering/mathml/MathOperator.cpp

    r202489 r203280  
    3939static inline FloatRect boundsForGlyph(const GlyphData& data)
    4040{
    41     return data.isValid() ? data.font->boundsForGlyph(data.glyph) : FloatRect();
     41    return data.font ? data.font->boundsForGlyph(data.glyph) : FloatRect();
    4242}
    4343
     
    5656static inline float advanceWidthForGlyph(const GlyphData& data)
    5757{
    58     return data.isValid() ? data.font->widthForGlyph(data.glyph) : 0;
     58    return data.font ? data.font->widthForGlyph(data.glyph) : 0;
    5959}
    6060
     
    125125{
    126126    baseGlyph = style.fontCascade().glyphDataForCharacter(m_baseCharacter, !style.isLeftToRightDirection());
    127     return baseGlyph.isValid() && baseGlyph.font == &style.fontCascade().primaryFont();
     127    return baseGlyph.font && baseGlyph.font == &style.fontCascade().primaryFont();
    128128}
    129129
    130130void MathOperator::setSizeVariant(const GlyphData& sizeVariant)
    131131{
    132     ASSERT(sizeVariant.isValid());
     132    ASSERT(sizeVariant.font);
    133133    ASSERT(sizeVariant.font->mathData());
    134134    m_stretchType = StretchType::SizeVariant;
     
    469469    ASSERT(m_operatorType == Type::VerticalOperator);
    470470    ASSERT(m_stretchType == StretchType::GlyphAssembly);
    471     ASSERT(m_assembly.extension.isValid());
     471    ASSERT(m_assembly.extension.font);
    472472    ASSERT(from.y() <= to.y());
    473473
     
    508508    ASSERT(m_operatorType == Type::HorizontalOperator);
    509509    ASSERT(m_stretchType == StretchType::GlyphAssembly);
    510     ASSERT(m_assembly.extension.isValid());
     510    ASSERT(m_assembly.extension.font);
    511511    ASSERT(from.x() <= to.x());
    512512    ASSERT(from.y() == to.y());
     
    546546    ASSERT(m_operatorType == Type::VerticalOperator);
    547547    ASSERT(m_stretchType == StretchType::GlyphAssembly);
    548     ASSERT(m_assembly.topOrRight.isValid());
    549     ASSERT(m_assembly.bottomOrLeft.isValid());
     548    ASSERT(m_assembly.topOrRight.font);
     549    ASSERT(m_assembly.bottomOrLeft.font);
    550550
    551551    // We are positioning the glyphs so that the edge of the tight glyph bounds line up exactly with the edges of our paint box.
     
    559559    LayoutRect bottomGlyphPaintRect = paintGlyph(style, info, m_assembly.bottomOrLeft, bottomGlyphOrigin, TrimTop);
    560560
    561     if (m_assembly.middle.isValid()) {
     561    if (m_assembly.middle.font) {
    562562        // Center the glyph origin between the start and end glyph paint extents. Then shift it half the paint height toward the bottom glyph.
    563563        FloatRect middleGlyphBounds = boundsForGlyph(m_assembly.middle);
     
    577577    ASSERT(m_operatorType == Type::HorizontalOperator);
    578578    ASSERT(m_stretchType == StretchType::GlyphAssembly);
    579     ASSERT(m_assembly.bottomOrLeft.isValid());
    580     ASSERT(m_assembly.topOrRight.isValid());
     579    ASSERT(m_assembly.bottomOrLeft.font);
     580    ASSERT(m_assembly.topOrRight.font);
    581581
    582582    // We are positioning the glyphs so that the edge of the tight glyph bounds line up exactly with the edges of our paint box.
     
    590590    LayoutRect rightGlyphPaintRect = paintGlyph(style, info, m_assembly.topOrRight, rightGlyphOrigin, TrimLeft);
    591591
    592     if (m_assembly.middle.isValid()) {
     592    if (m_assembly.middle.font) {
    593593        // Center the glyph origin between the start and end glyph paint extents.
    594594        LayoutPoint middleGlyphOrigin(operatorTopLeft.x(), baselineY);
     
    633633            return;
    634634    } else if (m_stretchType == StretchType::SizeVariant) {
    635         ASSERT(m_variant.isValid());
     635        ASSERT(m_variant.font);
    636636        glyphData = m_variant;
    637637    }
  • trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp

    r203072 r203280  
    247247            // In some fonts, glyphs for invisible operators have nonzero width. Consequently, we subtract that width here to avoid wide gaps.
    248248            GlyphData data = style().fontCascade().glyphDataForCharacter(m_textContent, false);
    249             float glyphWidth = data.isValid() ? data.font->widthForGlyph(data.glyph) : 0;
     249            float glyphWidth = data.font ? data.font->widthForGlyph(data.glyph) : 0;
    250250            ASSERT(glyphWidth <= preferredWidth);
    251251            preferredWidth -= glyphWidth;
  • trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp

    r203072 r203280  
    497497        updateMathVariantGlyph();
    498498
    499     if (m_mathVariantGlyph.isValid()) {
     499    if (m_mathVariantGlyph.font) {
    500500        m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph);
    501501        setPreferredLogicalWidthsDirty(false);
     
    547547Optional<int> RenderMathMLToken::firstLineBaseline() const
    548548{
    549     if (m_mathVariantGlyph.isValid())
     549    if (m_mathVariantGlyph.font)
    550550        return Optional<int>(static_cast<int>(lroundf(-m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).y())));
    551551    return RenderMathMLBlock::firstLineBaseline();
     
    559559        return;
    560560
    561     if (!m_mathVariantGlyph.isValid()) {
     561    if (!m_mathVariantGlyph.font) {
    562562        RenderMathMLBlock::layoutBlock(relayoutChildren, pageLogicalHeight);
    563563        return;
     
    578578
    579579    // FIXME: Instead of using DrawGlyph, we may consider using the more general TextPainter so that we can apply mathvariant to strings with an arbitrary number of characters and preserve advanced CSS effects (text-shadow, etc).
    580     if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantGlyph.isValid())
     580    if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantGlyph.font)
    581581        return;
    582582
     
    592592void RenderMathMLToken::paintChildren(PaintInfo& paintInfo, const LayoutPoint& paintOffset, PaintInfo& paintInfoForChild, bool usePrintRect)
    593593{
    594     if (m_mathVariantGlyph.isValid())
     594    if (m_mathVariantGlyph.font)
    595595        return;
    596596    RenderMathMLBlock::paintChildren(paintInfo, paintOffset, paintInfoForChild, usePrintRect);
Note: See TracChangeset for help on using the changeset viewer.