RenderNamedFlowThread should only support RenderElement children.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2014 20:15:05 +0000 (20:15 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2014 20:15:05 +0000 (20:15 +0000)
<https://webkit.org/b/128675>

Tighten up flow-thread rendering so that it only supports element
children directly. This means we don't have to worry about text
renderers on this code path.

Reviewed by Antti Koivisto.

* rendering/RenderElement.cpp:
(WebCore::RenderElement::insertedIntoTree):
(WebCore::RenderElement::willBeRemovedFromTree):
(WebCore::RenderElement::willBeDestroyed):
* rendering/RenderNamedFlowThread.cpp:
(WebCore::RenderNamedFlowThread::nextRendererForElement):
(WebCore::RenderNamedFlowThread::addFlowChild):
(WebCore::RenderNamedFlowThread::removeFlowChild):
* rendering/RenderNamedFlowThread.h:
* rendering/RenderObject.cpp:
(WebCore::RenderObject::willBeDestroyed):
(WebCore::RenderObject::insertedIntoTree):
(WebCore::RenderObject::willBeRemovedFromTree):
* style/StyleResolveTree.cpp:
(WebCore::Style::createRendererIfNeeded):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderNamedFlowThread.cpp
Source/WebCore/rendering/RenderNamedFlowThread.h
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/style/StyleResolveTree.cpp

index 5cf738848eab619b85cd75116fef984956e1915e..50c6c5a5c7c880750d87541ec5d538872060329e 100644 (file)
@@ -1,3 +1,30 @@
+2014-02-12  Andreas Kling  <akling@apple.com>
+
+        RenderNamedFlowThread should only support RenderElement children.
+        <https://webkit.org/b/128675>
+
+        Tighten up flow-thread rendering so that it only supports element
+        children directly. This means we don't have to worry about text
+        renderers on this code path.
+
+        Reviewed by Antti Koivisto.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::insertedIntoTree):
+        (WebCore::RenderElement::willBeRemovedFromTree):
+        (WebCore::RenderElement::willBeDestroyed):
+        * rendering/RenderNamedFlowThread.cpp:
+        (WebCore::RenderNamedFlowThread::nextRendererForElement):
+        (WebCore::RenderNamedFlowThread::addFlowChild):
+        (WebCore::RenderNamedFlowThread::removeFlowChild):
+        * rendering/RenderNamedFlowThread.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::willBeDestroyed):
+        (WebCore::RenderObject::insertedIntoTree):
+        (WebCore::RenderObject::willBeRemovedFromTree):
+        * style/StyleResolveTree.cpp:
+        (WebCore::Style::createRendererIfNeeded):
+
 2014-02-12  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Modernize missed inspector files
index 667b28ad1d09312046bd70dbc285587c5fc1ab09..c1486580199ac4b8e10d39687c931503b5c8619b 100644 (file)
@@ -33,6 +33,7 @@
 #include "Frame.h"
 #include "HTMLElement.h"
 #include "HTMLNames.h"
+#include "FlowThreadController.h"
 #include "RenderCounter.h"
 #include "RenderDeprecatedFlexibleBox.h"
 #include "RenderFlexibleBox.h"
@@ -967,6 +968,9 @@ void RenderElement::insertedIntoTree()
 {
     RenderObject::insertedIntoTree();
 
+    if (auto* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
+        containerFlowThread->addFlowChild(*this);
+
     // Keep our layer hierarchy updated. Optimize for the common case where we don't have any children
     // and don't have a layer attached to ourselves.
     RenderLayer* layer = nullptr;
@@ -1007,6 +1011,9 @@ void RenderElement::willBeRemovedFromTree()
     if (isOutOfFlowPositioned() && parent()->childrenInline())
         parent()->dirtyLinesFromChangedChild(this);
 
+    if (auto* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
+        containerFlowThread->removeFlowChild(*this);
+
     RenderObject::willBeRemovedFromTree();
 }
 
@@ -1017,6 +1024,16 @@ void RenderElement::willBeDestroyed()
     destroyLeftoverChildren();
 
     RenderObject::willBeDestroyed();
+
+#if !ASSERT_DISABLED
+    if (!documentBeingDestroyed() && view().hasRenderNamedFlowThreads()) {
+        // After remove, the object and the associated information should not be in any flow thread.
+        for (auto& flowThread : *view().flowThreadController().renderNamedFlowThreadList()) {
+            ASSERT(!flowThread->hasChild(*this));
+            ASSERT(!flowThread->hasChildInfo(this));
+        }
+    }
+#endif
 }
 
 void RenderElement::setNeedsPositionedMovementLayout(const RenderStyle* oldStyle)
index 102dfea22b88fc7b2e86175cd51332432e217f67..b61be569134a1654547357f4146db66511fb96e0 100644 (file)
@@ -97,11 +97,11 @@ void RenderNamedFlowThread::updateWritingMode()
     setStyle(std::move(newStyle));
 }
 
-RenderObject* RenderNamedFlowThread::nextRendererForNode(Node* node) const
+RenderElement* RenderNamedFlowThread::nextRendererForElement(Element& element) const
 {
     for (auto& child : m_flowThreadChildList) {
-        ASSERT(child->node());
-        unsigned short position = node->compareDocumentPosition(child->node());
+        ASSERT(!child->isAnonymous());
+        unsigned short position = element.compareDocumentPosition(child->element());
         if (position & Node::DOCUMENT_POSITION_FOLLOWING)
             return child;
     }
@@ -109,29 +109,24 @@ RenderObject* RenderNamedFlowThread::nextRendererForNode(Node* node) const
     return 0;
 }
 
-void RenderNamedFlowThread::addFlowChild(RenderObject* newChild)
+void RenderNamedFlowThread::addFlowChild(RenderElement& newChild)
 {
     // The child list is used to sort the flow thread's children render objects 
     // based on their corresponding nodes DOM order. The list is needed to avoid searching the whole DOM.
 
-    Node* childNode = newChild->node();
-
-    // Do not add anonymous objects.
-    if (!childNode)
+    if (newChild.isAnonymous())
         return;
 
-    ASSERT(childNode->isElementNode());
-
-    RenderObject* beforeChild = nextRendererForNode(childNode);
+    auto* beforeChild = nextRendererForElement(*newChild.element());
     if (beforeChild)
-        m_flowThreadChildList.insertBefore(beforeChild, newChild);
+        m_flowThreadChildList.insertBefore(beforeChild, &newChild);
     else
-        m_flowThreadChildList.add(newChild);
+        m_flowThreadChildList.add(&newChild);
 }
 
-void RenderNamedFlowThread::removeFlowChild(RenderObject* child)
+void RenderNamedFlowThread::removeFlowChild(RenderElement& child)
 {
-    m_flowThreadChildList.remove(child);
+    m_flowThreadChildList.remove(&child);
 }
 
 bool RenderNamedFlowThread::dependsOn(RenderNamedFlowThread* otherRenderFlowThread) const
index b147a2a706112f9766ec63df1de0e24c0e307d2e..9484cc5fc97357ab58697d548cb9846c94541fb6 100644 (file)
@@ -52,13 +52,13 @@ public:
 
     const RenderRegionList& invalidRenderRegionList() const { return m_invalidRegionList; }
 
-    RenderObject* nextRendererForNode(Node*) const;
+    RenderElement* nextRendererForElement(Element&) const;
 
-    void addFlowChild(RenderObject* newChild);
-    void removeFlowChild(RenderObject*);
+    void addFlowChild(RenderElement&);
+    void removeFlowChild(RenderElement&);
     bool hasChildren() const { return !m_flowThreadChildList.isEmpty(); }
 #ifndef NDEBUG
-    bool hasChild(RenderObject* child) const { return m_flowThreadChildList.contains(child); }
+    bool hasChild(RenderElement& child) const { return m_flowThreadChildList.contains(&child); }
 #endif
 
     void pushDependencies(RenderNamedFlowThreadList&);
@@ -129,7 +129,7 @@ private:
     RenderNamedFlowThreadCountedSet m_layoutBeforeThreadsSet;
 
     // Holds the sorted children of a named flow. This is the only way we can get the ordering right.
-    ListHashSet<RenderObject*> m_flowThreadChildList;
+    ListHashSet<RenderElement*> m_flowThreadChildList;
 
     NamedFlowContentElements m_contentElements;
 
index 90fd92bd03e0885cc39662ecef2f2d6cadc572bc..2939eb419fb7c3d5314b5d9c1db88147330610a4 100644 (file)
@@ -1855,18 +1855,6 @@ void RenderObject::willBeDestroyed()
     if (AXObjectCache* cache = document().existingAXObjectCache())
         cache->remove(this);
 
-#ifndef NDEBUG
-    if (!documentBeingDestroyed() && view().hasRenderNamedFlowThreads()) {
-        // After remove, the object and the associated information should not be in any flow thread.
-        const RenderNamedFlowThreadList* flowThreadList = view().flowThreadController().renderNamedFlowThreadList();
-        for (RenderNamedFlowThreadList::const_iterator iter = flowThreadList->begin(); iter != flowThreadList->end(); ++iter) {
-            const RenderNamedFlowThread* renderFlowThread = *iter;
-            ASSERT(!renderFlowThread->hasChild(this));
-            ASSERT(!renderFlowThread->hasChildInfo(this));
-        }
-    }
-#endif
-
     // If this renderer had a parent, remove should have destroyed any counters
     // attached to this renderer and marked the affected other counters for
     // reevaluation. This apparently redundant check is here for the case when
@@ -1891,9 +1879,6 @@ void RenderObject::insertedIntoTree()
 
     if (!isFloating() && parent()->childrenInline())
         parent()->dirtyLinesFromChangedChild(this);
-
-    if (RenderNamedFlowThread* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
-        containerFlowThread->addFlowChild(this);
 }
 
 void RenderObject::willBeRemovedFromTree()
@@ -1902,9 +1887,6 @@ void RenderObject::willBeRemovedFromTree()
 
     removeFromRenderFlowThread();
 
-    if (RenderNamedFlowThread* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
-        containerFlowThread->removeFlowChild(this);
-
     // Update cached boundaries in SVG renderers, if a child is removed.
     parent()->setNeedsBoundariesUpdate();
 }
index 2707b841a87ee48a46c6c01b3af23eb7a4ec3531..0837c1c8ec02a02247681a546d7d879fd89ea834 100644 (file)
@@ -226,7 +226,7 @@ static void createRendererIfNeeded(Element& element, PassRefPtr<RenderStyle> res
     RenderObject* nextRenderer;
     if (parentFlowRenderer) {
         parentRenderer = parentFlowRenderer;
-        nextRenderer = parentFlowRenderer->nextRendererForNode(&element);
+        nextRenderer = parentFlowRenderer->nextRendererForElement(element);
     } else {
         // FIXME: Make this path Element only, handle the root special case separately.
         parentRenderer = renderingParentNode->renderer();