Use WeakPtr for first-letter memory management

https://bugs.webkit.org/show_bug.cgi?id=177716

Reviewed by Darin Adler.

* rendering/RenderElement.cpp:
(WebCore::RenderElement::destroyLeftoverChildren):

    Remove first-letter special case.
    Use removeAndDestroyChild instead of calling destroy() directly. The latter should
    eventually stop calling takeChild and assert that the renderer is not in the tree.

* rendering/RenderTextFragment.cpp:
(WebCore::RenderTextFragment::willBeDestroyed):
(WebCore::RenderTextFragment::setText):
* rendering/RenderTextFragment.h:

    Use WeakPtr.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index a30ac85..a92dd82 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2017-10-01  Antti Koivisto  <antti@apple.com>
+
+        Use WeakPtr for first-letter memory management
+        https://bugs.webkit.org/show_bug.cgi?id=177716
+
+        Reviewed by Darin Adler.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::destroyLeftoverChildren):
+
+            Remove first-letter special case.
+            Use removeAndDestroyChild instead of calling destroy() directly. The latter should
+            eventually stop calling takeChild and assert that the renderer is not in the tree.
+
+        * rendering/RenderTextFragment.cpp:
+        (WebCore::RenderTextFragment::willBeDestroyed):
+        (WebCore::RenderTextFragment::setText):
+        * rendering/RenderTextFragment.h:
+
+            Use WeakPtr.
+
 2017-10-01  Sam Weinig  <sam@webkit.org>
 
         XMLHttpRequest's responseXML should be annotated with [Exposed=Window]

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
index a4d34de..91bdf6b 100644 (file)
--- a/Source/WebCore/rendering/RenderElement.cpp
+++ b/Source/WebCore/rendering/RenderElement.cpp
@@ -503,17 +503,9 @@ void RenderElement::removeAndDestroyChild(RenderObject& oldChild)
 void RenderElement::destroyLeftoverChildren()
 {
     while (m_firstChild) {
-        if (m_firstChild->style().styleType() == FIRST_LETTER && !m_firstChild->isText()) {
-            // FIXME: Memory management.
-            auto firstLetter = takeChild(*m_firstChild); // :first-letter fragment renderers are destroyed by their remaining text fragment.
-            auto* leakedPtr = firstLetter.leakPtr();
-            UNUSED_PARAM(leakedPtr);
-        } else {
-            // Destroy any anonymous children remaining in the render tree, as well as implicit (shadow) DOM elements like those used in the engine-based text fields.
-            if (m_firstChild->node())
-                m_firstChild->node()->setRenderer(nullptr);
-            m_firstChild->destroy();
-        }
+        if (auto* node = m_firstChild->node())
+            node->setRenderer(nullptr);
+        removeAndDestroyChild(*m_firstChild);
     }
 }
 

diff --git a/Source/WebCore/rendering/RenderTextFragment.cpp b/Source/WebCore/rendering/RenderTextFragment.cpp
index 5f767e8..124473b 100644 (file)
--- a/Source/WebCore/rendering/RenderTextFragment.cpp
+++ b/Source/WebCore/rendering/RenderTextFragment.cpp
@@ -75,7 +75,7 @@ void RenderTextFragment::styleDidChange(StyleDifference diff, const RenderStyle*
 void RenderTextFragment::willBeDestroyed()
 {
     if (m_firstLetter)
-        m_firstLetter->destroy();
+        m_firstLetter->removeFromParentAndDestroy();
     RenderText::willBeDestroyed();
 }
 
@@ -87,8 +87,8 @@ void RenderTextFragment::setText(const String& text, bool force)
     m_end = textLength();
     if (!m_firstLetter)
         return;
-    m_firstLetter->destroy();
-    m_firstLetter = 0;
+    m_firstLetter->removeFromParentAndDestroy();
+    ASSERT(!m_firstLetter);
     if (!textNode())
         return;
     ASSERT(!textNode()->renderer());

diff --git a/Source/WebCore/rendering/RenderTextFragment.h b/Source/WebCore/rendering/RenderTextFragment.h
index 7bf2fb1..08fba8f 100644 (file)
--- a/Source/WebCore/rendering/RenderTextFragment.h
+++ b/Source/WebCore/rendering/RenderTextFragment.h
@@ -43,8 +43,8 @@ public:
     unsigned start() const { return m_start; }
     unsigned end() const { return m_end; }
 
-    RenderBoxModelObject* firstLetter() const { return m_firstLetter; }
-    void setFirstLetter(RenderBoxModelObject& firstLetter) { m_firstLetter = &firstLetter; }
+    RenderBoxModelObject* firstLetter() const { return m_firstLetter.get(); }
+    void setFirstLetter(RenderBoxModelObject& firstLetter) { m_firstLetter = makeWeakPtr(firstLetter); }
     
     RenderBlock* blockForAccompanyingFirstLetter();
 
@@ -68,7 +68,7 @@ private:
     // Alternative description that can be used for accessibility instead of the native text.
     String m_altText;
     String m_contentString;
-    RenderBoxModelObject* m_firstLetter;
+    WeakPtr<RenderBoxModelObject> m_firstLetter;
 };
 
 } // namespace WebCore