<!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>[203280] trunk/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/203280">203280</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-07-15 10:03:45 -0700 (Fri, 15 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingmathmlMathOperatorcpp">trunk/Source/WebCore/rendering/mathml/MathOperator.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingmathmlRenderMathMLOperatorcpp">trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingmathmlRenderMathMLTokencpp">trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (203279 => 203280)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-07-15 16:51:56 UTC (rev 203279)
+++ trunk/Source/WebCore/ChangeLog 2016-07-15 17:03:45 UTC (rev 203280)
</span><span class="lines">@@ -1,5 +1,38 @@
</span><span class="cx"> 2016-07-15 Frederic Wang <fwang@igalia.com>
</span><span class="cx">
</span><ins>+ Check whether font is nonnull for GlyphData instead of calling GlyphData::isValid()
+ https://bugs.webkit.org/show_bug.cgi?id=159783
+
+ 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):
+
+2016-07-15 Frederic Wang <fwang@igalia.com>
+
</ins><span class="cx"> Add DejaVu Math TeX Gyre to the list of math fonts.
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=159805
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingmathmlMathOperatorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/mathml/MathOperator.cpp (203279 => 203280)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/mathml/MathOperator.cpp 2016-07-15 16:51:56 UTC (rev 203279)
+++ trunk/Source/WebCore/rendering/mathml/MathOperator.cpp 2016-07-15 17:03:45 UTC (rev 203280)
</span><span class="lines">@@ -38,7 +38,7 @@
</span><span class="cx">
</span><span class="cx"> static inline FloatRect boundsForGlyph(const GlyphData& data)
</span><span class="cx"> {
</span><del>- return data.isValid() ? data.font->boundsForGlyph(data.glyph) : FloatRect();
</del><ins>+ return data.font ? data.font->boundsForGlyph(data.glyph) : FloatRect();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static inline float heightForGlyph(const GlyphData& data)
</span><span class="lines">@@ -55,7 +55,7 @@
</span><span class="cx">
</span><span class="cx"> static inline float advanceWidthForGlyph(const GlyphData& data)
</span><span class="cx"> {
</span><del>- return data.isValid() ? data.font->widthForGlyph(data.glyph) : 0;
</del><ins>+ return data.font ? data.font->widthForGlyph(data.glyph) : 0;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // FIXME: This hardcoded data can be removed when OpenType MATH font are widely available (http://wkbug/156837).
</span><span class="lines">@@ -124,12 +124,12 @@
</span><span class="cx"> bool MathOperator::getBaseGlyph(const RenderStyle& style, GlyphData& baseGlyph) const
</span><span class="cx"> {
</span><span class="cx"> baseGlyph = style.fontCascade().glyphDataForCharacter(m_baseCharacter, !style.isLeftToRightDirection());
</span><del>- return baseGlyph.isValid() && baseGlyph.font == &style.fontCascade().primaryFont();
</del><ins>+ return baseGlyph.font && baseGlyph.font == &style.fontCascade().primaryFont();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MathOperator::setSizeVariant(const GlyphData& sizeVariant)
</span><span class="cx"> {
</span><del>- ASSERT(sizeVariant.isValid());
</del><ins>+ ASSERT(sizeVariant.font);
</ins><span class="cx"> ASSERT(sizeVariant.font->mathData());
</span><span class="cx"> m_stretchType = StretchType::SizeVariant;
</span><span class="cx"> m_variant = sizeVariant;
</span><span class="lines">@@ -468,7 +468,7 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_operatorType == Type::VerticalOperator);
</span><span class="cx"> ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><del>- ASSERT(m_assembly.extension.isValid());
</del><ins>+ ASSERT(m_assembly.extension.font);
</ins><span class="cx"> ASSERT(from.y() <= to.y());
</span><span class="cx">
</span><span class="cx"> // If there is no space for the extension glyph, we don't need to do anything.
</span><span class="lines">@@ -507,7 +507,7 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_operatorType == Type::HorizontalOperator);
</span><span class="cx"> ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><del>- ASSERT(m_assembly.extension.isValid());
</del><ins>+ ASSERT(m_assembly.extension.font);
</ins><span class="cx"> ASSERT(from.x() <= to.x());
</span><span class="cx"> ASSERT(from.y() == to.y());
</span><span class="cx">
</span><span class="lines">@@ -545,8 +545,8 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_operatorType == Type::VerticalOperator);
</span><span class="cx"> ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><del>- ASSERT(m_assembly.topOrRight.isValid());
- ASSERT(m_assembly.bottomOrLeft.isValid());
</del><ins>+ ASSERT(m_assembly.topOrRight.font);
+ ASSERT(m_assembly.bottomOrLeft.font);
</ins><span class="cx">
</span><span class="cx"> // We are positioning the glyphs so that the edge of the tight glyph bounds line up exactly with the edges of our paint box.
</span><span class="cx"> LayoutPoint operatorTopLeft = paintOffset;
</span><span class="lines">@@ -558,7 +558,7 @@
</span><span class="cx"> LayoutPoint bottomGlyphOrigin(operatorTopLeft.x(), operatorTopLeft.y() + stretchSize() - (bottomGlyphBounds.height() + bottomGlyphBounds.y()));
</span><span class="cx"> LayoutRect bottomGlyphPaintRect = paintGlyph(style, info, m_assembly.bottomOrLeft, bottomGlyphOrigin, TrimTop);
</span><span class="cx">
</span><del>- if (m_assembly.middle.isValid()) {
</del><ins>+ if (m_assembly.middle.font) {
</ins><span class="cx"> // Center the glyph origin between the start and end glyph paint extents. Then shift it half the paint height toward the bottom glyph.
</span><span class="cx"> FloatRect middleGlyphBounds = boundsForGlyph(m_assembly.middle);
</span><span class="cx"> LayoutPoint middleGlyphOrigin(operatorTopLeft.x(), topGlyphOrigin.y());
</span><span class="lines">@@ -576,8 +576,8 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_operatorType == Type::HorizontalOperator);
</span><span class="cx"> ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><del>- ASSERT(m_assembly.bottomOrLeft.isValid());
- ASSERT(m_assembly.topOrRight.isValid());
</del><ins>+ ASSERT(m_assembly.bottomOrLeft.font);
+ ASSERT(m_assembly.topOrRight.font);
</ins><span class="cx">
</span><span class="cx"> // We are positioning the glyphs so that the edge of the tight glyph bounds line up exactly with the edges of our paint box.
</span><span class="cx"> LayoutPoint operatorTopLeft = paintOffset;
</span><span class="lines">@@ -589,7 +589,7 @@
</span><span class="cx"> LayoutPoint rightGlyphOrigin(operatorTopLeft.x() + stretchSize() - rightGlyphBounds.width(), baselineY);
</span><span class="cx"> LayoutRect rightGlyphPaintRect = paintGlyph(style, info, m_assembly.topOrRight, rightGlyphOrigin, TrimLeft);
</span><span class="cx">
</span><del>- if (m_assembly.middle.isValid()) {
</del><ins>+ if (m_assembly.middle.font) {
</ins><span class="cx"> // Center the glyph origin between the start and end glyph paint extents.
</span><span class="cx"> LayoutPoint middleGlyphOrigin(operatorTopLeft.x(), baselineY);
</span><span class="cx"> middleGlyphOrigin.moveBy(LayoutPoint((rightGlyphPaintRect.x() - leftGlyphPaintRect.maxX()) / 2.0, 0));
</span><span class="lines">@@ -632,7 +632,7 @@
</span><span class="cx"> if (!getBaseGlyph(style, glyphData))
</span><span class="cx"> return;
</span><span class="cx"> } else if (m_stretchType == StretchType::SizeVariant) {
</span><del>- ASSERT(m_variant.isValid());
</del><ins>+ ASSERT(m_variant.font);
</ins><span class="cx"> glyphData = m_variant;
</span><span class="cx"> }
</span><span class="cx"> GlyphBuffer buffer;
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingmathmlRenderMathMLOperatorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp (203279 => 203280)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp 2016-07-15 16:51:56 UTC (rev 203279)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp 2016-07-15 17:03:45 UTC (rev 203280)
</span><span class="lines">@@ -246,7 +246,7 @@
</span><span class="cx"> if (isInvisibleOperator()) {
</span><span class="cx"> // In some fonts, glyphs for invisible operators have nonzero width. Consequently, we subtract that width here to avoid wide gaps.
</span><span class="cx"> GlyphData data = style().fontCascade().glyphDataForCharacter(m_textContent, false);
</span><del>- float glyphWidth = data.isValid() ? data.font->widthForGlyph(data.glyph) : 0;
</del><ins>+ float glyphWidth = data.font ? data.font->widthForGlyph(data.glyph) : 0;
</ins><span class="cx"> ASSERT(glyphWidth <= preferredWidth);
</span><span class="cx"> preferredWidth -= glyphWidth;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingmathmlRenderMathMLTokencpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp (203279 => 203280)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp 2016-07-15 16:51:56 UTC (rev 203279)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLToken.cpp 2016-07-15 17:03:45 UTC (rev 203280)
</span><span class="lines">@@ -496,7 +496,7 @@
</span><span class="cx"> if (m_mathVariantGlyphDirty)
</span><span class="cx"> updateMathVariantGlyph();
</span><span class="cx">
</span><del>- if (m_mathVariantGlyph.isValid()) {
</del><ins>+ if (m_mathVariantGlyph.font) {
</ins><span class="cx"> m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = m_mathVariantGlyph.font->widthForGlyph(m_mathVariantGlyph.glyph);
</span><span class="cx"> setPreferredLogicalWidthsDirty(false);
</span><span class="cx"> return;
</span><span class="lines">@@ -546,7 +546,7 @@
</span><span class="cx">
</span><span class="cx"> Optional<int> RenderMathMLToken::firstLineBaseline() const
</span><span class="cx"> {
</span><del>- if (m_mathVariantGlyph.isValid())
</del><ins>+ if (m_mathVariantGlyph.font)
</ins><span class="cx"> return Optional<int>(static_cast<int>(lroundf(-m_mathVariantGlyph.font->boundsForGlyph(m_mathVariantGlyph.glyph).y())));
</span><span class="cx"> return RenderMathMLBlock::firstLineBaseline();
</span><span class="cx"> }
</span><span class="lines">@@ -558,7 +558,7 @@
</span><span class="cx"> if (!relayoutChildren && simplifiedLayout())
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (!m_mathVariantGlyph.isValid()) {
</del><ins>+ if (!m_mathVariantGlyph.font) {
</ins><span class="cx"> RenderMathMLBlock::layoutBlock(relayoutChildren, pageLogicalHeight);
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="lines">@@ -577,7 +577,7 @@
</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.isValid())
</del><ins>+ if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !m_mathVariantGlyph.font)
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> GraphicsContextStateSaver stateSaver(info.context());
</span><span class="lines">@@ -591,7 +591,7 @@
</span><span class="cx">
</span><span class="cx"> void RenderMathMLToken::paintChildren(PaintInfo& paintInfo, const LayoutPoint& paintOffset, PaintInfo& paintInfoForChild, bool usePrintRect)
</span><span class="cx"> {
</span><del>- if (m_mathVariantGlyph.isValid())
</del><ins>+ if (m_mathVariantGlyph.font)
</ins><span class="cx"> return;
</span><span class="cx"> RenderMathMLBlock::paintChildren(paintInfo, paintOffset, paintInfoForChild, usePrintRect);
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>