<!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>[192413] trunk</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/192413">192413</a></dd>
<dt>Author</dt> <dd>svillar@igalia.com</dd>
<dt>Date</dt> <dd>2015-11-13 03:13:42 -0800 (Fri, 13 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>ASSERTION FAILED: m_isEngaged in WTF::Optional::value
https://bugs.webkit.org/show_bug.cgi?id=151094
Reviewed by Darin Adler.
Source/WebCore:
That ASSERT was hit because the precondition was incorrectly
met, i.e., we're considering that the main axis length was
definite when it was actually not. The problem is that to
determine whether it was indefinite or not we're using
RenderBox::hasDefiniteLogicalHeight() which did not perfectly
match RenderBox::computePercentageLogicalHeight() for the case
of RenderTables. So computePercentageLogicalHeight() was
returning Nullopt (i.e. indefinite) while
hasDefiniteLogicalHeight() was returning true (so definite).
Some checks were refactored in a helper function to solve the
inconsistency so that both functions behave the same way even
in this particular situation.
Test: css3/flexbox/inline-flex-percentage-height-in-table-crash.html
* rendering/RenderBox.cpp:
(WebCore::tableCellShouldHaveZeroInitialSize):
(WebCore::RenderBox::computePercentageLogicalHeight):
(WebCore::percentageLogicalHeightIsResolvable):
(WebCore::RenderBox::percentageLogicalHeightIsResolvableFromBlock):
* rendering/RenderBox.h:
LayoutTests:
* css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt: Added.
* css3/flexbox/inline-flex-percentage-height-in-table-crash.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBoxcpp">trunk/Source/WebCore/rendering/RenderBox.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBoxh">trunk/Source/WebCore/rendering/RenderBox.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestscss3flexboxinlineflexpercentageheightintablecrashexpectedtxt">trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestscss3flexboxinlineflexpercentageheightintablecrashhtml">trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (192412 => 192413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-11-13 10:47:32 UTC (rev 192412)
+++ trunk/LayoutTests/ChangeLog 2015-11-13 11:13:42 UTC (rev 192413)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2015-11-13 Sergio Villar Senin <svillar@igalia.com>
+
+ ASSERTION FAILED: m_isEngaged in WTF::Optional::value
+ https://bugs.webkit.org/show_bug.cgi?id=151094
+
+ Reviewed by Darin Adler.
+
+ * css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt: Added.
+ * css3/flexbox/inline-flex-percentage-height-in-table-crash.html: Added.
+
</ins><span class="cx"> 2015-11-12 Brady Eidson <beidson@apple.com>
</span><span class="cx">
</span><span class="cx"> Modern IDB: Pipe through cursor functions from client to server.
</span></span></pre></div>
<a id="trunkLayoutTestscss3flexboxinlineflexpercentageheightintablecrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt (0 => 192413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt 2015-11-13 11:13:42 UTC (rev 192413)
</span><span class="lines">@@ -0,0 +1,2 @@
</span><ins>+This test PASSES if it doesn't CRASH in DEBUG builds.
+
</ins></span></pre></div>
<a id="trunkLayoutTestscss3flexboxinlineflexpercentageheightintablecrashhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash.html (0 => 192413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash.html (rev 0)
+++ trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash.html 2015-11-13 11:13:42 UTC (rev 192413)
</span><span class="lines">@@ -0,0 +1,18 @@
</span><ins>+<!DOCTYPE html>
+<style>
+caption {
+ display: inline-flex;
+ flex-direction: column-reverse;
+ height: 0%;
+}
+</style>
+This test PASSES if it doesn't CRASH in DEBUG builds.
+<table>
+ <caption>
+ <div style="height: 0%"></div>
+ </caption>
+</table>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (192412 => 192413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-11-13 10:47:32 UTC (rev 192412)
+++ trunk/Source/WebCore/ChangeLog 2015-11-13 11:13:42 UTC (rev 192413)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2015-11-13 Sergio Villar Senin <svillar@igalia.com>
+
+ ASSERTION FAILED: m_isEngaged in WTF::Optional::value
+ https://bugs.webkit.org/show_bug.cgi?id=151094
+
+ Reviewed by Darin Adler.
+
+ That ASSERT was hit because the precondition was incorrectly
+ met, i.e., we're considering that the main axis length was
+ definite when it was actually not. The problem is that to
+ determine whether it was indefinite or not we're using
+ RenderBox::hasDefiniteLogicalHeight() which did not perfectly
+ match RenderBox::computePercentageLogicalHeight() for the case
+ of RenderTables. So computePercentageLogicalHeight() was
+ returning Nullopt (i.e. indefinite) while
+ hasDefiniteLogicalHeight() was returning true (so definite).
+
+ Some checks were refactored in a helper function to solve the
+ inconsistency so that both functions behave the same way even
+ in this particular situation.
+
+ Test: css3/flexbox/inline-flex-percentage-height-in-table-crash.html
+
+ * rendering/RenderBox.cpp:
+ (WebCore::tableCellShouldHaveZeroInitialSize):
+ (WebCore::RenderBox::computePercentageLogicalHeight):
+ (WebCore::percentageLogicalHeightIsResolvable):
+ (WebCore::RenderBox::percentageLogicalHeightIsResolvableFromBlock):
+ * rendering/RenderBox.h:
+
</ins><span class="cx"> 2015-11-13 Csaba Osztrogonác <ossy@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, really fix the Mac CMake build after r192376.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (192412 => 192413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderBox.cpp 2015-11-13 10:47:32 UTC (rev 192412)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2015-11-13 11:13:42 UTC (rev 192413)
</span><span class="lines">@@ -2904,10 +2904,23 @@
</span><span class="cx"> return !containingBlock->isTableCell() && !containingBlock->isOutOfFlowPositioned() && containingBlock->style().logicalHeight().isAuto() && isHorizontalWritingMode() == containingBlock->isHorizontalWritingMode();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static bool tableCellShouldHaveZeroInitialSize(const RenderBlock& block, bool scrollsOverflowY)
+{
+ // Normally we would let the cell size intrinsically, but scrolling overflow has to be
+ // treated differently, since WinIE lets scrolled overflow regions shrink as needed.
+ // While we can't get all cases right, we can at least detect when the cell has a specified
+ // height or when the table has a specified height. In these cases we want to initially have
+ // no size and allow the flexing of the table or the cell to its specified height to cause us
+ // to grow to fill the space. This could end up being wrong in some cases, but it is
+ // preferable to the alternative (sizing intrinsically and making the row end up too big).
+ const RenderTableCell& cell = downcast<RenderTableCell>(block);
+ return scrollsOverflowY && (!cell.style().logicalHeight().isAuto() || !cell.table()->style().logicalHeight().isAuto());
+}
+
</ins><span class="cx"> Optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length& height) const
</span><span class="cx"> {
</span><span class="cx"> Optional<LayoutUnit> availableHeight;
</span><del>-
</del><ins>+
</ins><span class="cx"> bool skippedAutoHeightContainingBlock = false;
</span><span class="cx"> RenderBlock* cb = containingBlock();
</span><span class="cx"> const RenderBox* containingBlockChild = this;
</span><span class="lines">@@ -2941,19 +2954,9 @@
</span><span class="cx"> // Table cells violate what the CSS spec says to do with heights. Basically we
</span><span class="cx"> // don't care if the cell specified a height or not. We just always make ourselves
</span><span class="cx"> // be a percentage of the cell's current content height.
</span><del>- if (!cb->hasOverrideLogicalContentHeight()) {
- // Normally we would let the cell size intrinsically, but scrolling overflow has to be
- // treated differently, since WinIE lets scrolled overflow regions shrink as needed.
- // While we can't get all cases right, we can at least detect when the cell has a specified
- // height or when the table has a specified height. In these cases we want to initially have
- // no size and allow the flexing of the table or the cell to its specified height to cause us
- // to grow to fill the space. This could end up being wrong in some cases, but it is
- // preferable to the alternative (sizing intrinsically and making the row end up too big).
- RenderTableCell& cell = downcast<RenderTableCell>(*cb);
- if (scrollsOverflowY() && (!cell.style().logicalHeight().isAuto() || !cell.table()->style().logicalHeight().isAuto()))
- return LayoutUnit(0);
- return Nullopt;
- }
</del><ins>+ if (!cb->hasOverrideLogicalContentHeight())
+ return tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY()) ? Optional<LayoutUnit>() : Nullopt;
+
</ins><span class="cx"> availableHeight = cb->overrideLogicalContentHeight();
</span><span class="cx"> includeBorderPadding = true;
</span><span class="cx"> }
</span><span class="lines">@@ -4624,10 +4627,10 @@
</span><span class="cx">
</span><span class="cx"> inline static bool percentageLogicalHeightIsResolvable(const RenderBox* box)
</span><span class="cx"> {
</span><del>- return RenderBox::percentageLogicalHeightIsResolvableFromBlock(box->containingBlock(), box->isOutOfFlowPositioned());
</del><ins>+ return RenderBox::percentageLogicalHeightIsResolvableFromBlock(box->containingBlock(), box->isOutOfFlowPositioned(), box->scrollsOverflowY());
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-bool RenderBox::percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool isOutOfFlowPositioned)
</del><ins>+bool RenderBox::percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool isOutOfFlowPositioned, bool scrollsOverflowY)
</ins><span class="cx"> {
</span><span class="cx"> // In quirks mode, blocks with auto height are skipped, and we keep looking for an enclosing
</span><span class="cx"> // block that may have a specified height and then use it. In strict mode, this violates the
</span><span class="lines">@@ -4636,6 +4639,7 @@
</span><span class="cx"> // only at explicit containers.
</span><span class="cx"> const RenderBlock* cb = containingBlock;
</span><span class="cx"> bool inQuirksMode = cb->document().inQuirksMode();
</span><ins>+ bool skippedAutoHeightContainingBlock = false;
</ins><span class="cx"> while (!cb->isRenderView() && !cb->isBody() && !cb->isTableCell() && !cb->isOutOfFlowPositioned() && cb->style().logicalHeight().isAuto()) {
</span><span class="cx"> if (!inQuirksMode && !cb->isAnonymousBlock())
</span><span class="cx"> break;
</span><span class="lines">@@ -4643,7 +4647,7 @@
</span><span class="cx"> if (cb->hasOverrideContainingBlockLogicalHeight())
</span><span class="cx"> return static_cast<bool>(cb->overrideContainingBlockContentLogicalHeight());
</span><span class="cx"> #endif
</span><del>-
</del><ins>+ skippedAutoHeightContainingBlock = true;
</ins><span class="cx"> cb = cb->containingBlock();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -4656,15 +4660,17 @@
</span><span class="cx"> // Table cells violate what the CSS spec says to do with heights. Basically we
</span><span class="cx"> // don't care if the cell specified a height or not. We just always make ourselves
</span><span class="cx"> // be a percentage of the cell's current content height.
</span><del>- if (cb->isTableCell())
- return true;
</del><ins>+ if (cb->isTableCell()) {
+ // Matches computePercentageLogicalHeight().
+ return skippedAutoHeightContainingBlock || cb->hasOverrideLogicalContentHeight() || tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY);
+ }
</ins><span class="cx">
</span><span class="cx"> // Otherwise we only use our percentage height if our containing block had a specified
</span><span class="cx"> // height.
</span><span class="cx"> if (cb->style().logicalHeight().isFixed())
</span><span class="cx"> return true;
</span><span class="cx"> if (cb->style().logicalHeight().isPercentOrCalculated() && !isOutOfFlowPositionedWithSpecifiedHeight)
</span><del>- return percentageLogicalHeightIsResolvableFromBlock(cb->containingBlock(), cb->isOutOfFlowPositioned());
</del><ins>+ return percentageLogicalHeightIsResolvableFromBlock(cb->containingBlock(), cb->isOutOfFlowPositioned(), cb->scrollsOverflowY());
</ins><span class="cx"> if (cb->isRenderView() || inQuirksMode || isOutOfFlowPositionedWithSpecifiedHeight)
</span><span class="cx"> return true;
</span><span class="cx"> if (cb->isDocumentElementRenderer() && isOutOfFlowPositioned) {
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBoxh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBox.h (192412 => 192413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderBox.h 2015-11-13 10:47:32 UTC (rev 192412)
+++ trunk/Source/WebCore/rendering/RenderBox.h 2015-11-13 11:13:42 UTC (rev 192413)
</span><span class="lines">@@ -438,7 +438,7 @@
</span><span class="cx"> virtual LayoutUnit computeReplacedLogicalHeight() const;
</span><span class="cx">
</span><span class="cx"> bool hasDefiniteLogicalWidth() const;
</span><del>- static bool percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool outOfFlowPositioned);
</del><ins>+ static bool percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool outOfFlowPositioned, bool scrollsOverflowY);
</ins><span class="cx"> bool hasDefiniteLogicalHeight() const;
</span><span class="cx"> Optional<LayoutUnit> computePercentageLogicalHeight(const Length& height) const;
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>