Title: [225255] trunk
- Revision
- 225255
- Author
- [email protected]
- Date
- 2017-11-28 20:12:32 -0800 (Tue, 28 Nov 2017)
Log Message
Clean up spanners before creating nested column context
https://bugs.webkit.org/show_bug.cgi?id=180107
<rdar://problem/35686655>
Reviewed by Antti Koivisto.
Source/WebCore:
When an existing spanner placeholder is moved into a newly constructed (and nested)
multicolumn context, we figure it's not valid anymore and end up destroying it
(see RenderMultiColumnFlow::fragmentedFlowDescendantInserted).
This is very unfortunate since as we climb back on the stack, we could hit this renderer as
the newly inserted child.
This patch proactively removes the invalid placeholders and moves the associated spanners back to their
original position.
Test: fast/multicol/crash-when-constructing-nested-columns.html
* rendering/RenderMultiColumnFlow.h:
* style/RenderTreeUpdaterMultiColumn.cpp:
(WebCore::RenderTreeUpdater::MultiColumn::createFragmentedFlow):
RenderTreeUpdater::MultiColumn::destroyFragmentedFlow still relies on the placeholder removal
logic in RenderMultiColumnFlow::fragmentedFlowDescendantInserted.
LayoutTests:
* fast/multicol/crash-when-constructing-nested-columns-expected.txt: Added.
* fast/multicol/crash-when-constructing-nested-columns.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225254 => 225255)
--- trunk/LayoutTests/ChangeLog 2017-11-29 02:59:47 UTC (rev 225254)
+++ trunk/LayoutTests/ChangeLog 2017-11-29 04:12:32 UTC (rev 225255)
@@ -1,3 +1,14 @@
+2017-11-28 Zalan Bujtas <[email protected]>
+
+ Clean up spanners before creating nested column context
+ https://bugs.webkit.org/show_bug.cgi?id=180107
+ <rdar://problem/35686655>
+
+ Reviewed by Antti Koivisto.
+
+ * fast/multicol/crash-when-constructing-nested-columns-expected.txt: Added.
+ * fast/multicol/crash-when-constructing-nested-columns.html: Added.
+
2017-11-28 Wenson Hsieh <[email protected]>
Allow attachment elements with no appearance to defer rendering to child nodes
Added: trunk/LayoutTests/fast/multicol/crash-when-constructing-nested-columns-expected.txt (0 => 225255)
--- trunk/LayoutTests/fast/multicol/crash-when-constructing-nested-columns-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/multicol/crash-when-constructing-nested-columns-expected.txt 2017-11-29 04:12:32 UTC (rev 225255)
@@ -0,0 +1,2 @@
+PASS if no crash.
+
Added: trunk/LayoutTests/fast/multicol/crash-when-constructing-nested-columns.html (0 => 225255)
--- trunk/LayoutTests/fast/multicol/crash-when-constructing-nested-columns.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/crash-when-constructing-nested-columns.html 2017-11-29 04:12:32 UTC (rev 225255)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+PASS if no crash.
+<div style="columns: 2;"><div id=nestedColumn><div style="column-span:all;"></div></div></div>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+document.body.offsetTop;
+nestedColumn.style.columns = "2";
+</script>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (225254 => 225255)
--- trunk/Source/WebCore/ChangeLog 2017-11-29 02:59:47 UTC (rev 225254)
+++ trunk/Source/WebCore/ChangeLog 2017-11-29 04:12:32 UTC (rev 225255)
@@ -1,3 +1,28 @@
+2017-11-28 Zalan Bujtas <[email protected]>
+
+ Clean up spanners before creating nested column context
+ https://bugs.webkit.org/show_bug.cgi?id=180107
+ <rdar://problem/35686655>
+
+ Reviewed by Antti Koivisto.
+
+ When an existing spanner placeholder is moved into a newly constructed (and nested)
+ multicolumn context, we figure it's not valid anymore and end up destroying it
+ (see RenderMultiColumnFlow::fragmentedFlowDescendantInserted).
+ This is very unfortunate since as we climb back on the stack, we could hit this renderer as
+ the newly inserted child.
+
+ This patch proactively removes the invalid placeholders and moves the associated spanners back to their
+ original position.
+
+ Test: fast/multicol/crash-when-constructing-nested-columns.html
+
+ * rendering/RenderMultiColumnFlow.h:
+ * style/RenderTreeUpdaterMultiColumn.cpp:
+ (WebCore::RenderTreeUpdater::MultiColumn::createFragmentedFlow):
+ RenderTreeUpdater::MultiColumn::destroyFragmentedFlow still relies on the placeholder removal
+ logic in RenderMultiColumnFlow::fragmentedFlowDescendantInserted.
+
2017-11-28 Tim Horton <[email protected]>
REGRESSION (High Sierra): Layout Test fast/multicol/newmulticol/spanner2.html is a flaky image failure on WK1
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnFlow.h (225254 => 225255)
--- trunk/Source/WebCore/rendering/RenderMultiColumnFlow.h 2017-11-29 02:59:47 UTC (rev 225254)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnFlow.h 2017-11-29 04:12:32 UTC (rev 225255)
@@ -96,6 +96,7 @@
bool shouldCheckColumnBreaks() const override;
typedef HashMap<RenderBox*, WeakPtr<RenderMultiColumnSpannerPlaceholder>> SpannerMap;
+ SpannerMap& spannerMap() { return *m_spannerMap; }
std::unique_ptr<SpannerMap> takeSpannerMap() { return WTFMove(m_spannerMap); }
private:
@@ -119,8 +120,6 @@
void handleSpannerRemoval(RenderObject& spanner);
RenderObject* processPossibleSpannerDescendant(RenderObject*& subtreeRoot, RenderObject& descendant);
- SpannerMap& spannerMap() { return *m_spannerMap; }
-
private:
std::unique_ptr<SpannerMap> m_spannerMap;
Modified: trunk/Source/WebCore/style/RenderTreeUpdaterMultiColumn.cpp (225254 => 225255)
--- trunk/Source/WebCore/style/RenderTreeUpdaterMultiColumn.cpp 2017-11-29 02:59:47 UTC (rev 225254)
+++ trunk/Source/WebCore/style/RenderTreeUpdaterMultiColumn.cpp 2017-11-29 04:12:32 UTC (rev 225255)
@@ -49,10 +49,36 @@
void RenderTreeUpdater::MultiColumn::createFragmentedFlow(RenderBlockFlow& flow)
{
+ flow.setChildrenInline(false); // Do this to avoid wrapping inline children that are just going to move into the flow thread.
+ flow.deleteLines();
+ // If this soon-to-be multicolumn flow is already part of a multicolumn context, we need to move back the descendant spanners
+ // to their original position before moving subtrees around.
+ auto* enclosingflow = flow.enclosingFragmentedFlow();
+ if (is<RenderMultiColumnFlow>(enclosingflow)) {
+ auto& spanners = downcast<RenderMultiColumnFlow>(enclosingflow)->spannerMap();
+ Vector<RenderMultiColumnSpannerPlaceholder*> placeholdersToDelete;
+ for (auto& spannerAndPlaceholder : spanners) {
+ auto& placeholder = *spannerAndPlaceholder.value;
+ if (!placeholder.isDescendantOf(&flow))
+ continue;
+ placeholdersToDelete.append(&placeholder);
+ }
+ for (auto* placeholder : placeholdersToDelete) {
+ auto* spanner = placeholder->spanner();
+ if (!spanner) {
+ ASSERT_NOT_REACHED();
+ continue;
+ }
+ // Move the spanner back to its original position.
+ auto& spannerOriginalParent = *placeholder->parent();
+ // Detaching the spanner takes care of removing the placeholder (and merges the RenderMultiColumnSets).
+ auto spannerToReInsert = spanner->parent()->takeChild(*spanner);
+ spannerOriginalParent.addChild(WTFMove(spannerToReInsert));
+ }
+ }
+
auto newFragmentedFlow = WebCore::createRenderer<RenderMultiColumnFlow>(flow.document(), RenderStyle::createAnonymousStyleWithDisplay(flow.style(), BLOCK));
newFragmentedFlow->initializeStyle();
- flow.setChildrenInline(false); // Do this to avoid wrapping inline children that are just going to move into the flow thread.
- flow.deleteLines();
auto& fragmentedFlow = *newFragmentedFlow;
flow.RenderBlock::addChild(WTFMove(newFragmentedFlow));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
