Title: [222710] trunk/Source/WebCore
Revision
222710
Author
an...@apple.com
Date
2017-10-02 09:00:28 -0700 (Mon, 02 Oct 2017)

Log Message

Fix memory leaks in RenderMultiColumnFlow
https://bugs.webkit.org/show_bug.cgi?id=177735

Reviewed by Zalan Bujtas.

* rendering/RenderMultiColumnFlow.cpp:
(WebCore::RenderMultiColumnFlow::evacuateAndDestroy):

    Switch from destroy() to removeFromParentAndDestroy() (they are currently equivalent in practice).

(WebCore::RenderMultiColumnFlow::fragmentedFlowDescendantInserted):

    Destroy the placeholders instead of leaking them.

(WebCore::RenderMultiColumnFlow::handleSpannerRemoval):

    Destroy the placeholder instead of leaking it.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (222709 => 222710)


--- trunk/Source/WebCore/ChangeLog	2017-10-02 14:59:28 UTC (rev 222709)
+++ trunk/Source/WebCore/ChangeLog	2017-10-02 16:00:28 UTC (rev 222710)
@@ -1,3 +1,23 @@
+2017-10-02  Antti Koivisto  <an...@apple.com>
+
+        Fix memory leaks in RenderMultiColumnFlow
+        https://bugs.webkit.org/show_bug.cgi?id=177735
+
+        Reviewed by Zalan Bujtas.
+
+        * rendering/RenderMultiColumnFlow.cpp:
+        (WebCore::RenderMultiColumnFlow::evacuateAndDestroy):
+
+            Switch from destroy() to removeFromParentAndDestroy() (they are currently equivalent in practice).
+
+        (WebCore::RenderMultiColumnFlow::fragmentedFlowDescendantInserted):
+
+            Destroy the placeholders instead of leaking them.
+
+        (WebCore::RenderMultiColumnFlow::handleSpannerRemoval):
+
+            Destroy the placeholder instead of leaking it.
+
 2017-10-02  Charles Turner  <ctur...@igalia.com>
 
         Try to play AVC codec even if H.264 decoder only advertises byte-stream profile.

Modified: trunk/Source/WebCore/rendering/RenderMultiColumnFlow.cpp (222709 => 222710)


--- trunk/Source/WebCore/rendering/RenderMultiColumnFlow.cpp	2017-10-02 14:59:28 UTC (rev 222709)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnFlow.cpp	2017-10-02 16:00:28 UTC (rev 222710)
@@ -189,7 +189,7 @@
         if (RenderMultiColumnSpannerPlaceholder* placeholder = it->value.get()) {
             RenderBlockFlow& originalContainer = downcast<RenderBlockFlow>(*placeholder->parent());
             originalContainer.addChild(WTFMove(takenSpanner), placeholder);
-            placeholder->destroy();
+            placeholder->removeFromParentAndDestroy();
         }
         m_spannerMap.remove(it);
     }
@@ -196,9 +196,9 @@
 
     // Remove all sets.
     while (RenderMultiColumnSet* columnSet = firstMultiColumnSet())
-        columnSet->destroy();
+        columnSet->removeFromParentAndDestroy();
     
-    destroy();
+    removeFromParentAndDestroy();
 }
 
 void RenderMultiColumnFlow::addFragmentToThread(RenderFragmentContainer* RenderFragmentContainer)
@@ -407,6 +407,9 @@
 {
     if (gShiftingSpanner || m_beingEvacuated || newDescendant.isInFlowRenderFragmentedFlow())
         return;
+
+    Vector<RenderPtr<RenderObject>> spannersToDelete;
+
     RenderObject* subtreeRoot = &newDescendant;
     for (auto* descendant = &newDescendant; descendant; descendant = (descendant ? descendant->nextInPreOrder(subtreeRoot) : nullptr)) {
         if (is<RenderMultiColumnSpannerPlaceholder>(*descendant)) {
@@ -430,10 +433,7 @@
                 // we shifted the placeholder down into this flow thread.
                 placeholder.fragmentedFlow()->m_spannerMap.remove(spanner);
 
-                auto takenChild = placeholder.parent()->takeChild(placeholder);
-                // FIXME: Memory management.
-                auto* leakenPtr = takenChild.leakPtr();
-                UNUSED_PARAM(leakenPtr);
+                spannersToDelete.append(placeholder.parent()->takeChild(placeholder));
 
                 if (subtreeRoot == descendant)
                     subtreeRoot = spanner;
@@ -454,20 +454,14 @@
 void RenderMultiColumnFlow::handleSpannerRemoval(RenderObject& spanner)
 {
     // The placeholder may already have been removed, but if it hasn't, do so now.
-    if (RenderMultiColumnSpannerPlaceholder* placeholder = m_spannerMap.get(&downcast<RenderBox>(spanner)).get()) {
-        auto takenChild = placeholder->parent()->takeChild(*placeholder);
-        // FIXME: Memory management.
-        auto* leakenPtr = takenChild.leakPtr();
-        UNUSED_PARAM(leakenPtr);
+    if (auto placeholder = m_spannerMap.take(&downcast<RenderBox>(spanner)))
+        placeholder->removeFromParentAndDestroy();
 
-        m_spannerMap.remove(&downcast<RenderBox>(spanner));
-    }
-
     if (RenderObject* next = spanner.nextSibling()) {
         if (RenderObject* previous = spanner.previousSibling()) {
             if (previous->isRenderMultiColumnSet() && next->isRenderMultiColumnSet()) {
                 // Merge two sets that no longer will be separated by a spanner.
-                next->destroy();
+                next->removeFromParentAndDestroy();
                 previous->setNeedsLayout();
             }
         }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to