Changeset 222932 in webkit

Timestamp:

Oct 5, 2017 2:14:44 PM (7 years ago)

Author:

Alan Bujtas

Message:

RenderButton should not hold raw pointers to its direct children.
​https://bugs.webkit.org/show_bug.cgi?id=177960
<rdar://problem/34840807>

Reviewed by Antti Koivisto.

The correct way of destroying a renderer is to call ::removeFromParentAndDestroy().

Covered by existing tests.

• rendering/RenderButton.cpp:

(WebCore::RenderButton::RenderButton):
(WebCore::RenderButton::addChild):
(WebCore::RenderButton::takeChild):
(WebCore::RenderButton::updateAnonymousChildStyle const):
(WebCore::RenderButton::setText):
(WebCore::RenderButton::text const):

• rendering/RenderButton.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderButton.cpp (modified) (5 diffs)
• rendering/RenderButton.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-05  Zalan Bujtas  <zalan@apple.com>
+
+        RenderButton should not hold raw pointers to its direct children.
+        https://bugs.webkit.org/show_bug.cgi?id=177960
+        <rdar://problem/34840807>
+
+        Reviewed by Antti Koivisto.
+
+        The correct way of destroying a renderer is to call ::removeFromParentAndDestroy().
+
+        Covered by existing tests.
+
+        * rendering/RenderButton.cpp:
+        (WebCore::RenderButton::RenderButton):
+        (WebCore::RenderButton::addChild):
+        (WebCore::RenderButton::takeChild):
+        (WebCore::RenderButton::updateAnonymousChildStyle const):
+        (WebCore::RenderButton::setText):
+        (WebCore::RenderButton::text const):
+        * rendering/RenderButton.h:
+
 2017-10-05  David Kilzer  <ddkilzer@apple.com>
 

trunk/Source/WebCore/rendering/RenderButton.cpp

@@ -40 +40 @@
 RenderButton::RenderButton(HTMLFormControlElement& element, RenderStyle&& style)
     : RenderFlexibleBox(element, WTFMove(style))
-    , m_buttonText(0)
-    , m_inner(0)
 {
 }
@@ -71 +69 @@
         auto newInner = createAnonymousBlock(style().display());
         updateAnonymousChildStyle(*newInner, newInner->mutableStyle());
-        m_inner = newInner.get();
+        m_inner = makeWeakPtr(*newInner);
         RenderFlexibleBox::addChild(WTFMove(newInner));
-    }
-    
+    }    
     m_inner->addChild(WTFMove(newChild), beforeChild);
 }
@@ -85 +82 @@
     if (&oldChild == m_inner || !m_inner || oldChild.parent() == this) {
         ASSERT(&oldChild == m_inner || !m_inner);
-        m_inner = nullptr;
         return RenderFlexibleBox::takeChild(oldChild);
     }
@@ -94 +90 @@
 {
     ASSERT_UNUSED(child, !m_inner || &child == m_inner);
-    
     childStyle.setFlexGrow(1.0f);
     // min-width: 0; is needed for correct shrinking.
@@ -121 +116 @@
 void RenderButton::setText(const String& str)
 {
-    if (str.isEmpty()) {
-        if (m_buttonText) {
-            m_buttonText->destroy();
-            m_buttonText = 0;
-        }
-    } else {
-        if (m_buttonText)
-            m_buttonText->setText(str.impl());
-        else {
-            auto newButtonText = createRenderer<RenderTextFragment>(document(), str);
-            m_buttonText = newButtonText.get();
-            addChild(WTFMove(newButtonText));
-        }
+    if (!m_buttonText && str.isEmpty())
+        return;
+
+    if (!m_buttonText) {
+        auto newButtonText = createRenderer<RenderTextFragment>(document(), str);
+        m_buttonText = makeWeakPtr(*newButtonText);
+        addChild(WTFMove(newButtonText));
+        return;
     }
+
+    if (!str.isEmpty()) {
+        m_buttonText->setText(str.impl());
+        return;
+    }
+    m_buttonText->removeFromParentAndDestroy();
 }
 
 String RenderButton::text() const
 {
-    return m_buttonText ? m_buttonText->text() : 0;
+    if (m_buttonText)
+        return m_buttonText->text();
+    return { };
 }
 

trunk/Source/WebCore/rendering/RenderButton.h

@@ -71 +71 @@
     bool isFlexibleBoxImpl() const override { return true; }
 
-    RenderTextFragment* m_buttonText;
-    RenderBlock* m_inner;
+    WeakPtr<RenderTextFragment> m_buttonText;
+    WeakPtr<RenderBlock> m_inner;
 };