Title: [219257] trunk
Revision
219257
Author
[email protected]
Date
2017-07-07 09:42:32 -0700 (Fri, 07 Jul 2017)

Log Message

[SVG] Leak in SVGAnimatedListPropertyTearOff
https://bugs.webkit.org/show_bug.cgi?id=172545

Reviewed by Said Abou-Hallawa.

SVGAnimatedListPropertyTearOff maintains a vector m_wrappers with references to
SVGPropertyTraits<PropertyType>::ListItemTearOff. Apart from that SVGPropertyTearOff has a
reference to SVGAnimatedProperty.

When SVGListProperty::getItemValuesAndWrappers() is called, it creates a
SVGPropertyTraits<PropertyType>::ListItemTearOff pointing to the same SVGAnimatedProperty (a
SVGAnimatedListPropertyTearOff) which stores the m_wrappers vector where the ListItemTearOff
is going to be added to. This effectively creates a reference cycle between the
SVGAnimatedListPropertyTearOff and all the ListItemTearOff it stores in m_wrappers.

We should detach those wrappers in propertyWillBeDeleted() in order to break the cycle.

* svg/properties/SVGAnimatedListPropertyTearOff.h:

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/svg/animations/animation-leak-list-property-instances-expected.txt (0 => 219257)


--- trunk/LayoutTests/svg/animations/animation-leak-list-property-instances-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/animations/animation-leak-list-property-instances-expected.txt	2017-07-07 16:42:32 UTC (rev 219257)
@@ -0,0 +1,2 @@
+PASS: all nodes were properly removed.
+
Property changes on: trunk/LayoutTests/svg/animations/animation-leak-list-property-instances-expected.txt
___________________________________________________________________

Added: svn:eol-style

+LF \ No newline at end of property

Added: trunk/LayoutTests/svg/animations/animation-leak-list-property-instances.html (0 => 219257)


--- trunk/LayoutTests/svg/animations/animation-leak-list-property-instances.html	                        (rev 0)
+++ trunk/LayoutTests/svg/animations/animation-leak-list-property-instances.html	2017-07-07 16:42:32 UTC (rev 219257)
@@ -0,0 +1,72 @@
+<!DOCTYPE html>
+<script>
+
+ var xmlns = "http://www.w3.org/2000/svg";
+
+ function log(message) {
+     var logDiv = document.getElementById('log');
+     logDiv.appendChild(document.createTextNode(message));
+ }
+
+ function addRect()
+ {
+     var elem = document.createElementNS(xmlns, "rect");
+     elem.setAttributeNS(null,"id", "rect");
+     elem.setAttributeNS(null,"x",50);
+     elem.setAttributeNS(null,"y",50);
+     elem.setAttributeNS(null,"width",50);
+     elem.setAttributeNS(null,"height",50);
+     elem.setAttributeNS(null,"fill", "blue");
+
+     document.getElementById("rootSVG").appendChild(elem);
+ }
+
+ function applyTransform()
+ {
+     var svgroot = document.getElementById("rootSVG");
+     var transformList = document.getElementById("rect").transform.baseVal;
+     var rotate = svgroot.createSVGTransform();
+     rotate.setRotate(15,0,0);
+     transformList.appendItem(rotate);
+ }
+
+ function removeRect()
+ {
+     document.getElementById("rootSVG").removeChild(document.getElementById("rect"));
+ }
+
+ var originalLiveElements = 0;
+
+ function test()
+ {
+     if (!window.testRunner) {
+         log("This test requires testRunner");
+         return;
+     }
+
+     testRunner.dumpAsText();
+     testRunner.waitUntilDone();
+
+     GCController.collect();
+     originalLiveElements = window.internals.numberOfLiveNodes();
+
+     addRect();
+     applyTransform();
+     removeRect();
+
+     GCController.collect();
+     var delta = window.internals.numberOfLiveNodes() - originalLiveElements;
+     if (delta == 0)
+         log("PASS: all nodes were properly removed.");
+     else
+         log("FAIL: " + delta + " nodes leaked.")
+
+     testRunner.notifyDone();
+ }
+</script>
+
+<body _onload_="test()">
+    <div id="log"></div>
+    <svg id="rootSVG" width="300" height="300" xmlns="http://www.w3.org/2000/svg" version="1.1">
+    </svg>
+</body>
Property changes on: trunk/LayoutTests/svg/animations/animation-leak-list-property-instances.html
___________________________________________________________________

Added: svn:eol-style

+LF \ No newline at end of property

Added: svn:mime-type

+text/html \ No newline at end of property

Modified: trunk/Source/WebCore/ChangeLog (219256 => 219257)


--- trunk/Source/WebCore/ChangeLog	2017-07-07 15:41:40 UTC (rev 219256)
+++ trunk/Source/WebCore/ChangeLog	2017-07-07 16:42:32 UTC (rev 219257)
@@ -1,3 +1,24 @@
+2017-05-24  Sergio Villar Senin  <[email protected]>
+
+        [SVG] Leak in SVGAnimatedListPropertyTearOff
+        https://bugs.webkit.org/show_bug.cgi?id=172545
+
+        Reviewed by Said Abou-Hallawa.
+
+        SVGAnimatedListPropertyTearOff maintains a vector m_wrappers with references to
+        SVGPropertyTraits<PropertyType>::ListItemTearOff. Apart from that SVGPropertyTearOff has a
+        reference to SVGAnimatedProperty.
+
+        When SVGListProperty::getItemValuesAndWrappers() is called, it creates a
+        SVGPropertyTraits<PropertyType>::ListItemTearOff pointing to the same SVGAnimatedProperty (a
+        SVGAnimatedListPropertyTearOff) which stores the m_wrappers vector where the ListItemTearOff
+        is going to be added to. This effectively creates a reference cycle between the
+        SVGAnimatedListPropertyTearOff and all the ListItemTearOff it stores in m_wrappers.
+
+        We should detach those wrappers in propertyWillBeDeleted() in order to break the cycle.
+
+        * svg/properties/SVGAnimatedListPropertyTearOff.h:
+
 2017-07-07  Charlie Turner  <[email protected]>
 
         [GStreamer] vid.me videos do not play

Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h (219256 => 219257)


--- trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h	2017-07-07 15:41:40 UTC (rev 219256)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h	2017-07-07 16:42:32 UTC (rev 219257)
@@ -73,6 +73,8 @@
             m_baseVal = nullptr;
         else if (&property == m_animVal)
             m_animVal = nullptr;
+        if (!m_baseVal && !m_animVal)
+            detachListWrappers(m_values.size());
     }
 
     int findItem(SVGProperty* property)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to