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.
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
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 0ab3b50..9b153df 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -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
index 0000000..4179f59
--- /dev/null
+++ b/LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt
@@ -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
index 0000000..bc24318
--- /dev/null
+++ b/LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation.html
@@ -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>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 8f04a82..045beb2 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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
diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp
index 8c1cf06..fb69567 100644
--- a/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp
+++ b/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp
@@ -107,9 +107,18 @@
// 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;
