<!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>[208586] 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/208586">208586</a></dd>
<dt>Author</dt> <dd>rego@igalia.com</dd>
<dt>Date</dt> <dd>2016-11-11 03:08:39 -0800 (Fri, 11 Nov 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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.</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="#trunkSourceWebCorerenderingRenderBlockcpp">trunk/Source/WebCore/rendering/RenderBlock.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBlockh">trunk/Source/WebCore/rendering/RenderBlock.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderGridcpp">trunk/Source/WebCore/rendering/RenderGrid.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderGridh">trunk/Source/WebCore/rendering/RenderGrid.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgridpositioneditemdynamicchangeexpectedhtml">trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgridpositioneditemdynamicchangehtml">trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgridsimplifiedlayoutpositionedexpectedhtml">trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgridsimplifiedlayoutpositionedhtml">trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (208585 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-11-11 09:48:37 UTC (rev 208585)
+++ trunk/LayoutTests/ChangeLog 2016-11-11 11:08:39 UTC (rev 208586)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2016-11-11 Manuel Rego Casasnovas <rego@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.
+
</ins><span class="cx"> 2016-11-11 Antoine Quint <graouts@apple.com>
</span><span class="cx">
</span><span class="cx"> [Modern Media Controls] Media Controller: media tracks control support
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgridpositioneditemdynamicchangeexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html (0 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgridpositioneditemdynamicchangehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html (0 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,44 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgridsimplifiedlayoutpositionedexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html (0 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgridsimplifiedlayoutpositionedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned.html (0 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,17 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (208585 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,36 @@
</span><ins>+2016-11-11 Manuel Rego Casasnovas <rego@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.
+
</ins><span class="cx"> 2016-11-11 Antoine Quint <graouts@apple.com>
</span><span class="cx">
</span><span class="cx"> [Modern Media Controls] Media Controller: media tracks control support
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (208585 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1336,9 +1336,14 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool RenderBlock::canPerformSimplifiedLayout() const
+{
+ return (posChildNeedsLayout() || needsSimplifiedNormalFlowLayout()) && !normalChildNeedsLayout() && !selfNeedsLayout();
+}
+
</ins><span class="cx"> bool RenderBlock::simplifiedLayout()
</span><span class="cx"> {
</span><del>- if ((!posChildNeedsLayout() && !needsSimplifiedNormalFlowLayout()) || normalChildNeedsLayout() || selfNeedsLayout())
</del><ins>+ if (!canPerformSimplifiedLayout())
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> LayoutStateMaintainer statePusher(view(), *this, locationOffset(), hasTransform() || hasReflection() || style().isFlippedBlocksWritingMode());
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBlock.h (208585 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -360,6 +360,7 @@
</span><span class="cx">
</span><span class="cx"> virtual bool hasLineIfEmpty() const;
</span><span class="cx">
</span><ins>+ virtual bool canPerformSimplifiedLayout() const;
</ins><span class="cx"> bool simplifiedLayout();
</span><span class="cx"> virtual void simplifiedNormalFlowLayout();
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (208585 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -446,6 +446,16 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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();
+}
+
</ins><span class="cx"> void RenderGrid::layoutBlock(bool relayoutChildren, LayoutUnit)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(needsLayout());
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.h (208585 => 208586)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -110,6 +110,7 @@
</span><span class="cx"> GridTrackSizingDirection autoPlacementMajorAxisDirection() const;
</span><span class="cx"> GridTrackSizingDirection autoPlacementMinorAxisDirection() const;
</span><span class="cx">
</span><ins>+ bool canPerformSimplifiedLayout() const final;
</ins><span class="cx"> void prepareChildForPositionedLayout(RenderBox&);
</span><span class="cx"> void layoutPositionedObject(RenderBox&, bool relayoutChildren, bool fixedPositionObjectsOnly) override;
</span><span class="cx"> void offsetAndBreadthForPositionedChild(const RenderBox&, GridTrackSizingDirection, LayoutUnit& offset, LayoutUnit& breadth);
</span></span></pre>
</div>
</div>
</body>
</html>