Breaking several cyclic references between SVG animated properties.

Based on upstream fix by sabouhallawa@apple.com at http://trac.webkit.org/changeset/196268

The leak happens because of cyclic reference between SVGListPropertyTearOff
and SVGAnimatedListPropertyTearOff which is derived from SVGAnimatedProperty.
There is also cyclic reference between SVGAnimatedProperty and SVGElement and
this causes the whole document to be leaked. So if the JS requests, for
example, an instance of SVGPolylineElement.points, the whole document will be
leaked.

The fix depends on having the cyclic reference as is since the owning and the
owned classes have to live together if any of them is referenced. But the owning
class caches a raw 'ref-counted' pointer of the owned class. If it is requested
for an instance of the owned class it returned a RefPtr<> of it. Once the owned
class is not used, it can delete itself. The only thing needed here is to notify
the owner class of the deletion so it cleans its caches and be able to create a
new pointer if it is requested for an instance of the owned class later.

Revert the change of r181345 in SVGAnimatedProperty::lookupOrCreateWrapper()
to break the cyclic reference between SVGElement and SVGAnimatedProperty.

Also apply the same approach in SVGAnimatedListPropertyTearOff::baseVal() and
animVal() to break cyclic reference between SVGListPropertyTearOff and
SVGAnimatedListPropertyTearOff.

Change-Id: Ied6a077299e47855feb235a1c9310f1a58aad91b
Reviewed-by: Konstantin Tokarev <annulen@yandex.ru>
Reviewed-by: Michael Brüning <michael.bruning@theqtcompany.com>

15 files changed, 145 insertions, 94 deletions

diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
index df4234e23..2cca58703 100644
--- a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
+++ b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
@@ -3624,7 +3624,11 @@ sub NativeToJSValue
         return "toJSNewlyCreated(exec, $globalObject, WTF::getPtr($value))";
     }
 
-    if ($codeGenerator->IsSVGAnimatedType($interfaceName) or $interfaceName eq "SVGViewSpec") {
+    # $type has to be used here because SVGViewSpec.transform is of type SVGTransformList.
+    if ($codeGenerator->IsSVGTypeNeedingTearOff($type) and $type =~ /(?<!String)List$/) {
+        # Convert from abstract RefPtr<ListProperty> to real type, so the right toJS() method can be invoked.
+        $value = "static_cast<" . GetNativeType($type) . ">($value" . ".get())";
+    } elsif ($codeGenerator->IsSVGAnimatedType($interfaceName) or $interfaceName eq "SVGViewSpec") {
         # Convert from abstract SVGProperty to real type, so the right toJS() method can be invoked.
         $value = "static_cast<" . GetNativeType($type) . ">($value)";
     } elsif ($codeGenerator->IsSVGTypeNeedingTearOff($type) and not $interfaceName =~ /List$/) {
diff --git a/Source/WebCore/svg/SVGPathElement.cpp b/Source/WebCore/svg/SVGPathElement.cpp
index 3324d0b5d..b030c9a83 100644
--- a/Source/WebCore/svg/SVGPathElement.cpp
+++ b/Source/WebCore/svg/SVGPathElement.cpp
@@ -310,10 +310,15 @@ void SVGPathElement::removedFrom(ContainerNode* rootParent)
 
 SVGPathByteStream* SVGPathElement::pathByteStream() const
 {
-    SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(this, dPropertyInfo());
+    RefPtr<SVGAnimatedProperty> property = SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(this, dPropertyInfo());
     if (!property || !property->isAnimating())
         return m_pathByteStream.get();
-    return static_cast<SVGAnimatedPathSegListPropertyTearOff*>(property)->animatedPathByteStream();
+
+    SVGPathByteStream* animatedPathByteStream = static_cast<SVGAnimatedPathSegListPropertyTearOff*>(property.get())->animatedPathByteStream();
+     if (!animatedPathByteStream)
+         return m_pathByteStream.get();
+
+     return animatedPathByteStream;
 }
 
 PassRefPtr<SVGAnimatedProperty> SVGPathElement::lookupOrCreateDWrapper(SVGElement* contextElement)
@@ -321,7 +326,7 @@ PassRefPtr<SVGAnimatedProperty> SVGPathElement::lookupOrCreateDWrapper(SVGElemen
     ASSERT(contextElement);
     SVGPathElement* ownerType = toSVGPathElement(contextElement);
 
-    if (SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(ownerType, dPropertyInfo()))
+    if (RefPtr<SVGAnimatedProperty> property = SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(ownerType, dPropertyInfo()))
         return property;
 
     // Build initial SVGPathSegList.
@@ -340,26 +345,26 @@ void SVGPathElement::synchronizeD(SVGElement* contextElement)
     ownerType->m_pathSegList.synchronize(ownerType, dPropertyInfo()->attributeName, ownerType->m_pathSegList.value.valueAsString());
 }
 
-SVGPathSegListPropertyTearOff* SVGPathElement::pathSegList()
+RefPtr<SVGPathSegListPropertyTearOff> SVGPathElement::pathSegList()
 {
     m_pathSegList.shouldSynchronize = true;
-    return static_cast<SVGPathSegListPropertyTearOff*>(static_pointer_cast<SVGAnimatedPathSegListPropertyTearOff>(lookupOrCreateDWrapper(this))->baseVal());
+    return static_cast<SVGPathSegListPropertyTearOff*>(static_pointer_cast<SVGAnimatedPathSegListPropertyTearOff>(lookupOrCreateDWrapper(this))->baseVal().get());
 }
 
-SVGPathSegListPropertyTearOff* SVGPathElement::normalizedPathSegList()
+RefPtr<SVGPathSegListPropertyTearOff> SVGPathElement::normalizedPathSegList()
 {
     // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists!
     return 0;
 }
 
-SVGPathSegListPropertyTearOff* SVGPathElement::animatedPathSegList()
+RefPtr<SVGPathSegListPropertyTearOff> SVGPathElement::animatedPathSegList()
 {
     m_pathSegList.shouldSynchronize = true;
     m_isAnimValObserved = true;
-    return static_cast<SVGPathSegListPropertyTearOff*>(static_pointer_cast<SVGAnimatedPathSegListPropertyTearOff>(lookupOrCreateDWrapper(this))->animVal());
+    return static_cast<SVGPathSegListPropertyTearOff*>(static_pointer_cast<SVGAnimatedPathSegListPropertyTearOff>(lookupOrCreateDWrapper(this))->animVal().get());
 }
 
-SVGPathSegListPropertyTearOff* SVGPathElement::animatedNormalizedPathSegList()
+RefPtr<SVGPathSegListPropertyTearOff> SVGPathElement::animatedNormalizedPathSegList()
 {
     // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists!
     return 0;
diff --git a/Source/WebCore/svg/SVGPathElement.h b/Source/WebCore/svg/SVGPathElement.h
index c81ead198..0ca66ac11 100644
--- a/Source/WebCore/svg/SVGPathElement.h
+++ b/Source/WebCore/svg/SVGPathElement.h
@@ -83,10 +83,10 @@ public:
     PassRefPtr<SVGPathSegCurvetoQuadraticSmoothRel> createSVGPathSegCurvetoQuadraticSmoothRel(float x, float y, SVGPathSegRole role = PathSegUndefinedRole);
 
     // Used in the bindings only.
-    SVGPathSegListPropertyTearOff* pathSegList();
-    SVGPathSegListPropertyTearOff* animatedPathSegList();
-    SVGPathSegListPropertyTearOff* normalizedPathSegList();
-    SVGPathSegListPropertyTearOff* animatedNormalizedPathSegList();
+    RefPtr<SVGPathSegListPropertyTearOff> pathSegList();
+    RefPtr<SVGPathSegListPropertyTearOff> animatedPathSegList();
+    RefPtr<SVGPathSegListPropertyTearOff> normalizedPathSegList();
+    RefPtr<SVGPathSegListPropertyTearOff> animatedNormalizedPathSegList();
 
     SVGPathByteStream* pathByteStream() const;
 
diff --git a/Source/WebCore/svg/SVGPathSegWithContext.h b/Source/WebCore/svg/SVGPathSegWithContext.h
index 701067227..4ba1447bf 100644
--- a/Source/WebCore/svg/SVGPathSegWithContext.h
+++ b/Source/WebCore/svg/SVGPathSegWithContext.h
@@ -33,7 +33,7 @@ public:
     {
     }
 
-    SVGAnimatedProperty* animatedProperty() const
+    RefPtr<SVGAnimatedProperty> animatedProperty() const
     {
         switch (m_role) {
         case PathSegUndefinedRole:
diff --git a/Source/WebCore/svg/SVGPolyElement.cpp b/Source/WebCore/svg/SVGPolyElement.cpp
index fd5c652f1..977234d5e 100644
--- a/Source/WebCore/svg/SVGPolyElement.cpp
+++ b/Source/WebCore/svg/SVGPolyElement.cpp
@@ -88,8 +88,8 @@ void SVGPolyElement::parseAttribute(const QualifiedName& name, const AtomicStrin
         if (!pointsListFromSVGData(newList, value))
             document()->accessSVGExtensions()->reportError("Problem parsing points=\"" + value + "\"");
 
-        if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper<SVGPolyElement, SVGAnimatedPointList>(this, pointsPropertyInfo()))
-            static_cast<SVGAnimatedPointList*>(wrapper)->detachListWrappers(newList.size());
+        if (RefPtr<SVGAnimatedProperty> wrapper = SVGAnimatedProperty::lookupWrapper<SVGPolyElement, SVGAnimatedPointList>(this, pointsPropertyInfo()))
+            static_pointer_cast<SVGAnimatedPointList>(wrapper)->detachListWrappers(newList.size());
 
         m_points.value = newList;
         return;
@@ -147,16 +147,16 @@ PassRefPtr<SVGAnimatedProperty> SVGPolyElement::lookupOrCreatePointsWrapper(SVGE
            (ownerType, pointsPropertyInfo(), ownerType->m_points.value);
 }
 
-SVGListPropertyTearOff<SVGPointList>* SVGPolyElement::points()
+RefPtr<SVGListPropertyTearOff<SVGPointList> > SVGPolyElement::points()
 {
     m_points.shouldSynchronize = true;
-    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedPointList>(lookupOrCreatePointsWrapper(this))->baseVal());
+    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedPointList>(lookupOrCreatePointsWrapper(this))->baseVal().get());
 }
 
-SVGListPropertyTearOff<SVGPointList>* SVGPolyElement::animatedPoints()
+RefPtr<SVGListPropertyTearOff<SVGPointList> > SVGPolyElement::animatedPoints()
 {
     m_points.shouldSynchronize = true;
-    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedPointList>(lookupOrCreatePointsWrapper(this))->animVal());
+    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedPointList>(lookupOrCreatePointsWrapper(this))->animVal().get());
 }
 
 }
diff --git a/Source/WebCore/svg/SVGPolyElement.h b/Source/WebCore/svg/SVGPolyElement.h
index bd0dd49aa..f399a427e 100644
--- a/Source/WebCore/svg/SVGPolyElement.h
+++ b/Source/WebCore/svg/SVGPolyElement.h
@@ -33,8 +33,8 @@ namespace WebCore {
 class SVGPolyElement : public SVGGraphicsElement
                      , public SVGExternalResourcesRequired {
 public:
-    SVGListPropertyTearOff<SVGPointList>* points();
-    SVGListPropertyTearOff<SVGPointList>* animatedPoints();
+    RefPtr<SVGListPropertyTearOff<SVGPointList> > points();
+    RefPtr<SVGListPropertyTearOff<SVGPointList> > animatedPoints();
 
     SVGPointList& pointList() const { return m_points.value; }
 
diff --git a/Source/WebCore/svg/SVGViewSpec.cpp b/Source/WebCore/svg/SVGViewSpec.cpp
index 253c082fb..5ac02dbdd 100644
--- a/Source/WebCore/svg/SVGViewSpec.cpp
+++ b/Source/WebCore/svg/SVGViewSpec.cpp
@@ -117,8 +117,8 @@ void SVGViewSpec::setTransformString(const String& transform)
     SVGTransformList newList;
     newList.parse(transform);
 
-    if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper<SVGElement, SVGAnimatedTransformList>(m_contextElement, transformPropertyInfo()))
-        static_cast<SVGAnimatedTransformList*>(wrapper)->detachListWrappers(newList.size());
+    if (RefPtr<SVGAnimatedProperty> wrapper = SVGAnimatedProperty::lookupWrapper<SVGElement, SVGAnimatedTransformList>(m_contextElement, transformPropertyInfo()))
+        static_pointer_cast<SVGAnimatedTransformList>(wrapper)->detachListWrappers(newList.size());
 
     m_transform = newList;
 }
@@ -148,12 +148,12 @@ SVGElement* SVGViewSpec::viewTarget() const
     return toSVGElement(element);
 }
 
-SVGTransformListPropertyTearOff* SVGViewSpec::transform()
+RefPtr<SVGTransformListPropertyTearOff> SVGViewSpec::transform()
 {
     if (!m_contextElement)
         return 0;
     // Return the animVal here, as its readonly by default - which is exactly what we want here.
-    return static_cast<SVGTransformListPropertyTearOff*>(static_pointer_cast<SVGAnimatedTransformList>(lookupOrCreateTransformWrapper(this))->animVal());
+    return static_cast<SVGTransformListPropertyTearOff*>(static_pointer_cast<SVGAnimatedTransformList>(lookupOrCreateTransformWrapper(this))->animVal().get());
 }
 
 PassRefPtr<SVGAnimatedRect> SVGViewSpec::viewBoxAnimated()
diff --git a/Source/WebCore/svg/SVGViewSpec.h b/Source/WebCore/svg/SVGViewSpec.h
index f8634afd6..3887db7f5 100644
--- a/Source/WebCore/svg/SVGViewSpec.h
+++ b/Source/WebCore/svg/SVGViewSpec.h
@@ -69,7 +69,7 @@ public:
     void resetContextElement() { m_contextElement = 0; }
 
     // Custom non-animated 'transform' property.
-    SVGTransformListPropertyTearOff* transform();
+    RefPtr<SVGTransformListPropertyTearOff> transform();
     SVGTransformList transformBaseValue() const { return m_transform; }
 
     // Custom animated 'viewBox' property.
diff --git a/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h
index 65ee9e58d..136ba8253 100644
--- a/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h
+++ b/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h
@@ -40,35 +40,49 @@ public:
     typedef SVGListPropertyTearOff<PropertyType> ListPropertyTearOff;
     typedef PropertyType ContentType;
 
-    virtual ListProperty* baseVal()
+    virtual PassRefPtr<ListProperty> baseVal()
     {
-        if (!m_baseVal)
-            m_baseVal = ListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers);
-        return static_cast<ListProperty*>(m_baseVal.get());
+        if (m_baseVal)
+            return m_baseVal;
+
+        RefPtr<ListProperty> property = ListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers).get();
+        m_baseVal = property.get();
+        return property.release();
+    }
+
+    virtual PassRefPtr<ListProperty> animVal()
+    {
+        if (m_animVal)
+            return m_animVal;
+
+        RefPtr<ListProperty> property = ListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers).get();
+        m_animVal = property.get();
+        return property.release();
     }
 
-    virtual ListProperty* animVal()
+    void propertyWillBeDeleted(const ListProperty& property)
     {
-        if (!m_animVal)
-            m_animVal = ListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers);
-        return static_cast<ListProperty*>(m_animVal.get());
+        if (&property == m_baseVal)
+            m_baseVal = 0;
+        else if (&property == m_animVal)
+            m_animVal = 0;
     }
 
     virtual bool isAnimatedListTearOff() const { return true; }
 
-    int findItem(SVGProperty* property) const
+    int findItem(SVGProperty* property)
     {
         // This should ever be called for our baseVal, as animVal can't modify the list.
         // It's safe to cast to ListPropertyTearOff here as all classes inheriting from us supply their own removeItemFromList() method.
         typedef SVGPropertyTearOff<typename SVGPropertyTraits<PropertyType>::ListItemType> ListItemTearOff;
-        return static_cast<ListPropertyTearOff*>(m_baseVal.get())->findItem(static_cast<ListItemTearOff*>(property));
+        return static_pointer_cast<ListPropertyTearOff>(baseVal())->findItem(static_cast<ListItemTearOff*>(property));
     }
 
     void removeItemFromList(size_t itemIndex, bool shouldSynchronizeWrappers)
     {
         // This should ever be called for our baseVal, as animVal can't modify the list.
         // It's safe to cast to ListPropertyTearOff here as all classes inheriting from us supply their own removeItemFromList() method.
-        static_cast<ListPropertyTearOff*>(m_baseVal.get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers);
+        static_pointer_cast<ListPropertyTearOff>(baseVal())->removeItemFromList(itemIndex, shouldSynchronizeWrappers);
     }
 
     void detachListWrappers(unsigned newListSize)
@@ -79,8 +93,8 @@ public:
     PropertyType& currentAnimatedValue()
     {
         ASSERT(m_isAnimating);
-        ASSERT(m_animVal);
-        return static_cast<ListProperty*>(m_animVal.get())->values();
+        ASSERT(m_animatingAnimVal);
+        return static_pointer_cast<ListProperty>(m_animatingAnimVal)->values();
     }
 
     const PropertyType& currentBaseValue() const
@@ -91,6 +105,7 @@ public:
     void animationStarted(PropertyType* newAnimVal, bool shouldOwnValues = false)
     {
         ASSERT(!m_isAnimating);
+        ASSERT(!m_animatingAnimVal);
         ASSERT(newAnimVal);
         ASSERT(m_values.size() == m_wrappers.size());
         ASSERT(m_animatedWrappers.isEmpty());
@@ -99,49 +114,49 @@ public:
         if (!newAnimVal->isEmpty())
             m_animatedWrappers.fill(0, newAnimVal->size());
 
-        ListProperty* animVal = static_cast<ListProperty*>(this->animVal());
-        animVal->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues);
-        ASSERT(animVal->values().size() == animVal->wrappers().size());
-        ASSERT(animVal->wrappers().size() == m_animatedWrappers.size());
+        m_animatingAnimVal = animVal();
+        m_animatingAnimVal->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues);
+        ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size());
+        ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size());
         m_isAnimating = true;
     }
 
     void animationEnded()
     {
         ASSERT(m_isAnimating);
-        ASSERT(m_animVal);
+        ASSERT(m_animatingAnimVal);
         ASSERT(m_values.size() == m_wrappers.size());
 
-        ListProperty* animVal = static_cast<ListProperty*>(m_animVal.get());
-        ASSERT(animVal->values().size() == animVal->wrappers().size());
-        ASSERT(animVal->wrappers().size() == m_animatedWrappers.size());
+        ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size());
+        ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size());
 
-        animVal->setValuesAndWrappers(&m_values, &m_wrappers, false);
-        ASSERT(animVal->values().size() == animVal->wrappers().size());
-        ASSERT(animVal->wrappers().size() == m_wrappers.size());
+        m_animatingAnimVal->setValuesAndWrappers(&m_values, &m_wrappers, false);
+        ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size());
+        ASSERT(m_animatingAnimVal->wrappers().size() == m_wrappers.size());
 
         m_animatedWrappers.clear();
+        m_animatingAnimVal = 0;
         m_isAnimating = false;
     }
 
     void synchronizeWrappersIfNeeded()
     {
+        ASSERT(m_isAnimating);
+        ASSERT(m_animatingAnimVal);
+
         // Eventually the wrapper list needs synchronization because any SVGAnimateLengthList::calculateAnimatedValue() call may
         // mutate the length of our values() list, and thus the wrapper() cache needs synchronization, to have the same size.
         // Also existing wrappers which point directly at elements in the existing SVGLengthList have to be detached (so a copy
         // of them is created, so existing animVal variables in JS are kept-alive). If we'd detach them later the underlying
         // SVGLengthList was already mutated, and our list item wrapper tear offs would point nowhere. Assertions would fire.
-        ListProperty* animVal = static_cast<ListProperty*>(m_animVal.get());
-        animVal->detachListWrappers(animVal->values().size());
+        m_animatingAnimVal->detachListWrappers(m_animatingAnimVal->values().size());
 
-        ASSERT(animVal->values().size() == animVal->wrappers().size());
-        ASSERT(animVal->wrappers().size() == m_animatedWrappers.size());
+        ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size());
+        ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size());
     }
 
     void animValWillChange()
     {
-        ASSERT(m_isAnimating);
-        ASSERT(m_animVal);
         ASSERT(m_values.size() == m_wrappers.size());
         synchronizeWrappersIfNeeded();
     }
@@ -164,6 +179,8 @@ protected:
     SVGAnimatedListPropertyTearOff(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, PropertyType& values)
         : SVGAnimatedProperty(contextElement, attributeName, animatedPropertyType)
         , m_values(values)
+        , m_baseVal(0)
+        , m_animVal(0)
     {
         if (!values.isEmpty())
             m_wrappers.fill(0, values.size());
@@ -174,8 +191,13 @@ protected:
     ListWrapperCache m_wrappers;
     ListWrapperCache m_animatedWrappers;
 
-    RefPtr<SVGProperty> m_baseVal;
-    RefPtr<SVGProperty> m_animVal;
+    // Cache the raw pointer but return a RefPtr<>. This will break the cyclic reference
+    // between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff once the property
+    // pointer is not needed.
+    ListProperty* m_baseVal;
+    ListProperty* m_animVal;
+
+    RefPtr<ListProperty> m_animatingAnimVal;
 };
 
 }
diff --git a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h
index eb6dd19e3..1cfe0230e 100644
--- a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h
+++ b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h
@@ -32,32 +32,36 @@ namespace WebCore {
 
 class SVGAnimatedPathSegListPropertyTearOff : public SVGAnimatedListPropertyTearOff<SVGPathSegList> {
 public:
-    virtual SVGListProperty<SVGPathSegList>* baseVal()
+    virtual PassRefPtr<ListProperty> baseVal() OVERRIDE
     {
-        if (!m_baseVal)
-            m_baseVal = SVGPathSegListPropertyTearOff::create(this, BaseValRole, PathSegUnalteredRole, m_values, m_wrappers);
-        return static_cast<SVGListProperty<SVGPathSegList>*>(m_baseVal.get());
+        if (m_baseVal)
+            return m_baseVal;
+
+        RefPtr<ListProperty> property = SVGPathSegListPropertyTearOff::create(this, BaseValRole, PathSegUnalteredRole, m_values, m_wrappers);
+        m_baseVal = property.get();
+        return property.release();
     }
 
-    virtual SVGListProperty<SVGPathSegList>* animVal()
+    virtual PassRefPtr<ListProperty> animVal() OVERRIDE
     {
-        if (!m_animVal)
-            m_animVal = SVGPathSegListPropertyTearOff::create(this, AnimValRole, PathSegUnalteredRole, m_values, m_wrappers);
-        return static_cast<SVGListProperty<SVGPathSegList>*>(m_animVal.get());
+        if (m_animVal)
+            return m_animVal;
+
+        RefPtr<ListProperty> property = SVGPathSegListPropertyTearOff::create(this, AnimValRole, PathSegUnalteredRole, m_values, m_wrappers);
+        m_animVal = property.get();
+        return property.release();
     }
 
-    int findItem(const RefPtr<SVGPathSeg>& segment) const
+    int findItem(const RefPtr<SVGPathSeg>& segment)
     {
         // This should ever be called for our baseVal, as animVal can't modify the list.
-        ASSERT(m_baseVal);
-        return static_cast<SVGPathSegListPropertyTearOff*>(m_baseVal.get())->findItem(segment);
+        return static_cast<SVGPathSegListPropertyTearOff*>(baseVal().get())->findItem(segment);
     }
 
     void removeItemFromList(size_t itemIndex, bool shouldSynchronizeWrappers)
     {
         // This should ever be called for our baseVal, as animVal can't modify the list.
-        ASSERT(m_baseVal);
-        static_cast<SVGPathSegListPropertyTearOff*>(m_baseVal.get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers);
+        static_cast<SVGPathSegListPropertyTearOff*>(baseVal().get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers);
     }
 
     static PassRefPtr<SVGAnimatedPathSegListPropertyTearOff> create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, SVGPathSegList& values)
diff --git a/Source/WebCore/svg/properties/SVGAnimatedProperty.h b/Source/WebCore/svg/properties/SVGAnimatedProperty.h
index 93f076b2b..d40a50434 100644
--- a/Source/WebCore/svg/properties/SVGAnimatedProperty.h
+++ b/Source/WebCore/svg/properties/SVGAnimatedProperty.h
@@ -53,18 +53,22 @@ public:
     {
         ASSERT(info);
         SVGAnimatedPropertyDescription key(element, info->propertyIdentifier);
-        RefPtr<SVGAnimatedProperty> wrapper = animatedPropertyCache()->get(key);
-        if (!wrapper) {
-            wrapper = TearOffType::create(element, info->attributeName, info->animatedPropertyType, property);
-            if (info->animatedPropertyState == PropertyIsReadOnly)
-                wrapper->setIsReadOnly();
-            animatedPropertyCache()->set(key, wrapper.get());
-        }
+        Cache::AddResult result = animatedPropertyCache()->add(key, 0);
+        if (!result.isNewEntry)
+            return static_cast<TearOffType*>(result.iterator->value);
+
+        RefPtr<SVGAnimatedProperty> wrapper = TearOffType::create(element, info->attributeName, info->animatedPropertyType, property);
+        if (info->animatedPropertyState == PropertyIsReadOnly)
+            wrapper->setIsReadOnly();
+
+        // Cache the raw pointer but return a RefPtr<>. This will break the cyclic reference
+        // between SVGAnimatedProperty and SVGElement once the property pointer is not needed.
+        result.iterator->value = wrapper.get();
         return static_pointer_cast<TearOffType>(wrapper);
     }
 
     template<typename OwnerType, typename TearOffType>
-    static TearOffType* lookupWrapper(OwnerType* element, const SVGPropertyInfo* info)
+    static PassRefPtr<TearOffType> lookupWrapper(OwnerType* element, const SVGPropertyInfo* info)
     {
         ASSERT(info);
         SVGAnimatedPropertyDescription key(element, info->propertyIdentifier);
@@ -72,7 +76,7 @@ public:
     }
 
     template<typename OwnerType, typename TearOffType>
-    static TearOffType* lookupWrapper(const OwnerType* element, const SVGPropertyInfo* info)
+    static PassRefPtr<TearOffType> lookupWrapper(const OwnerType* element, const SVGPropertyInfo* info)
     {
         return lookupWrapper<OwnerType, TearOffType>(const_cast<OwnerType*>(element), info);
     }
diff --git a/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h b/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h
index 1ddbc5c6c..e31015e52 100644
--- a/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h
+++ b/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h
@@ -118,7 +118,7 @@ public: \
     static const SVGPropertyInfo* LowerProperty##PropertyInfo(); \
     PropertyType& LowerProperty() const \
     { \
-        if (TearOffType* wrapper = SVGAnimatedProperty::lookupWrapper<UseOwnerType, TearOffType>(this, LowerProperty##PropertyInfo())) { \
+        if (RefPtr<TearOffType> wrapper = SVGAnimatedProperty::lookupWrapper<UseOwnerType, TearOffType>(this, LowerProperty##PropertyInfo())) { \
             if (wrapper->isAnimating()) \
                 return wrapper->currentAnimatedValue(); \
         } \
@@ -179,7 +179,7 @@ private: \
 DECLARE_ANIMATED_PROPERTY(TearOffType, PropertyType, UpperProperty, LowerProperty) \
 void detachAnimated##UpperProperty##ListWrappers(unsigned newListSize) \
 { \
-    if (TearOffType* wrapper = SVGAnimatedProperty::lookupWrapper<UseOwnerType, TearOffType>(this, LowerProperty##PropertyInfo())) \
+    if (RefPtr<TearOffType> wrapper = SVGAnimatedProperty::lookupWrapper<UseOwnerType, TearOffType>(this, LowerProperty##PropertyInfo())) \
         wrapper->detachListWrappers(newListSize); \
 }
 
diff --git a/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h
index 6f643e87c..2953f2ff6 100644
--- a/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h
+++ b/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h
@@ -29,18 +29,24 @@ namespace WebCore {
 
 class SVGAnimatedTransformListPropertyTearOff : public SVGAnimatedListPropertyTearOff<SVGTransformList> {
 public:
-    virtual SVGListPropertyTearOff<SVGTransformList>* baseVal()
+    virtual PassRefPtr<ListProperty> baseVal() OVERRIDE
     {
-        if (!m_baseVal)
-            m_baseVal = SVGTransformListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers);
-        return static_cast<SVGListPropertyTearOff<SVGTransformList>*>(m_baseVal.get());
+        if (m_baseVal)
+            return m_baseVal;
+
+        RefPtr<ListProperty> property = SVGTransformListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers);
+        m_baseVal = property.get();
+        return property.release();
     }
 
-    virtual SVGListPropertyTearOff<SVGTransformList>* animVal()
+    virtual PassRefPtr<ListProperty> animVal() OVERRIDE
     {
-        if (!m_animVal)
-            m_animVal = SVGTransformListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers);
-        return static_cast<SVGListPropertyTearOff<SVGTransformList>*>(m_animVal.get());
+        if (m_animVal)
+            return m_animVal;
+
+        RefPtr<ListProperty> property = SVGTransformListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers);
+        m_animVal = property.get();
+        return property.release();
     }
 
     static PassRefPtr<SVGAnimatedTransformListPropertyTearOff> create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, SVGTransformList& values)
diff --git a/Source/WebCore/svg/properties/SVGListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGListPropertyTearOff.h
index 3c6552467..326a89a74 100644
--- a/Source/WebCore/svg/properties/SVGListPropertyTearOff.h
+++ b/Source/WebCore/svg/properties/SVGListPropertyTearOff.h
@@ -121,6 +121,12 @@ protected:
     {
     }
 
+    virtual ~SVGListPropertyTearOff()
+    {
+        if (m_animatedProperty)
+            m_animatedProperty->propertyWillBeDeleted(*this);
+    }
+
     virtual bool isReadOnly() const
     {
         if (m_role == AnimValRole)
diff --git a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp
index 573d453a4..51c48b4c3 100644
--- a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp
+++ b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp
@@ -95,7 +95,7 @@ SVGPathElement* SVGPathSegListPropertyTearOff::contextElement() const
 bool SVGPathSegListPropertyTearOff::processIncomingListItemValue(const ListItemType& newItem, unsigned* indexToModify)
 {
     SVGPathSegWithContext* newItemWithContext = static_cast<SVGPathSegWithContext*>(newItem.get());
-    SVGAnimatedProperty* animatedPropertyOfItem = newItemWithContext->animatedProperty();
+    RefPtr<SVGAnimatedProperty> animatedPropertyOfItem = newItemWithContext->animatedProperty();
 
     // Alter the role, after calling animatedProperty(), as that may influence the returned animated property.
     newItemWithContext->setContextAndRole(contextElement(), m_pathSegRole);
@@ -111,7 +111,7 @@ bool SVGPathSegListPropertyTearOff::processIncomingListItemValue(const ListItemT
     // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list.
     // 'newItem' is already living in another list. If it's not our list, synchronize the other lists wrappers after the removal.
     bool livesInOtherList = animatedPropertyOfItem != m_animatedProperty;
-    SVGAnimatedPathSegListPropertyTearOff* propertyTearOff = static_cast<SVGAnimatedPathSegListPropertyTearOff*>(animatedPropertyOfItem);
+    RefPtr<SVGAnimatedPathSegListPropertyTearOff> propertyTearOff = static_pointer_cast<SVGAnimatedPathSegListPropertyTearOff>(animatedPropertyOfItem);
     int indexToRemove = propertyTearOff->findItem(newItem.get());
     ASSERT(indexToRemove != -1);