Title: [224273] trunk
Revision
224273
Author
[email protected]
Date
2017-11-01 00:57:02 -0700 (Wed, 01 Nov 2017)

Log Message

Remove empty continuations in RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers
https://bugs.webkit.org/show_bug.cgi?id=179014

Reviewed by Geoff Garen.

Source/WebCore:

Treat continuation similarly to other anonymous wrappers. This makes things more understandable
and allows removal of some questionable code in RenderBlock::takeChild.

The patch also makes continuation chain a double linked so we can efficiently remove single
continuations from the chain. It also gets rid of algorithms that recurse in continuation chain.

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::firstChildInContinuation):
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::styleDidChange):

    Don't add and remove continuations from the chain when updating style. Prevent recursion by walking
    the chain only in the (non-continuation) head renderer.

(WebCore::RenderBlock::dropAnonymousBoxChild):

    Make a member function.

(WebCore::RenderBlock::takeChild):

    Remove code that destroyed empty continuations and caused the parent to destroy itself.
    Empty continuations are now removed by RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers.

* rendering/RenderBlock.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::ContinuationChainNode::ContinuationChainNode):
(WebCore::RenderBoxModelObject::ContinuationChainNode::~ContinuationChainNode):
(WebCore::RenderBoxModelObject::ContinuationChainNode::insertAfter):

    Track continuations with double linked lists.

(WebCore::continuationChainNodeMap):
(WebCore::RenderBoxModelObject::willBeDestroyed):

    Don't recurse to destroy continuation chain.
    Destroy all continuations iteratively if this is the head of the chain.
    When destroying a continuation renderer simply remove it from the chain.

(WebCore::RenderBoxModelObject::continuation const):
(WebCore::RenderBoxModelObject::insertIntoContinuationChainAfter):
(WebCore::RenderBoxModelObject::removeFromContinuationChain):
(WebCore::RenderBoxModelObject::ensureContinuationChainNode):
(WebCore::continuationMap): Deleted.
(WebCore::RenderBoxModelObject::setContinuation): Deleted.
* rendering/RenderBoxModelObject.h:
* rendering/RenderElement.cpp:
(WebCore::RenderElement::RenderElement):
(WebCore::RenderElement::removeAnonymousWrappersForInlinesIfNecessary):

    Make this a function of the parent renderer itself instead of getting 'parent()' as first operation and
    then using it.
    Don't remove continuations (isAnonymousBlockContinuation() test gives wrong result for the last continuation of the chain).

(WebCore::RenderElement::styleDidChange):

    removeAnonymousWrappersForInlinesIfNecessary is no function of the parent.

(WebCore::RenderElement::updateOutlineAutoAncestor):
* rendering/RenderElement.h:
(WebCore::RenderElement::hasContinuationChainNode const):
(WebCore::RenderElement::setHasContinuationChainNode):
(WebCore::RenderElement::hasContinuation const): Deleted.
(WebCore::RenderElement::setHasContinuation): Deleted.
* rendering/RenderInline.cpp:
(WebCore::RenderInline::styleDidChange):

    Don't add and remove continuations from the chain when updating style. Prevent recursion by walking
    the chain only in the (non-continuation) head renderer.

(WebCore::RenderInline::addChildIgnoringContinuation):

    Remove the old continuation from the chain. splitFlow() will add it back into the right place.

(WebCore::RenderInline::splitInlines):
(WebCore::RenderInline::addChildToContinuation):
(WebCore::RenderInline::childBecameNonInline):

    Remove the old continuation from the chain. splitFlow() will add it back into the right place.

* rendering/RenderInline.h:
* rendering/RenderObject.cpp:
(WebCore::RenderObject::propagateRepaintToParentWithOutlineAutoIfNeeded const):
(WebCore::RenderObject::outputRenderObject const):
(WebCore::findDestroyRootIncludingAnonymous):

    Allow anonymous continuations as destroy roots.

(WebCore::RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers):

    Removing a continuation may leave behind unnecessary anonymous sibling wrappers.
    Call removeAnonymousWrappersForInlinesIfNecessary() on parent after removal to get rid of them.

LayoutTests:

* fast/ruby/float-overhang-from-ruby-text-expected.txt:
* platform/mac/fast/ruby/rubyDOM-remove-rt1-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (224272 => 224273)


--- trunk/LayoutTests/ChangeLog	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/LayoutTests/ChangeLog	2017-11-01 07:57:02 UTC (rev 224273)
@@ -1,3 +1,13 @@
+2017-11-01  Antti Koivisto  <[email protected]>
+
+        Remove empty continuations in RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers
+        https://bugs.webkit.org/show_bug.cgi?id=179014
+
+        Reviewed by Geoff Garen.
+
+        * fast/ruby/float-overhang-from-ruby-text-expected.txt:
+        * platform/mac/fast/ruby/rubyDOM-remove-rt1-expected.txt:
+
 2017-10-31  Chris Dumez  <[email protected]>
 
         Fix a couple of service worker tests and unskip them

Modified: trunk/LayoutTests/fast/ruby/float-overhang-from-ruby-text-expected.txt (224272 => 224273)


--- trunk/LayoutTests/fast/ruby/float-overhang-from-ruby-text-expected.txt	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/LayoutTests/fast/ruby/float-overhang-from-ruby-text-expected.txt	2017-11-01 07:57:02 UTC (rev 224273)
@@ -17,7 +17,6 @@
       RenderBlock {DIV} at (0,75) size 784x50
         RenderRuby (inline) {RUBY} at (0,0) size 100x50
           RenderRubyRun (anonymous) at (0,0) size 100x50
-            RenderRubyBase (anonymous) at (0,0) size 100x50
-              RenderText {#text} at (0,0) size 100x50
-                text run at (0,0) width 100: "aa"
+            RenderText {#text} at (0,0) size 100x50
+              text run at (0,0) width 100: "aa"
         RenderText {#text} at (0,0) size 0x0

Modified: trunk/LayoutTests/platform/mac/fast/ruby/rubyDOM-remove-rt1-expected.txt (224272 => 224273)


--- trunk/LayoutTests/platform/mac/fast/ruby/rubyDOM-remove-rt1-expected.txt	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/LayoutTests/platform/mac/fast/ruby/rubyDOM-remove-rt1-expected.txt	2017-11-01 07:57:02 UTC (rev 224273)
@@ -24,9 +24,8 @@
               RenderText {#text} at (30,0) size 47x18
                 text run at (30,0) width 47: "HTML"
           RenderRubyRun (anonymous) at (259,10) size 9x18
-            RenderRubyBase (anonymous) at (0,0) size 8x18
-              RenderText {#text} at (0,0) size 8x18
-                text run at (0,0) width 8: "5"
+            RenderText {#text} at (0,0) size 8x18
+              text run at (0,0) width 8: "5"
         RenderText {#text} at (267,10) size 38x18
           text run at (267,10) width 38: " spec."
       RenderBlock {P} at (0,164) size 784x28

Modified: trunk/Source/WebCore/ChangeLog (224272 => 224273)


--- trunk/Source/WebCore/ChangeLog	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/ChangeLog	2017-11-01 07:57:02 UTC (rev 224273)
@@ -1,3 +1,102 @@
+2017-11-01  Antti Koivisto  <[email protected]>
+
+        Remove empty continuations in RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers
+        https://bugs.webkit.org/show_bug.cgi?id=179014
+
+        Reviewed by Geoff Garen.
+
+        Treat continuation similarly to other anonymous wrappers. This makes things more understandable
+        and allows removal of some questionable code in RenderBlock::takeChild.
+
+        The patch also makes continuation chain a double linked so we can efficiently remove single
+        continuations from the chain. It also gets rid of algorithms that recurse in continuation chain.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::firstChildInContinuation):
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::styleDidChange):
+
+            Don't add and remove continuations from the chain when updating style. Prevent recursion by walking
+            the chain only in the (non-continuation) head renderer.
+
+        (WebCore::RenderBlock::dropAnonymousBoxChild):
+
+            Make a member function.
+
+        (WebCore::RenderBlock::takeChild):
+
+            Remove code that destroyed empty continuations and caused the parent to destroy itself.
+            Empty continuations are now removed by RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers.
+
+        * rendering/RenderBlock.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::ContinuationChainNode::ContinuationChainNode):
+        (WebCore::RenderBoxModelObject::ContinuationChainNode::~ContinuationChainNode):
+        (WebCore::RenderBoxModelObject::ContinuationChainNode::insertAfter):
+
+            Track continuations with double linked lists.
+
+        (WebCore::continuationChainNodeMap):
+        (WebCore::RenderBoxModelObject::willBeDestroyed):
+
+            Don't recurse to destroy continuation chain. 
+            Destroy all continuations iteratively if this is the head of the chain.
+            When destroying a continuation renderer simply remove it from the chain.
+
+        (WebCore::RenderBoxModelObject::continuation const):
+        (WebCore::RenderBoxModelObject::insertIntoContinuationChainAfter):
+        (WebCore::RenderBoxModelObject::removeFromContinuationChain):
+        (WebCore::RenderBoxModelObject::ensureContinuationChainNode):
+        (WebCore::continuationMap): Deleted.
+        (WebCore::RenderBoxModelObject::setContinuation): Deleted.
+        * rendering/RenderBoxModelObject.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        (WebCore::RenderElement::removeAnonymousWrappersForInlinesIfNecessary):
+
+            Make this a function of the parent renderer itself instead of getting 'parent()' as first operation and
+            then using it.
+            Don't remove continuations (isAnonymousBlockContinuation() test gives wrong result for the last continuation of the chain).
+
+        (WebCore::RenderElement::styleDidChange):
+
+            removeAnonymousWrappersForInlinesIfNecessary is no function of the parent.
+
+        (WebCore::RenderElement::updateOutlineAutoAncestor):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::hasContinuationChainNode const):
+        (WebCore::RenderElement::setHasContinuationChainNode):
+        (WebCore::RenderElement::hasContinuation const): Deleted.
+        (WebCore::RenderElement::setHasContinuation): Deleted.
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::styleDidChange):
+
+            Don't add and remove continuations from the chain when updating style. Prevent recursion by walking
+            the chain only in the (non-continuation) head renderer.
+
+        (WebCore::RenderInline::addChildIgnoringContinuation):
+
+            Remove the old continuation from the chain. splitFlow() will add it back into the right place.
+
+        (WebCore::RenderInline::splitInlines):
+        (WebCore::RenderInline::addChildToContinuation):
+        (WebCore::RenderInline::childBecameNonInline):
+
+            Remove the old continuation from the chain. splitFlow() will add it back into the right place.
+
+        * rendering/RenderInline.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::propagateRepaintToParentWithOutlineAutoIfNeeded const):
+        (WebCore::RenderObject::outputRenderObject const):
+        (WebCore::findDestroyRootIncludingAnonymous):
+
+            Allow anonymous continuations as destroy roots.
+
+        (WebCore::RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers):
+
+            Removing a continuation may leave behind unnecessary anonymous sibling wrappers.
+            Call removeAnonymousWrappersForInlinesIfNecessary() on parent after removal to get rid of them.
+
 2017-10-31  Said Abou-Hallawa  <[email protected]>
 
         updateMaskedAncestorShouldIsolateBlending() should check the Nullability of the computedStyle() of the element's ancestors

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (224272 => 224273)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2017-11-01 07:57:02 UTC (rev 224273)
@@ -163,7 +163,7 @@
 
 static inline RenderObject* firstChildInContinuation(RenderInline& renderer)
 {
-    auto continuation = renderer.continuation();
+    auto* continuation = renderer.continuation();
 
     while (continuation) {
         if (is<RenderBlock>(*continuation))

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2017-11-01 07:57:02 UTC (rev 224273)
@@ -435,14 +435,10 @@
         adjustFragmentedFlowStateOnContainingBlockChangeIfNeeded();
 
     auto& newStyle = style();
-    if (!isAnonymousBlock()) {
+    if (!isAnonymousBlock() && !isContinuation()) {
         // Ensure that all of our continuation blocks pick up the new style.
-        for (RenderBlock* currCont = blockElementContinuation(); currCont; currCont = currCont->blockElementContinuation()) {
-            RenderBoxModelObject* nextCont = currCont->continuation();
-            currCont->setContinuation(0);
+        for (RenderBlock* currCont = blockElementContinuation(); currCont; currCont = currCont->blockElementContinuation())
             currCont->setStyle(RenderStyle::clone(newStyle));
-            currCont->setContinuation(nextCont);
-        }
     }
 
     propagateStyleToAnonymousChildren(PropagateToBlockChildrenOnly);
@@ -818,14 +814,14 @@
     return true;
 }
 
-void RenderBlock::dropAnonymousBoxChild(RenderBlock& parent, RenderBlock& child)
+void RenderBlock::dropAnonymousBoxChild(RenderBlock& child)
 {
-    parent.setNeedsLayoutAndPrefWidthsRecalc();
-    parent.setChildrenInline(child.childrenInline());
+    setNeedsLayoutAndPrefWidthsRecalc();
+    setChildrenInline(child.childrenInline());
     RenderObject* nextSibling = child.nextSibling();
 
-    auto toBeDeleted = parent.takeChildInternal(child, child.hasLayer() ? NotifyChildren : DontNotifyChildren);
-    child.moveAllChildrenTo(&parent, nextSibling, child.hasLayer());
+    auto toBeDeleted = takeChildInternal(child, child.hasLayer() ? NotifyChildren : DontNotifyChildren);
+    child.moveAllChildrenTo(this, nextSibling, child.hasLayer());
     // Delete the now-empty block's lines and nuke it.
     child.deleteLines();
 }
@@ -893,7 +889,7 @@
     if (canMergeAnonymousBlocks && child && !child->previousSibling() && !child->nextSibling() && canDropAnonymousBlockChild()) {
         // The removal has knocked us down to containing only a single anonymous
         // box. We can pull the content right back up into our box.
-        dropAnonymousBoxChild(*this, downcast<RenderBlock>(*child));
+        dropAnonymousBoxChild(downcast<RenderBlock>(*child));
     } else if (((prev && prev->isAnonymousBlock()) || (next && next->isAnonymousBlock())) && canDropAnonymousBlockChild()) {
         // It's possible that the removal has knocked us down to a single anonymous
         // block with floating siblings.
@@ -909,7 +905,7 @@
                 }
             }
             if (dropAnonymousBlock)
-                dropAnonymousBoxChild(*this, anonBlock);
+                dropAnonymousBoxChild(anonBlock);
         }
     }
 
@@ -917,31 +913,6 @@
         // If this was our last child be sure to clear out our line boxes.
         if (childrenInline())
             deleteLines();
-
-        // If we are an empty anonymous block in the continuation chain,
-        // we need to remove ourself and fix the continuation chain.
-        if (!beingDestroyed() && isAnonymousBlockContinuation() && !oldChild.isListMarker()) {
-            auto containingBlockIgnoringAnonymous = containingBlock();
-            while (containingBlockIgnoringAnonymous && containingBlockIgnoringAnonymous->isAnonymousBlock())
-                containingBlockIgnoringAnonymous = containingBlockIgnoringAnonymous->containingBlock();
-            for (RenderObject* current = this; current; current = current->previousInPreOrder(containingBlockIgnoringAnonymous)) {
-                if (!is<RenderBoxModelObject>(current) || downcast<RenderBoxModelObject>(*current).continuation() != this)
-                    continue;
-                // Found our previous continuation. We just need to point it to
-                // |this|'s next continuation.
-                auto* nextContinuation = continuation();
-                if (is<RenderInline>(*current))
-                    downcast<RenderInline>(*current).setContinuation(nextContinuation);
-                else if (is<RenderBlock>(*current))
-                    downcast<RenderBlock>(*current).setContinuation(nextContinuation);
-                else
-                    ASSERT_NOT_REACHED();
-                break;
-            }
-            setContinuation(nullptr);
-            // FIXME: This is dangerous.
-            removeFromParentAndDestroy();
-        }
     }
     return takenChild;
 }

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2017-11-01 07:57:02 UTC (rev 224273)
@@ -192,12 +192,9 @@
     WEBCORE_EXPORT RenderInline* inlineElementContinuation() const;
     RenderBlock* blockElementContinuation() const;
 
-    using RenderBoxModelObject::continuation;
-    using RenderBoxModelObject::setContinuation;
-
     static RenderPtr<RenderBlock> createAnonymousWithParentRendererAndDisplay(const RenderBox& parent, EDisplay = BLOCK);
     RenderPtr<RenderBlock> createAnonymousBlock(EDisplay = BLOCK) const;
-    static void dropAnonymousBoxChild(RenderBlock& parent, RenderBlock& child);
+    void dropAnonymousBoxChild(RenderBlock& child);
 
     RenderPtr<RenderBox> createAnonymousBoxWithSameTypeAs(const RenderBox&) const override;
 

Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp	2017-11-01 07:57:02 UTC (rev 224273)
@@ -77,10 +77,54 @@
 // an anonymous block (that houses other blocks) or it will be an inline flow.
 // <b><i><p>Hello</p></i></b>. In this example the <i> will have a block as
 // its continuation but the <b> will just have an inline as its continuation.
-typedef HashMap<const RenderBoxModelObject*, WeakPtr<RenderBoxModelObject>> ContinuationMap;
-static ContinuationMap& continuationMap()
+
+struct RenderBoxModelObject::ContinuationChainNode {
+    WeakPtr<RenderBoxModelObject> renderer;
+    ContinuationChainNode* previous { nullptr };
+    ContinuationChainNode* next { nullptr };
+
+    ContinuationChainNode(RenderBoxModelObject&);
+    ~ContinuationChainNode();
+
+    void insertAfter(ContinuationChainNode&);
+
+    WTF_MAKE_FAST_ALLOCATED;
+};
+
+RenderBoxModelObject::ContinuationChainNode::ContinuationChainNode(RenderBoxModelObject& renderer)
+    : renderer(makeWeakPtr(renderer))
 {
-    static NeverDestroyed<ContinuationMap> map;
+}
+
+RenderBoxModelObject::ContinuationChainNode::~ContinuationChainNode()
+{
+    if (next) {
+        ASSERT(previous);
+        ASSERT(next->previous == this);
+        next->previous = previous;
+    }
+    if (previous) {
+        ASSERT(previous->next == this);
+        previous->next = next;
+    }
+}
+
+void RenderBoxModelObject::ContinuationChainNode::insertAfter(ContinuationChainNode& after)
+{
+    ASSERT(!previous);
+    ASSERT(!next);
+    if ((next = after.next)) {
+        ASSERT(next->previous = &after);
+        next->previous = this;
+    }
+    previous = &after;
+    after.next = this;
+}
+
+typedef HashMap<const RenderBoxModelObject*, std::unique_ptr<RenderBoxModelObject::ContinuationChainNode>> ContinuationChainNodeMap;
+static ContinuationChainNodeMap& continuationChainNodeMap()
+{
+    static NeverDestroyed<ContinuationChainNodeMap> map;
     return map;
 }
 
@@ -187,10 +231,12 @@
 
 void RenderBoxModelObject::willBeDestroyed()
 {
-    if (hasContinuation()) {
-        continuation()->removeFromParentAndDestroy();
-        setContinuation(nullptr);
+    if (continuation() && !isContinuation()) {
+        removeAndDestroyAllContinuations();
+        ASSERT(!continuation());
     }
+    if (hasContinuationChainNode())
+        removeFromContinuationChain();
 
     // If this is a first-letter object with a remaining text fragment then the
     // entry needs to be cleared from the map.
@@ -2447,21 +2493,51 @@
 
 RenderBoxModelObject* RenderBoxModelObject::continuation() const
 {
-    if (!hasContinuation())
+    if (!hasContinuationChainNode())
         return nullptr;
-    return continuationMap().get(this).get();
+
+    auto& continuationChainNode = *continuationChainNodeMap().get(this);
+    if (!continuationChainNode.next)
+        return nullptr;
+    return continuationChainNode.next->renderer.get();
 }
 
-void RenderBoxModelObject::setContinuation(RenderBoxModelObject* continuation)
+void RenderBoxModelObject::insertIntoContinuationChainAfter(RenderBoxModelObject& afterRenderer)
 {
-    ASSERT(!continuation || continuation->isContinuation());
-    if (continuation)
-        continuationMap().set(this, makeWeakPtr(continuation));
-    else if (hasContinuation())
-        continuationMap().remove(this);
-    setHasContinuation(!!continuation);
+    ASSERT(isContinuation());
+    ASSERT(!continuationChainNodeMap().contains(this));
+
+    auto& after = afterRenderer.ensureContinuationChainNode();
+    ensureContinuationChainNode().insertAfter(after);
 }
 
+void RenderBoxModelObject::removeFromContinuationChain()
+{
+    ASSERT(hasContinuationChainNode());
+    ASSERT(continuationChainNodeMap().contains(this));
+    setHasContinuationChainNode(false);
+    continuationChainNodeMap().remove(this);
+}
+
+auto RenderBoxModelObject::ensureContinuationChainNode() -> ContinuationChainNode&
+{
+    setHasContinuationChainNode(true);
+    return *continuationChainNodeMap().ensure(this, [&] {
+        return std::make_unique<ContinuationChainNode>(*this);
+    }).iterator->value;
+}
+
+void RenderBoxModelObject::removeAndDestroyAllContinuations()
+{
+    ASSERT(!isContinuation());
+    ASSERT(hasContinuationChainNode());
+    ASSERT(continuationChainNodeMap().contains(this));
+    auto& continuationChainNode = *continuationChainNodeMap().get(this);
+    while (continuationChainNode.next)
+        continuationChainNode.next->renderer->removeFromParentAndDestroy();
+    removeFromContinuationChain();
+}
+
 RenderTextFragment* RenderBoxModelObject::firstLetterRemainingText() const
 {
     if (!firstLetterRemainingTextMap)

Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.h (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderBoxModelObject.h	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.h	2017-11-01 07:57:02 UTC (rev 224273)
@@ -235,6 +235,9 @@
     void suspendAnimations(double time = 0);
 
     RenderBoxModelObject* continuation() const;
+    
+    void insertIntoContinuationChainAfter(RenderBoxModelObject&);
+    void removeFromContinuationChain();
 
     virtual LayoutRect paintRectToClipOutFromBorder(const LayoutRect&) { return LayoutRect(); };
     
@@ -256,8 +259,6 @@
 
     InterpolationQuality chooseInterpolationQuality(GraphicsContext&, Image&, const void*, const LayoutSize&);
 
-    void setContinuation(RenderBoxModelObject*);
-
     LayoutRect localCaretRectForEmptyElement(LayoutUnit width, LayoutUnit textIndentOffset);
 
     static bool shouldAntialiasLines(GraphicsContext&);
@@ -301,7 +302,12 @@
 
     RenderBlock* containingBlockForAutoHeightDetection(Length logicalHeight) const;
 
+    struct ContinuationChainNode;
+
 private:
+    ContinuationChainNode& ensureContinuationChainNode();
+    void removeAndDestroyAllContinuations();
+
     LayoutUnit computedCSSPadding(const Length&) const;
     
     virtual LayoutRect frameRectForStickyPositioning() const = 0;

Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderElement.cpp	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp	2017-11-01 07:57:02 UTC (rev 224273)
@@ -101,7 +101,7 @@
     , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_hasCounterNodeMap(false)
-    , m_hasContinuation(false)
+    , m_hasContinuationChainNode(false)
     , m_isContinuation(false)
     , m_hasValidCachedFirstLineStyle(false)
     , m_renderBlockHasMarginBeforeQuirk(false)
@@ -961,9 +961,12 @@
 
 void RenderElement::removeAnonymousWrappersForInlinesIfNecessary()
 {
-    RenderBlock& parentBlock = downcast<RenderBlock>(*parent());
-    if (!parentBlock.canDropAnonymousBlockChild())
+    // FIXME: Move to RenderBlock.
+    if (!is<RenderBlock>(*this))
         return;
+    RenderBlock& thisBlock = downcast<RenderBlock>(*this);
+    if (!thisBlock.canDropAnonymousBlockChild())
+        return;
 
     // We have changed to floated or out-of-flow positioning so maybe all our parent's
     // children can be inline now. Bail if there are any block children left on the line,
@@ -970,8 +973,8 @@
     // otherwise we can proceed to stripping solitary anonymous wrappers from the inlines.
     // FIXME: We should also handle split inlines here - we exclude them at the moment by returning
     // if we find a continuation.
-    RenderObject* current = parent()->firstChild();
-    while (current && ((current->isAnonymousBlock() && !downcast<RenderBlock>(*current).isAnonymousBlockContinuation()) || current->style().isFloating() || current->style().hasOutOfFlowPosition()))
+    RenderObject* current = firstChild();
+    while (current && ((current->isAnonymousBlock() && !downcast<RenderBlock>(*current).isContinuation()) || current->style().isFloating() || current->style().hasOutOfFlowPosition()))
         current = current->nextSibling();
 
     if (current)
@@ -978,10 +981,10 @@
         return;
 
     RenderObject* next;
-    for (current = parent()->firstChild(); current; current = next) {
+    for (current = firstChild(); current; current = next) {
         next = current->nextSibling();
         if (current->isAnonymousBlock())
-            parentBlock.dropAnonymousBoxChild(parentBlock, downcast<RenderBlock>(*current));
+            thisBlock.dropAnonymousBoxChild(downcast<RenderBlock>(*current));
     }
 }
 
@@ -1010,7 +1013,7 @@
         handleDynamicFloatPositionChange();
 
     if (s_noLongerAffectsParentBlock)
-        removeAnonymousWrappersForInlinesIfNecessary();
+        parent()->removeAnonymousWrappersForInlinesIfNecessary();
 
     SVGRenderSupport::styleChanged(*this, oldStyle);
 
@@ -2152,8 +2155,10 @@
             continue;
         downcast<RenderElement>(child).updateOutlineAutoAncestor(hasOutlineAuto);
     }
-    if (hasContinuation())
-        downcast<RenderBoxModelObject>(*this).continuation()->updateOutlineAutoAncestor(hasOutlineAuto);
+    if (is<RenderBoxModelObject>(*this)) {
+        if (auto* continuation = downcast<RenderBoxModelObject>(*this).continuation())
+            continuation->updateOutlineAutoAncestor(hasOutlineAuto);
+    }
 }
 
 bool RenderElement::hasOutlineAnnotation() const

Modified: trunk/Source/WebCore/rendering/RenderElement.h (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderElement.h	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderElement.h	2017-11-01 07:57:02 UTC (rev 224273)
@@ -221,7 +221,9 @@
     // the child.
     virtual void updateAnonymousChildStyle(const RenderObject&, RenderStyle&) const { };
 
-    bool hasContinuation() const { return m_hasContinuation; }
+    void removeAnonymousWrappersForInlinesIfNecessary();
+
+    bool hasContinuationChainNode() const { return m_hasContinuationChainNode; }
     bool isContinuation() const { return m_isContinuation; }
     void setIsContinuation() { m_isContinuation = true; }
     bool isElementContinuation() const { return isContinuation() && !isAnonymous(); }
@@ -264,7 +266,7 @@
     void setRenderInlineAlwaysCreatesLineBoxes(bool b) { m_renderInlineAlwaysCreatesLineBoxes = b; }
     bool renderInlineAlwaysCreatesLineBoxes() const { return m_renderInlineAlwaysCreatesLineBoxes; }
 
-    void setHasContinuation(bool b) { m_hasContinuation = b; }
+    void setHasContinuationChainNode(bool b) { m_hasContinuationChainNode = b; }
 
     void setRenderBlockHasMarginBeforeQuirk(bool b) { m_renderBlockHasMarginBeforeQuirk = b; }
     void setRenderBlockHasMarginAfterQuirk(bool b) { m_renderBlockHasMarginAfterQuirk = b; }
@@ -303,7 +305,6 @@
     // again.  We have to make sure the render tree updates as needed to accommodate the new
     // normal flow object.
     void handleDynamicFloatPositionChange();
-    void removeAnonymousWrappersForInlinesIfNecessary();
 
     bool shouldRepaintForStyleDifference(StyleDifference) const;
     bool hasImmediateNonWhitespaceTextChildOrBorderOrOutline() const;
@@ -336,7 +337,7 @@
     unsigned m_renderBoxNeedsLazyRepaint : 1;
     unsigned m_hasPausedImageAnimations : 1;
     unsigned m_hasCounterNodeMap : 1;
-    unsigned m_hasContinuation : 1;
+    unsigned m_hasContinuationChainNode : 1;
     unsigned m_isContinuation : 1;
     mutable unsigned m_hasValidCachedFirstLineStyle : 1;
 

Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderInline.cpp	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp	2017-11-01 07:57:02 UTC (rev 224273)
@@ -189,13 +189,9 @@
     // need to pass its style on to anyone else.
     auto& newStyle = style();
     RenderInline* continuation = inlineElementContinuation();
-    if (continuation) {
-        for (RenderInline* currCont = continuation; currCont; currCont = currCont->inlineElementContinuation()) {
-            RenderBoxModelObject* nextCont = currCont->continuation();
-            currCont->setContinuation(nullptr);
+    if (continuation && !isContinuation()) {
+        for (RenderInline* currCont = continuation; currCont; currCont = currCont->inlineElementContinuation())
             currCont->setStyle(RenderStyle::clone(newStyle));
-            currCont->setContinuation(nextCont);
-        }
         // If an inline's in-flow positioning has changed and it is part of an active continuation as a descendant of an anonymous containing block,
         // then any descendant blocks will need to change their in-flow positioning accordingly.
         // Do this by updating the position of the descendant blocks' containing anonymous blocks - there may be more than one.
@@ -344,7 +340,9 @@
         newBox->initializeStyle();
         newBox->setIsContinuation();
         RenderBoxModelObject* oldContinuation = continuation();
-        setContinuation(newBox.get());
+        if (oldContinuation)
+            oldContinuation->removeFromContinuationChain();
+        newBox->insertIntoContinuationChainAfter(*this);
 
         splitFlow(beforeChild, WTFMove(newBox), WTFMove(newChild), oldContinuation);
         return;
@@ -417,9 +415,10 @@
         rendererToMove->setNeedsLayoutAndPrefWidthsRecalc();
         rendererToMove = nextSibling;
     }
-    cloneInline->setContinuation(oldCont);
     // Hook |clone| up as the continuation of the middle block.
-    middleBlock->setContinuation(cloneInline.get());
+    cloneInline->insertIntoContinuationChainAfter(*middleBlock);
+    if (oldCont)
+        oldCont->insertIntoContinuationChainAfter(*cloneInline);
 
     // We have been reparented and are now under the fromBlock.  We need
     // to walk up our inline parent chain until we hit the containing block.
@@ -443,19 +442,16 @@
             cloneInline->addChildIgnoringContinuation(WTFMove(cloneChild));
 
             // Hook the clone up as a continuation of |curr|.
-            RenderInline& currentInline = downcast<RenderInline>(*current);
-            oldCont = currentInline.continuation();
-            currentInline.setContinuation(cloneInline.get());
-            cloneInline->setContinuation(oldCont);
+            cloneInline->insertIntoContinuationChainAfter(*current);
 
             // Now we need to take all of the children starting from the first child
             // *after* currentChild and append them all to the clone.
-            for (auto* current = currentChild->nextSibling(); current;) {
-                auto* next = current->nextSibling();
-                auto childToMove = currentInline.takeChildInternal(*current, NotifyChildren);
+            for (auto* sibling = currentChild->nextSibling(); sibling;) {
+                auto* next = sibling->nextSibling();
+                auto childToMove = current->takeChildInternal(*sibling, NotifyChildren);
                 cloneInline->addChildIgnoringContinuation(WTFMove(childToMove));
-                current->setNeedsLayoutAndPrefWidthsRecalc();
-                current = next;
+                sibling->setNeedsLayoutAndPrefWidthsRecalc();
+                sibling = next;
             }
         }
         
@@ -576,7 +572,7 @@
         auto* parent = beforeChild->parent();
         while (parent && parent->parent() && parent->parent()->isAnonymous()) {
             // The ancestor candidate needs to be inside the continuation.
-            if (parent->hasContinuation())
+            if (parent->isContinuation())
                 break;
             parent = parent->parent();
         }
@@ -1379,7 +1375,9 @@
     auto newBox = containingBlock()->createAnonymousBlock();
     newBox->setIsContinuation();
     RenderBoxModelObject* oldContinuation = continuation();
-    setContinuation(newBox.get());
+    if (oldContinuation)
+        oldContinuation->removeFromContinuationChain();
+    newBox->insertIntoContinuationChainAfter(*this);
     RenderObject* beforeChild = child.nextSibling();
     auto removedChild = takeChildInternal(child, NotifyChildren);
     splitFlow(beforeChild, WTFMove(newBox), WTFMove(removedChild), oldContinuation);

Modified: trunk/Source/WebCore/rendering/RenderInline.h (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderInline.h	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderInline.h	2017-11-01 07:57:02 UTC (rev 224273)
@@ -87,9 +87,6 @@
     void addFocusRingRects(Vector<LayoutRect>&, const LayoutPoint& additionalOffset, const RenderLayerModelObject* paintContainer = 0) final;
     void paintOutline(PaintInfo&, const LayoutPoint&);
 
-    using RenderBoxModelObject::continuation;
-    using RenderBoxModelObject::setContinuation;
-
     bool alwaysCreateLineBoxes() const { return renderInlineAlwaysCreatesLineBoxes(); }
     void setAlwaysCreateLineBoxes() { setRenderInlineAlwaysCreatesLineBoxes(true); }
     void updateAlwaysCreateLineBoxes(bool fullLayout);

Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (224272 => 224273)


--- trunk/Source/WebCore/rendering/RenderObject.cpp	2017-11-01 06:15:59 UTC (rev 224272)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp	2017-11-01 07:57:02 UTC (rev 224273)
@@ -824,7 +824,7 @@
         bool rendererHasOutlineAutoAncestor = renderer->hasOutlineAutoAncestor();
         ASSERT(rendererHasOutlineAutoAncestor
             || renderer->outlineStyleForRepaint().outlineStyleIsAuto()
-            || (is<RenderElement>(*renderer) && downcast<RenderElement>(*renderer).hasContinuation()));
+            || (is<RenderBoxModelObject>(*renderer) && downcast<RenderBoxModelObject>(*renderer).isContinuation()));
         if (renderer == &repaintContainer && rendererHasOutlineAutoAncestor)
             repaintRectNeedsConverting = true;
         if (rendererHasOutlineAutoAncestor)
@@ -1175,7 +1175,7 @@
     }
     if (is<RenderBoxModelObject>(*this)) {
         auto& renderer = downcast<RenderBoxModelObject>(*this);
-        if (renderer.hasContinuation())
+        if (renderer.continuation())
             stream << " continuation->(" << renderer.continuation() << ")";
     }
     outputRegionsInformation(stream);
@@ -1466,9 +1466,6 @@
             break;
         if (destroyRootParent->isRenderFragmentedFlow())
             break;
-        // FIXME: Destroy continuations here too.
-        if (destroyRootParent->isContinuation())
-            break;
         bool destroyingOnlyChild = destroyRootParent->firstChild() == destroyRoot && destroyRootParent->lastChild() == destroyRoot;
         if (!destroyingOnlyChild)
             break;
@@ -1490,7 +1487,9 @@
     if (is<RenderTableRow>(destroyRoot))
         downcast<RenderTableRow>(destroyRoot).collapseAndDestroyAnonymousSiblingRows();
 
-    destroyRoot.removeFromParentAndDestroy();
+    auto& destroyRootParent = *destroyRoot.parent();
+    destroyRootParent.removeAndDestroyChild(destroyRoot);
+    destroyRootParent.removeAnonymousWrappersForInlinesIfNecessary();
     // WARNING: |this| is deleted here.
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to