Changeset 205031 in webkit


Ignore:
Timestamp:
Aug 26, 2016 11:12:56 AM (8 years ago)
Author:
Brent Fulgham
Message:

Crash when getting font bounding rect
​https://bugs.webkit.org/show_bug.cgi?id=161202
<rdar://problem/27986981>

Reviewed by Myles C. Maxfield.

We should never store GlyphData objects for later use, because they contain raw pointers to Font elements
contained in caches, and those font caches get periodically purged.

Instead, we should hold onto the ‘key’ representing the GlyphData, and simply ask the system for the
GlyphData the next time it is needed.

Tested by existing MathML tests under ASAN and GuardMalloc.

  • rendering/mathml/RenderMathMLToken.cpp:

(WebCore::RenderMathMLToken::RenderMathMLToken): Clean up constructors.
(WebCore::RenderMathMLToken::computePreferredLogicalWidths): Use keys to get correct GlyphData when needed.
(WebCore::RenderMathMLToken::updateMathVariantGlyph): Ditto.
(WebCore::RenderMathMLToken::firstLineBaseline): Ditto.
(WebCore::RenderMathMLToken::layoutBlock): Ditto.
(WebCore::RenderMathMLToken::paint): Ditto.
(WebCore::RenderMathMLToken::paintChildren): Ditto.

  • rendering/mathml/RenderMathMLToken.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r205030 r205031  
     12016-08-25  Brent Fulgham  <bfulgham@apple.com>
     2
     3        Crash when getting font bounding rect
     4        https://bugs.webkit.org/show_bug.cgi?id=161202
     5        <rdar://problem/27986981>
     6
     7        Reviewed by Myles C. Maxfield.
     8
     9        We should never store GlyphData objects for later use, because they contain raw pointers to Font elements
     10        contained in caches, and those font caches get periodically purged.
     11
     12        Instead, we should hold onto the ‘key’ representing the GlyphData, and simply ask the system for the
     13        GlyphData the next time it is needed.
     14
     15        Tested by existing MathML tests under ASAN and GuardMalloc.
     16
     17        * rendering/mathml/RenderMathMLToken.cpp:
     18        (WebCore::RenderMathMLToken::RenderMathMLToken): Clean up constructors.
     19        (WebCore::RenderMathMLToken::computePreferredLogicalWidths): Use keys to get correct GlyphData when needed.
     20        (WebCore::RenderMathMLToken::updateMathVariantGlyph): Ditto.
     21        (WebCore::RenderMathMLToken::firstLineBaseline): Ditto.
     22        (WebCore::RenderMathMLToken::layoutBlock): Ditto.
     23        (WebCore::RenderMathMLToken::paint): Ditto.
     24        (WebCore::RenderMathMLToken::paintChildren): Ditto.
     25        * rendering/mathml/RenderMathMLToken.h:
     26
    1272016-08-26  Chris Dumez  <cdumez@apple.com>
    228

  • trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp

    r205011 r205031  
    22 * Copyright (C) 2014 Frédéric Wang (fred.wang@free.fr). All rights reserved.
    33 * Copyright (C) 2016 Igalia S.L.
     4 * Copyright (C) 2016 Apple Inc.  All rights reserved.
    45 *
    56 * Redistribution and use in source and binary forms, with or without
    … …  
    4344RenderMathMLToken::RenderMathMLToken(Element& element, RenderStyle&& style)
    4445    : RenderMathMLBlock(element, WTFMove(style))
    45     , m_mathVariantGlyph()
    46     , m_mathVariantGlyphDirty(false)
    4746{
    4847}
    … …  
    5049RenderMathMLToken::RenderMathMLToken(Document& document, RenderStyle&& style)
    5150    : RenderMathMLBlock(document, WTFMove(style))
    52     , m_mathVariantGlyph()
    53     , m_mathVariantGlyphDirty(false)
    5451{
    5552}
    … …  
    504501        updateMathVariantGlyph();
    505502
    506     if (m_mathVariantGlyph.font) {
    507         m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph);
    508         setPreferredLogicalWidthsDirty(false);
    509         return;
     503    if (m_mathVariantCodePoint) {
     504        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     505        if (mathVariantGlyph.font) {
     506            m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = mathVariantGlyph.font->widthForGlyph(mathVariantGlyph.glyph);
     507            setPreferredLogicalWidthsDirty(false);
     508            return;
     509        }
    510510    }
    511511
    … …  
    517517    ASSERT(m_mathVariantGlyphDirty);
    518518
    519     m_mathVariantGlyph = GlyphData();
     519    m_mathVariantCodePoint = Nullopt;
    520520    m_mathVariantGlyphDirty = false;
    521521
    … …  
    533533            mathvariant = tokenElement.hasTagName(MathMLNames::miTag) ? MathMLElement::MathVariant::Italic : MathMLElement::MathVariant::Normal;
    534534        UChar32 transformedCodePoint = mathVariant(codePoint.value(), mathvariant);
    535         if (transformedCodePoint != codePoint.value())
    536             m_mathVariantGlyph = style().fontCascade().glyphDataForCharacter(transformedCodePoint, !style().isLeftToRightDirection());
     535        if (transformedCodePoint != codePoint.value()) {
     536            m_mathVariantCodePoint = mathVariant(codePoint.value(), mathvariant);
     537            m_mathVariantIsMirrored = !style().isLeftToRightDirection();
     538        }
    537539    }
    538540}
    … …  
    552554Optional<int> RenderMathMLToken::firstLineBaseline() const
    553555{
    554     if (m_mathVariantGlyph.font)
    555         return Optional<int>(static_cast<int>(lroundf(-m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).y())));
     556    if (m_mathVariantCodePoint) {
     557        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     558        if (mathVariantGlyph.font)
     559            return Optional<int>(static_cast<int>(lroundf(-mathVariantGlyph.font->boundsForGlyph(mathVariantGlyph.glyph).y())));
     560    }
    556561    return RenderMathMLBlock::firstLineBaseline();
    557562}
    … …  
    564569        return;
    565570
    566     if (!m_mathVariantGlyph.font) {
     571    GlyphData mathVariantGlyph;
     572    if (m_mathVariantCodePoint)
     573        mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     574
     575    if (!mathVariantGlyph.font) {
    567576        RenderMathMLBlock::layoutBlock(relayoutChildren, pageLogicalHeight);
    568577        return;
    … …  
    572581        child->layoutIfNeeded();
    573582
    574     setLogicalWidth(m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph));
    575     setLogicalHeight(m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).height());
     583    setLogicalWidth(mathVariantGlyph.font->widthForGlyph(mathVariantGlyph.glyph));
     584    setLogicalHeight(mathVariantGlyph.font->boundsForGlyph(mathVariantGlyph.glyph).height());
    576585
    577586    clearNeedsLayout();
    … …  
    583592
    584593    // 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).
    585     if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantGlyph.font)
     594    if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantCodePoint)
     595        return;
     596
     597    auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     598    if (!mathVariantGlyph.font)
    586599        return;
    587600
    … …  
    590603
    591604    GlyphBuffer buffer;
    592     buffer.add(m_mathVariantGlyph.glyph, m_mathVariantGlyph.font, m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph));
    593     LayoutUnit glyphAscent = static_cast<int>(lroundf(-m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).y()));
    594     info.context().drawGlyphs(style().fontCascade(), *m_mathVariantGlyph.font, buffer, 0, 1, paintOffset + location() + LayoutPoint(0, glyphAscent));
     605    buffer.add(mathVariantGlyph.glyph, mathVariantGlyph.font, mathVariantGlyph.font->widthForGlyph(mathVariantGlyph.glyph));
     606    LayoutUnit glyphAscent = static_cast<int>(lroundf(-mathVariantGlyph.font->boundsForGlyph(mathVariantGlyph.glyph).y()));
     607    info.context().drawGlyphs(style().fontCascade(), *mathVariantGlyph.font, buffer, 0, 1, paintOffset + location() + LayoutPoint(0, glyphAscent));
    595608}
    596609
    597610void RenderMathMLToken::paintChildren(PaintInfo& paintInfo, const LayoutPoint& paintOffset, PaintInfo& paintInfoForChild, bool usePrintRect)
    598611{
    599     if (m_mathVariantGlyph.font)
    600         return;
     612    if (m_mathVariantCodePoint) {
     613        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     614        if (mathVariantGlyph.font)
     615            return;
     616    }
     617
    601618    RenderMathMLBlock::paintChildren(paintInfo, paintOffset, paintInfoForChild, usePrintRect);
    602619}

  • trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.h

    r204715 r205031  
    22 * Copyright (C) 2014 Frédéric Wang (fred.wang@free.fr). All rights reserved.
    33 * Copyright (C) 2016 Igalia S.L.
     4 * Copyright (C) 2016 Apple Inc.  All rights reserved.
    45 *
    56 * Redistribution and use in source and binary forms, with or without
    … …  
    6364        setNeedsLayoutAndPrefWidthsRecalc();
    6465    }
    65     GlyphData m_mathVariantGlyph;
    66     bool m_mathVariantGlyphDirty;
     66    Optional<UChar32> m_mathVariantCodePoint { Nullopt };
     67    bool m_mathVariantIsMirrored { false };
     68    bool m_mathVariantGlyphDirty { false };
    6769};
    6870
Note: See TracChangeset for help on using the changeset viewer.