<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[207271] branches/safari-602-branch/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/207271">207271</a></dd>
<dt>Author</dt> <dd>matthew_hanson@apple.com</dd>
<dt>Date</dt> <dd>2016-10-12 19:28:46 -0700 (Wed, 12 Oct 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/205031">r205031</a>. rdar://problem/28216249

    2016-08-25  Brent Fulgham  &lt;bfulgham@apple.com&gt;

    Crash when getting font bounding rect
    https://bugs.webkit.org/show_bug.cgi?id=161202
    &lt;rdar://problem/27986981&gt;

    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 &lt;bfulgham@apple.com&gt; on 2016-08-25</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#branchessafari602branchSourceWebCoreChangeLog">branches/safari-602-branch/Source/WebCore/ChangeLog</a></li>
<li><a href="#branchessafari602branchSourceWebCorerenderingmathmlRenderMathMLTokencpp">branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp</a></li>
<li><a href="#branchessafari602branchSourceWebCorerenderingmathmlRenderMathMLTokenh">branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="branchessafari602branchSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: branches/safari-602-branch/Source/WebCore/ChangeLog (207270 => 207271)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-602-branch/Source/WebCore/ChangeLog        2016-10-13 02:28:42 UTC (rev 207270)
+++ branches/safari-602-branch/Source/WebCore/ChangeLog        2016-10-13 02:28:46 UTC (rev 207271)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2016-08-25  Brent Fulgham  &lt;bfulgham@apple.com&gt;
+
+        Merge r205031. rdar://problem/28216249
+
+    2016-08-25  Brent Fulgham  &lt;bfulgham@apple.com&gt;
+
+            Crash when getting font bounding rect
+            https://bugs.webkit.org/show_bug.cgi?id=161202
+            &lt;rdar://problem/27986981&gt;
+
+            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:
+
</ins><span class="cx"> 2016-10-12  Matthew Hanson  &lt;matthew_hanson@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Merge r206975. rdar://problem/28545012
</span></span></pre></div>
<a id="branchessafari602branchSourceWebCorerenderingmathmlRenderMathMLTokencpp"></a>
<div class="modfile"><h4>Modified: branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp (207270 => 207271)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp        2016-10-13 02:28:42 UTC (rev 207270)
+++ branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp        2016-10-13 02:28:46 UTC (rev 207271)
</span><span class="lines">@@ -1,6 +1,7 @@
</span><span class="cx"> /*
</span><span class="cx">  * Copyright (C) 2014 Frédéric Wang (fred.wang@free.fr). All rights reserved.
</span><span class="cx">  * Copyright (C) 2016 Igalia S.L.
</span><ins>+ * Copyright (C) 2016 Apple Inc.  All rights reserved.
</ins><span class="cx">  *
</span><span class="cx">  * Redistribution and use in source and binary forms, with or without
</span><span class="cx">  * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -40,15 +41,11 @@
</span><span class="cx"> 
</span><span class="cx"> RenderMathMLToken::RenderMathMLToken(Element&amp; element, RenderStyle&amp;&amp; style)
</span><span class="cx">     : RenderMathMLBlock(element, WTFMove(style))
</span><del>-    , m_mathVariantGlyph()
-    , m_mathVariantGlyphDirty(false)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> RenderMathMLToken::RenderMathMLToken(Document&amp; document, RenderStyle&amp;&amp; style)
</span><span class="cx">     : RenderMathMLBlock(document, WTFMove(style))
</span><del>-    , m_mathVariantGlyph()
-    , m_mathVariantGlyphDirty(false)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -496,10 +493,13 @@
</span><span class="cx">     if (m_mathVariantGlyphDirty)
</span><span class="cx">         updateMathVariantGlyph();
</span><span class="cx"> 
</span><del>-    if (m_mathVariantGlyph.font) {
-        m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = m_mathVariantGlyph.font-&gt;widthForGlyph(m_mathVariantGlyph.glyph);
-        setPreferredLogicalWidthsDirty(false);
-        return;
</del><ins>+    if (m_mathVariantCodePoint) {
+        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
+        if (mathVariantGlyph.font) {
+            m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = mathVariantGlyph.font-&gt;widthForGlyph(mathVariantGlyph.glyph);
+            setPreferredLogicalWidthsDirty(false);
+            return;
+        }
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     RenderMathMLBlock::computePreferredLogicalWidths();
</span><span class="lines">@@ -509,7 +509,7 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(m_mathVariantGlyphDirty);
</span><span class="cx"> 
</span><del>-    m_mathVariantGlyph = GlyphData();
</del><ins>+    m_mathVariantCodePoint = Nullopt;
</ins><span class="cx">     m_mathVariantGlyphDirty = false;
</span><span class="cx"> 
</span><span class="cx">     // Early return if the token element contains RenderElements.
</span><span class="lines">@@ -527,8 +527,10 @@
</span><span class="cx">         if (mathvariant == MathMLStyle::None)
</span><span class="cx">             mathvariant = tokenElement.hasTagName(MathMLNames::miTag) ? MathMLStyle::Italic : MathMLStyle::Normal;
</span><span class="cx">         UChar32 transformedCodePoint = mathVariant(codePoint, mathvariant);
</span><del>-        if (transformedCodePoint != codePoint)
-            m_mathVariantGlyph = style().fontCascade().glyphDataForCharacter(transformedCodePoint, !style().isLeftToRightDirection());
</del><ins>+        if (transformedCodePoint != codePoint) {
+            m_mathVariantCodePoint = mathVariant(codePoint, mathvariant);
+            m_mathVariantIsMirrored = !style().isLeftToRightDirection();
+        }
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -546,8 +548,11 @@
</span><span class="cx"> 
</span><span class="cx"> Optional&lt;int&gt; RenderMathMLToken::firstLineBaseline() const
</span><span class="cx"> {
</span><del>-    if (m_mathVariantGlyph.font)
-        return Optional&lt;int&gt;(static_cast&lt;int&gt;(lroundf(-m_mathVariantGlyph.font-&gt;boundsForGlyph(m_mathVariantGlyph.glyph).y())));
</del><ins>+    if (m_mathVariantCodePoint) {
+        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
+        if (mathVariantGlyph.font)
+            return Optional&lt;int&gt;(static_cast&lt;int&gt;(lroundf(-mathVariantGlyph.font-&gt;boundsForGlyph(mathVariantGlyph.glyph).y())));
+    }
</ins><span class="cx">     return RenderMathMLBlock::firstLineBaseline();
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -558,7 +563,11 @@
</span><span class="cx">     if (!relayoutChildren &amp;&amp; simplifiedLayout())
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (!m_mathVariantGlyph.font) {
</del><ins>+    GlyphData mathVariantGlyph;
+    if (m_mathVariantCodePoint)
+        mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
+
+    if (!mathVariantGlyph.font) {
</ins><span class="cx">         RenderMathMLBlock::layoutBlock(relayoutChildren, pageLogicalHeight);
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="lines">@@ -566,8 +575,8 @@
</span><span class="cx">     for (auto* child = firstChildBox(); child; child = child-&gt;nextSiblingBox())
</span><span class="cx">         child-&gt;layoutIfNeeded();
</span><span class="cx"> 
</span><del>-    setLogicalWidth(m_mathVariantGlyph.font-&gt;widthForGlyph(m_mathVariantGlyph.glyph));
-    setLogicalHeight(m_mathVariantGlyph.font-&gt;boundsForGlyph(m_mathVariantGlyph.glyph).height());
</del><ins>+    setLogicalWidth(mathVariantGlyph.font-&gt;widthForGlyph(mathVariantGlyph.glyph));
+    setLogicalHeight(mathVariantGlyph.font-&gt;boundsForGlyph(mathVariantGlyph.glyph).height());
</ins><span class="cx"> 
</span><span class="cx">     clearNeedsLayout();
</span><span class="cx"> }
</span><span class="lines">@@ -577,22 +586,30 @@
</span><span class="cx">     RenderMathMLBlock::paint(info, paintOffset);
</span><span class="cx"> 
</span><span class="cx">     // 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).
</span><del>-    if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantGlyph.font)
</del><ins>+    if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantCodePoint)
</ins><span class="cx">         return;
</span><span class="cx"> 
</span><ins>+    auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
+    if (!mathVariantGlyph.font)
+        return;
+
</ins><span class="cx">     GraphicsContextStateSaver stateSaver(info.context());
</span><span class="cx">     info.context().setFillColor(style().visitedDependentColor(CSSPropertyColor));
</span><span class="cx"> 
</span><span class="cx">     GlyphBuffer buffer;
</span><del>-    buffer.add(m_mathVariantGlyph.glyph, m_mathVariantGlyph.font, m_mathVariantGlyph.font-&gt;widthForGlyph(m_mathVariantGlyph.glyph));
-    LayoutUnit glyphAscent = static_cast&lt;int&gt;(lroundf(-m_mathVariantGlyph.font-&gt;boundsForGlyph(m_mathVariantGlyph.glyph).y()));
-    info.context().drawGlyphs(style().fontCascade(), *m_mathVariantGlyph.font, buffer, 0, 1, paintOffset + location() + LayoutPoint(0, glyphAscent));
</del><ins>+    buffer.add(mathVariantGlyph.glyph, mathVariantGlyph.font, mathVariantGlyph.font-&gt;widthForGlyph(mathVariantGlyph.glyph));
+    LayoutUnit glyphAscent = static_cast&lt;int&gt;(lroundf(-mathVariantGlyph.font-&gt;boundsForGlyph(mathVariantGlyph.glyph).y()));
+    info.context().drawGlyphs(style().fontCascade(), *mathVariantGlyph.font, buffer, 0, 1, paintOffset + location() + LayoutPoint(0, glyphAscent));
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void RenderMathMLToken::paintChildren(PaintInfo&amp; paintInfo, const LayoutPoint&amp; paintOffset, PaintInfo&amp; paintInfoForChild, bool usePrintRect)
</span><span class="cx"> {
</span><del>-    if (m_mathVariantGlyph.font)
-        return;
</del><ins>+    if (m_mathVariantCodePoint) {
+        auto mathVariantGlyph = style().fontCascade().glyphDataForCharacter(m_mathVariantCodePoint.value(), m_mathVariantIsMirrored);
+        if (mathVariantGlyph.font)
+            return;
+    }
+
</ins><span class="cx">     RenderMathMLBlock::paintChildren(paintInfo, paintOffset, paintInfoForChild, usePrintRect);
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="branchessafari602branchSourceWebCorerenderingmathmlRenderMathMLTokenh"></a>
<div class="modfile"><h4>Modified: branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.h (207270 => 207271)</h4>
<pre class="diff"><span>
<span class="info">--- branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.h        2016-10-13 02:28:42 UTC (rev 207270)
+++ branches/safari-602-branch/Source/WebCore/rendering/mathml/RenderMathMLToken.h        2016-10-13 02:28:46 UTC (rev 207271)
</span><span class="lines">@@ -1,6 +1,7 @@
</span><span class="cx"> /*
</span><span class="cx">  * Copyright (C) 2014 Frédéric Wang (fred.wang@free.fr). All rights reserved.
</span><span class="cx">  * Copyright (C) 2016 Igalia S.L.
</span><ins>+ * Copyright (C) 2016 Apple Inc.  All rights reserved.
</ins><span class="cx">  *
</span><span class="cx">  * Redistribution and use in source and binary forms, with or without
</span><span class="cx">  * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -61,8 +62,9 @@
</span><span class="cx">         m_mathVariantGlyphDirty = true;
</span><span class="cx">         setNeedsLayoutAndPrefWidthsRecalc();
</span><span class="cx">     }
</span><del>-    GlyphData m_mathVariantGlyph;
-    bool m_mathVariantGlyphDirty;
</del><ins>+    Optional&lt;UChar32&gt; m_mathVariantCodePoint { Nullopt };
+    bool m_mathVariantIsMirrored { false };
+    bool m_mathVariantGlyphDirty { false };
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> } // namespace WebCore
</span></span></pre>
</div>
</div>

</body>
</html>