Changeset 208586 in webkit

Timestamp:

Nov 11, 2016 3:08:39 AM (7 years ago)

Author:

Manuel Rego Casasnovas

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html (added)
• LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html (added)
• LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html (added)
• LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.h (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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.
+
 2016-11-11  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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.
+
 2016-11-11  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -1337 +1337 @@
 }
 
+bool RenderBlock::canPerformSimplifiedLayout() const
+{
+    return (posChildNeedsLayout() || needsSimplifiedNormalFlowLayout()) && !normalChildNeedsLayout() && !selfNeedsLayout();
+}
+
 bool RenderBlock::simplifiedLayout()
 {
-    if ((!posChildNeedsLayout() && !needsSimplifiedNormalFlowLayout()) || normalChildNeedsLayout() || selfNeedsLayout())
+    if (!canPerformSimplifiedLayout())
         return false;
 

trunk/Source/WebCore/rendering/RenderBlock.h

@@ -361 +361 @@
     virtual bool hasLineIfEmpty() const;
     
+    virtual bool canPerformSimplifiedLayout() const;
     bool simplifiedLayout();
     virtual void simplifiedNormalFlowLayout();

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -445 +445 @@
         computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows);
     }
+}
+
+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();
 }
 

trunk/Source/WebCore/rendering/RenderGrid.h

@@ -111 +111 @@
     GridTrackSizingDirection autoPlacementMinorAxisDirection() const;
 
+    bool canPerformSimplifiedLayout() const final;
     void prepareChildForPositionedLayout(RenderBox&);
     void layoutPositionedObject(RenderBox&, bool relayoutChildren, bool fixedPositionObjectsOnly) override;