Changeset 207271 in webkit


Ignore:
Timestamp:
Oct 12, 2016 7:28:46 PM (8 years ago)
Author:
matthew_hanson@apple.com
Message:

Merge r205031. rdar://problem/28216249

2016-08-25 Brent Fulgham <Brent Fulgham>

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:

Patch by Brent Fulgham <Brent Fulgham> on 2016-08-25

Location:
branches/safari-602-branch/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-602-branch/Source/WebCore/ChangeLog

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

  • branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp

    r203280 r207271  
    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
    … …  
    4142RenderMathMLToken::RenderMathMLToken(Element& element, RenderStyle&& style)
    4243    : RenderMathMLBlock(element, WTFMove(style))
    43     , m_mathVariantGlyph()
    44     , m_mathVariantGlyphDirty(false)
    4544{
    4645}
    … …  
    4847RenderMathMLToken::RenderMathMLToken(Document& document, RenderStyle&& style)
    4948    : RenderMathMLBlock(document, WTFMove(style))
    50     , m_mathVariantGlyph()
    51     , m_mathVariantGlyphDirty(false)
    5249{
    5350}
    … …  
    497494        updateMathVariantGlyph();
    498495
    499     if (m_mathVariantGlyph.font) {
    500         m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph);
    501         setPreferredLogicalWidthsDirty(false);
    502         return;
     496    if (m_mathVariantCodePoint) {
     497        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     498        if (mathVariantGlyph.font) {
     499            m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = mathVariantGlyph.font->widthForGlyph(mathVariantGlyph.glyph);
     500            setPreferredLogicalWidthsDirty(false);
     501            return;
     502        }
    503503    }
    504504
    … …  
    510510    ASSERT(m_mathVariantGlyphDirty);
    511511
    512     m_mathVariantGlyph = GlyphData();
     512    m_mathVariantCodePoint = Nullopt;
    513513    m_mathVariantGlyphDirty = false;
    514514
    … …  
    528528            mathvariant = tokenElement.hasTagName(MathMLNames::miTag) ? MathMLStyle::Italic : MathMLStyle::Normal;
    529529        UChar32 transformedCodePoint = mathVariant(codePoint, mathvariant);
    530         if (transformedCodePoint != codePoint)
    531             m_mathVariantGlyph = style().fontCascade().glyphDataForCharacter(transformedCodePoint, !style().isLeftToRightDirection());
     530        if (transformedCodePoint != codePoint) {
     531            m_mathVariantCodePoint = mathVariant(codePoint, mathvariant);
     532            m_mathVariantIsMirrored = !style().isLeftToRightDirection();
     533        }
    532534    }
    533535}
    … …  
    547549Optional<int> RenderMathMLToken::firstLineBaseline() const
    548550{
    549     if (m_mathVariantGlyph.font)
    550         return Optional<int>(static_cast<int>(lroundf(-m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).y())));
     551    if (m_mathVariantCodePoint) {
     552        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     553        if (mathVariantGlyph.font)
     554            return Optional<int>(static_cast<int>(lroundf(-mathVariantGlyph.font->boundsForGlyph(mathVariantGlyph.glyph).y())));
     555    }
    551556    return RenderMathMLBlock::firstLineBaseline();
    552557}
    … …  
    559564        return;
    560565
    561     if (!m_mathVariantGlyph.font) {
     566    GlyphData mathVariantGlyph;
     567    if (m_mathVariantCodePoint)
     568        mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     569
     570    if (!mathVariantGlyph.font) {
    562571        RenderMathMLBlock::layoutBlock(relayoutChildren, pageLogicalHeight);
    563572        return;
    … …  
    567576        child->layoutIfNeeded();
    568577
    569     setLogicalWidth(m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph));
    570     setLogicalHeight(m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).height());
     578    setLogicalWidth(mathVariantGlyph.font->widthForGlyph(mathVariantGlyph.glyph));
     579    setLogicalHeight(mathVariantGlyph.font->boundsForGlyph(mathVariantGlyph.glyph).height());
    571580
    572581    clearNeedsLayout();
    … …  
    578587
    579588    // 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.font)
     589    if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantCodePoint)
     590        return;
     591
     592    auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     593    if (!mathVariantGlyph.font)
    581594        return;
    582595
    … …  
    585598
    586599    GlyphBuffer buffer;
    587     buffer.add(m_mathVariantGlyph.glyph, m_mathVariantGlyph.font, m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph));
    588     LayoutUnit glyphAscent = static_cast<int>(lroundf(-m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).y()));
    589     info.context().drawGlyphs(style().fontCascade(), *m_mathVariantGlyph.font, buffer, 0, 1, paintOffset + location() + LayoutPoint(0, glyphAscent));
     600    buffer.add(mathVariantGlyph.glyph, mathVariantGlyph.font, mathVariantGlyph.font->widthForGlyph(mathVariantGlyph.glyph));
     601    LayoutUnit glyphAscent = static_cast<int>(lroundf(-mathVariantGlyph.font->boundsForGlyph(mathVariantGlyph.glyph).y()));
     602    info.context().drawGlyphs(style().fontCascade(), *mathVariantGlyph.font, buffer, 0, 1, paintOffset + location() + LayoutPoint(0, glyphAscent));
    590603}
    591604
    592605void RenderMathMLToken::paintChildren(PaintInfo& paintInfo, const LayoutPoint& paintOffset, PaintInfo& paintInfoForChild, bool usePrintRect)
    593606{
    594     if (m_mathVariantGlyph.font)
    595         return;
     607    if (m_mathVariantCodePoint) {
     608        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
     609        if (mathVariantGlyph.font)
     610            return;
     611    }
     612
    596613    RenderMathMLBlock::paintChildren(paintInfo, paintOffset, paintInfoForChild, usePrintRect);
    597614}

  • branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.h

    r203228 r207271  
    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
    … …  
    6263        setNeedsLayoutAndPrefWidthsRecalc();
    6364    }
    64     GlyphData m_mathVariantGlyph;
    65     bool m_mathVariantGlyphDirty;
     65    Optional<UChar32> m_mathVariantCodePoint { Nullopt };
     66    bool m_mathVariantIsMirrored { false };
     67    bool m_mathVariantGlyphDirty { false };
    6668};
    6769
Note: See TracChangeset for help on using the changeset viewer.