Title: [214756] releases/WebKitGTK/webkit-2.16
Revision
214756
Author
carlo...@webkit.org
Date
2017-04-03 03:05:29 -0700 (Mon, 03 Apr 2017)

Log Message

Merge r214039 - [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.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (214755 => 214756)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog	2017-04-03 09:59:57 UTC (rev 214755)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog	2017-04-03 10:05:29 UTC (rev 214756)
@@ -1,3 +1,16 @@
+2017-03-16  Manuel Rego Casasnovas  <r...@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-15  Zalan Bujtas  <za...@apple.com>
 
         Do not reparent floating object until after intruding/overhanging dependency is cleared.

Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item-expected.txt (0 => 214756)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item-expected.txt	2017-04-03 10:05:29 UTC (rev 214756)
@@ -0,0 +1,5 @@
+webkit.org/b/169739 - [css-grid] Crash on debug removing a positioned child
+
+This test has PASSED if it does not CRASH on debug.
+
+item

Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item.html (0 => 214756)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/css-grid-layout/grid-crash-remove-positioned-item.html	2017-04-03 10:05:29 UTC (rev 214756)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<script>
+  if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<p>webkit.org/b/169739 - [css-grid] Crash on debug removing a positioned child</p>
+<p>This test has PASSED if it does not CRASH on debug.</p>
+<div id="grid" style="display: grid;">
+  <!-- This grid item with some text is needed, otherwise RenderGrid::paintChildren()
+       won't be called after removing the positioned item. -->
+  <div>item</div>
+</div>
+<script>
+  var abspositem = document.createElement("div");
+  abspositem.style.position = "absolute";
+  var grid = document.getElementById("grid");
+  grid.appendChild(abspositem);
+  document.body.offsetLeft;
+  grid.removeChild(abspositem);
+</script>

Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (214755 => 214756)


--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog	2017-04-03 09:59:57 UTC (rev 214755)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog	2017-04-03 10:05:29 UTC (rev 214756)
@@ -1,3 +1,26 @@
+2017-03-16  Manuel Rego Casasnovas  <r...@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-15  Zalan Bujtas  <za...@apple.com>
 
         Do not reparent floating object until after intruding/overhanging dependency is cleared.

Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/rendering/RenderGrid.cpp (214755 => 214756)


--- releases/WebKitGTK/webkit-2.16/Source/WebCore/rendering/RenderGrid.cpp	2017-04-03 09:59:57 UTC (rev 214755)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/rendering/RenderGrid.cpp	2017-04-03 10:05:29 UTC (rev 214756)
@@ -109,6 +109,11 @@
 {
     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.
     dirtyGrid();
@@ -118,6 +123,11 @@
 {
     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
     // will change their position.
     dirtyGrid();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to