Nullptr deref in WebCore::RenderTreeBuilder::Block::attachIgnoringContinuation when...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 04:03:20 +0000 (04:03 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 04:03:20 +0000 (04:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212116
<rdar://problem/62993844>

Reviewed by Simon Fraser.

Source/WebCore:

This patch fixes the case when a nested fragmented context has a spanner and we try to form a continuation while this nested fragmented context is being destroyed.

1. The continuation is triggered by a style change that turns a previously out-of-flow block container into an inflow box
(and the parent inline level container can't have the box as a direct child anymore).
2. An unrelated style change nukes the nested fragmented context. We need to "re-assign" the spanner to the parent fragment.

These 2 changes are split into 2 phases; first we take care of the tree mutation triggered by the continuation (updateRendererStyle), while
we do the fragmented context cleanup (updateAfterDescendants) in a separate step.
This 2 phase setup confuses the "where to put this spanner" logic.

This patch addresses the issue by keeping the spanner inside the about-to-be-destroyed fragmented context while forming the continuation (phase #1) and let the second phase (updateAfterDescendants)
deal with the spanner moving.

Test: fast/multicol/nested-multicol-with-spanner-and-continuation.html

* rendering/updating/RenderTreeBuilderMultiColumn.cpp:
(WebCore::isValidColumnSpanner):

LayoutTests:

* fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt: Added.
* fast/multicol/nested-multicol-with-spanner-and-continuation.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt [new file with mode: 0644]
LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp

index 0ab3b50..9b153df 100644 (file)
@@ -1,3 +1,14 @@
+2020-05-22  Zalan Bujtas  <zalan@apple.com>
+
+        Nullptr deref in WebCore::RenderTreeBuilder::Block::attachIgnoringContinuation when parent and beforeChild are siblings
+        https://bugs.webkit.org/show_bug.cgi?id=212116
+        <rdar://problem/62993844>
+
+        Reviewed by Simon Fraser.
+
+        * fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt: Added.
+        * fast/multicol/nested-multicol-with-spanner-and-continuation.html: Added.
+
 2020-05-22  Kenneth Russell  <kbr@chromium.org>
 
         [ANGLE - iOS] webgl/1.0.3/conformance/extensions/ext-sRGB.html is failing
diff --git a/LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt b/LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt
new file mode 100644 (file)
index 0000000..4179f59
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if no crash when the nested fragmented context is being destroyed with a spanner in it while forming a new continuation.
+
diff --git a/LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation.html b/LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation.html
new file mode 100644 (file)
index 0000000..bc24318
--- /dev/null
@@ -0,0 +1,17 @@
+<style>
+.container { 
+    columns: 2;
+}
+#innerContainer { 
+    position: absolute;
+    columns: 2;
+}
+</style>
+PASS if no crash when the nested fragmented context is being destroyed with a spanner in it while forming a new continuation.
+<div class=container><span><div id=innerContainer><div style="column-span: all;"></div></div></span></div><script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+document.body.offsetHeight;
+innerContainer.style.position = "static";
+innerContainer.style.webkitColumns = "1";
+</script>
index 8f04a82..045beb2 100644 (file)
@@ -1,3 +1,29 @@
+2020-05-22  Zalan Bujtas  <zalan@apple.com>
+
+        Nullptr deref in WebCore::RenderTreeBuilder::Block::attachIgnoringContinuation when parent and beforeChild are siblings
+        https://bugs.webkit.org/show_bug.cgi?id=212116
+        <rdar://problem/62993844>
+
+        Reviewed by Simon Fraser.
+
+        This patch fixes the case when a nested fragmented context has a spanner and we try to form a continuation while this nested fragmented context is being destroyed.
+
+        1. The continuation is triggered by a style change that turns a previously out-of-flow block container into an inflow box
+        (and the parent inline level container can't have the box as a direct child anymore).
+        2. An unrelated style change nukes the nested fragmented context. We need to "re-assign" the spanner to the parent fragment.
+
+        These 2 changes are split into 2 phases; first we take care of the tree mutation triggered by the continuation (updateRendererStyle), while
+        we do the fragmented context cleanup (updateAfterDescendants) in a separate step.
+        This 2 phase setup confuses the "where to put this spanner" logic.
+
+        This patch addresses the issue by keeping the spanner inside the about-to-be-destroyed fragmented context while forming the continuation (phase #1) and let the second phase (updateAfterDescendants)
+        deal with the spanner moving.
+
+        Test: fast/multicol/nested-multicol-with-spanner-and-continuation.html
+
+        * rendering/updating/RenderTreeBuilderMultiColumn.cpp:
+        (WebCore::isValidColumnSpanner):
+
 2020-05-22  Chris Dumez  <cdumez@apple.com>
 
         Revoking an object URL immediately after triggering navigation causes navigation to fail
index 8c1cf06..fb69567 100644 (file)
@@ -107,9 +107,18 @@ static bool isValidColumnSpanner(const RenderMultiColumnFlow& fragmentedFlow, co
             // implement (not to mention specify behavior).
             return ancestor == &fragmentedFlow;
         }
-        // This ancestor (descendent of the fragmentedFlow) will create columns later. The spanner belongs to it.
-        if (is<RenderBlockFlow>(*ancestor) && downcast<RenderBlockFlow>(*ancestor).willCreateColumns())
-            return false;
+        if (is<RenderBlockFlow>(*ancestor)) {
+            auto& blockFlowAncestor = downcast<RenderBlockFlow>(*ancestor);
+            if (blockFlowAncestor.willCreateColumns()) {
+                // This ancestor (descendent of the fragmentedFlow) will create columns later. The spanner belongs to it.
+                return false;
+            }
+            if (blockFlowAncestor.multiColumnFlow()) {
+                // While this ancestor (descendent of the fragmentedFlow) has a fragmented flow context, this context is being destroyed.
+                // However the spanner still belongs to it (will most likely be moved to the parent fragmented context as the next step).
+                return false;
+            }
+        }
         ASSERT(ancestor->style().columnSpan() != ColumnSpan::All || !isValidColumnSpanner(fragmentedFlow, *ancestor));
         if (ancestor->isUnsplittableForPagination())
             return false;