Crash in RenderTreeBuilder::Table::findOrCreateParentForChild with multicol spanner
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Feb 2020 14:10:25 +0000 (14:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Feb 2020 14:10:25 +0000 (14:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=206917

Patch by Doug Kelly <dougk@apple.com> on 2020-02-08
Reviewed by Zalan Bujtas.

Source/WebCore:

During render tree construction, multi-column spanners are moved from their original tree position to
next to the enclosing anonymous fragmented flow and we mark their previous location with a spanner placeholder.
If we try to add a new renderer right before the spanner (beforeChild is the spanner renderer), we need to use
the spanner's original position as the insertion point.
This patch addresses the mismatching position issue by adjusting the spanner beforeChild right before
we start searching for the final insertion point.

Test: fast/multicol/spanner-crash-when-finding-table-parent.html

* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::attach):
* rendering/updating/RenderTreeBuilderTable.cpp:
(WebCore::RenderTreeBuilder::Table::findOrCreateParentForChild):

LayoutTests:

* fast/multicol/spanner-crash-when-finding-table-parent-expected.txt: Added.
* fast/multicol/spanner-crash-when-finding-table-parent.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/multicol/spanner-crash-when-finding-table-parent-expected.txt [new file with mode: 0644]
LayoutTests/fast/multicol/spanner-crash-when-finding-table-parent.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
Source/WebCore/rendering/updating/RenderTreeBuilderTable.cpp

index 65a6530..bf7447f 100644 (file)
@@ -1,3 +1,13 @@
+2020-02-08  Doug Kelly  <dougk@apple.com>
+
+        Crash in RenderTreeBuilder::Table::findOrCreateParentForChild with multicol spanner
+        https://bugs.webkit.org/show_bug.cgi?id=206917
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/multicol/spanner-crash-when-finding-table-parent-expected.txt: Added.
+        * fast/multicol/spanner-crash-when-finding-table-parent.html: Added.
+
 2020-02-07  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS] Double tapping shouldn't scroll the page when the body has `overflow: hidden`
diff --git a/LayoutTests/fast/multicol/spanner-crash-when-finding-table-parent-expected.txt b/LayoutTests/fast/multicol/spanner-crash-when-finding-table-parent-expected.txt
new file mode 100644 (file)
index 0000000..fec1728
--- /dev/null
@@ -0,0 +1 @@
+This test verifies that adding an element which is a sibling to a multicol spanner finds the correct table row. Test passes if WebKit does not crash. PASS
diff --git a/LayoutTests/fast/multicol/spanner-crash-when-finding-table-parent.html b/LayoutTests/fast/multicol/spanner-crash-when-finding-table-parent.html
new file mode 100644 (file)
index 0000000..95fce18
--- /dev/null
@@ -0,0 +1,21 @@
+<style>
+body {
+    display: table-header-group;
+    overflow-y: -webkit-paged-x;
+}
+div {
+    column-span: all;
+}
+</style>
+<body>
+    <span id=span></span>
+    <div></div>
+    <script>
+        document.body.offsetHeight;
+        span.outerText = "remove";
+        document.body.offsetHeight;
+        document.body.innerText = "This test verifies that adding an element which is a sibling to a multicol spanner finds the correct table row. Test passes if WebKit does not crash. PASS";
+        if (window.testRunner)
+            testRunner.dumpAsText();
+    </script>
+</body>
index 51e545a..ae63801 100644 (file)
@@ -1,3 +1,24 @@
+2020-02-08  Doug Kelly  <dougk@apple.com>
+
+        Crash in RenderTreeBuilder::Table::findOrCreateParentForChild with multicol spanner
+        https://bugs.webkit.org/show_bug.cgi?id=206917
+
+        Reviewed by Zalan Bujtas.
+
+        During render tree construction, multi-column spanners are moved from their original tree position to
+        next to the enclosing anonymous fragmented flow and we mark their previous location with a spanner placeholder.
+        If we try to add a new renderer right before the spanner (beforeChild is the spanner renderer), we need to use
+        the spanner's original position as the insertion point.
+        This patch addresses the mismatching position issue by adjusting the spanner beforeChild right before
+        we start searching for the final insertion point.
+
+        Test: fast/multicol/spanner-crash-when-finding-table-parent.html
+
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::RenderTreeBuilder::attach):
+        * rendering/updating/RenderTreeBuilderTable.cpp:
+        (WebCore::RenderTreeBuilder::Table::findOrCreateParentForChild):
+
 2020-02-07  Jon Lee  <jonlee@apple.com>
 
         Web Inspector: Revert slim toolbar
index 1871808..6fed50b 100644 (file)
@@ -38,6 +38,7 @@
 #include "RenderMathMLFenced.h"
 #include "RenderMenuList.h"
 #include "RenderMultiColumnFlow.h"
+#include "RenderMultiColumnSpannerPlaceholder.h"
 #include "RenderRuby.h"
 #include "RenderRubyBase.h"
 #include "RenderRubyRun.h"
@@ -208,6 +209,20 @@ void RenderTreeBuilder::attach(RenderElement& parent, RenderPtr<RenderObject> ch
     if (is<RenderText>(beforeChild)) {
         if (auto* wrapperInline = downcast<RenderText>(*beforeChild).inlineWrapperForDisplayContents())
             beforeChild = wrapperInline;
+    } else if (is<RenderBox>(beforeChild)) {
+        // Adjust the beforeChild if it happens to be a spanner and the its actual location is inside the fragmented flow.
+        auto& beforeChildBox = downcast<RenderBox>(*beforeChild);
+        if (auto* enclosingFragmentedFlow = parent.enclosingFragmentedFlow()) {
+            auto columnSpannerPlaceholderForBeforeChild = [&]() -> RenderMultiColumnSpannerPlaceholder* {
+                if (!is<RenderMultiColumnFlow>(enclosingFragmentedFlow))
+                    return nullptr;
+                auto& multiColumnFlow = downcast<RenderMultiColumnFlow>(*enclosingFragmentedFlow);
+                return multiColumnFlow.findColumnSpannerPlaceholder(&beforeChildBox);
+            };
+
+            if (auto* spannerPlaceholder = columnSpannerPlaceholderForBeforeChild())
+                beforeChild = spannerPlaceholder;
+        }
     }
 
     if (is<RenderTableRow>(parent)) {
index 18dd26c..5aa18d1 100644 (file)
@@ -107,7 +107,7 @@ RenderElement& RenderTreeBuilder::Table::findOrCreateParentForChild(RenderTableS
     // If beforeChild is inside an anonymous cell/row, insert into the cell or into
     // the anonymous row containing it, if there is one.
     auto* parentCandidate = lastChild;
-    while (parentCandidate && parentCandidate->parent()->isAnonymous() && !is<RenderTableRow>(*parentCandidate))
+    while (parentCandidate && parentCandidate->parent() && parentCandidate->parent()->isAnonymous() && !is<RenderTableRow>(*parentCandidate))
         parentCandidate = parentCandidate->parent();
     if (is<RenderTableRow>(parentCandidate) && parentCandidate->isAnonymous() && !parentCandidate->isBeforeOrAfterContent())
         return downcast<RenderElement>(*parentCandidate);