Changeset 214039 in webkit

Timestamp:

Mar 16, 2017 6:13:01 AM (7 years ago)

Author:

Manuel Rego Casasnovas

Message:

[css-grid] Crash on debug removing a positioned child
​https://bugs.webkit.org/show_bug.cgi?id=169739

Reviewed by Sergio Villar Senin.

Source/WebCore:

When we add or remove a positioned item we don't need to mark
the grid as dirty, because positioned items do not affect the layout
of the grid at all.

This was causing a crash when a positioned item was removed
after a layout. As after the positioned item was removed,
the method RenderGrid::layoutBlock() was not called,
so when the grid was repainted we got a crash.

Test: fast/css-grid-layout/grid-crash-remove-positioned-item.html

• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::addChild): Add early return to avoid marking
the grid as dirty for positioned grid items.
(WebCore::RenderGrid::removeChild): Ditto.

LayoutTests:

Add new test that checks that adding and removing a positioned grid item
doesn't cause any crashes.

• fast/css-grid-layout/grid-crash-remove-positioned-item-expected.txt: Added.
• fast/css-grid-layout/grid-crash-remove-positioned-item.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item-expected.txt (added)
• LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-03-16  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Crash on debug removing a positioned child
+        https://bugs.webkit.org/show_bug.cgi?id=169739
+
+        Reviewed by Sergio Villar Senin.
+
+        Add new test that checks that adding and removing a positioned grid item
+        doesn't cause any crashes.
+
+        * fast/css-grid-layout/grid-crash-remove-positioned-item-expected.txt: Added.
+        * fast/css-grid-layout/grid-crash-remove-positioned-item.html: Added.
+
 2017-03-16  Caio Lima  <ticaiolima@gmail.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-03-16  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Crash on debug removing a positioned child
+        https://bugs.webkit.org/show_bug.cgi?id=169739
+
+        Reviewed by Sergio Villar Senin.
+
+        When we add or remove a positioned item we don't need to mark
+        the grid as dirty, because positioned items do not affect the layout
+        of the grid at all.
+
+        This was causing a crash when a positioned item was removed
+        after a layout. As after the positioned item was removed,
+        the method RenderGrid::layoutBlock() was not called,
+        so when the grid was repainted we got a crash.
+
+        Test: fast/css-grid-layout/grid-crash-remove-positioned-item.html
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::addChild): Add early return to avoid marking
+        the grid as dirty for positioned grid items.
+        (WebCore::RenderGrid::removeChild): Ditto.
+
 2017-03-16  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -70 +70 @@
     RenderBlock::addChild(newChild, beforeChild);
 
+    // Positioned grid items do not take up space or otherwise participate in the layout of the grid,
+    // for that reason we don't need to mark the grid as dirty when they are added.
+    if (newChild->isOutOfFlowPositioned())
+        return;
+
     // The grid needs to be recomputed as it might contain auto-placed items that
     // will change their position.
@@ -78 +83 @@
 {
     RenderBlock::removeChild(child);
+
+    // Positioned grid items do not take up space or otherwise participate in the layout of the grid,
+    // for that reason we don't need to mark the grid as dirty when they are removed.
+    if (child.isOutOfFlowPositioned())
+        return;
 
     // The grid needs to be recomputed as it might contain auto-placed items that