RenderFlowThread state reset cleanup.

https://bugs.webkit.org/show_bug.cgi?id=164426

Reviewed by Simon Fraser.

RenderFlowThread state reset is spread across several functions. This patch groups them
together in RenderObject::resetFlowThreadState().

No change in functionality.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::removeLeftoverAnonymousBlock):
(WebCore::RenderBlock::dropAnonymousBoxChild): This is now part of resetFlowThreadState() since resetFlowThreadState
gets called even when NotifyChildren is false.
* rendering/RenderElement.cpp:
(WebCore::RenderElement::insertChildInternal): Initialize the thread state before we notify the child.
(WebCore::RenderElement::removeChildInternal): Reset the state even when NotifyChildren is false.
(WebCore::RenderElement::willBeRemovedFromTree): This code is moved to removeFromRenderFlowThread().
(WebCore::RenderElement::removeFromRenderFlowThread):
* rendering/RenderObject.cpp:
(WebCore::RenderObject::initializeFlowThreadState): This is in transition for webkit.org/b/164428 (RenderFlowThread state initialization cleanup.)
(WebCore::RenderObject::resetFlowThreadState):
(WebCore::RenderObject::setParent): This was seemingly a random place to put flow state initialization.
(WebCore::RenderObject::willBeRemovedFromTree): resetFlowThreadState() takes care of it now.
* rendering/RenderObject.h:


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

Author: [email protected]

Source/WebCore/ChangeLog

@@ -1,3 +1,31 @@
+2016-11-04  Zalan Bujtas  <[email protected]>
+
+        RenderFlowThread state reset cleanup.
+        https://bugs.webkit.org/show_bug.cgi?id=164426
+
+        Reviewed by Simon Fraser.
+
+        RenderFlowThread state reset is spread across several functions. This patch groups them
+        together in RenderObject::resetFlowThreadState().
+
+        No change in functionality.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::removeLeftoverAnonymousBlock):
+        (WebCore::RenderBlock::dropAnonymousBoxChild): This is now part of resetFlowThreadState() since resetFlowThreadState
+        gets called even when NotifyChildren is false.
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::insertChildInternal): Initialize the thread state before we notify the child.
+        (WebCore::RenderElement::removeChildInternal): Reset the state even when NotifyChildren is false.
+        (WebCore::RenderElement::willBeRemovedFromTree): This code is moved to removeFromRenderFlowThread().
+        (WebCore::RenderElement::removeFromRenderFlowThread):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::initializeFlowThreadState): This is in transition for webkit.org/b/164428 (RenderFlowThread state initialization cleanup.)
+        (WebCore::RenderObject::resetFlowThreadState):
+        (WebCore::RenderObject::setParent): This was seemingly a random place to put flow state initialization. 
+        (WebCore::RenderObject::willBeRemovedFromTree): resetFlowThreadState() takes care of it now.
+        * rendering/RenderObject.h:
+
 2016-11-04  Yusuke Suzuki  <[email protected]>
 
         [DOMJIT] Add DOMJIT::Signature annotation to Document::getElementById

Source/WebCore/rendering/RenderBlock.cpp

@@ -768,7 +768,7 @@ void RenderBlock::removeLeftoverAnonymousBlock(RenderBlock* child)
     child->m_next = 0;
 
     // Remove all the information in the flow thread associated with the leftover anonymous block.
-    child->removeFromRenderFlowThread();
+    child->resetFlowThreadStateOnRemoval();
 
     child->setParent(0);
     child->setPreviousSibling(0);
@@ -812,9 +812,6 @@ void RenderBlock::dropAnonymousBoxChild(RenderBlock& parent, RenderBlock& child)
 {
     parent.setNeedsLayoutAndPrefWidthsRecalc();
     parent.setChildrenInline(child.childrenInline());
-    if (auto* childFlowThread = child.flowThreadContainingBlock())
-        childFlowThread->removeFlowChildInfo(&child);
-
     RenderObject* nextSibling = child.nextSibling();
     parent.removeChildInternal(child, child.hasLayer() ? NotifyChildren : DontNotifyChildren);
     child.moveAllChildrenTo(&parent, nextSibling, child.hasLayer());

Source/WebCore/rendering/RenderElement.cpp

@@ -572,6 +572,7 @@ void RenderElement::insertChildInternal(RenderObject* newChild, RenderObject* be
         m_lastChild = newChild;
     }
 
+    newChild->initializeFlowThreadStateOnInsertion();
     if (!documentBeingDestroyed()) {
         if (notifyChildren == NotifyChildren)
             newChild->insertedIntoTree();
@@ -626,6 +627,8 @@ void RenderElement::removeChildInternal(RenderObject& oldChild, NotifyChildrenTy
     if (!documentBeingDestroyed() && notifyChildren == NotifyChildren)
         oldChild.willBeRemovedFromTree();
 
+    oldChild.resetFlowThreadStateOnRemoval();
+
     // WARNING: There should be no code running between willBeRemovedFromTree and the actual removal below.
     // This is needed to avoid race conditions where willBeRemovedFromTree would dirty the tree's structure
     // and the code running here would force an untimely rebuilding, leaving |oldChild| dangling.
@@ -1085,11 +1088,6 @@ void RenderElement::willBeRemovedFromTree()
     if (isOutOfFlowPositioned() && parent()->childrenInline())
         parent()->dirtyLinesFromChangedChild(*this);
 
-    if (auto* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
-        containerFlowThread->removeFlowChild(*this);
-
-    removeFromRenderFlowThread();
-
     RenderObject::willBeRemovedFromTree();
 }
 
@@ -2208,9 +2206,9 @@ RespectImageOrientationEnum RenderElement::shouldRespectImageOrientation() const
 
 void RenderElement::removeFromRenderFlowThread()
 {
-    if (flowThreadState() == NotInsideFlowThread)
-        return;
-
+    ASSERT(flowThreadState() != NotInsideFlowThread);
+    if (auto* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
+        containerFlowThread->removeFlowChild(*this);
     // Sometimes we remove the element from the flow, but it's not destroyed at that time.
     // It's only until later when we actually destroy it and remove all the children from it.
     // Currently, that happens for firstLetter elements and list markers.

Source/WebCore/rendering/RenderObject.cpp

@@ -177,16 +177,40 @@ void RenderObject::setFlowThreadStateIncludingDescendants(FlowThreadState state)
     }
 }
 
+void RenderObject::initializeFlowThreadStateOnInsertion()
+{
+    ASSERT(parent());
+
+    if (flowThreadState() == parent()->flowThreadState())
+        return;
+
+    // A RenderFlowThread is always considered to be inside itself, so it never has to change its state in response to parent changes.
+    if (isRenderFlowThread())
+        return;
+
+    setFlowThreadStateIncludingDescendants(parent()->flowThreadState());
+}
+
+void RenderObject::resetFlowThreadStateOnRemoval()
+{
+    if (flowThreadState() == NotInsideFlowThread)
+        return;
+
+    if (!documentBeingDestroyed() && is<RenderElement>(*this)) {
+        downcast<RenderElement>(*this).removeFromRenderFlowThread();
+        return;
+    }
+
+    // A RenderFlowThread is always considered to be inside itself, so it never has to change its state in response to parent changes.
+    if (isRenderFlowThread())
+        return;
+
+    setFlowThreadStateIncludingDescendants(NotInsideFlowThread);
+}
+
 void RenderObject::setParent(RenderElement* parent)
 {
     m_parent = parent;
-
-    // Only update if our flow thread state is different from our new parent and if we're not a RenderFlowThread.
-    // A RenderFlowThread is always considered to be inside itself, so it never has to change its state
-    // in response to parent changes.
-    FlowThreadState newState = parent ? parent->flowThreadState() : NotInsideFlowThread;
-    if (newState != flowThreadState() && !isRenderFlowThread())
-        setFlowThreadStateIncludingDescendants(newState);
 }
 
 void RenderObject::removeFromParent()
@@ -1450,9 +1474,6 @@ void RenderObject::insertedIntoTree()
 void RenderObject::willBeRemovedFromTree()
 {
     // FIXME: We should ASSERT(isRooted()) but we have some out-of-order removals which would need to be fixed first.
-
-    setFlowThreadState(NotInsideFlowThread);
-
     // Update cached boundaries in SVG renderers, if a child is removed.
     parent()->setNeedsBoundariesUpdate();
 }

Source/WebCore/rendering/RenderObject.h

@@ -818,6 +818,9 @@ class RenderObject : public CachedImageClient {
     virtual RenderFlowThread* locateFlowThreadContainingBlock() const;
     static void calculateBorderStyleColor(const EBorderStyle&, const BoxSide&, Color&);
 
+    void initializeFlowThreadStateOnInsertion();
+    void resetFlowThreadStateOnRemoval();
+
 private:
 #ifndef NDEBUG
     bool isSetNeedsLayoutForbidden() const { return m_setNeedsLayoutForbidden; }