RenderMultiColumnFlowThread::processPossibleSpannerDescendant should take RenderObjec...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Oct 2016 21:19:34 +0000 (21:19 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Oct 2016 21:19:34 +0000 (21:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164072

Reviewed by Simon Fraser.

No change in functionality.

* rendering/RenderFlowThread.h:
* rendering/RenderMultiColumnFlowThread.cpp:
(WebCore::findSetRendering):
(WebCore::isValidColumnSpanner):
(WebCore::RenderMultiColumnFlowThread::processPossibleSpannerDescendant):
(WebCore::RenderMultiColumnFlowThread::flowThreadDescendantInserted):
(WebCore::RenderMultiColumnFlowThread::findSetRendering): Deleted.
* rendering/RenderMultiColumnFlowThread.h:
* rendering/RenderMultiColumnSet.cpp:
(WebCore::precedesRenderer):
(WebCore::RenderMultiColumnSet::containsRendererInFlowThread):
* rendering/RenderMultiColumnSet.h:
* rendering/RenderMultiColumnSpannerPlaceholder.cpp:
(WebCore::RenderMultiColumnSpannerPlaceholder::createAnonymous):
(WebCore::RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder):
* rendering/RenderMultiColumnSpannerPlaceholder.h:
* rendering/RenderObject.cpp:
(WebCore::RenderObject::insertedIntoTree):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlowThread.h
Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp
Source/WebCore/rendering/RenderMultiColumnFlowThread.h
Source/WebCore/rendering/RenderMultiColumnSet.cpp
Source/WebCore/rendering/RenderMultiColumnSet.h
Source/WebCore/rendering/RenderMultiColumnSpannerPlaceholder.cpp
Source/WebCore/rendering/RenderMultiColumnSpannerPlaceholder.h
Source/WebCore/rendering/RenderObject.cpp

index 2925f70..65f4aac 100644 (file)
@@ -1,3 +1,31 @@
+2016-10-27  Zalan Bujtas  <zalan@apple.com>
+
+        RenderMultiColumnFlowThread::processPossibleSpannerDescendant should take RenderObject& instead of RenderObject*
+        https://bugs.webkit.org/show_bug.cgi?id=164072
+
+        Reviewed by Simon Fraser.
+
+        No change in functionality.
+
+        * rendering/RenderFlowThread.h:
+        * rendering/RenderMultiColumnFlowThread.cpp:
+        (WebCore::findSetRendering):
+        (WebCore::isValidColumnSpanner):
+        (WebCore::RenderMultiColumnFlowThread::processPossibleSpannerDescendant):
+        (WebCore::RenderMultiColumnFlowThread::flowThreadDescendantInserted):
+        (WebCore::RenderMultiColumnFlowThread::findSetRendering): Deleted.
+        * rendering/RenderMultiColumnFlowThread.h:
+        * rendering/RenderMultiColumnSet.cpp:
+        (WebCore::precedesRenderer):
+        (WebCore::RenderMultiColumnSet::containsRendererInFlowThread):
+        * rendering/RenderMultiColumnSet.h:
+        * rendering/RenderMultiColumnSpannerPlaceholder.cpp:
+        (WebCore::RenderMultiColumnSpannerPlaceholder::createAnonymous):
+        (WebCore::RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder):
+        * rendering/RenderMultiColumnSpannerPlaceholder.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::insertedIntoTree):
+
 2016-10-27  Brent Fulgham  <bfulgham@apple.com>
 
         Prevent hit tests from being performed on an invalid render tree
index 7494eb7..1771931 100644 (file)
@@ -92,7 +92,7 @@ public:
     // location in the tree.
     virtual RenderObject* resolveMovedChild(RenderObject* child) const { return child; }
     // Called when a descendant of the flow thread has been inserted.
-    virtual void flowThreadDescendantInserted(RenderObject*) { }
+    virtual void flowThreadDescendantInserted(RenderObject&) { }
     // Called when a sibling or descendant of the flow thread is about to be removed.
     virtual void flowThreadRelativeWillBeRemoved(RenderObject*) { }
     // Called when a descendant box's layout is finished and it has been positioned within its container.
index ef1b58e..71dcb93 100644 (file)
@@ -133,9 +133,10 @@ void RenderMultiColumnFlowThread::layout()
     m_lastSetWorkedOn = nullptr;
 }
 
-RenderMultiColumnSet* RenderMultiColumnFlowThread::findSetRendering(RenderObject* renderer) const
+static RenderMultiColumnSet* findSetRendering(const RenderMultiColumnFlowThread& flowThread, const RenderObject& renderer)
 {
-    for (RenderMultiColumnSet* multicolSet = firstMultiColumnSet(); multicolSet; multicolSet = multicolSet->nextSiblingMultiColumnSet()) {
+    // Find the set inside which the specified renderer would be rendered.
+    for (auto* multicolSet = flowThread.firstMultiColumnSet(); multicolSet; multicolSet = multicolSet->nextSiblingMultiColumnSet()) {
         if (multicolSet->containsRendererInFlowThread(renderer))
             return multicolSet;
     }
@@ -233,36 +234,36 @@ RenderObject* RenderMultiColumnFlowThread::resolveMovedChild(RenderObject* child
     return child;
 }
 
-static bool isValidColumnSpanner(RenderMultiColumnFlowThread* flowThread, RenderObject* descendant)
+static bool isValidColumnSpanner(const RenderMultiColumnFlowThread& flowThread, const RenderObject& descendant)
 {
     // We assume that we're inside the flow thread. This function is not to be called otherwise.
-    ASSERT(descendant->isDescendantOf(flowThread));
+    ASSERT(descendant.isDescendantOf(&flowThread));
 
     // First make sure that the renderer itself has the right properties for becoming a spanner.
-    auto& style = descendant->style();
-    if (style.columnSpan() != ColumnSpanAll || !is<RenderBox>(*descendant) || descendant->isFloatingOrOutOfFlowPositioned())
+    auto& style = descendant.style();
+    if (style.columnSpan() != ColumnSpanAll || !is<RenderBox>(descendant) || descendant.isFloatingOrOutOfFlowPositioned())
         return false;
 
-    RenderElement* container = descendant->parent();
+    RenderElement* container = descendant.parent();
     if (!is<RenderBlockFlow>(*container) || container->childrenInline()) {
         // Needs to be block-level.
         return false;
     }
     
     // We need to have the flow thread as the containing block. A spanner cannot break out of the flow thread.
-    RenderFlowThread* enclosingFlowThread = descendant->flowThreadContainingBlock();
-    if (enclosingFlowThread != flowThread)
+    RenderFlowThread* enclosingFlowThread = descendant.flowThreadContainingBlock();
+    if (enclosingFlowThread != &flowThread)
         return false;
 
     // This looks like a spanner, but if we're inside something unbreakable, it's not to be treated as one.
-    for (RenderBox* ancestor = downcast<RenderBox>(*descendant).containingBlock(); ancestor && !is<RenderView>(*ancestor); ancestor = ancestor->containingBlock()) {
+    for (auto* ancestor = downcast<RenderBox>(descendant).containingBlock(); ancestor && !is<RenderView>(*ancestor); ancestor = ancestor->containingBlock()) {
         if (ancestor->isRenderFlowThread()) {
             // Don't allow any intervening non-multicol fragmentation contexts. The spec doesn't say
             // anything about disallowing this, but it's just going to be too complicated to
             // implement (not to mention specify behavior).
-            return ancestor == flowThread;
+            return ancestor == &flowThread;
         }
-        ASSERT(ancestor->style().columnSpan() != ColumnSpanAll || !isValidColumnSpanner(flowThread, ancestor));
+        ASSERT(ancestor->style().columnSpan() != ColumnSpanAll || !isValidColumnSpanner(flowThread, *ancestor));
         if (ancestor->isUnsplittableForPagination())
             return false;
     }
@@ -294,23 +295,23 @@ static RenderObject* spannerPlacehoderCandidate(const RenderObject& renderer, co
     return nullptr;
 }
 
-RenderObject* RenderMultiColumnFlowThread::processPossibleSpannerDescendant(RenderObject*& subtreeRoot, RenderObject* descendant)
+RenderObject* RenderMultiColumnFlowThread::processPossibleSpannerDescendant(RenderObject*& subtreeRoot, RenderObject& descendant)
 {
     RenderBlockFlow* multicolContainer = multiColumnBlockFlow();
-    RenderObject* nextRendererInFlowThread = spannerPlacehoderCandidate(*descendant, *this);
+    RenderObject* nextRendererInFlowThread = spannerPlacehoderCandidate(descendant, *this);
     RenderObject* insertBeforeMulticolChild = nullptr;
-    RenderObject* nextDescendant = descendant;
+    RenderObject* nextDescendant = &descendant;
 
-    if (isValidColumnSpanner(this, descendant)) {
+    if (isValidColumnSpanner(*this, descendant)) {
         // This is a spanner (column-span:all). Such renderers are moved from where they would
         // otherwise occur in the render tree to becoming a direct child of the multicol container,
         // so that they live among the column sets. This simplifies the layout implementation, and
         // basically just relies on regular block layout done by the RenderBlockFlow that
         // establishes the multicol container.
-        RenderBlockFlow* container = downcast<RenderBlockFlow>(descendant->parent());
+        RenderBlockFlow* container = downcast<RenderBlockFlow>(descendant.parent());
         RenderMultiColumnSet* setToSplit = nullptr;
         if (nextRendererInFlowThread) {
-            setToSplit = findSetRendering(descendant);
+            setToSplit = findSetRendering(*this, descendant);
             if (setToSplit) {
                 setToSplit->setNeedsLayout();
                 insertBeforeMulticolChild = setToSplit->nextSibling();
@@ -322,20 +323,21 @@ RenderObject* RenderMultiColumnFlowThread::processPossibleSpannerDescendant(Rend
         // content before and after the spanner, so that it becomes separate line boxes. Secondly,
         // this placeholder serves as a break point for column sets, so that, when encountered, we
         // end flowing one column set and move to the next one.
-        RenderMultiColumnSpannerPlaceholder* placeholder = RenderMultiColumnSpannerPlaceholder::createAnonymous(this, downcast<RenderBox>(descendant), &container->style());
-        container->addChild(placeholder, descendant->nextSibling());
-        container->removeChild(*descendant);
+        RenderMultiColumnSpannerPlaceholder* placeholder = RenderMultiColumnSpannerPlaceholder::createAnonymous(this,
+            downcast<RenderBox>(descendant), &container->style());
+        container->addChild(placeholder, descendant.nextSibling());
+        container->removeChild(descendant);
         
         // This is a guard to stop an ancestor flow thread from processing the spanner.
         gShiftingSpanner = true;
-        multicolContainer->RenderBlock::addChild(descendant, insertBeforeMulticolChild);
+        multicolContainer->RenderBlock::addChild(&descendant, 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
         // creation of column sets or anything like that. Continue at its original position in
         // the tree, i.e. where the placeholder was just put.
-        if (subtreeRoot == descendant)
+        if (subtreeRoot == &descendant)
             subtreeRoot = placeholder;
         nextDescendant = placeholder;
     } else {
@@ -375,12 +377,12 @@ RenderObject* RenderMultiColumnFlowThread::processPossibleSpannerDescendant(Rend
     return nextDescendant;
 }
 
-void RenderMultiColumnFlowThread::flowThreadDescendantInserted(RenderObject* descendant)
+void RenderMultiColumnFlowThread::flowThreadDescendantInserted(RenderObject& newDescendant)
 {
-    if (gShiftingSpanner || m_beingEvacuated || descendant->isInFlowRenderFlowThread())
+    if (gShiftingSpanner || m_beingEvacuated || newDescendant.isInFlowRenderFlowThread())
         return;
-    RenderObject* subtreeRoot = descendant;
-    for (; descendant; descendant = (descendant ? descendant->nextInPreOrder(subtreeRoot) : nullptr)) {
+    RenderObject* subtreeRoot = &newDescendant;
+    for (auto* descendant = &newDescendant; descendant; descendant = (descendant ? descendant->nextInPreOrder(subtreeRoot) : nullptr)) {
         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
@@ -406,7 +408,7 @@ void RenderMultiColumnFlowThread::flowThreadDescendantInserted(RenderObject* des
                 if (subtreeRoot == descendant)
                     subtreeRoot = spanner;
                 // Now we process the spanner.
-                descendant = processPossibleSpannerDescendant(subtreeRoot, spanner);
+                descendant = processPossibleSpannerDescendant(subtreeRoot, *spanner);
                 continue;
             }
             
@@ -415,8 +417,7 @@ void RenderMultiColumnFlowThread::flowThreadDescendantInserted(RenderObject* des
             ASSERT(!placeholder.firstChild()); // There should be no children here, but if there are, we ought to skip them.
             continue;
         }
-        
-        descendant = processPossibleSpannerDescendant(subtreeRoot, descendant);
+        descendant = processPossibleSpannerDescendant(subtreeRoot, *descendant);
     }
 }
 
index cdc0f74..ec99dce 100644 (file)
@@ -53,9 +53,6 @@ public:
 
     void layout() override;
 
-    // Find the set inside which the specified renderer would be rendered.
-    RenderMultiColumnSet* findSetRendering(RenderObject*) const;
-
     // Populate the flow thread with what's currently its siblings. Called when a regular block
     // becomes a multicol container.
     void populate();
@@ -116,7 +113,7 @@ private:
     void addRegionToThread(RenderRegion*) override;
     void willBeRemovedFromTree() override;
     RenderObject* resolveMovedChild(RenderObject* child) const override;
-    void flowThreadDescendantInserted(RenderObject*) override;
+    void flowThreadDescendantInserted(RenderObject&) override;
     void flowThreadRelativeWillBeRemoved(RenderObject*) override;
     void flowThreadDescendantBoxLaidOut(RenderBox*) override;
     void computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logicalTop, LogicalExtentComputedValues&) const override;
@@ -129,7 +126,7 @@ private:
     bool isPageLogicalHeightKnown() const override;
 
     void handleSpannerRemoval(RenderObject* spanner);
-    RenderObject* processPossibleSpannerDescendant(RenderObject*& subtreeRoot, RenderObject* descendant);
+    RenderObject* processPossibleSpannerDescendant(RenderObject*& subtreeRoot, RenderObject& descendant);
     
 private:
     typedef HashMap<RenderBox*, RenderMultiColumnSpannerPlaceholder*> SpannerMap;
index 8ea6fee..7d2fb43 100644 (file)
@@ -91,7 +91,7 @@ RenderObject* RenderMultiColumnSet::lastRendererInFlowThread() const
     return flowThread()->lastLeafChild();
 }
 
-static bool precedesRenderer(RenderObject* renderer, RenderObject* boundary)
+static bool precedesRenderer(const RenderObject* renderer, const RenderObject* boundary)
 {
     for (; renderer; renderer = renderer->nextInPreOrder()) {
         if (renderer == boundary)
@@ -100,11 +100,11 @@ static bool precedesRenderer(RenderObject* renderer, RenderObject* boundary)
     return false;
 }
 
-bool RenderMultiColumnSet::containsRendererInFlowThread(RenderObject* renderer) const
+bool RenderMultiColumnSet::containsRendererInFlowThread(const RenderObject& renderer) const
 {
     if (!previousSiblingMultiColumnSet() && !nextSiblingMultiColumnSet()) {
         // There is only one set. This is easy, then.
-        return renderer->isDescendantOf(m_flowThread);
+        return renderer.isDescendantOf(m_flowThread);
     }
 
     RenderObject* firstRenderer = firstRendererInFlowThread();
@@ -113,7 +113,7 @@ bool RenderMultiColumnSet::containsRendererInFlowThread(RenderObject* renderer)
     ASSERT(lastRenderer);
 
     // This is SLOW! But luckily very uncommon.
-    return precedesRenderer(firstRenderer, renderer) && precedesRenderer(renderer, lastRenderer);
+    return precedesRenderer(firstRenderer, &renderer) && precedesRenderer(&renderer, lastRenderer);
 }
 
 void RenderMultiColumnSet::setLogicalTopInFlowThread(LayoutUnit logicalTop)
index 4115477..7c99423 100644 (file)
@@ -60,7 +60,7 @@ public:
     RenderObject* lastRendererInFlowThread() const;
 
     // Return true if the specified renderer (descendant of the flow thread) is inside this column set.
-    bool containsRendererInFlowThread(RenderObject*) const;
+    bool containsRendererInFlowThread(const RenderObject&) const;
 
     void setLogicalTopInFlowThread(LayoutUnit);
     LayoutUnit logicalTopInFlowThread() const { return isHorizontalWritingMode() ? flowThreadPortionRect().y() : flowThreadPortionRect().x(); }
index 9de2d68..8fb0d40 100644 (file)
@@ -34,7 +34,7 @@
 
 namespace WebCore {
 
-RenderMultiColumnSpannerPlaceholder* RenderMultiColumnSpannerPlaceholder::createAnonymous(RenderMultiColumnFlowThread* flowThread, RenderBox* spanner, const RenderStyle* parentStyle)
+RenderMultiColumnSpannerPlaceholder* RenderMultiColumnSpannerPlaceholder::createAnonymous(RenderMultiColumnFlowThread* flowThread, RenderBox& spanner, const RenderStyle* parentStyle)
 {
     auto newStyle = RenderStyle::createAnonymousStyleWithDisplay(*parentStyle, BLOCK);
     newStyle.setClear(CBOTH); // We don't want floats in the row preceding the spanner to continue on the other side.
@@ -43,9 +43,9 @@ RenderMultiColumnSpannerPlaceholder* RenderMultiColumnSpannerPlaceholder::create
     return placeholder;
 }
 
-RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder(RenderMultiColumnFlowThread* flowThread, RenderBox* spanner, RenderStyle&& style)
+RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder(RenderMultiColumnFlowThread* flowThread, RenderBox& spanner, RenderStyle&& style)
     : RenderBox(flowThread->document(), WTFMove(style), RenderBoxModelObjectFlag)
-    , m_spanner(spanner)
+    , m_spanner(&spanner)
     , m_flowThread(flowThread)
 {
 }
index eb466d9..395529e 100644 (file)
@@ -38,13 +38,13 @@ class RenderMultiColumnFlowThread;
 
 class RenderMultiColumnSpannerPlaceholder final : public RenderBox {
 public:
-    static RenderMultiColumnSpannerPlaceholder* createAnonymous(RenderMultiColumnFlowThread*, RenderBox* spanner, const RenderStyle* parentStyle);
+    static RenderMultiColumnSpannerPlaceholder* createAnonymous(RenderMultiColumnFlowThread*, RenderBox& spanner, const RenderStyle* parentStyle);
 
     RenderBox* spanner() const { return m_spanner; }
     RenderMultiColumnFlowThread* flowThread() const { return m_flowThread; }
 
 private:
-    RenderMultiColumnSpannerPlaceholder(RenderMultiColumnFlowThread*, RenderBox* spanner, RenderStyle&&);
+    RenderMultiColumnSpannerPlaceholder(RenderMultiColumnFlowThread*, RenderBox& spanner, RenderStyle&&);
     bool isRenderMultiColumnSpannerPlaceholder() const override { return true; }
 
     bool canHaveChildren() const override { return false; }
index 502a776..99ecc76 100644 (file)
@@ -1421,7 +1421,7 @@ void RenderObject::insertedIntoTree()
         parent()->dirtyLinesFromChangedChild(*this);
 
     if (RenderFlowThread* flowThread = flowThreadContainingBlock())
-        flowThread->flowThreadDescendantInserted(this);
+        flowThread->flowThreadDescendantInserted(*this);
 }
 
 void RenderObject::willBeRemovedFromTree()