Move RenderNamedFlowThread nextRendererForElement logic to RenderTreeUpdater.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Nov 2016 20:38:28 +0000 (20:38 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Nov 2016 20:38:28 +0000 (20:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164503

Reviewed by Antti Koivisto.

When we insert a renderer into the render tree, we need to know both its parent
and its next sibling. Normally the parent and the sibling are based on the DOM, but
when this renderer is part of a flow thread, its insertion sibling is not necessarily the DOM sibling.
To find the correct sibling, we call RenderNamedFlowThread's nextRendererForElement().
RenderNamedFlowThread keeps track of its children so that it can compute the next sibling
for the insertion point.

This patch eliminates the need for keeping track of the child renderers of each
flow by moving the 'next sibling' logic to RenderTreePosition.

No change in functionality.

* rendering/RenderElement.cpp:
(WebCore::RenderElement::insertedIntoTree):
(WebCore::RenderElement::willBeDestroyed):
(WebCore::RenderElement::removeFromRenderFlowThread):
(WebCore::RenderElement::renderNamedFlowThreadWrapper): Deleted.
* rendering/RenderElement.h:
* rendering/RenderNamedFlowThread.cpp:
(WebCore::RenderNamedFlowThread::nextRendererForElement): Deleted.
(WebCore::RenderNamedFlowThread::addFlowChild): Deleted.
(WebCore::RenderNamedFlowThread::removeFlowChild): Deleted.
* rendering/RenderNamedFlowThread.h:
* style/RenderTreePosition.cpp:
(WebCore::RenderTreePosition::previousSiblingRenderer):
(WebCore::RenderTreePosition::flowThreadInsertionContext):
* style/RenderTreePosition.h:
(WebCore::RenderTreePosition::RenderTreePosition):
(WebCore::RenderTreePosition::parent):
* style/RenderTreeUpdater.cpp:
(WebCore::registerElementForFlowThreadIfNeeded): We need to registed the element even when it does not create renderer (display: none).
(WebCore::RenderTreeUpdater::createRenderer):
(WebCore::moveToFlowThreadIfNeeded): Deleted.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderNamedFlowThread.cpp
Source/WebCore/rendering/RenderNamedFlowThread.h
Source/WebCore/style/RenderTreePosition.cpp
Source/WebCore/style/RenderTreePosition.h
Source/WebCore/style/RenderTreeUpdater.cpp

index 019dd77..f35776e 100644 (file)
@@ -1,3 +1,44 @@
+2016-11-09  Zalan Bujtas  <zalan@apple.com>
+
+        Move RenderNamedFlowThread nextRendererForElement logic to RenderTreeUpdater.
+        https://bugs.webkit.org/show_bug.cgi?id=164503
+
+        Reviewed by Antti Koivisto.
+
+        When we insert a renderer into the render tree, we need to know both its parent
+        and its next sibling. Normally the parent and the sibling are based on the DOM, but
+        when this renderer is part of a flow thread, its insertion sibling is not necessarily the DOM sibling.
+        To find the correct sibling, we call RenderNamedFlowThread's nextRendererForElement().
+        RenderNamedFlowThread keeps track of its children so that it can compute the next sibling
+        for the insertion point.
+
+        This patch eliminates the need for keeping track of the child renderers of each
+        flow by moving the 'next sibling' logic to RenderTreePosition.
+
+        No change in functionality.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::insertedIntoTree):
+        (WebCore::RenderElement::willBeDestroyed):
+        (WebCore::RenderElement::removeFromRenderFlowThread):
+        (WebCore::RenderElement::renderNamedFlowThreadWrapper): Deleted.
+        * rendering/RenderElement.h:
+        * rendering/RenderNamedFlowThread.cpp:
+        (WebCore::RenderNamedFlowThread::nextRendererForElement): Deleted.
+        (WebCore::RenderNamedFlowThread::addFlowChild): Deleted.
+        (WebCore::RenderNamedFlowThread::removeFlowChild): Deleted.
+        * rendering/RenderNamedFlowThread.h:
+        * style/RenderTreePosition.cpp:
+        (WebCore::RenderTreePosition::previousSiblingRenderer):
+        (WebCore::RenderTreePosition::flowThreadInsertionContext):
+        * style/RenderTreePosition.h:
+        (WebCore::RenderTreePosition::RenderTreePosition):
+        (WebCore::RenderTreePosition::parent):
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::registerElementForFlowThreadIfNeeded): We need to registed the element even when it does not create renderer (display: none).
+        (WebCore::RenderTreeUpdater::createRenderer):
+        (WebCore::moveToFlowThreadIfNeeded): Deleted.
+
 2016-11-09  Per Arne Vollan  <pvollan@apple.com>
 
         [Win][Direct2D] Incomplete image decoding.
index 10a833d..2d3a3d1 100644 (file)
@@ -1044,9 +1044,6 @@ void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldS
 
 void RenderElement::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;
@@ -1124,7 +1121,6 @@ void RenderElement::willBeDestroyed()
     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));
         }
     }
@@ -1525,14 +1521,6 @@ bool RenderElement::repaintForPausedImageAnimationsIfNeeded(const IntRect& visib
     return true;
 }
 
-RenderNamedFlowThread* RenderElement::renderNamedFlowThreadWrapper()
-{
-    auto* renderer = this;
-    while (renderer && renderer->isAnonymousBlock() && !is<RenderNamedFlowThread>(*renderer))
-        renderer = renderer->parent();
-    return is<RenderNamedFlowThread>(renderer) ? downcast<RenderNamedFlowThread>(renderer) : nullptr;
-}
-
 const RenderStyle* RenderElement::getCachedPseudoStyle(PseudoId pseudo, const RenderStyle* parentStyle) const
 {
     if (pseudo < FIRST_INTERNAL_PSEUDOID && !style().hasPseudoStyle(pseudo))
@@ -2207,8 +2195,6 @@ RespectImageOrientationEnum RenderElement::shouldRespectImageOrientation() const
 void RenderElement::removeFromRenderFlowThread()
 {
     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.
index 00b3d71..ae3aa2c 100644 (file)
@@ -199,8 +199,6 @@ public:
     bool hasPausedImageAnimations() const { return m_hasPausedImageAnimations; }
     void setHasPausedImageAnimations(bool b) { m_hasPausedImageAnimations = b; }
 
-    RenderNamedFlowThread* renderNamedFlowThreadWrapper();
-
     void setRenderBoxNeedsLazyRepaint(bool b) { m_renderBoxNeedsLazyRepaint = b; }
     bool renderBoxNeedsLazyRepaint() const { return m_renderBoxNeedsLazyRepaint; }
 
index 941b0ea..8af080a 100644 (file)
@@ -97,40 +97,6 @@ void RenderNamedFlowThread::updateWritingMode()
     setStyle(WTFMove(newStyle));
 }
 
-RenderElement* RenderNamedFlowThread::nextRendererForElement(Element& element) const
-{
-    for (auto& child : m_flowThreadChildList) {
-        ASSERT(!child->isAnonymous());
-        ASSERT_WITH_MESSAGE(child->element(), "Can only be null for anonymous renderers");
-
-        unsigned short position = element.compareDocumentPosition(*child->element());
-        if (position & Node::DOCUMENT_POSITION_FOLLOWING)
-            return child;
-    }
-
-    return nullptr;
-}
-
-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.
-
-    if (newChild.isAnonymous())
-        return;
-
-    auto* beforeChild = nextRendererForElement(*newChild.element());
-    if (beforeChild)
-        m_flowThreadChildList.insertBefore(beforeChild, &newChild);
-    else
-        m_flowThreadChildList.add(&newChild);
-}
-
-void RenderNamedFlowThread::removeFlowChild(RenderElement& child)
-{
-    m_flowThreadChildList.remove(&child);
-}
-
 bool RenderNamedFlowThread::dependsOn(RenderNamedFlowThread* otherRenderFlowThread) const
 {
     if (m_layoutBeforeThreadsSet.contains(otherRenderFlowThread))
index 584686a..8c1ef0a 100644 (file)
@@ -53,15 +53,6 @@ public:
 
     const RenderRegionList& invalidRenderRegionList() const { return m_invalidRegionList; }
 
-    RenderElement* nextRendererForElement(Element&) const;
-
-    void addFlowChild(RenderElement&);
-    void removeFlowChild(RenderElement&);
-    bool hasChildren() const { return !m_flowThreadChildList.isEmpty(); }
-#ifndef NDEBUG
-    bool hasChild(RenderElement& child) const { return m_flowThreadChildList.contains(&child); }
-#endif
-
     static RenderBlock* fragmentFromRenderBoxAsRenderBlock(RenderBox*, const IntPoint& absolutePoint, const RenderBox& flowedBox);
 
     void pushDependencies(RenderNamedFlowThreadList&);
@@ -141,9 +132,6 @@ private:
     // easy to sort the order of threads layout.
     RenderNamedFlowThreadCountedSet m_layoutBeforeThreadsSet;
 
-    // Holds the sorted children of a named flow. This is the only way we can get the ordering right.
-    ListHashSet<RenderElement*> m_flowThreadChildList;
-
     NamedFlowContentElements m_contentElements;
 
     RenderRegionList m_invalidRegionList;
index 8854ecc..2b90d68 100644 (file)
@@ -27,6 +27,7 @@
 #include "RenderTreePosition.h"
 
 #include "ComposedTreeIterator.h"
+#include "FlowThreadController.h"
 #include "PseudoElement.h"
 #include "RenderObject.h"
 #include "ShadowRoot.h"
@@ -66,7 +67,7 @@ RenderObject* RenderTreePosition::previousSiblingRenderer(const Text& textNode)
     auto composedChildren = composedTreeChildren(*parentElement);
     for (auto it = composedChildren.at(textNode), end = composedChildren.end(); it != end; --it) {
         RenderObject* renderer = it->renderer();
-        if (renderer && !RenderTreePosition::isRendererReparented(*renderer))
+        if (renderer && !isRendererReparented(*renderer))
             return renderer;
     }
     if (auto* before = parentElement->beforePseudoElement())
@@ -104,6 +105,50 @@ RenderObject* RenderTreePosition::nextSiblingRenderer(const Node& node) const
     return nullptr;
 }
 
+#if ENABLE(CSS_REGIONS)
+RenderTreePosition RenderTreePosition::insertionPositionForFlowThread(Element* insertionParent, Element& element, const RenderStyle& style)
+{
+    ASSERT(element.shouldMoveToFlowThread(style));
+    auto& parentFlowThread = element.document().renderView()->flowThreadController().ensureRenderFlowThreadWithName(style.flowThread());
+
+    if (!insertionParent)
+        return { parentFlowThread, nullptr };
+
+    auto composedDescendants = composedTreeDescendants(*insertionParent);
+    auto it = element.isBeforePseudoElement() ? composedDescendants.begin() : composedDescendants.at(element);
+    auto end = composedDescendants.end();
+    while (it != end) {
+        auto& currentNode = *it;
+        bool hasDisplayContents = is<Element>(currentNode) && downcast<Element>(currentNode).hasDisplayContents();
+        if (hasDisplayContents) {
+            it.traverseNext();
+            continue;
+        }
+
+        auto* renderer = currentNode.renderer();
+        if (!renderer) {
+            it.traverseNextSkippingChildren();
+            continue;
+        }
+
+        if (!is<RenderElement>(*renderer)) {
+            it.traverseNext();
+            continue;
+        }
+
+        // We are the last child in this flow.
+        if (!isRendererReparented(*renderer))
+            return { parentFlowThread, nullptr };
+
+        if (renderer->style().hasFlowInto() && style.flowThread() == renderer->style().flowThread())
+            return { parentFlowThread, downcast<RenderElement>(renderer) };
+        // Nested flows, skip.
+        it.traverseNextSkippingChildren();
+    }
+    return { parentFlowThread, nullptr };
+}
+#endif
+    
 bool RenderTreePosition::isRendererReparented(const RenderObject& renderer)
 {
     if (!renderer.node()->isElementNode())
index f379e93..b8fba4c 100644 (file)
@@ -27,6 +27,7 @@
 #define RenderTreePosition_h
 
 #include "RenderElement.h"
+#include "RenderNamedFlowThread.h"
 #include "RenderText.h"
 #include "RenderView.h"
 
@@ -45,15 +46,11 @@ public:
     {
     }
     
-    RenderTreePosition(RenderElement& parent, RenderObject* nextSibling)
-        : m_parent(parent)
-        , m_nextSibling(nextSibling)
-        , m_hasValidNextSibling(true)
-    {
-    }
+#if ENABLE(CSS_REGIONS)
+    static RenderTreePosition insertionPositionForFlowThread(Element* insertionParent, Element& child, const RenderStyle&);
+#endif
 
     RenderElement& parent() const { return m_parent; }
-
     void insert(RenderObject&);
     bool canInsert(RenderElement&) const;
     bool canInsert(RenderText&) const;
@@ -67,6 +64,15 @@ public:
     static bool isRendererReparented(const RenderObject&);
 
 private:
+#if ENABLE(CSS_REGIONS)
+    RenderTreePosition(RenderFlowThread& parent, RenderObject* nextSibling)
+        : m_parent(parent)
+        , m_nextSibling(nextSibling)
+        , m_hasValidNextSibling(true)
+    {
+    }
+#endif
+
     RenderElement& m_parent;
     RenderObject* m_nextSibling { nullptr };
     bool m_hasValidNextSibling { false };
index c0b533f..3a39e23 100644 (file)
@@ -306,37 +306,38 @@ void RenderTreeUpdater::updateElementRenderer(Element& element, Style::ElementUp
 }
 
 #if ENABLE(CSS_REGIONS)
-static RenderNamedFlowThread* moveToFlowThreadIfNeeded(Element& element, const RenderStyle& style)
+static void registerElementForFlowThreadIfNeeded(Element& element, const RenderStyle& style)
 {
     if (!element.shouldMoveToFlowThread(style))
-        return nullptr;
-
+        return;
     FlowThreadController& flowThreadController = element.document().renderView()->flowThreadController();
-    RenderNamedFlowThread& parentFlowRenderer = flowThreadController.ensureRenderFlowThreadWithName(style.flowThread());
-    flowThreadController.registerNamedFlowContentElement(element, parentFlowRenderer);
-    return &parentFlowRenderer;
+    flowThreadController.registerNamedFlowContentElement(element, flowThreadController.ensureRenderFlowThreadWithName(style.flowThread()));
 }
 #endif
 
 void RenderTreeUpdater::createRenderer(Element& element, RenderStyle&& style)
 {
+    auto computeInsertionPosition = [this, &element, &style] () {
+#if ENABLE(CSS_REGIONS)
+        if (element.shouldMoveToFlowThread(style))
+            return RenderTreePosition::insertionPositionForFlowThread(renderTreePosition().parent().element(), element, style);
+#endif
+        renderTreePosition().computeNextSibling(element);
+        return renderTreePosition();
+    };
+    
     if (!shouldCreateRenderer(element, renderTreePosition().parent()))
         return;
 
-    RenderNamedFlowThread* parentFlowRenderer = nullptr;
 #if ENABLE(CSS_REGIONS)
-    parentFlowRenderer = moveToFlowThreadIfNeeded(element, style);
+    // Even display: none elements need to be registered in FlowThreadController.
+    registerElementForFlowThreadIfNeeded(element, style);
 #endif
 
     if (!element.rendererIsNeeded(style))
         return;
 
-    renderTreePosition().computeNextSibling(element);
-
-    RenderTreePosition insertionPosition = parentFlowRenderer
-        ? RenderTreePosition(*parentFlowRenderer, parentFlowRenderer->nextRendererForElement(element))
-        : renderTreePosition();
-
+    RenderTreePosition insertionPosition = computeInsertionPosition();
     RenderElement* newRenderer = element.createElementRenderer(WTFMove(style), insertionPosition).leakPtr();
     if (!newRenderer)
         return;