- Revision
- 197967
- Author
- s...@apple.com
- Date
- 2016-03-10 16:32:51 -0800 (Thu, 10 Mar 2016)
Log Message
REGRESSION: GuardMallloc crash in SVGListPropertyTearOff<SVGPointList>::processIncomingListItemWrapper
https://bugs.webkit.org/show_bug.cgi?id=154969
Patch by Said Abou-Hallawa <sabouhall...@apple.com> on 2016-03-10
Reviewed by Darin Adler.
The life cycle of the SVGAnimatedPropertyTearOff::m_baseVal and m_animVal
was not correct. Like what was done in SVGAnimatedListPropertyTearOff,
m_baseVal and m_animVal have to be raw RefCounted pointers. When requested
through, SVGAnimatedPropertyTearOff::baseVal() and animVal() they are
encapsulated in a RefPtr to ensure they existence as long as they are
referenced. When the animated property object (which is stored in either
m_baseVal or m_animVal) is not referenced by anyone, it is going to be
deleted. In the destructor of their class, SVGAnimatedPropertyTearOff
will be notified of this deletion through propertyWillBeDeleted() to clean
its member m_baseVal or m_animVal.
* bindings/scripts/CodeGeneratorJS.pm:
(NativeToJSValue): Now all the SVG animated property return RefPtrs. In
addition to that, SVGViewSpec.transform also returns
RefPtr<SVGTransformListPropertyTearOff>.
* svg/properties/SVGAnimatedListPropertyTearOff.h:
(WebCore::SVGAnimatedListPropertyTearOff::animVal):
(WebCore::SVGAnimatedListPropertyTearOff::currentAnimatedValue):
(WebCore::SVGAnimatedListPropertyTearOff::animationStarted):
(WebCore::SVGAnimatedListPropertyTearOff::animationEnded):
(WebCore::SVGAnimatedListPropertyTearOff::synchronizeWrappersIfNeeded):
(WebCore::SVGAnimatedListPropertyTearOff::isAnimating):
(WebCore::SVGAnimatedListPropertyTearOff::propertyWillBeDeleted):
Change propertyWillBeDeleted() to be virtual and make it takes an SVGProperty*.
Rename m_animatingAnimVal to be m_animatedProperty. Add isAnimating() which
returns true if m_animatedProperty is not null. Use isAnimating() instead of
m_isAnimating because it's deleted from the base class.
* svg/properties/SVGAnimatedProperty.cpp:
(WebCore::SVGAnimatedProperty::SVGAnimatedProperty):
(WebCore::SVGAnimatedProperty::~SVGAnimatedProperty):
* svg/properties/SVGAnimatedProperty.h:
(WebCore::SVGAnimatedProperty::isAnimating):
(WebCore::SVGAnimatedProperty::propertyWillBeDeleted):
Delete m_isAnimating since its value can be deduced from the value of
m_animatedProperty in the derived class. Add propertyWillBeDeleted() and
isAnimating() as virtual functions with the default behavior.
* svg/properties/SVGAnimatedPropertyTearOff.h:
(WebCore::SVGAnimatedPropertyTearOff::baseVal):
(WebCore::SVGAnimatedPropertyTearOff::animVal):
Like SVGAnimatedListPropertyTearOff::baseVal() and animVal() create the
value if it does not exist. Keep a raw RefCounted pointer but return a
RefPtr.
(WebCore::SVGAnimatedPropertyTearOff::isAnimating):
(WebCore::SVGAnimatedPropertyTearOff::propertyWillBeDeleted):
Override virtual functions.
(WebCore::SVGAnimatedPropertyTearOff::currentAnimatedValue):
(WebCore::SVGAnimatedPropertyTearOff::animationStarted):
(WebCore::SVGAnimatedPropertyTearOff::animationEnded):
(WebCore::SVGAnimatedPropertyTearOff::animValWillChange):
(WebCore::SVGAnimatedPropertyTearOff::animValDidChange):
Replace m_isAnimating with isAnimating(). Ensure that we get a new animated
property through animVal() and store it in a RefPtr to ensure it will not
go away while animating.
* svg/properties/SVGAnimatedStaticPropertyTearOff.h:
(WebCore::SVGAnimatedStaticPropertyTearOff::isAnimating):
(WebCore::SVGAnimatedStaticPropertyTearOff::currentAnimatedValue):
(WebCore::SVGAnimatedStaticPropertyTearOff::animationStarted):
(WebCore::SVGAnimatedStaticPropertyTearOff::animationEnded):
(WebCore::SVGAnimatedStaticPropertyTearOff::animValWillChange):
(WebCore::SVGAnimatedStaticPropertyTearOff::animValDidChange):
Add isAnimating() and replace all the instances of m_isAnimating with calls
to isAnimating().
* svg/properties/SVGPropertyTearOff.h:
(WebCore::SVGPropertyTearOff::animatedProperty):
(WebCore::SVGPropertyTearOff::setAnimatedProperty):
(WebCore::SVGPropertyTearOff::contextElement):
(WebCore::SVGPropertyTearOff::SVGPropertyTearOff):
(WebCore::SVGPropertyTearOff::~SVGPropertyTearOff):
SVGPropertyTearOff is what SVGAnimatedPropertyTearOff creates for its
baseVal() and animVal() values. These values can be null anytime once
they are not referenced. The SVGAnimatedPropertyTearOff holds only raw
RefCounted pointer for them. So (1) SVGPropertyTearOff needs to hold a
RefPtr for its SVGAnimatedProperty and (2) it needs to notify its
SVGAnimatedProperty when it's deleted by calling propertyWillBeDeleted()
from the destructor. Also there is no need to get the contextElement()
and save it in class member, m_contextElement since it can be always be
retrieved from SVGAnimatedProperty::contextElement().
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (197966 => 197967)
--- trunk/Source/WebCore/ChangeLog 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/ChangeLog 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,3 +1,95 @@
+2016-03-10 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ REGRESSION: GuardMallloc crash in SVGListPropertyTearOff<SVGPointList>::processIncomingListItemWrapper
+ https://bugs.webkit.org/show_bug.cgi?id=154969
+
+ Reviewed by Darin Adler.
+
+ The life cycle of the SVGAnimatedPropertyTearOff::m_baseVal and m_animVal
+ was not correct. Like what was done in SVGAnimatedListPropertyTearOff,
+ m_baseVal and m_animVal have to be raw RefCounted pointers. When requested
+ through, SVGAnimatedPropertyTearOff::baseVal() and animVal() they are
+ encapsulated in a RefPtr to ensure they existence as long as they are
+ referenced. When the animated property object (which is stored in either
+ m_baseVal or m_animVal) is not referenced by anyone, it is going to be
+ deleted. In the destructor of their class, SVGAnimatedPropertyTearOff
+ will be notified of this deletion through propertyWillBeDeleted() to clean
+ its member m_baseVal or m_animVal.
+
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (NativeToJSValue): Now all the SVG animated property return RefPtrs. In
+ addition to that, SVGViewSpec.transform also returns
+ RefPtr<SVGTransformListPropertyTearOff>.
+
+ * svg/properties/SVGAnimatedListPropertyTearOff.h:
+ (WebCore::SVGAnimatedListPropertyTearOff::animVal):
+ (WebCore::SVGAnimatedListPropertyTearOff::currentAnimatedValue):
+ (WebCore::SVGAnimatedListPropertyTearOff::animationStarted):
+ (WebCore::SVGAnimatedListPropertyTearOff::animationEnded):
+ (WebCore::SVGAnimatedListPropertyTearOff::synchronizeWrappersIfNeeded):
+ (WebCore::SVGAnimatedListPropertyTearOff::isAnimating):
+ (WebCore::SVGAnimatedListPropertyTearOff::propertyWillBeDeleted):
+ Change propertyWillBeDeleted() to be virtual and make it takes an SVGProperty*.
+ Rename m_animatingAnimVal to be m_animatedProperty. Add isAnimating() which
+ returns true if m_animatedProperty is not null. Use isAnimating() instead of
+ m_isAnimating because it's deleted from the base class.
+
+ * svg/properties/SVGAnimatedProperty.cpp:
+ (WebCore::SVGAnimatedProperty::SVGAnimatedProperty):
+ (WebCore::SVGAnimatedProperty::~SVGAnimatedProperty):
+ * svg/properties/SVGAnimatedProperty.h:
+ (WebCore::SVGAnimatedProperty::isAnimating):
+ (WebCore::SVGAnimatedProperty::propertyWillBeDeleted):
+ Delete m_isAnimating since its value can be deduced from the value of
+ m_animatedProperty in the derived class. Add propertyWillBeDeleted() and
+ isAnimating() as virtual functions with the default behavior.
+
+ * svg/properties/SVGAnimatedPropertyTearOff.h:
+ (WebCore::SVGAnimatedPropertyTearOff::baseVal):
+ (WebCore::SVGAnimatedPropertyTearOff::animVal):
+ Like SVGAnimatedListPropertyTearOff::baseVal() and animVal() create the
+ value if it does not exist. Keep a raw RefCounted pointer but return a
+ RefPtr.
+
+ (WebCore::SVGAnimatedPropertyTearOff::isAnimating):
+ (WebCore::SVGAnimatedPropertyTearOff::propertyWillBeDeleted):
+ Override virtual functions.
+
+ (WebCore::SVGAnimatedPropertyTearOff::currentAnimatedValue):
+ (WebCore::SVGAnimatedPropertyTearOff::animationStarted):
+ (WebCore::SVGAnimatedPropertyTearOff::animationEnded):
+ (WebCore::SVGAnimatedPropertyTearOff::animValWillChange):
+ (WebCore::SVGAnimatedPropertyTearOff::animValDidChange):
+ Replace m_isAnimating with isAnimating(). Ensure that we get a new animated
+ property through animVal() and store it in a RefPtr to ensure it will not
+ go away while animating.
+
+ * svg/properties/SVGAnimatedStaticPropertyTearOff.h:
+ (WebCore::SVGAnimatedStaticPropertyTearOff::isAnimating):
+ (WebCore::SVGAnimatedStaticPropertyTearOff::currentAnimatedValue):
+ (WebCore::SVGAnimatedStaticPropertyTearOff::animationStarted):
+ (WebCore::SVGAnimatedStaticPropertyTearOff::animationEnded):
+ (WebCore::SVGAnimatedStaticPropertyTearOff::animValWillChange):
+ (WebCore::SVGAnimatedStaticPropertyTearOff::animValDidChange):
+ Add isAnimating() and replace all the instances of m_isAnimating with calls
+ to isAnimating().
+
+ * svg/properties/SVGPropertyTearOff.h:
+ (WebCore::SVGPropertyTearOff::animatedProperty):
+ (WebCore::SVGPropertyTearOff::setAnimatedProperty):
+ (WebCore::SVGPropertyTearOff::contextElement):
+ (WebCore::SVGPropertyTearOff::SVGPropertyTearOff):
+ (WebCore::SVGPropertyTearOff::~SVGPropertyTearOff):
+ SVGPropertyTearOff is what SVGAnimatedPropertyTearOff creates for its
+ baseVal() and animVal() values. These values can be null anytime once
+ they are not referenced. The SVGAnimatedPropertyTearOff holds only raw
+ RefCounted pointer for them. So (1) SVGPropertyTearOff needs to hold a
+ RefPtr for its SVGAnimatedProperty and (2) it needs to notify its
+ SVGAnimatedProperty when it's deleted by calling propertyWillBeDeleted()
+ from the destructor. Also there is no need to get the contextElement()
+ and save it in class member, m_contextElement since it can be always be
+ retrieved from SVGAnimatedProperty::contextElement().
+
2016-03-10 Jonathan Davis <j...@apple.com>
Fixed broken link for "WebGL 2" on the Feature Status page
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (197966 => 197967)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2016-03-11 00:32:51 UTC (rev 197967)
@@ -4317,11 +4317,10 @@
return "toJSNewlyCreated(state, $globalObject, WTF::getPtr($value))";
}
- # $type has to be used here because SVGViewSpec.transform is of type SVGTransformList.
- if ($codeGenerator->IsSVGTypeNeedingTearOff($type) and $type =~ /(?<!String)List$/) {
+ if ($codeGenerator->IsSVGAnimatedType($interfaceName) or ($interfaceName eq "SVGViewSpec" and $type eq "SVGTransformList")) {
# 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") {
+ } elsif ($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$/) {
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h (197966 => 197967)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,5 +1,6 @@
/*
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -58,8 +59,10 @@
m_animVal = property.ptr();
return WTFMove(property);
}
-
- void propertyWillBeDeleted(const ListProperty& property)
+
+ bool isAnimating() const override { return m_animatedProperty; }
+ bool isAnimatedListTearOff() const override { return true; }
+ void propertyWillBeDeleted(const SVGProperty& property) override
{
if (&property == m_baseVal)
m_baseVal = nullptr;
@@ -67,8 +70,6 @@
m_animVal = nullptr;
}
- bool isAnimatedListTearOff() const override { return true; }
-
int findItem(SVGProperty* property)
{
// This should ever be called for our baseVal, as animVal can't modify the list.
@@ -91,9 +92,8 @@
PropertyType& currentAnimatedValue()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animatingAnimVal);
- return static_pointer_cast<ListProperty>(m_animatingAnimVal)->values();
+ ASSERT(isAnimating());
+ return m_animatedProperty->values();
}
const PropertyType& currentBaseValue() const
@@ -103,8 +103,7 @@
void animationStarted(PropertyType* newAnimVal, bool shouldOwnValues = false)
{
- ASSERT(!m_isAnimating);
- ASSERT(!m_animatingAnimVal);
+ ASSERT(!isAnimating());
ASSERT(newAnimVal);
ASSERT(m_values.size() == m_wrappers.size());
ASSERT(m_animatedWrappers.isEmpty());
@@ -113,45 +112,41 @@
if (!newAnimVal->isEmpty())
m_animatedWrappers.fill(0, newAnimVal->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;
+ m_animatedProperty = animVal();
+ m_animatedProperty->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues);
+ ASSERT(m_animatedProperty->values().size() == m_animatedProperty->wrappers().size());
+ ASSERT(m_animatedProperty->wrappers().size() == m_animatedWrappers.size());
}
void animationEnded()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animatingAnimVal);
+ ASSERT(isAnimating());
ASSERT(m_values.size() == m_wrappers.size());
- ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size());
- ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size());
+ ASSERT(m_animatedProperty->values().size() == m_animatedProperty->wrappers().size());
+ ASSERT(m_animatedProperty->wrappers().size() == m_animatedWrappers.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_animatedProperty->setValuesAndWrappers(&m_values, &m_wrappers, false);
+ ASSERT(m_animatedProperty->values().size() == m_animatedProperty->wrappers().size());
+ ASSERT(m_animatedProperty->wrappers().size() == m_wrappers.size());
m_animatedWrappers.clear();
- m_animatingAnimVal = nullptr;
- m_isAnimating = false;
+ m_animatedProperty = nullptr;
}
void synchronizeWrappersIfNeeded()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animatingAnimVal);
+ ASSERT(isAnimating());
// 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.
- m_animatingAnimVal->detachListWrappers(m_animatingAnimVal->values().size());
+ m_animatedProperty->detachListWrappers(m_animatedProperty->values().size());
- ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size());
- ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size());
+ ASSERT(m_animatedProperty->values().size() == m_animatedProperty->wrappers().size());
+ ASSERT(m_animatedProperty->wrappers().size() == m_animatedWrappers.size());
}
void animValWillChange()
@@ -192,7 +187,7 @@
ListProperty* m_baseVal { nullptr };
ListProperty* m_animVal { nullptr };
- RefPtr<ListProperty> m_animatingAnimVal;
+ RefPtr<ListProperty> m_animatedProperty;
};
}
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedProperty.cpp (197966 => 197967)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedProperty.cpp 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedProperty.cpp 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,6 +1,7 @@
/*
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
* Copyright (C) 2013 Samsung Electronics. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -29,7 +30,6 @@
: m_contextElement(contextElement)
, m_attributeName(attributeName)
, m_animatedPropertyType(animatedPropertyType)
- , m_isAnimating(false)
, m_isReadOnly(false)
{
}
@@ -45,7 +45,7 @@
}
// Assure that animationEnded() was called, if animationStarted() was called before.
- ASSERT(!m_isAnimating);
+ ASSERT(!isAnimating());
}
void SVGAnimatedProperty::commitChange()
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedProperty.h (197966 => 197967)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedProperty.h 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedProperty.h 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,6 +1,7 @@
/*
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
* Copyright (C) 2013 Samsung Electronics. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -28,19 +29,21 @@
namespace WebCore {
class SVGElement;
+class SVGProperty;
class SVGAnimatedProperty : public RefCounted<SVGAnimatedProperty> {
public:
SVGElement* contextElement() const { return m_contextElement.get(); }
const QualifiedName& attributeName() const { return m_attributeName; }
AnimatedPropertyType animatedPropertyType() const { return m_animatedPropertyType; }
- bool isAnimating() const { return m_isAnimating; }
bool isReadOnly() const { return m_isReadOnly; }
void setIsReadOnly() { m_isReadOnly = true; }
void commitChange();
+ virtual bool isAnimating() const { return false; }
virtual bool isAnimatedListTearOff() const { return false; }
+ virtual void propertyWillBeDeleted(const SVGProperty&) { }
// Caching facilities.
typedef HashMap<SVGAnimatedPropertyDescription, SVGAnimatedProperty*, SVGAnimatedPropertyDescriptionHash, SVGAnimatedPropertyDescriptionHashTraits> Cache;
@@ -92,7 +95,6 @@
AnimatedPropertyType m_animatedPropertyType;
protected:
- bool m_isAnimating;
bool m_isReadOnly;
};
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyTearOff.h (197966 => 197967)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyTearOff.h 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyTearOff.h 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,5 +1,6 @@
/*
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -31,30 +32,34 @@
typedef SVGPropertyTearOff<PropertyType> PropertyTearOff;
typedef PropertyType ContentType;
- virtual ~SVGAnimatedPropertyTearOff()
+ RefPtr<PropertyTearOff> baseVal()
{
- if (m_baseVal) {
- ASSERT(m_baseVal->animatedProperty() == this);
- m_baseVal->setAnimatedProperty(nullptr);
- }
- if (m_animVal) {
- ASSERT(m_animVal->animatedProperty() == this);
- m_animVal->setAnimatedProperty(nullptr);
- }
+ if (m_baseVal)
+ return m_baseVal;
+
+ auto property = PropertyTearOff::create(this, BaseValRole, m_property);
+ m_baseVal = property.ptr();
+ return WTFMove(property);
}
- PropertyTearOff* baseVal()
+ RefPtr<PropertyTearOff> animVal()
{
- if (!m_baseVal)
- m_baseVal = PropertyTearOff::create(this, BaseValRole, m_property);
- return m_baseVal.get();
+ if (m_animVal)
+ return m_animVal;
+
+ auto property = PropertyTearOff::create(this, AnimValRole, m_property);
+ m_animVal = property.ptr();
+ return WTFMove(property);
}
- PropertyTearOff* animVal()
+ bool isAnimating() const override { return m_animatedProperty; }
+
+ void propertyWillBeDeleted(const SVGProperty& property) override
{
- if (!m_animVal)
- m_animVal = PropertyTearOff::create(this, AnimValRole, m_property);
- return m_animVal.get();
+ if (&property == m_baseVal)
+ m_baseVal = nullptr;
+ else if (&property == m_animVal)
+ m_animVal = nullptr;
}
static Ref<SVGAnimatedPropertyTearOff<PropertyType>> create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, PropertyType& property)
@@ -65,9 +70,8 @@
PropertyType& currentAnimatedValue()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animVal);
- return m_animVal->propertyReference();
+ ASSERT(isAnimating());
+ return m_animatedProperty->propertyReference();
}
const PropertyType& currentBaseValue() const
@@ -77,32 +81,29 @@
void animationStarted(PropertyType* newAnimVal)
{
- ASSERT(!m_isAnimating);
+ ASSERT(!isAnimating());
ASSERT(newAnimVal);
- animVal()->setValue(*newAnimVal);
- m_isAnimating = true;
+ m_animatedProperty = animVal();
+ m_animatedProperty->setValue(*newAnimVal);
}
void animationEnded()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animVal);
- m_animVal->setValue(m_property);
- m_isAnimating = false;
+ ASSERT(isAnimating());
+ m_animatedProperty->setValue(m_property);
+ m_animatedProperty = nullptr;
}
void animValWillChange()
{
// no-op for non list types.
- ASSERT(m_isAnimating);
- ASSERT(m_animVal);
+ ASSERT(isAnimating());
}
void animValDidChange()
{
// no-op for non list types.
- ASSERT(m_isAnimating);
- ASSERT(m_animVal);
+ ASSERT(isAnimating());
}
private:
@@ -113,8 +114,10 @@
}
PropertyType& m_property;
- RefPtr<PropertyTearOff> m_baseVal;
- RefPtr<PropertyTearOff> m_animVal;
+ PropertyTearOff* m_baseVal { nullptr };
+ PropertyTearOff* m_animVal { nullptr };
+
+ RefPtr<PropertyTearOff> m_animatedProperty;
};
}
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedStaticPropertyTearOff.h (197966 => 197967)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedStaticPropertyTearOff.h 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedStaticPropertyTearOff.h 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,5 +1,6 @@
/*
* Copyright (C) Research In Motion Limited 2010-2012. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -48,6 +49,8 @@
commitChange();
}
+ bool isAnimating() const override { return m_animatedProperty; }
+
static Ref<SVGAnimatedStaticPropertyTearOff<PropertyType>> create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, PropertyType& property)
{
ASSERT(contextElement);
@@ -56,8 +59,7 @@
PropertyType& currentAnimatedValue()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animatedProperty);
+ ASSERT(isAnimating());
return *m_animatedProperty;
}
@@ -68,33 +70,27 @@
void animationStarted(PropertyType* newAnimVal)
{
- ASSERT(!m_isAnimating);
- ASSERT(!m_animatedProperty);
+ ASSERT(!isAnimating());
ASSERT(newAnimVal);
m_animatedProperty = newAnimVal;
- m_isAnimating = true;
}
void animationEnded()
{
- ASSERT(m_isAnimating);
- ASSERT(m_animatedProperty);
+ ASSERT(isAnimating());
m_animatedProperty = nullptr;
- m_isAnimating = false;
}
void animValWillChange()
{
// no-op for non list types.
- ASSERT(m_isAnimating);
- ASSERT(m_animatedProperty);
+ ASSERT(isAnimating());
}
void animValDidChange()
{
// no-op for non list types.
- ASSERT(m_isAnimating);
- ASSERT(m_animatedProperty);
+ ASSERT(isAnimating());
}
protected:
Modified: trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h (197966 => 197967)
--- trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h 2016-03-11 00:05:58 UTC (rev 197966)
+++ trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h 2016-03-11 00:32:51 UTC (rev 197967)
@@ -1,5 +1,6 @@
/*
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -57,7 +58,7 @@
}
virtual PropertyType& propertyReference() { return *m_value; }
- SVGAnimatedProperty* animatedProperty() const { return m_animatedProperty; }
+ SVGAnimatedProperty* animatedProperty() const { return m_animatedProperty.get(); }
virtual void setValue(PropertyType& value)
{
@@ -72,16 +73,13 @@
void setAnimatedProperty(SVGAnimatedProperty* animatedProperty)
{
m_animatedProperty = animatedProperty;
-
- if (m_animatedProperty)
- m_contextElement = m_animatedProperty->contextElement();
}
SVGElement* contextElement() const
{
if (!m_animatedProperty || m_valueIsCopy)
- return 0;
- return m_contextElement.get();
+ return nullptr;
+ return m_animatedProperty->contextElement();
}
void addChild(WeakPtr<SVGPropertyTearOffBase> child)
@@ -131,11 +129,6 @@
, m_value(&value)
, m_valueIsCopy(false)
{
- // Using operator & is completely fine, as SVGAnimatedProperty owns this reference,
- // and we're guaranteed to live as long as SVGAnimatedProperty does.
-
- if (m_animatedProperty)
- m_contextElement = m_animatedProperty->contextElement();
}
SVGPropertyTearOff(const PropertyType& initialValue)
@@ -157,6 +150,9 @@
detachChildren();
delete m_value;
}
+
+ if (m_animatedProperty)
+ m_animatedProperty->propertyWillBeDeleted(*this);
}
void detachChildren()
@@ -168,8 +164,7 @@
m_childTearOffs.clear();
}
- RefPtr<SVGElement> m_contextElement;
- SVGAnimatedProperty* m_animatedProperty;
+ RefPtr<SVGAnimatedProperty> m_animatedProperty;
SVGPropertyRole m_role;
PropertyType* m_value;
Vector<WeakPtr<SVGPropertyTearOffBase>> m_childTearOffs;
