Title: [208470] trunk/Source/WebCore
Revision
208470
Author
za...@apple.com
Date
2016-11-09 12:38:28 -0800 (Wed, 09 Nov 2016)

Log 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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (208469 => 208470)


--- trunk/Source/WebCore/ChangeLog	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/ChangeLog	2016-11-09 20:38:28 UTC (rev 208470)
@@ -1,3 +1,44 @@
+2016-11-09  Zalan Bujtas  <za...@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  <pvol...@apple.com>
 
         [Win][Direct2D] Incomplete image decoding.

Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (208469 => 208470)


--- trunk/Source/WebCore/rendering/RenderElement.cpp	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp	2016-11-09 20:38:28 UTC (rev 208470)
@@ -1044,9 +1044,6 @@
 
 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 @@
     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 @@
     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 @@
 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.

Modified: trunk/Source/WebCore/rendering/RenderElement.h (208469 => 208470)


--- trunk/Source/WebCore/rendering/RenderElement.h	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/rendering/RenderElement.h	2016-11-09 20:38:28 UTC (rev 208470)
@@ -199,8 +199,6 @@
     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; }
 

Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp (208469 => 208470)


--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp	2016-11-09 20:38:28 UTC (rev 208470)
@@ -97,40 +97,6 @@
     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))

Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.h (208469 => 208470)


--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.h	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.h	2016-11-09 20:38:28 UTC (rev 208470)
@@ -53,15 +53,6 @@
 
     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 @@
     // 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;

Modified: trunk/Source/WebCore/style/RenderTreePosition.cpp (208469 => 208470)


--- trunk/Source/WebCore/style/RenderTreePosition.cpp	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/style/RenderTreePosition.cpp	2016-11-09 20:38:28 UTC (rev 208470)
@@ -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 @@
     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 @@
     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())

Modified: trunk/Source/WebCore/style/RenderTreePosition.h (208469 => 208470)


--- trunk/Source/WebCore/style/RenderTreePosition.h	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/style/RenderTreePosition.h	2016-11-09 20:38:28 UTC (rev 208470)
@@ -27,6 +27,7 @@
 #define RenderTreePosition_h
 
 #include "RenderElement.h"
+#include "RenderNamedFlowThread.h"
 #include "RenderText.h"
 #include "RenderView.h"
 
@@ -45,15 +46,11 @@
     {
     }
     
-    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 @@
     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 };

Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (208469 => 208470)


--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp	2016-11-09 20:29:23 UTC (rev 208469)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp	2016-11-09 20:38:28 UTC (rev 208470)
@@ -306,37 +306,38 @@
 }
 
 #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;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to