- Revision
- 208586
- Author
- r...@igalia.com
- Date
- 2016-11-11 03:08:39 -0800 (Fri, 11 Nov 2016)
Log Message
[css-grid] ASSERTION FAILED: !m_gridIsDirty in WebCore::RenderGrid::gridRowCount
https://bugs.webkit.org/show_bug.cgi?id=163450
Reviewed by Darin Adler.
Source/WebCore:
The issue is that in the test case a simplifiedLayout() is performed.
So in RenderGrid::layoutBlock() we early return and the grid is not populated,
so the m_gridIsDirty flag is not cleared when we try to check the size of the grid
in RenderGrid::layoutPositionedObject().
We should avoid to do a simplified layout if we have to layout
some positioned grid items and the grid is dirty.
The problem was not only the ASSERT, but the current behavior was wrong too.
As we didn't do a proper layout of the grid container, the positioned item
won't be placed on the expected position. Added tests verifying this.
Tests: fast/css-grid-layout/grid-positioned-item-dynamic-change.html
fast/css-grid-layout/grid-simplified-layout-positioned.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::canPerformSimplifiedLayout): Check if we can perform or not
a simplified layout.
(WebCore::RenderBlock::simplifiedLayout): Extract initial check
into canPerformSimplifiedLayout().
* rendering/RenderBlock.h: Add new header for canPerformSimplifiedLayout().
* rendering/RenderGrid.cpp: Implement our own version of canPerformSimplifiedLayout()
to verify that the grid is not dirty if we have to layout some positioned items.
(WebCore::RenderGrid::canPerformSimplifiedLayout):
* rendering/RenderGrid.h: Add canPerformSimplifiedLayout() header.
LayoutTests:
The tests shouldn't crash in debug to verify that the bug is fixed.
On top of that the positioned grid items should appear in the right position too.
* fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html: Added.
* fast/css-grid-layout/grid-positioned-item-dynamic-change.html: Added.
* fast/css-grid-layout/grid-simplified-layout-positioned-expected.html: Added.
* fast/css-grid-layout/grid-simplified-layout-positioned.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (208585 => 208586)
--- trunk/LayoutTests/ChangeLog 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/LayoutTests/ChangeLog 2016-11-11 11:08:39 UTC (rev 208586)
@@ -1,3 +1,18 @@
+2016-11-11 Manuel Rego Casasnovas <r...@igalia.com>
+
+ [css-grid] ASSERTION FAILED: !m_gridIsDirty in WebCore::RenderGrid::gridRowCount
+ https://bugs.webkit.org/show_bug.cgi?id=163450
+
+ Reviewed by Darin Adler.
+
+ The tests shouldn't crash in debug to verify that the bug is fixed.
+ On top of that the positioned grid items should appear in the right position too.
+
+ * fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html: Added.
+ * fast/css-grid-layout/grid-positioned-item-dynamic-change.html: Added.
+ * fast/css-grid-layout/grid-simplified-layout-positioned-expected.html: Added.
+ * fast/css-grid-layout/grid-simplified-layout-positioned.html: Added.
+
2016-11-11 Antoine Quint <grao...@apple.com>
[Modern Media Controls] Media Controller: media tracks control support
Added: trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html (0 => 208586)
--- trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html 2016-11-11 11:08:39 UTC (rev 208586)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<style>
+
+.grid {
+ width: 100px;
+ height: 100px;
+}
+
+.green {
+ background: green;
+}
+
+</style>
+
+<p>This test checks that positioned items can be dynamically changed.</p>
+<p>The test passes if you see a 100x100 green square and no red.</p>
+
+<div class="grid green">
+</div>
Added: trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html (0 => 208586)
--- trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html 2016-11-11 11:08:39 UTC (rev 208586)
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<style>
+
+.grid {
+ display: grid;
+ grid: 50px 50px / 50px 50px;
+ position: relative;
+}
+
+.green {
+ background: green;
+}
+
+.red {
+ background: red;
+}
+
+#item {
+ position: absolute;
+ width: 100%;
+ height: 100%;
+ grid-column: 1 / 2;
+ grid-row: 1 / 2;
+}
+</style>
+
+<p>This test checks that positioned items can be dynamically changed.</p>
+<p>The test passes if you see a 100x100 green square and no red.</p>
+
+<div class="grid">
+ <div class="green"></div>
+ <div class="green"></div>
+ <div class="green"></div>
+ <div class="red"></div>
+ <div id="item" class="green"></div>
+</div>
+
+<script>
+document.body.offsetLeft;
+
+var item = document.getElementById("item");
+item.style.gridColumn = "2 / 3";
+item.style.gridRow = "2 / 3";
+</script>
Added: trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html (0 => 208586)
--- trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html 2016-11-11 11:08:39 UTC (rev 208586)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+ body {
+ position: absolute;
+ left: 100px;
+ top: 100px;
+ width: 400px;
+ }
+</style>
+This text and input should appear at 100x100 position.
+Like if they have a left and top margin of 100px.
+<input autofocus>
Added: trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned.html (0 => 208586)
--- trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned.html 2016-11-11 11:08:39 UTC (rev 208586)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<style>
+ html {
+ display: grid;
+ position: absolute;
+ grid: 100px 400px / 100px 400px;
+ }
+ body {
+ position: absolute;
+ grid-column: 2 / 3;
+ grid-row: 2 / 3;
+ width: 100%;
+ }
+</style>
+This text and input should appear at 100x100 position.
+Like if they have a left and top margin of 100px.
+<input autofocus>
Modified: trunk/Source/WebCore/ChangeLog (208585 => 208586)
--- trunk/Source/WebCore/ChangeLog 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/Source/WebCore/ChangeLog 2016-11-11 11:08:39 UTC (rev 208586)
@@ -1,3 +1,36 @@
+2016-11-11 Manuel Rego Casasnovas <r...@igalia.com>
+
+ [css-grid] ASSERTION FAILED: !m_gridIsDirty in WebCore::RenderGrid::gridRowCount
+ https://bugs.webkit.org/show_bug.cgi?id=163450
+
+ Reviewed by Darin Adler.
+
+ The issue is that in the test case a simplifiedLayout() is performed.
+ So in RenderGrid::layoutBlock() we early return and the grid is not populated,
+ so the m_gridIsDirty flag is not cleared when we try to check the size of the grid
+ in RenderGrid::layoutPositionedObject().
+
+ We should avoid to do a simplified layout if we have to layout
+ some positioned grid items and the grid is dirty.
+
+ The problem was not only the ASSERT, but the current behavior was wrong too.
+ As we didn't do a proper layout of the grid container, the positioned item
+ won't be placed on the expected position. Added tests verifying this.
+
+ Tests: fast/css-grid-layout/grid-positioned-item-dynamic-change.html
+ fast/css-grid-layout/grid-simplified-layout-positioned.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::canPerformSimplifiedLayout): Check if we can perform or not
+ a simplified layout.
+ (WebCore::RenderBlock::simplifiedLayout): Extract initial check
+ into canPerformSimplifiedLayout().
+ * rendering/RenderBlock.h: Add new header for canPerformSimplifiedLayout().
+ * rendering/RenderGrid.cpp: Implement our own version of canPerformSimplifiedLayout()
+ to verify that the grid is not dirty if we have to layout some positioned items.
+ (WebCore::RenderGrid::canPerformSimplifiedLayout):
+ * rendering/RenderGrid.h: Add canPerformSimplifiedLayout() header.
+
2016-11-11 Antoine Quint <grao...@apple.com>
[Modern Media Controls] Media Controller: media tracks control support
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (208585 => 208586)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2016-11-11 11:08:39 UTC (rev 208586)
@@ -1336,9 +1336,14 @@
}
}
+bool RenderBlock::canPerformSimplifiedLayout() const
+{
+ return (posChildNeedsLayout() || needsSimplifiedNormalFlowLayout()) && !normalChildNeedsLayout() && !selfNeedsLayout();
+}
+
bool RenderBlock::simplifiedLayout()
{
- if ((!posChildNeedsLayout() && !needsSimplifiedNormalFlowLayout()) || normalChildNeedsLayout() || selfNeedsLayout())
+ if (!canPerformSimplifiedLayout())
return false;
LayoutStateMaintainer statePusher(view(), *this, locationOffset(), hasTransform() || hasReflection() || style().isFlippedBlocksWritingMode());
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (208585 => 208586)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2016-11-11 11:08:39 UTC (rev 208586)
@@ -360,6 +360,7 @@
virtual bool hasLineIfEmpty() const;
+ virtual bool canPerformSimplifiedLayout() const;
bool simplifiedLayout();
virtual void simplifiedNormalFlowLayout();
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (208585 => 208586)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2016-11-11 11:08:39 UTC (rev 208586)
@@ -446,6 +446,16 @@
}
}
+bool RenderGrid::canPerformSimplifiedLayout() const
+{
+ // We cannot perform a simplified layout if the grid is dirty and we have
+ // some positioned items to be laid out.
+ if (m_gridIsDirty && posChildNeedsLayout())
+ return false;
+
+ return RenderBlock::canPerformSimplifiedLayout();
+}
+
void RenderGrid::layoutBlock(bool relayoutChildren, LayoutUnit)
{
ASSERT(needsLayout());
Modified: trunk/Source/WebCore/rendering/RenderGrid.h (208585 => 208586)
--- trunk/Source/WebCore/rendering/RenderGrid.h 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/Source/WebCore/rendering/RenderGrid.h 2016-11-11 11:08:39 UTC (rev 208586)
@@ -110,6 +110,7 @@
GridTrackSizingDirection autoPlacementMajorAxisDirection() const;
GridTrackSizingDirection autoPlacementMinorAxisDirection() const;
+ bool canPerformSimplifiedLayout() const final;
void prepareChildForPositionedLayout(RenderBox&);
void layoutPositionedObject(RenderBox&, bool relayoutChildren, bool fixedPositionObjectsOnly) override;
void offsetAndBreadthForPositionedChild(const RenderBox&, GridTrackSizingDirection, LayoutUnit& offset, LayoutUnit& breadth);
