Changeset 224976 in webkit

Timestamp:

Nov 17, 2017 11:34:32 AM (6 years ago)

Author:

Antti Koivisto

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderBlockFlow.cpp (modified) (1 diff)
• rendering/RenderElement.cpp (modified) (1 diff)
• rendering/RenderElement.h (modified) (2 diffs)
• rendering/RenderInline.cpp (modified) (1 diff)
• rendering/RenderLayer.cpp (modified) (1 diff)
• rendering/RenderObject.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderBlockFlow.cpp

@@ -133 +133 @@
 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()) {

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -1127 +1127 @@
         view().frameView().removeSlowRepaintObject(this);
 
-    destroyLeftoverChildren();
-
     unregisterForVisibleInViewportCallback();
 

trunk/Source/WebCore/rendering/RenderElement.h

@@ -230 +230 @@
     void setIsFirstLetter() { m_isFirstLetter = true; }
 
+    void destroyLeftoverChildren();
+
 protected:
     enum BaseTypeFlag {
@@ -255 +257 @@
     void setFirstChild(RenderObject* child) { m_firstChild = child; }
     void setLastChild(RenderObject* child) { m_lastChild = child; }
-    void destroyLeftoverChildren();
 
     virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle);

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -85 +85 @@
 #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()) {

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -436 +436 @@
 
     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);
 }
 

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -1520 +1520 @@
     RELEASE_ASSERT(!m_bitfields.beingDestroyed());
 
+    if (is<RenderElement>(*this))
+        downcast<RenderElement>(*this).destroyLeftoverChildren();
+
     m_bitfields.setBeingDestroyed(true);