Move destroyLeftoverChildren call to RenderObject::destroy
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Nov 2017 19:34:32 +0000 (19:34 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Nov 2017 19:34:32 +0000 (19:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179819

Reviewed by Zalan Bujtas.

This is currently called inconsistenly from various willBeDestroyed implementations.
We should always call it before invoking willBeDestroyed.

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::willBeDestroyed):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::willBeDestroyed):
* rendering/RenderElement.h:
(WebCore::RenderElement::setLastChild):
* rendering/RenderInline.cpp:
(WebCore::RenderInline::willBeDestroyed):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::~RenderLayer):

    Add some release asserts verifying layer has been detached before destruction.
    This would reveal cases where destroyLeftoverChildren was called too late.

* rendering/RenderObject.cpp:
(WebCore::RenderObject::destroy):

    Call destroyLeftoverChildren.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderInline.cpp
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderObject.cpp

index bba49dd..6bfa626 100644 (file)
@@ -1,3 +1,32 @@
+2017-11-17  Antti Koivisto  <antti@apple.com>
+
+        Move destroyLeftoverChildren call to RenderObject::destroy
+        https://bugs.webkit.org/show_bug.cgi?id=179819
+
+        Reviewed by Zalan Bujtas.
+
+        This is currently called inconsistenly from various willBeDestroyed implementations.
+        We should always call it before invoking willBeDestroyed.
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::willBeDestroyed):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::willBeDestroyed):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::setLastChild):
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::willBeDestroyed):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::~RenderLayer):
+
+            Add some release asserts verifying layer has been detached before destruction.
+            This would reveal cases where destroyLeftoverChildren was called too late.
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::destroy):
+
+            Call destroyLeftoverChildren.
+
 2017-11-17  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         SVG scrolling anchor should be reset if the fragmentIdentifier does not exist or is not provided
index 9c67711..79757e7 100644 (file)
@@ -132,10 +132,6 @@ void RenderBlockFlow::insertedIntoTree()
 
 void RenderBlockFlow::willBeDestroyed()
 {
-    // Make sure to destroy anonymous children first while they are still connected to the rest of the tree, so that they will
-    // properly dirty line boxes that they are removed from. Effects that do :before/:after only on hover could crash otherwise.
-    destroyLeftoverChildren();
-
     if (!renderTreeBeingDestroyed()) {
         if (firstRootBox()) {
             // We can't wait for RenderBox::destroy to clear the selection,
index 5629c57..1f5f66d 100644 (file)
@@ -1126,8 +1126,6 @@ void RenderElement::willBeDestroyed()
     if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
         view().frameView().removeSlowRepaintObject(this);
 
-    destroyLeftoverChildren();
-
     unregisterForVisibleInViewportCallback();
 
     if (hasCounterNodeMap())
index d182d40..a867d2d 100644 (file)
@@ -229,6 +229,8 @@ public:
     bool isFirstLetter() const { return m_isFirstLetter; }
     void setIsFirstLetter() { m_isFirstLetter = true; }
 
+    void destroyLeftoverChildren();
+
 protected:
     enum BaseTypeFlag {
         RenderLayerModelObjectFlag  = 1 << 0,
@@ -254,7 +256,6 @@ protected:
 
     void setFirstChild(RenderObject* child) { m_firstChild = child; }
     void setLastChild(RenderObject* child) { m_lastChild = child; }
-    void destroyLeftoverChildren();
 
     virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle);
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
index cb30bc5..f6fb45b 100644 (file)
@@ -84,10 +84,6 @@ void RenderInline::willBeDestroyed()
     }
 #endif
 
-    // Make sure to destroy anonymous children first while they are still connected to the rest of the tree, so that they will
-    // properly dirty line boxes that they are removed from.  Effects that do :before/:after only on hover could crash otherwise.
-    destroyLeftoverChildren();
-    
     if (!renderTreeBeingDestroyed()) {
         if (firstLineBox()) {
             // We can't wait for RenderBoxModelObject::destroy to clear the selection,
index 2f0f18b..598fae9 100644 (file)
@@ -435,6 +435,10 @@ RenderLayer::~RenderLayer()
     // we don't need to delete them ourselves.
 
     clearBacking(true);
+
+    // Layer and all its children should be removed from the tree before destruction.
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(renderer().renderTreeBeingDestroyed() || !m_parent);
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(renderer().renderTreeBeingDestroyed() || !m_first);
 }
 
 String RenderLayer::name() const
index 3c937e3..79c098c 100644 (file)
@@ -1519,6 +1519,9 @@ 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)