1 files changed, 54 insertions, 32 deletions

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;
 };
 
 }