Cherry-pick 260286.15@webkit-2023.2-embargoed (028f984). https://bugs…

….webkit.org/show_bug.cgi?id=245374 Fix spanner reset logic https://bugs.webkit.org/show_bug.cgi?id=245374 Reviewed by Alan Baradlay. In restoreColumnSpannersForContainer we want to reset the spanners to their original position and remove the placeholders, however in some cases the attach step will call multiColumnDescendantInserted and re-insert placeholders. To fix this, prevent calling the spanner processing logic by multiColumnDescendantInserted by introducing a new flag gRestoringColumnSpannersForContainer. * LayoutTests/fast/multicol/crash-when-constructing-nested-columns2-expected.txt: Added. * LayoutTests/fast/multicol/crash-when-constructing-nested-columns2.html: Added. * Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp: (WebCore::RenderTreeBuilder::MultiColumn::restoreColumnSpannersForContainer): (WebCore::RenderTreeBuilder::MultiColumn::multiColumnDescendantInserted): (WebCore::RenderTreeBuilder::MultiColumn::processPossibleSpannerDescendant): Canonical link: https://commits.webkit.org/260286.15@webkit-2023.2-embargoed Canonical link: https://commits.webkit.org/263022.7@webkit-2023.4-embargoed
WebKit · Apr 17, 2023 · c5710a2 · c5710a2
1 parent 8efd98e
commit c5710a2
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 3 deletions.
diff --git a/LayoutTests/fast/multicol/crash-when-constructing-nested-columns2-expected.txt b/LayoutTests/fast/multicol/crash-when-constructing-nested-columns2-expected.txt
@@ -0,0 +1,2 @@
+PASS if no crash.
+
diff --git a/LayoutTests/fast/multicol/crash-when-constructing-nested-columns2.html b/LayoutTests/fast/multicol/crash-when-constructing-nested-columns2.html
@@ -0,0 +1,17 @@
+<style>
+dialog { -webkit-mask-box-image-source: url(about:url); }
+div { column-span: all; }
+html { overflow: -webkit-paged-x; -webkit-perspective: 0vw; }
+</style>
+<script>
+if (window.testRunner)
+  testRunner.dumpAsText();
+function runTest() {
+  dialog.appendChild(document.createElement("div"));
+  target.style.setProperty("overflow-wrap", "break-word");
+  target.style.setProperty("-webkit-column-width", "1px");
+}
+</script>
+<body id="target" onload=runTest()>
+  <dialog id="dialog" open="true">PASS if no crash.</dialog>
+</body>
diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp
@@ -33,6 +33,7 @@
 #include "RenderTreeBuilder.h"
 #include "RenderTreeBuilderBlock.h"
 #include "RenderView.h"
+#include <wtf/SetForScope.h>
 
 namespace WebCore {
 
@@ -177,6 +178,8 @@ void RenderTreeBuilder::MultiColumn::createFragmentedFlow(RenderBlockFlow& flow)
     flow.setMultiColumnFlow(fragmentedFlow);
 }
 
+static bool gRestoringColumnSpannersForContainer = false;
+
 void RenderTreeBuilder::MultiColumn::restoreColumnSpannersForContainer(const RenderElement& container, RenderMultiColumnFlow& multiColumnFlow)
 {
     auto& spanners = multiColumnFlow.spannerMap();
@@ -197,6 +200,7 @@ void RenderTreeBuilder::MultiColumn::restoreColumnSpannersForContainer(const Ren
         auto& spannerOriginalParent = *placeholder->parent();
         // Detaching the spanner takes care of removing the placeholder (and merges the RenderMultiColumnSets).
         auto spannerToReInsert = m_builder.detach(*spanner->parent(), *spanner);
+        auto restoreColumnSpannersScope = SetForScope { gRestoringColumnSpannersForContainer, true };
         m_builder.attach(spannerOriginalParent, WTFMove(spannerToReInsert));
     }
 }
@@ -278,7 +282,7 @@ static bool gShiftingSpanner = false;
 
 void RenderTreeBuilder::MultiColumn::multiColumnDescendantInserted(RenderMultiColumnFlow& flow, RenderObject& newDescendant)
 {
-    if (gShiftingSpanner || newDescendant.isRenderFragmentedFlow())
+    if (gShiftingSpanner || gRestoringColumnSpannersForContainer || newDescendant.isRenderFragmentedFlow())
         return;
 
     auto* subtreeRoot = &newDescendant;
@@ -341,9 +345,8 @@ RenderObject* RenderTreeBuilder::MultiColumn::processPossibleSpannerDescendant(R
         auto takenDescendant = m_builder.detach(*container, descendant);
 
         // This is a guard to stop an ancestor flow thread from processing the spanner.
-        gShiftingSpanner = true;
+        auto shiftingSpannerScope = SetForScope { gShiftingSpanner, true };
         m_builder.blockBuilder().attach(*multicolContainer, WTFMove(takenDescendant), insertBeforeMulticolChild);
-        gShiftingSpanner = false;
 
         // The spanner has now been moved out from the flow thread, but we don't want to
         // examine its children anyway. They are all part of the spanner and shouldn't trigger