[RenderTreeBuilder] Remove RenderElement::destroyLeftoverChildren.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Feb 2018 19:52:45 +0000 (19:52 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Feb 2018 19:52:45 +0000 (19:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182518
<rdar://problem/37256035>

Reviewed by Antti Koivisto.

Remove leftover children before we call takeChild() on the parent (as opposed to when
we finally call destroy() on the parent).
This patch also explicitly destroys the top level pagination renderers.

Covered by existing tests.

* rendering/RenderElement.cpp:
(WebCore::RenderElement::removeAndDestroyChild):
(WebCore::RenderElement::destroyLeftoverChildren): Deleted.
* rendering/RenderElement.h:
* rendering/RenderObject.cpp:
(WebCore::RenderObject::destroy):
* rendering/updating/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::tearDownRenderers):
(WebCore::RenderTreeUpdater::tearDownLeftoverPaginationRenderersIfNeeded):
* rendering/updating/RenderTreeUpdater.h:

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderView.cpp
Source/WebCore/rendering/updating/RenderTreeUpdater.cpp
Source/WebCore/rendering/updating/RenderTreeUpdater.h

index 60e974f..91c58d5 100644 (file)
@@ -1,3 +1,28 @@
+2018-02-07  Zalan Bujtas  <zalan@apple.com>
+
+        [RenderTreeBuilder] Remove RenderElement::destroyLeftoverChildren.
+        https://bugs.webkit.org/show_bug.cgi?id=182518
+        <rdar://problem/37256035>
+
+        Reviewed by Antti Koivisto.
+
+        Remove leftover children before we call takeChild() on the parent (as opposed to when
+        we finally call destroy() on the parent).
+        This patch also explicitly destroys the top level pagination renderers.
+
+        Covered by existing tests.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::removeAndDestroyChild):
+        (WebCore::RenderElement::destroyLeftoverChildren): Deleted.
+        * rendering/RenderElement.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::destroy):
+        * rendering/updating/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::tearDownRenderers):
+        (WebCore::RenderTreeUpdater::tearDownLeftoverPaginationRenderersIfNeeded):
+        * rendering/updating/RenderTreeUpdater.h:
+
 2018-02-07  Daniel Bates  <dabates@apple.com>
 
         Log error when authentication challenge is blocked due to an insecure request
index 3cb609e..7380df6 100644 (file)
@@ -136,6 +136,7 @@ RenderElement::RenderElement(Document& document, RenderStyle&& style, BaseTypeFl
 RenderElement::~RenderElement()
 {
     // Do not add any code here. Add it to willBeDestroyed() instead.
+    ASSERT(!m_firstChild);
 }
 
 RenderPtr<RenderElement> RenderElement::createFor(Element& element, RenderStyle&& style, RendererCreationType creationType)
@@ -487,16 +488,16 @@ RenderPtr<RenderObject> RenderElement::takeChild(RenderTreeBuilder&, RenderObjec
 
 void RenderElement::removeAndDestroyChild(RenderTreeBuilder& builder, RenderObject& oldChild)
 {
-    auto toDestroy = takeChild(builder, oldChild);
-}
-
-void RenderElement::destroyLeftoverChildren()
-{
-    while (m_firstChild) {
-        if (auto* node = m_firstChild->node())
-            node->setRenderer(nullptr);
-        removeAndDestroyChild(*RenderTreeBuilder::current(), *m_firstChild);
+    if (is<RenderElement>(oldChild)) {
+        auto& child = downcast<RenderElement>(oldChild);
+        while (child.firstChild()) {
+            auto& firstChild = *child.firstChild();
+            if (auto* node = firstChild.node())
+                node->setRenderer(nullptr);
+            child.removeAndDestroyChild(builder, firstChild);
+        }
     }
+    auto toDestroy = takeChild(builder, oldChild);
 }
 
 RenderObject* RenderElement::attachRendererInternal(RenderPtr<RenderObject> child, RenderObject* beforeChild)
index 3269ef2..907d219 100644 (file)
@@ -228,7 +228,6 @@ public:
     bool isFirstLetter() const { return m_isFirstLetter; }
     void setIsFirstLetter() { m_isFirstLetter = true; }
 
-    void destroyLeftoverChildren();
     RenderObject* attachRendererInternal(RenderPtr<RenderObject> child, RenderObject* beforeChild);
     RenderPtr<RenderObject> detachRendererInternal(RenderObject&);
 
index f7c5c11..67066ac 100644 (file)
@@ -1477,9 +1477,6 @@ void RenderObject::destroy()
     RELEASE_ASSERT(!m_previous);
     RELEASE_ASSERT(!m_bitfields.beingDestroyed());
 
-    if (is<RenderElement>(*this))
-        downcast<RenderElement>(*this).destroyLeftoverChildren();
-
     m_bitfields.setBeingDestroyed(true);
 
 #if PLATFORM(IOS)
index 67959ae..59416ba 100644 (file)
@@ -47,6 +47,7 @@
 #include "RenderMultiColumnSet.h"
 #include "RenderMultiColumnSpannerPlaceholder.h"
 #include "RenderQuote.h"
+#include "RenderTreeBuilder.h"
 #include "RenderWidget.h"
 #include "ScrollbarTheme.h"
 #include "Settings.h"
@@ -613,6 +614,12 @@ bool RenderView::isScrollableOrRubberbandableBox() const
 void RenderView::willBeDestroyed()
 {
     RenderBlockFlow::willBeDestroyed();
+    // FIXME: This is a workaround for leftover content (see webkit.org/b/182547).
+    if (firstChild()) {
+        RenderTreeBuilder builder(*this);
+        while (firstChild())
+            removeAndDestroyChild(builder, *firstChild());
+    }
 
     ASSERT_WITH_MESSAGE(m_rendererCount == 1, "All other renderers in this render tree should have been destroyed");
 }
index 069028e..e824483 100644 (file)
@@ -579,6 +579,8 @@ void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownTy
     }
 
     pop(0);
+
+    tearDownLeftoverPaginationRenderersIfNeeded(root);
 }
 
 void RenderTreeUpdater::tearDownTextRenderer(Text& text)
@@ -590,6 +592,18 @@ void RenderTreeUpdater::tearDownTextRenderer(Text& text)
     text.setRenderer(nullptr);
 }
 
+void RenderTreeUpdater::tearDownLeftoverPaginationRenderersIfNeeded(Element& root)
+{
+    if (&root != root.document().documentElement())
+        return;
+    for (auto* child = root.document().renderView()->firstChild(); child;) {
+        auto* nextSibling = child->nextSibling();
+        if (is<RenderMultiColumnFlow>(*child) || is<RenderMultiColumnSet>(*child))
+            RenderTreeBuilder::current()->removeFromParentAndDestroyCleaningUpAnonymousWrappers(*child);
+        child = nextSibling;
+    }
+}
+
 void RenderTreeUpdater::tearDownLeftoverShadowHostChildren(Element& host)
 {
     for (auto* hostChild = host.firstChild(); hostChild; hostChild = hostChild->nextSibling()) {
index d4421fd..7d05a96 100644 (file)
@@ -89,6 +89,7 @@ private:
     static void tearDownRenderers(Element&, TeardownType);
     static void tearDownTextRenderer(Text&);
     static void tearDownLeftoverShadowHostChildren(Element&);
+    static void tearDownLeftoverPaginationRenderersIfNeeded(Element&);
 
     RenderView& renderView();