- Revision
- 192413
- Author
- svil...@igalia.com
- Date
- 2015-11-13 03:13:42 -0800 (Fri, 13 Nov 2015)
Log Message
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (192412 => 192413)
--- trunk/LayoutTests/ChangeLog 2015-11-13 10:47:32 UTC (rev 192412)
+++ trunk/LayoutTests/ChangeLog 2015-11-13 11:13:42 UTC (rev 192413)
@@ -1,3 +1,13 @@
+2015-11-13 Sergio Villar Senin <svil...@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.
+
2015-11-12 Brady Eidson <beid...@apple.com>
Modern IDB: Pipe through cursor functions from client to server.
Added: trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash-expected.txt (0 => 192413)
--- 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)
@@ -0,0 +1,2 @@
+This test PASSES if it doesn't CRASH in DEBUG builds.
+
Added: trunk/LayoutTests/css3/flexbox/inline-flex-percentage-height-in-table-crash.html (0 => 192413)
--- 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)
@@ -0,0 +1,18 @@
+<!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>
Modified: trunk/Source/WebCore/ChangeLog (192412 => 192413)
--- 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)
@@ -1,3 +1,33 @@
+2015-11-13 Sergio Villar Senin <svil...@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:
+
2015-11-13 Csaba Osztrogonác <o...@webkit.org>
Unreviewed, really fix the Mac CMake build after r192376.
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (192412 => 192413)
--- 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)
@@ -2904,10 +2904,23 @@
return !containingBlock->isTableCell() && !containingBlock->isOutOfFlowPositioned() && containingBlock->style().logicalHeight().isAuto() && isHorizontalWritingMode() == containingBlock->isHorizontalWritingMode();
}
+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());
+}
+
Optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length& height) const
{
Optional<LayoutUnit> availableHeight;
-
+
bool skippedAutoHeightContainingBlock = false;
RenderBlock* cb = containingBlock();
const RenderBox* containingBlockChild = this;
@@ -2941,19 +2954,9 @@
// Table cells violate what the CSS spec says to do with heights. Basically we
// don't care if the cell specified a height or not. We just always make ourselves
// be a percentage of the cell's current content height.
- 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;
- }
+ if (!cb->hasOverrideLogicalContentHeight())
+ return tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY()) ? Optional<LayoutUnit>() : Nullopt;
+
availableHeight = cb->overrideLogicalContentHeight();
includeBorderPadding = true;
}
@@ -4624,10 +4627,10 @@
inline static bool percentageLogicalHeightIsResolvable(const RenderBox* box)
{
- return RenderBox::percentageLogicalHeightIsResolvableFromBlock(box->containingBlock(), box->isOutOfFlowPositioned());
+ return RenderBox::percentageLogicalHeightIsResolvableFromBlock(box->containingBlock(), box->isOutOfFlowPositioned(), box->scrollsOverflowY());
}
-bool RenderBox::percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool isOutOfFlowPositioned)
+bool RenderBox::percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool isOutOfFlowPositioned, bool scrollsOverflowY)
{
// In quirks mode, blocks with auto height are skipped, and we keep looking for an enclosing
// block that may have a specified height and then use it. In strict mode, this violates the
@@ -4636,6 +4639,7 @@
// only at explicit containers.
const RenderBlock* cb = containingBlock;
bool inQuirksMode = cb->document().inQuirksMode();
+ bool skippedAutoHeightContainingBlock = false;
while (!cb->isRenderView() && !cb->isBody() && !cb->isTableCell() && !cb->isOutOfFlowPositioned() && cb->style().logicalHeight().isAuto()) {
if (!inQuirksMode && !cb->isAnonymousBlock())
break;
@@ -4643,7 +4647,7 @@
if (cb->hasOverrideContainingBlockLogicalHeight())
return static_cast<bool>(cb->overrideContainingBlockContentLogicalHeight());
#endif
-
+ skippedAutoHeightContainingBlock = true;
cb = cb->containingBlock();
}
@@ -4656,15 +4660,17 @@
// Table cells violate what the CSS spec says to do with heights. Basically we
// don't care if the cell specified a height or not. We just always make ourselves
// be a percentage of the cell's current content height.
- if (cb->isTableCell())
- return true;
+ if (cb->isTableCell()) {
+ // Matches computePercentageLogicalHeight().
+ return skippedAutoHeightContainingBlock || cb->hasOverrideLogicalContentHeight() || tableCellShouldHaveZeroInitialSize(*cb, scrollsOverflowY);
+ }
// Otherwise we only use our percentage height if our containing block had a specified
// height.
if (cb->style().logicalHeight().isFixed())
return true;
if (cb->style().logicalHeight().isPercentOrCalculated() && !isOutOfFlowPositionedWithSpecifiedHeight)
- return percentageLogicalHeightIsResolvableFromBlock(cb->containingBlock(), cb->isOutOfFlowPositioned());
+ return percentageLogicalHeightIsResolvableFromBlock(cb->containingBlock(), cb->isOutOfFlowPositioned(), cb->scrollsOverflowY());
if (cb->isRenderView() || inQuirksMode || isOutOfFlowPositionedWithSpecifiedHeight)
return true;
if (cb->isDocumentElementRenderer() && isOutOfFlowPositioned) {
Modified: trunk/Source/WebCore/rendering/RenderBox.h (192412 => 192413)
--- 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)
@@ -438,7 +438,7 @@
virtual LayoutUnit computeReplacedLogicalHeight() const;
bool hasDefiniteLogicalWidth() const;
- static bool percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool outOfFlowPositioned);
+ static bool percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool outOfFlowPositioned, bool scrollsOverflowY);
bool hasDefiniteLogicalHeight() const;
Optional<LayoutUnit> computePercentageLogicalHeight(const Length& height) const;
