Changeset 208470 in webkit

Timestamp:

Nov 9, 2016 12:38:28 PM (7 years ago)

Author:

Alan Bujtas

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderElement.cpp (modified) (4 diffs)
• rendering/RenderElement.h (modified) (1 diff)
• rendering/RenderNamedFlowThread.cpp (modified) (1 diff)
• rendering/RenderNamedFlowThread.h (modified) (2 diffs)
• style/RenderTreePosition.cpp (modified) (3 diffs)
• style/RenderTreePosition.h (modified) (3 diffs)
• style/RenderTreeUpdater.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -1045 +1045 @@
 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.
@@ -1125 +1122 @@
         // 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));
         }
@@ -1524 +1520 @@
 
     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;
 }
 
@@ -2208 +2196 @@
 {
     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.

trunk/Source/WebCore/rendering/RenderElement.h

@@ -200 +200 @@
     void setHasPausedImageAnimations(bool b) { m_hasPausedImageAnimations = b; }
 
-    RenderNamedFlowThread* renderNamedFlowThreadWrapper();
-
     void setRenderBoxNeedsLazyRepaint(bool b) { m_renderBoxNeedsLazyRepaint = b; }
     bool renderBoxNeedsLazyRepaint() const { return m_renderBoxNeedsLazyRepaint; }

trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp

@@ -96 +96 @@
     newStyle.setWritingMode(firstFragment->style().writingMode());
     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);
 }
 

trunk/Source/WebCore/rendering/RenderNamedFlowThread.h

@@ -53 +53 @@
 
     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);
@@ -142 +133 @@
     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;
 

trunk/Source/WebCore/style/RenderTreePosition.cpp

@@ -28 +28 @@
 
 #include "ComposedTreeIterator.h"
+#include "FlowThreadController.h"
 #include "PseudoElement.h"
 #include "RenderObject.h"
@@ -67 +68 @@
     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;
     }
@@ -105 +106 @@
 }
 
+#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)
 {

trunk/Source/WebCore/style/RenderTreePosition.h

@@ -28 +28 @@
 
 #include "RenderElement.h"
+#include "RenderNamedFlowThread.h"
 #include "RenderText.h"
 #include "RenderView.h"
@@ -46 +47 @@
     }
     
-    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;
@@ -68 +65 @@
 
 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 };

trunk/Source/WebCore/style/RenderTreeUpdater.cpp

@@ -307 +307 @@
 
 #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
@@ -321 +318 @@
 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
 
@@ -332 +338 @@
         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)