Title: [225506] trunk/Source/WebCore
- Revision
- 225506
- Author
- [email protected]
- Date
- 2017-12-04 16:51:01 -0800 (Mon, 04 Dec 2017)
Log Message
RenderMultiColumnFlow::fragmentedFlowDescendantInserted should not destroy incoming newDescendant
https://bugs.webkit.org/show_bug.cgi?id=180181
Reviewed by Antti Koivisto.
This is in preparation for having all multicolumn related tree mutation in RenderTreeUpdaterMultiColumn.
Covered by fast/multicol/column-span-range-crash.html
* rendering/RenderMultiColumnFlow.cpp:
(WebCore::RenderMultiColumnFlow::fragmentedFlowDescendantInserted):
* rendering/RenderMultiColumnFlow.h:
* style/RenderTreeUpdaterMultiColumn.cpp:
(WebCore::RenderTreeUpdater::MultiColumn::destroyFragmentedFlow):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (225505 => 225506)
--- trunk/Source/WebCore/ChangeLog 2017-12-05 00:13:09 UTC (rev 225505)
+++ trunk/Source/WebCore/ChangeLog 2017-12-05 00:51:01 UTC (rev 225506)
@@ -1,3 +1,20 @@
+2017-12-04 Zalan Bujtas <[email protected]>
+
+ RenderMultiColumnFlow::fragmentedFlowDescendantInserted should not destroy incoming newDescendant
+ https://bugs.webkit.org/show_bug.cgi?id=180181
+
+ Reviewed by Antti Koivisto.
+
+ This is in preparation for having all multicolumn related tree mutation in RenderTreeUpdaterMultiColumn.
+
+ Covered by fast/multicol/column-span-range-crash.html
+
+ * rendering/RenderMultiColumnFlow.cpp:
+ (WebCore::RenderMultiColumnFlow::fragmentedFlowDescendantInserted):
+ * rendering/RenderMultiColumnFlow.h:
+ * style/RenderTreeUpdaterMultiColumn.cpp:
+ (WebCore::RenderTreeUpdater::MultiColumn::destroyFragmentedFlow):
+
2017-12-04 JF Bastien <[email protected]>
Update std::expected to match libc++ coding style
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnFlow.cpp (225505 => 225506)
--- trunk/Source/WebCore/rendering/RenderMultiColumnFlow.cpp 2017-12-05 00:13:09 UTC (rev 225505)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnFlow.cpp 2017-12-05 00:51:01 UTC (rev 225506)
@@ -350,46 +350,26 @@
if (gShiftingSpanner || newDescendant.isInFlowRenderFragmentedFlow())
return;
- Vector<RenderPtr<RenderObject>> spannersToDelete;
-
- RenderObject* subtreeRoot = &newDescendant;
- for (auto* descendant = &newDescendant; descendant; descendant = (descendant ? descendant->nextInPreOrder(subtreeRoot) : nullptr)) {
+ auto* subtreeRoot = &newDescendant;
+ auto* descendant = subtreeRoot;
+ while (descendant) {
+ // Skip nested multicolumn flows.
+ if (is<RenderMultiColumnFlow>(*descendant)) {
+ descendant = descendant->nextSibling();
+ continue;
+ }
if (is<RenderMultiColumnSpannerPlaceholder>(*descendant)) {
// A spanner's placeholder has been inserted. The actual spanner renderer is moved from
// where it would otherwise occur (if it weren't a spanner) to becoming a sibling of the
// column sets.
RenderMultiColumnSpannerPlaceholder& placeholder = downcast<RenderMultiColumnSpannerPlaceholder>(*descendant);
- if (placeholder.fragmentedFlow() != this) {
- // This isn't our spanner! It shifted here from an ancestor multicolumn block. It's going to end up
- // becoming our spanner instead, but for it to do that we first have to nuke the original spanner,
- // and get the spanner content back into this flow thread.
- RenderBox* spanner = placeholder.spanner();
-
- // Insert after the placeholder, but don't let a notification happen.
- gShiftingSpanner = true;
- RenderBlockFlow& ancestorBlock = downcast<RenderBlockFlow>(*spanner->parent());
- ancestorBlock.moveChildTo(placeholder.parentBox(), spanner, placeholder.nextSibling(), RenderBoxModelObject::NormalizeAfterInsertion::Yes);
- gShiftingSpanner = false;
-
- // We have to nuke the placeholder, since the ancestor already lost the mapping to it when
- // we shifted the placeholder down into this flow thread.
- placeholder.fragmentedFlow()->spannerMap().remove(spanner);
-
- spannersToDelete.append(placeholder.parent()->takeChild(placeholder));
-
- if (subtreeRoot == descendant)
- subtreeRoot = spanner;
- // Now we process the spanner.
- descendant = processPossibleSpannerDescendant(subtreeRoot, *spanner);
- continue;
- }
-
ASSERT(!spannerMap().get(placeholder.spanner()));
- spannerMap().add(placeholder.spanner(), makeWeakPtr(placeholder));
+ spannerMap().add(placeholder.spanner(), makeWeakPtr(downcast<RenderMultiColumnSpannerPlaceholder>(descendant)));
ASSERT(!placeholder.firstChild()); // There should be no children here, but if there are, we ought to skip them.
- continue;
- }
- descendant = processPossibleSpannerDescendant(subtreeRoot, *descendant);
+ } else
+ descendant = processPossibleSpannerDescendant(subtreeRoot, *descendant);
+ if (descendant)
+ descendant = descendant->nextInPreOrder(subtreeRoot);
}
}
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnFlow.h (225505 => 225506)
--- trunk/Source/WebCore/rendering/RenderMultiColumnFlow.h 2017-12-05 00:13:09 UTC (rev 225505)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnFlow.h 2017-12-05 00:51:01 UTC (rev 225506)
@@ -97,7 +97,6 @@
typedef HashMap<RenderBox*, WeakPtr<RenderMultiColumnSpannerPlaceholder>> SpannerMap;
SpannerMap& spannerMap() { return *m_spannerMap; }
- std::unique_ptr<SpannerMap> takeSpannerMap() { return WTFMove(m_spannerMap); }
private:
bool isRenderMultiColumnFlow() const override { return true; }
Modified: trunk/Source/WebCore/style/RenderTreeUpdaterMultiColumn.cpp (225505 => 225506)
--- trunk/Source/WebCore/style/RenderTreeUpdaterMultiColumn.cpp 2017-12-05 00:13:09 UTC (rev 225505)
+++ trunk/Source/WebCore/style/RenderTreeUpdaterMultiColumn.cpp 2017-12-05 00:51:01 UTC (rev 225506)
@@ -97,26 +97,31 @@
void RenderTreeUpdater::MultiColumn::destroyFragmentedFlow(RenderBlockFlow& flow)
{
- auto& fragmentedFlow = *flow.multiColumnFlow();
- flow.clearMultiColumnFlow();
+ auto& multiColumnFlow = *flow.multiColumnFlow();
+ multiColumnFlow.deleteLines();
- fragmentedFlow.deleteLines();
- fragmentedFlow.moveAllChildrenTo(&flow, RenderBoxModelObject::NormalizeAfterInsertion::Yes);
-
// Move spanners back to their original DOM position in the tree, and destroy the placeholders.
- auto spannerMap = fragmentedFlow.takeSpannerMap();
- for (auto& spannerAndPlaceholder : *spannerMap) {
- RenderBox& spanner = *spannerAndPlaceholder.key;
- auto& placeholder = *spannerAndPlaceholder.value;
- auto takenSpanner = flow.takeChild(spanner);
- placeholder.parent()->addChild(WTFMove(takenSpanner), &placeholder);
- placeholder.removeFromParentAndDestroy();
+ auto& spanners = multiColumnFlow.spannerMap();
+ Vector<RenderMultiColumnSpannerPlaceholder*> placeholdersToDelete;
+ for (auto& spannerAndPlaceholder : spanners)
+ placeholdersToDelete.append(spannerAndPlaceholder.value.get());
+ Vector<std::pair<RenderElement*, RenderPtr<RenderObject>>> parentAndSpannerList;
+ for (auto* placeholder : placeholdersToDelete) {
+ auto* spannerOriginalParent = placeholder->parent();
+ if (spannerOriginalParent == &multiColumnFlow)
+ spannerOriginalParent = &flow;
+ // Detaching the spanner takes care of removing the placeholder (and merges the RenderMultiColumnSets).
+ auto* spanner = placeholder->spanner();
+ parentAndSpannerList.append(std::make_pair(spannerOriginalParent, spanner->parent()->takeChild(*spanner)));
}
-
- while (auto* columnSet = fragmentedFlow.firstMultiColumnSet())
+ while (auto* columnSet = multiColumnFlow.firstMultiColumnSet())
columnSet->removeFromParentAndDestroy();
- fragmentedFlow.removeFromParentAndDestroy();
+ flow.clearMultiColumnFlow();
+ multiColumnFlow.moveAllChildrenTo(&flow, RenderBoxModelObject::NormalizeAfterInsertion::Yes);
+ multiColumnFlow.removeFromParentAndDestroy();
+ for (auto& parentAndSpanner : parentAndSpannerList)
+ parentAndSpanner.first->addChild(WTFMove(parentAndSpanner.second));
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
