<rdar://62993844> 0 com.apple.WebCore 0x0000000107af46f3 WebCore::RenderTreeBuilder::Block::attachIgnoringContinuation(WebCore::RenderBlock&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 51 1 com.apple.WebCore 0x0000000107af3e15 WebCore::RenderTreeBuilder::Block::attach(WebCore::RenderBlock&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 421 2 com.apple.WebCore 0x0000000107af3bfa WebCore::RenderTreeBuilder::BlockFlow::attach(WebCore::RenderBlockFlow&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 426 3 com.apple.WebCore 0x0000000107af3836 WebCore::RenderTreeBuilder::attachInternal(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 2278 4 com.apple.WebCore 0x0000000107af3b89 WebCore::RenderTreeBuilder::BlockFlow::attach(WebCore::RenderBlockFlow&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 313 5 com.apple.WebCore 0x0000000107af3836 WebCore::RenderTreeBuilder::attachInternal(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 2278 6 com.apple.WebCore 0x0000000107afe8bc WebCore::RenderTreeBuilder::MultiColumn::multiColumnDescendantInserted(WebCore::RenderMultiColumnFlow&, WebCore::RenderObject&) + 2476 7 com.apple.WebCore 0x0000000107af6ae3 WebCore::RenderTreeBuilder::attachToRenderElementInternal(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 355 8 com.apple.WebCore 0x0000000107af4071 WebCore::RenderTreeBuilder::attachToRenderElement(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 353 9 com.apple.WebCore 0x0000000107af48df WebCore::RenderTreeBuilder::Block::attachIgnoringContinuation(WebCore::RenderBlock&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 543 10 com.apple.WebCore 0x0000000107af3e15 WebCore::RenderTreeBuilder::Block::attach(WebCore::RenderBlock&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 421 11 com.apple.WebCore 0x0000000107af3bfa WebCore::RenderTreeBuilder::BlockFlow::attach(WebCore::RenderBlockFlow&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 426 12 com.apple.WebCore 0x0000000107af3836 WebCore::RenderTreeBuilder::attachInternal(WebCore::RenderElement&, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderObject*) + 2278 13 com.apple.WebCore 0x0000000107afba88 WebCore::RenderTreeBuilder::Inline::splitFlow(WebCore::RenderInline&, WebCore::RenderObject*, std::__1::unique_ptr<WebCore::RenderBlock, WebCore::RenderObjectDeleter>, std::__1::unique_ptr<WebCore::RenderObject, WebCore::RenderObjectDeleter>, WebCore::RenderBoxModelObject*) + 2200 14 com.apple.WebCore 0x0000000107af742e WebCore::RenderTreeBuilder::normalizeTreeAfterStyleChange(WebCore::RenderElement&, WebCore::RenderStyle&) + 1166 15 com.apple.WebCore 0x0000000107b05754 WebCore::RenderTreeUpdater::updateRendererStyle(WebCore::RenderElement&, WebCore::RenderStyle&&, WebCore::StyleDifference) + 164 16 com.apple.WebCore 0x0000000107b0508d WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&) + 1293 17 com.apple.WebCore 0x0000000107b03634 WebCore::RenderTreeUpdater::commit(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >) + 2404 18 com.apple.WebCore 0x00000001070fa635 WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType) + 1381 19 com.apple.WebCore 0x0000000105dd9154 WebCore::Document::updateStyleIfNeeded() + 468 20 com.apple.WebCore 0x0000000105e4fc68 WebCore::Document::updateLayout() + 360
Test case: <style> ol { position: fixed; } ol, body { -webkit-columns: 2; } </style> <script> function run() { q.appendChild(ol); window.requestIdleCallback(idleCallback); } function idleCallback() { range = document.createElement("main").ownerDocument.createRange(); range.setEndBefore(li); range.extractContents(); } </script> <body onload=run()><ol id=ol><li id=li style="border-top-width: medium; column-span: all;"></ol><q id=q></q>
Minimized test case: <style> ol { position: fixed; } ol, body { -webkit-columns: 2; } </style> <script> function run() { range = document.createElement("main").ownerDocument.createRange(); range.setEndBefore(li); range.extractContents(); } </script> <body onload=run()><q><ol><li id=li style="column-span: all;"></ol></q>
Cause of the crash: 1. After the document is loaded, updateStyleIfNeeded is called and render tree is being updated. 2. When walking through the nodes in updateRenderTree(), we find that OL is not inline and needs to be splitter and be attached to a RenderBlock. 3. In RenderTreeBuilder::BlockFlow::attach() we find that OL has multiColumnFlow, so we call RenderTreeBuilder::Block::attachIgnoringContinuation() and pass the RenderMultiColumnFlowThread in OL as parent and RenderMultiColumnSet as before child. 4. In Block::attachIgnoringContinuation(), we try to find the immediate child of parent by looking at ancestors of RenderMultiColumnSet. 5. However, since RenderMultiColumnFlowThread and RenderMultiColumnSet are siblings, the logic exhausts the render tree and deref nullptr. Step 2: childBecameNonInline: parent=0x542fe00b0, child=0x542fe0400, beforeChild=0x542fe0c30 (B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y, (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer, (C)omposited, (+)Dirty style, (+)Dirty layout B---YGLC -+ RenderView at (0,0) size 0x0 renderer->(0x542f5d210) layout->[normal child][positioned child] B-----L- -+ HTML RenderBlock at (0,0) size 0x0 renderer->(0x542f5d6c0) node->(0x542f5f800) layout->[self][normal child] B------- -+ BODY RenderBody at (0,0) size 0x0 renderer->(0x542f5d7f0) node->(0x542f5fc30) layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0f20) [Rs:0x0 Re:0x0] layout->[self][normal child] I------- -+* Q RenderInline renderer->(0x542fe00b0) node->(0x542f5fcc0) continuation->(0x542fe1610) layout->[self][normal child] I----G-- -+ <pseudo> RenderInline renderer->(0x542fe0190) node->(0x542f5fee0) layout->[self][normal child] I---YG-- -+ RenderQuote renderer->(0x542fe0270) layout->[self][normal child] I---YG-- -+ RenderText renderer->(0x542fe0360) layout->[self] B------- -+ OL RenderBlock at (0,0) size 0x0 renderer->(0x542fe0400) node->(0x542f5fd50) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0790) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderMultiColumnSpannerPlaceholder at (0,0) size 0x0 renderer->(0x542fe0980) [Rs:0x0 Re:0x0] layout->[self] B------- -+ LI RenderListItem at (0,0) size 0x0 renderer->(0x542fe0530) node->(0x542f5fdf0) [Rs:0x0 Re:0x0] layout->[self][normal child] I---YG-- -+ RenderListMarker at (0,0) size 0x0 renderer->(0x542fe0670) layout->[self] B---YG-- -+ RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe0a90) [Rs:0x0 Re:0x0] layout->[self] I----G-- -+ <pseudo> RenderInline renderer->(0x542fe0c30) node->(0x542fe0010) layout->[self][normal child] I---YG-- -+ RenderQuote renderer->(0x542fe0d10) layout->[self][normal child] I---YG-- -+ RenderText renderer->(0x542fe0e00) layout->[self] I------- -+ #text RenderText renderer->(0x542fe0ea0) node->(0x542f5fe80) length->(1) "\n" layout->[self] B---YG-- -+ RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe1110) layout->[self] Step 3: BlockFlow::attach: parent=0x542fe0400, child=0x542fe1a80, beforeChild=0x542fe0a90 (B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y, (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer, (C)omposited, (+)Dirty style, (+)Dirty layout B---YGLC -+ RenderView at (0,0) size 0x0 renderer->(0x542f5d210) layout->[normal child][positioned child] B-----L- -+ HTML RenderBlock at (0,0) size 0x0 renderer->(0x542f5d6c0) node->(0x542f5f800) layout->[self][normal child] B------- -+ BODY RenderBody at (0,0) size 0x0 renderer->(0x542f5d7f0) node->(0x542f5fc30) layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0f20) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderBlock at (0,0) size 0x0 renderer->(0x542fe1740) [Rs:0x0 Re:0x0] layout->[self][normal child] I------- -+ Q RenderInline renderer->(0x542fe00b0) node->(0x542f5fcc0) continuation->(0x542fe1610) layout->[self][normal child] I----G-- -+ <pseudo> RenderInline renderer->(0x542fe0190) node->(0x542f5fee0) layout->[self][normal child] I---YG-- -+ RenderQuote renderer->(0x542fe0270) layout->[self][normal child] I---YG-- -+ RenderText renderer->(0x542fe0360) layout->[self] B---YG-- -+ RenderBlock at (0,0) size 0x0 renderer->(0x542fe1610) continuation->(0x542fe19a0) [Rs:0x0 Re:0x0] layout->[self] B------- -+ OL RenderBlock at (0,0) size 0x0 renderer->(0x542fe0400) node->(0x542f5fd50) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0790) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderMultiColumnSpannerPlaceholder at (0,0) size 0x0 renderer->(0x542fe0980) [Rs:0x0 Re:0x0] layout->[self] B------- -+ LI RenderListItem at (0,0) size 0x0 renderer->(0x542fe0530) node->(0x542f5fdf0) [Rs:0x0 Re:0x0] layout->[self][normal child] I---YG-- -+ RenderListMarker at (0,0) size 0x0 renderer->(0x542fe0670) [Rs:0x0 Re:0x0] layout->[self] B---YG-- -+* RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe0a90) [Rs:0x0 Re:0x0] layout->[self] B---YG-- -+ RenderBlock at (0,0) size 0x0 renderer->(0x542fe1870) [Rs:0x0 Re:0x0] layout->[self][normal child] I------- -+ Q RenderInline renderer->(0x542fe19a0) node->(0x542f5fcc0) layout->[self][normal child] I----G-- -+ <pseudo> RenderInline renderer->(0x542fe0c30) node->(0x542fe0010) layout->[self][normal child] I---YG-- -+ RenderQuote renderer->(0x542fe0d10) layout->[self][normal child] I---YG-- -+ RenderText renderer->(0x542fe0e00) layout->[self] I------- -+ #text RenderText renderer->(0x542fe0ea0) node->(0x542f5fe80) length->(1) "\n" layout->[self] B---YG-- -+ RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe1110) layout->[self] Step 4: beforeChildContainer = 0x0, parent = 0x542fe0790, beforeChild = 0x542fe0a90 (B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y, (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer, (C)omposited, (+)Dirty style, (+)Dirty layout B---YGLC -+ RenderView at (0,0) size 0x0 renderer->(0x542f5d210) layout->[normal child][positioned child] B-----L- -+ HTML RenderBlock at (0,0) size 0x0 renderer->(0x542f5d6c0) node->(0x542f5f800) layout->[self][normal child] B------- -+ BODY RenderBody at (0,0) size 0x0 renderer->(0x542f5d7f0) node->(0x542f5fc30) layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0f20) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderBlock at (0,0) size 0x0 renderer->(0x542fe1740) [Rs:0x0 Re:0x0] layout->[self][normal child] I------- -+ Q RenderInline renderer->(0x542fe00b0) node->(0x542f5fcc0) continuation->(0x542fe1610) layout->[self][normal child] I----G-- -+ <pseudo> RenderInline renderer->(0x542fe0190) node->(0x542f5fee0) layout->[self][normal child] I---YG-- -+ RenderQuote renderer->(0x542fe0270) layout->[self][normal child] I---YG-- -+ RenderText renderer->(0x542fe0360) layout->[self] B---YG-- -+ RenderBlock at (0,0) size 0x0 renderer->(0x542fe1610) continuation->(0x542fe19a0) [Rs:0x0 Re:0x0] layout->[self] B------- -+ OL RenderBlock at (0,0) size 0x0 renderer->(0x542fe0400) node->(0x542f5fd50) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0790) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderMultiColumnSpannerPlaceholder at (0,0) size 0x0 renderer->(0x542fe0980) [Rs:0x0 Re:0x0] layout->[self] B------- -+ LI RenderListItem at (0,0) size 0x0 renderer->(0x542fe0530) node->(0x542f5fdf0) [Rs:0x0 Re:0x0] layout->[self][normal child] I---YG-- -+ RenderListMarker at (0,0) size 0x0 renderer->(0x542fe0670) [Rs:0x0 Re:0x0] layout->[self] B---YG-- -+* RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe0a90) [Rs:0x0 Re:0x0] layout->[self] B---YG-- -+ RenderBlock at (0,0) size 0x0 renderer->(0x542fe1870) [Rs:0x0 Re:0x0] layout->[self][normal child] I------- -+ Q RenderInline renderer->(0x542fe19a0) node->(0x542f5fcc0) layout->[self][normal child] I----G-- -+ <pseudo> RenderInline renderer->(0x542fe0c30) node->(0x542fe0010) layout->[self][normal child] I---YG-- -+ RenderQuote renderer->(0x542fe0d10) layout->[self][normal child] I---YG-- -+ RenderText renderer->(0x542fe0e00) layout->[self] I------- -+ #text RenderText renderer->(0x542fe0ea0) node->(0x542f5fe80) length->(1) "\n" layout->[self] B---YG-- -+ RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe1110) layout->[self]
Created attachment 399802 [details] Patch
The patch is for discussion. In this patch, beforeChild is set to nullptr before calling m_builder.attach to avoid passing siblings as parent and beforeChild. However, doing so triggers the assertion below: ASSERTION FAILED: !flow.spannerMap().get(placeholder.spanner()) ./rendering/updating/RenderTreeBuilderMultiColumn.cpp(270) : void WebCore::RenderTreeBuilder::MultiColumn::multiColumnDescendantInserted(WebCore::RenderMultiColumnFlow &, WebCore::RenderObject &) If the code returns without calling m_builder.attach, there is no assertion or crash. (In reply to Jack from comment #4) > Created attachment 399802 [details] > Patch
Comment on attachment 399802 [details] Patch Looks like EWS tests are crashing.
Comment on attachment 399802 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399802&action=review > Source/WebCore/rendering/updating/RenderTreeBuilderBlockFlow.cpp:47 > + if (beforeChild->parent() == &parent) > + beforeChild = nullptr; This clearly needs some explanation. In many cases (probably the most common) the beforeChild's parent is the host for the insertion point.
Thanks. Yeah, this patch is to demonstrate the test case. Please see the complete render tree dump in comment #3. In this case we are passing parent.multiColumnFlow() and beforeChild to m_builder.attach. if (parent.multiColumnFlow() && (!parent.isFieldset() || !child->isLegend())) { ... return m_builder.attach(*parent.multiColumnFlow(), WTFMove(child), beforeChild); } And the render tree show that parent.multiColumnFlow() and beforeChild are siblings. BlockFlow::attach: parent=0x542fe0400, child=0x542fe1a80, beforeChild=0x542fe0a90 B------- -+ OL RenderBlock at (0,0) size 0x0 renderer->(0x542fe0400) node->(0x542f5fd50) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0790) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderMultiColumnSpannerPlaceholder at (0,0) size 0x0 renderer->(0x542fe0980) [Rs:0x0 Re:0x0] layout->[self] B------- -+ LI RenderListItem at (0,0) size 0x0 renderer->(0x542fe0530) node->(0x542f5fdf0) [Rs:0x0 Re:0x0] layout->[self][normal child] I---YG-- -+ RenderListMarker at (0,0) size 0x0 renderer->(0x542fe0670) [Rs:0x0 Re:0x0] layout->[self] B---YG-- -+* RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe0a90) [Rs:0x0 Re:0x0] layout->[self] And when the code enters RenderTreeBuilder::Block::attachIgnoringContinuation(), the following while loop will exhaust the ancestors and crashes when beforeChildContainer becomes nullptr. if (beforeChild && beforeChild->parent() != &parent) { RenderElement* beforeChildContainer = beforeChild->parent(); while (beforeChildContainer->parent() != &parent) beforeChildContainer = beforeChildContainer->parent(); (In reply to zalan from comment #7) > Comment on attachment 399802 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399802&action=review > > > Source/WebCore/rendering/updating/RenderTreeBuilderBlockFlow.cpp:47 > > + if (beforeChild->parent() == &parent) > > + beforeChild = nullptr; > > This clearly needs some explanation. In many cases (probably the most > common) the beforeChild's parent is the host for the insertion point.
Besides, I was wondering if RenderTreeBuilder::attachInternal should move beforeChild to the RenderMultiColumnSpannerPlaceholder since it is the child of RenderMultiColumnFlowThread? B------- -+ OL RenderBlock at (0,0) size 0x0 renderer->(0x542fe0400) node->(0x542f5fd50) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YGL- -+ RenderMultiColumnFlowThread at (0,0) size 0x0 renderer->(0x542fe0790) [Rs:0x0 Re:0x0] layout->[self][normal child] B---YG-- -+ RenderMultiColumnSpannerPlaceholder at (0,0) size 0x0 renderer->(0x542fe0980) [Rs:0x0 Re:0x0] layout->[self] B------- -+ LI RenderListItem at (0,0) size 0x0 renderer->(0x542fe0530) node->(0x542f5fdf0) [Rs:0x0 Re:0x0] layout->[self][normal child] I---YG-- -+ RenderListMarker at (0,0) size 0x0 renderer->(0x542fe0670) [Rs:0x0 Re:0x0] layout->[self] B---YG-- -+* RenderMultiColumnSet at (0,0) size 0x0 renderer->(0x542fe0a90) [Rs:0x0 Re:0x0] layout->[self] However, in the following code, findColumnSpannerPlaceholder cannot find the spanner in the spannerMap so return nullptr, therefore beforeChild is not moved to spannerPlaceholder. } 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; } }
It seems there is something wrong when we pass siblings as parent and beforeChild into RenderTreeBuilder::Block::attachIgnoringContinuation since it assumes that parent needs to be an ancestor of beforeChild, otherwise many places in that function would assert or crash. Does this seem like a case that we need to support, or just a corner case that a patch is enough? The simplest patch is to bail out in attachIgnoringContinuation when the while loop exhaust ancestors.
Created attachment 399848 [details] Patch
Comment on attachment 399848 [details] Patch This is papering over the real issue (not clearing the inner fragmented context when the column style gets unset).
Created attachment 400075 [details] Test reduction
<rdar://problem/62993844>
Created attachment 400095 [details] Patch
Comment on attachment 400095 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400095&action=review > LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation.html:3 > + -webkit-columns: 2; Is the prefix required?
(In reply to Simon Fraser (smfr) from comment #16) > Comment on attachment 400095 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=400095&action=review > > > LayoutTests/fast/multicol/nested-multicol-with-spanner-and-continuation.html:3 > > + -webkit-columns: 2; > > Is the prefix required? Yeah, certainly not Thanks.
Created attachment 400105 [details] Patch
Committed r262093: <https://trac.webkit.org/changeset/262093> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400105 [details].