Remove empty continuations in RenderObject::removeFromParentAndDestroyCleaningUpAnony...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Nov 2017 11:31:06 +0000 (11:31 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Nov 2017 11:31:06 +0000 (11:31 +0000)
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.
    If takeChild/removeAnonymousWrappersForInlinesIfNecessary leaves us with empty anonymous parent destroy that too.

* rendering/RenderRubyRun.cpp:
(WebCore::RenderRubyRun::takeChild):

    Similar to RenderBlock::takeChild, remove the code that would make the renderer destroy itself.
    Cleaning up RenderRubyRuns is now handled by removeFromParentAndDestroyCleaningUpAnonymousWrappers.

LayoutTests:

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

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/ruby/float-overhang-from-ruby-text-expected.txt
LayoutTests/platform/mac/fast/ruby/rubyDOM-remove-rt1-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderBoxModelObject.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderInline.cpp
Source/WebCore/rendering/RenderInline.h
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderRubyRun.cpp

index 93fafb4..027957c 100644 (file)
@@ -1,3 +1,13 @@
+2017-11-02  Antti Koivisto  <antti@apple.com>
+
+        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-11-02  Ms2ger  <Ms2ger@igalia.com>
 
         [GTK] Test gardening.
index bbff5f5..0e9a2e7 100644 (file)
@@ -17,7 +17,6 @@ layer at (0,0) size 800x600
       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
index 8c9c6aa..af8c365 100644 (file)
@@ -24,9 +24,8 @@ layer at (0,0) size 800x600
               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
index 48a535f..51a03ac 100644 (file)
@@ -1,3 +1,109 @@
+2017-11-02  Antti Koivisto  <antti@apple.com>
+
+        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.
+            If takeChild/removeAnonymousWrappersForInlinesIfNecessary leaves us with empty anonymous parent destroy that too.
+
+        * rendering/RenderRubyRun.cpp:
+        (WebCore::RenderRubyRun::takeChild):
+
+            Similar to RenderBlock::takeChild, remove the code that would make the renderer destroy itself.
+            Cleaning up RenderRubyRuns is now handled by removeFromParentAndDestroyCleaningUpAnonymousWrappers.
+
 2017-11-02  Ryosuke Niwa  <rniwa@webkit.org>
 
         REGRESSION(r224053): Crash in WebCore::Node::moveTreeToNewScope
index 50ec8ff..ba2352f 100644 (file)
@@ -163,7 +163,7 @@ static inline bool isInlineWithContinuation(RenderObject& object)
 
 static inline RenderObject* firstChildInContinuation(RenderInline& renderer)
 {
-    auto continuation = renderer.continuation();
+    auto* continuation = renderer.continuation();
 
     while (continuation) {
         if (is<RenderBlock>(*continuation))
index 47ee5d8..68635c3 100644 (file)
@@ -435,14 +435,10 @@ void RenderBlock::styleDidChange(StyleDifference diff, const RenderStyle* oldSty
         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 @@ static bool canMergeContiguousAnonymousBlocks(RenderObject& oldChild, RenderObje
     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 @@ RenderPtr<RenderObject> RenderBlock::takeChild(RenderObject& oldChild)
     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 @@ RenderPtr<RenderObject> RenderBlock::takeChild(RenderObject& oldChild)
                 }
             }
             if (dropAnonymousBlock)
-                dropAnonymousBoxChild(*this, anonBlock);
+                dropAnonymousBoxChild(anonBlock);
         }
     }
 
@@ -917,31 +913,6 @@ RenderPtr<RenderObject> RenderBlock::takeChild(RenderObject& oldChild)
         // 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;
 }
index 881ea0c..c8b3e65 100644 (file)
@@ -192,12 +192,9 @@ public:
     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;
 
index 2c405ed..564b9c1 100644 (file)
@@ -77,10 +77,55 @@ using namespace HTMLNames;
 // 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))
+{
+}
+
+RenderBoxModelObject::ContinuationChainNode::~ContinuationChainNode()
 {
-    static NeverDestroyed<ContinuationMap> map;
+    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;
+}
+
+using ContinuationChainNodeMap = HashMap<const RenderBoxModelObject*, std::unique_ptr<RenderBoxModelObject::ContinuationChainNode>>;
+
+static ContinuationChainNodeMap& continuationChainNodeMap()
+{
+    static NeverDestroyed<ContinuationChainNodeMap> map;
     return map;
 }
 
@@ -190,10 +235,12 @@ RenderBoxModelObject::~RenderBoxModelObject()
 
 void RenderBoxModelObject::willBeDestroyed()
 {
-    if (hasContinuation()) {
-        continuation()->removeFromParentAndDestroy();
-        setContinuation(nullptr);
+    if (continuation() && !isContinuation()) {
+        removeAndDestroyAllContinuations();
+        ASSERT(!continuation());
     }
+    if (hasContinuationChainNode())
+        removeFromContinuationChain();
 
     if (isFirstLetter())
         clearFirstLetterRemainingText();
@@ -2448,19 +2495,51 @@ LayoutUnit RenderBoxModelObject::containingBlockLogicalWidthForContent() const
 
 RenderBoxModelObject* RenderBoxModelObject::continuation() const
 {
-    if (!hasContinuation())
+    if (!hasContinuationChainNode())
+        return nullptr;
+
+    auto& continuationChainNode = *continuationChainNodeMap().get(this);
+    if (!continuationChainNode.next)
         return nullptr;
-    return continuationMap().get(this).get();
+    return continuationChainNode.next->renderer.get();
+}
+
+void RenderBoxModelObject::insertIntoContinuationChainAfter(RenderBoxModelObject& afterRenderer)
+{
+    ASSERT(isContinuation());
+    ASSERT(!continuationChainNodeMap().contains(this));
+
+    auto& after = afterRenderer.ensureContinuationChainNode();
+    ensureContinuationChainNode().insertAfter(after);
 }
 
-void RenderBoxModelObject::setContinuation(RenderBoxModelObject* continuation)
+void RenderBoxModelObject::removeFromContinuationChain()
 {
-    ASSERT(!continuation || continuation->isContinuation());
-    if (continuation)
-        continuationMap().set(this, makeWeakPtr(continuation));
-    else if (hasContinuation())
-        continuationMap().remove(this);
-    setHasContinuation(!!continuation);
+    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) {
+        ASSERT(!continuationChainNode.next->renderer->firstChild());
+        continuationChainNode.next->renderer->removeFromParentAndDestroy();
+    }
+    removeFromContinuationChain();
 }
 
 RenderTextFragment* RenderBoxModelObject::firstLetterRemainingText() const
index 8c9a35d..d574b20 100644 (file)
@@ -235,6 +235,9 @@ public:
     void suspendAnimations(double time = 0);
 
     RenderBoxModelObject* continuation() const;
+    
+    void insertIntoContinuationChainAfter(RenderBoxModelObject&);
+    void removeFromContinuationChain();
 
     virtual LayoutRect paintRectToClipOutFromBorder(const LayoutRect&) { return LayoutRect(); };
     
@@ -256,8 +259,6 @@ protected:
 
     InterpolationQuality chooseInterpolationQuality(GraphicsContext&, Image&, const void*, const LayoutSize&);
 
-    void setContinuation(RenderBoxModelObject*);
-
     LayoutRect localCaretRectForEmptyElement(LayoutUnit width, LayoutUnit textIndentOffset);
 
     static bool shouldAntialiasLines(GraphicsContext&);
@@ -302,7 +303,12 @@ public:
 
     RenderBlock* containingBlockForAutoHeightDetection(Length logicalHeight) const;
 
+    struct ContinuationChainNode;
+
 private:
+    ContinuationChainNode& ensureContinuationChainNode();
+    void removeAndDestroyAllContinuations();
+
     LayoutUnit computedCSSPadding(const Length&) const;
     
     virtual LayoutRect frameRectForStickyPositioning() const = 0;
index 86f7bb9..0a6aae7 100644 (file)
@@ -101,7 +101,7 @@ inline RenderElement::RenderElement(ContainerNode& elementOrDocument, RenderStyl
     , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_hasCounterNodeMap(false)
-    , m_hasContinuation(false)
+    , m_hasContinuationChainNode(false)
     , m_isContinuation(false)
     , m_isFirstLetter(false)
     , m_hasValidCachedFirstLineStyle(false)
@@ -962,8 +962,11 @@ void RenderElement::handleDynamicFloatPositionChange()
 
 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
@@ -971,18 +974,18 @@ void RenderElement::removeAnonymousWrappersForInlinesIfNecessary()
     // 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)
         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));
     }
 }
 
@@ -1011,7 +1014,7 @@ void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldS
         handleDynamicFloatPositionChange();
 
     if (s_noLongerAffectsParentBlock)
-        removeAnonymousWrappersForInlinesIfNecessary();
+        parent()->removeAnonymousWrappersForInlinesIfNecessary();
 
     SVGRenderSupport::styleChanged(*this, oldStyle);
 
@@ -2153,8 +2156,10 @@ void RenderElement::updateOutlineAutoAncestor(bool hasOutlineAuto)
             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
index 39c1ba0..4cdeceb 100644 (file)
@@ -221,7 +221,9 @@ public:
     // 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(); }
@@ -266,7 +268,7 @@ protected:
     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; }
@@ -305,7 +307,6 @@ private:
     // 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;
@@ -338,7 +339,7 @@ private:
     unsigned m_renderBoxNeedsLazyRepaint : 1;
     unsigned m_hasPausedImageAnimations : 1;
     unsigned m_hasCounterNodeMap : 1;
-    unsigned m_hasContinuation : 1;
+    unsigned m_hasContinuationChainNode : 1;
     unsigned m_isContinuation : 1;
     unsigned m_isFirstLetter : 1;
     mutable unsigned m_hasValidCachedFirstLineStyle : 1;
index 82039c6..f5c444d 100644 (file)
@@ -189,13 +189,9 @@ void RenderInline::styleDidChange(StyleDifference diff, const RenderStyle* oldSt
     // 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 @@ void RenderInline::addChildIgnoringContinuation(RenderPtr<RenderObject> newChild
         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 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
         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 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
             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 @@ void RenderInline::addChildToContinuation(RenderPtr<RenderObject> newChild, Rend
         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 @@ void RenderInline::childBecameNonInline(RenderElement& child)
     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);
index 5137aa5..7090f95 100644 (file)
@@ -87,9 +87,6 @@ public:
     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);
index a63a0bf..a7cc073 100644 (file)
@@ -824,7 +824,7 @@ void RenderObject::propagateRepaintToParentWithOutlineAutoIfNeeded(const RenderL
         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 @@ void RenderObject::outputRenderObject(TextStream& stream, bool mark, int depth)
     }
     if (is<RenderBoxModelObject>(*this)) {
         auto& renderer = downcast<RenderBoxModelObject>(*this);
-        if (renderer.hasContinuation())
+        if (renderer.continuation())
             stream << " continuation->(" << renderer.continuation() << ")";
     }
     outputRegionsInformation(stream);
@@ -1455,24 +1455,26 @@ void RenderObject::willBeRemovedFromTree()
     parent()->setNeedsBoundariesUpdate();
 }
 
+static bool isAnonymousAndSafeToDelete(RenderElement& element)
+{
+    if (!element.isAnonymous())
+        return false;
+    if (element.isRenderView() || element.isRenderFragmentedFlow())
+        return false;
+    return true;
+}
+
 static RenderObject& findDestroyRootIncludingAnonymous(RenderObject& renderer)
 {
     auto* destroyRoot = &renderer;
     while (true) {
-        auto* destroyRootParent = destroyRoot->parent();
-        if (!destroyRootParent->isAnonymous())
+        auto& destroyRootParent = *destroyRoot->parent();
+        if (!isAnonymousAndSafeToDelete(destroyRootParent))
             break;
-        if (destroyRootParent->isRenderView())
-            break;
-        if (destroyRootParent->isRenderFragmentedFlow())
-            break;
-        // FIXME: Destroy continuations here too.
-        if (destroyRootParent->isContinuation())
-            break;
-        bool destroyingOnlyChild = destroyRootParent->firstChild() == destroyRoot && destroyRootParent->lastChild() == destroyRoot;
+        bool destroyingOnlyChild = destroyRootParent.firstChild() == destroyRoot && destroyRootParent.lastChild() == destroyRoot;
         if (!destroyingOnlyChild)
             break;
-        destroyRoot = destroyRootParent;
+        destroyRoot = &destroyRootParent;
     }
     return *destroyRoot;
 }
@@ -1490,7 +1492,14 @@ void RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers()
     if (is<RenderTableRow>(destroyRoot))
         downcast<RenderTableRow>(destroyRoot).collapseAndDestroyAnonymousSiblingRows();
 
-    destroyRoot.removeFromParentAndDestroy();
+    auto& destroyRootParent = *destroyRoot.parent();
+    destroyRootParent.removeAndDestroyChild(destroyRoot);
+    destroyRootParent.removeAnonymousWrappersForInlinesIfNecessary();
+
+    // Anonymous parent might have become empty, try to delete it too.
+    if (isAnonymousAndSafeToDelete(destroyRootParent) && !destroyRootParent.firstChild())
+        destroyRootParent.removeFromParentAndDestroyCleaningUpAnonymousWrappers();
+
     // WARNING: |this| is deleted here.
 }
 
index dd24a99..0615b83 100644 (file)
@@ -181,12 +181,6 @@ RenderPtr<RenderObject> RenderRubyRun::takeChild(RenderObject& child)
             auto takenBase = RenderBlockFlow::takeChild(*base);
             base->deleteLines();
         }
-
-        // If any of the above leaves the run empty, destroy it as well.
-        if (!hasRubyText() && !hasRubyBase()) {
-            auto takenThis = parent()->takeChild(*this);
-            deleteLines();
-        }
     }
 
     return takenChild;