[css-grid] Crash on debug changing the style of a positioned element
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 08:17:10 +0000 (08:17 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 08:17:10 +0000 (08:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191473

Reviewed by Dean Jackson and Zalan Bujtas.

Source/WebCore:

When an box becomes {out-of,in}-flow, it may be re-parented and it may become a grid
item. In that case, we must mark the RenderGrid as dirty, so that the grid items
placement logic is executed again.

Test: fast/css-grid-layout/grid-crash-out-of-flow-positioned-element.html

* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::childFlowStateChangesAndAffectsParentBlock): Consider the case of a box's new parent being a grid container.

LayoutTests:

Regression test to ensure that the grid placement logic is executed
when a positioned item becomes a grid item.

* fast/css-grid-layout/grid-crash-out-of-flow-positioned-element-expected.txt:
* fast/css-grid-layout/grid-crash-out-of-flow-positioned-element.html:
* TestExpectations: Remove a Skip entry, since the test doesn't crash anymore.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238888 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/css-grid-layout/grid-crash-out-of-flow-positioned-element-expected.txt
LayoutTests/fast/css-grid-layout/grid-crash-out-of-flow-positioned-element.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/updating/RenderTreeBuilder.cpp

index 2eff5fd..4606e2c 100644 (file)
@@ -1,3 +1,17 @@
+2018-12-05  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Crash on debug changing the style of a positioned element
+        https://bugs.webkit.org/show_bug.cgi?id=191473
+
+        Reviewed by Dean Jackson and Zalan Bujtas.
+
+        Regression test to ensure that the grid placement logic is executed
+        when a positioned item becomes a grid item.
+
+        * fast/css-grid-layout/grid-crash-out-of-flow-positioned-element-expected.txt:
+        * fast/css-grid-layout/grid-crash-out-of-flow-positioned-element.html:
+        * TestExpectations: Remove a Skip entry, since the test doesn't crash anymore.
+
 2018-12-04  Simon Fraser  <simon.fraser@apple.com>
 
         Attempt to de-flake this test by scrolling a bit more.
index e90ed88..7400ae2 100644 (file)
@@ -617,7 +617,6 @@ webkit.org/b/191461 imported/w3c/web-platform-tests/css/css-grid/grid-items/perc
 webkit.org/b/191462 imported/w3c/web-platform-tests/css/css-grid/grid-items/percentage-size-replaced-subitems-001.html [ ImageOnlyFailure ]
 webkit.org/b/191463 imported/w3c/web-platform-tests/css/css-grid/grid-items/explicitly-sized-grid-item-as-table.html
 webkit.org/b/191627 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001.html [ Failure ]
-webkit.org/b/191473 fast/css-grid-layout/grid-crash-out-of-flow-positioned-element.html [ Skip ]
 webkit.org/b/149890 fast/css-grid-layout/grid-shorthands-style-format.html [ Failure ]
 webkit.org/b/191506 fast/css-grid-layout/grid-item-scroll-position.html [ Failure ]
 webkit.org/b/191507 fast/css-grid-layout/positioned-grid-container-percentage-tracks.html [ Failure ]
index 0135769..e72824c 100644 (file)
@@ -3,15 +3,14 @@
 <style>
 .absolutelyPositioned { position: absolute; }
 </style>
-crbug.com/280451 - Heap-use-after-free in WebCore::LayoutGrid::computePreferredTrackWidth</br>
 This test has passed if it didn't crash.
 <script>
 if (window.testRunner)
     testRunner.dumpAsText();
 
 var cell = document.createElement("cell");
-cell.setAttribute("class", "absolutelyPositioned");
+cell.classList.add("absolutelyPositioned");
 document.body.appendChild(cell);
-window.scrollBy(98, 28);
-cell.setAttribute("class", "nonExistent");
+document.body.offsetLeft;
+cell.classList.remove("absolutelyPositioned");
 </script>
index 438c7e7..4a8106b 100644 (file)
@@ -1,3 +1,19 @@
+2018-12-05  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Crash on debug changing the style of a positioned element
+        https://bugs.webkit.org/show_bug.cgi?id=191473
+
+        Reviewed by Dean Jackson and Zalan Bujtas.
+
+        When an box becomes {out-of,in}-flow, it may be re-parented and it may become a grid
+        item. In that case, we must mark the RenderGrid as dirty, so that the grid items
+        placement logic is executed again.
+
+        Test: fast/css-grid-layout/grid-crash-out-of-flow-positioned-element.html
+
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::childFlowStateChangesAndAffectsParentBlock): Consider the case of a box's new parent being a grid container.
+
 2018-12-04  Frederic Wang  <fwang@igalia.com>
 
         Always pass scrollingGeometry to update*ScrollingNode functions
index 1f9faa6..214f47d 100644 (file)
@@ -653,6 +653,13 @@ void RenderTreeBuilder::childFlowStateChangesAndAffectsParentBlock(RenderElement
             blockBuilder().childBecameNonInline(downcast<RenderBlock>(*parent), child);
         else if (is<RenderInline>(*parent))
             inlineBuilder().childBecameNonInline(downcast<RenderInline>(*parent), child);
+
+        // childBecameNonInline might have re-parented us.
+        if (auto* newParent = child.parent()) {
+            // We need to re-run the grid items placement if it had gained a new item.
+            if (newParent != parent && is<RenderGrid>(*newParent))
+                downcast<RenderGrid>(*newParent).dirtyGrid();
+        }
     } else {
         // An anonymous block must be made to wrap this inline.
         auto newBlock = downcast<RenderBlock>(*parent).createAnonymousBlock();