- Revision
- 136906
- Author
- [email protected]
- Date
- 2012-12-06 16:21:57 -0800 (Thu, 06 Dec 2012)
Log Message
Unify SVG's animation and target tracking systems.
https://bugs.webkit.org/show_bug.cgi?id=102655
Reviewed by Dirk Schulze.
Source/WebCore:
This patch unifies our animation target tracking system and regular target tracking system.
This simplifies the code, fixes a bug, and cleans up a historically security-sensitive area.
Background: When <use>, <mpath>, <animate>, etc. reference another element using
xlink:href="" we need to track when #id changes to #otherId, when #id is removed, etc.
This bookkeeping of element -> target is done in SVGDocumentExtensions. Additionally, when
a target changes that causes layout (e.g., rect.x is changed), all dependent elements with
renderers are notified (<animate> has no renderer and will not use this).
Previously, xlink:href changes were lazily resolved when targetElement() was called, target
changes were tracked using the animation tracking framework, and pending targets did not
work (e.g., <animate xlink:href="" is now pending #p --><rect id="p"/>).
After this patch, we no longer lazily resolve targetElement() but instead change it when
our xlink:href attribute changes. Instead of using the animation tracking framework in
SVGDocumentExtensions, we now use the regular target tracking framework. Lastly, by using
the regular target tracking framework we are able to hook into the pending resource handling
which fixes a bug (see the test).
A test has been added to test that the order of animation elements does not matter. A second
test has been added to show we do not regress a pending-while-pending case.
Tests: svg/animations/svg-animation-order.html
svg/custom/svg-pending-twice.html
* svg/SVGAnimateElement.cpp:
(WebCore::SVGAnimateElement::setTargetElement):
setTargetElement and setAttributeName now work similarly. When the corresponding attribute
changes, we update our internal state (target or attributeName) and save it instead of
looking these values up on each iteration.
(WebCore::SVGAnimateElement::setAttributeName):
(WebCore):
(WebCore::SVGAnimateElement::resetAnimatedPropertyType):
* svg/SVGAnimateElement.h:
(SVGAnimateElement):
* svg/SVGAnimationElement.cpp:
(WebCore::SVGAnimationElement::setAttributeType):
(WebCore::SVGAnimationElement::setTargetElement):
(WebCore::SVGAnimationElement::setAttributeName):
* svg/SVGAnimationElement.h:
(SVGAnimationElement):
* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::~SVGDocumentExtensions):
(WebCore):
* svg/SVGDocumentExtensions.h:
(SVGDocumentExtensions):
* svg/SVGElement.cpp:
(WebCore::SVGElement::~SVGElement):
(WebCore::SVGElement::removedFrom):
(WebCore::SVGElement::attributeChanged):
* svg/SVGElementInstance.cpp:
(WebCore::SVGElementInstance::invalidateAllInstancesOfElement):
This can be removed after r131631 landed.
* svg/SVGMPathElement.cpp:
(WebCore::SVGMPathElement::buildPendingResource):
* svg/SVGTextPathElement.cpp:
(WebCore::SVGTextPathElement::buildPendingResource):
A bug was discovered in review with our resource tracking in a pending-while-pending
case. SVGMpathElement and SVGTextPathElement have been updated to fix this as well.
* svg/animation/SVGSMILElement.cpp:
The changes in SVGSMILElement should look very similar to SVGFEImageElement,
SVGMPathElement, etc. The idea is to build pending resources when added or
removed from the document, or when the href attribute changes.
(WebCore::SVGSMILElement::~SVGSMILElement):
(WebCore):
(WebCore::SVGSMILElement::clearResourceReferences):
(WebCore::SVGSMILElement::buildPendingResource):
(WebCore::SVGSMILElement::insertedInto):
(WebCore::SVGSMILElement::removedFrom):
(WebCore::SVGSMILElement::svgAttributeChanged):
(WebCore::SVGSMILElement::setAttributeName):
(WebCore::SVGSMILElement::setTargetElement):
* svg/animation/SVGSMILElement.h:
(WebCore):
(WebCore::SVGSMILElement::targetElement):
(SVGSMILElement):
LayoutTests:
* svg/animations/svg-animation-order-expected.html: Added.
* svg/animations/svg-animation-order.html: Added.
* svg/custom/svg-pending-twice-expected.html: Added.
* svg/custom/svg-pending-twice.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (136905 => 136906)
--- trunk/LayoutTests/ChangeLog 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/LayoutTests/ChangeLog 2012-12-07 00:21:57 UTC (rev 136906)
@@ -1,3 +1,15 @@
+2012-12-06 Philip Rogers <[email protected]>
+
+ Unify SVG's animation and target tracking systems.
+ https://bugs.webkit.org/show_bug.cgi?id=102655
+
+ Reviewed by Dirk Schulze.
+
+ * svg/animations/svg-animation-order-expected.html: Added.
+ * svg/animations/svg-animation-order.html: Added.
+ * svg/custom/svg-pending-twice-expected.html: Added.
+ * svg/custom/svg-pending-twice.html: Added.
+
2012-12-06 James Simonsen <[email protected]>
[Chromium] Enable Resource Timing and User Timing
Added: trunk/LayoutTests/svg/animations/svg-animation-order-expected.html (0 => 136906)
--- trunk/LayoutTests/svg/animations/svg-animation-order-expected.html (rev 0)
+++ trunk/LayoutTests/svg/animations/svg-animation-order-expected.html 2012-12-07 00:21:57 UTC (rev 136906)
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<p id="description">Test that the order of elements does not affect animations.</p>
+<svg id="svg" width="300" height="300">
+ <rect x="0" y="0" width="100" height="100" fill="green"/>
+</svg>
+</body>
+</html>
Added: trunk/LayoutTests/svg/animations/svg-animation-order.html (0 => 136906)
--- trunk/LayoutTests/svg/animations/svg-animation-order.html (rev 0)
+++ trunk/LayoutTests/svg/animations/svg-animation-order.html 2012-12-07 00:21:57 UTC (rev 136906)
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<html>
+<script>
+function runTest() {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ // Defer to allow animations to run.
+ setTimeout(function() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+}
+</script>
+<body _onload_="runTest()">
+<p id="description">Test that the order of elements does not affect animations.</p>
+<svg id="svg" width="300" height="300">
+ <!-- This red square should be covered up if we pass. -->
+ <rect x="0" y="0" width="100" height="100" fill="red"/>
+
+ <!-- Animate one green rectangle over the top half of the red square. -->
+ <rect id="rect1" x="-100" y="0" width="100" height="50" fill="green" />
+ <set attributeName="x" xlink:href="" to="0"/>
+
+ <!-- Animate a second green rectangle over the bottom half of the red square. -->
+ <set attributeName="x" xlink:href="" to="0"/>
+ <rect id="rect2" x="-100" y="50" width="100" height="50" fill="green" />
+</svg>
+</body>
+</html>
Added: trunk/LayoutTests/svg/custom/svg-pending-twice-expected.html (0 => 136906)
--- trunk/LayoutTests/svg/custom/svg-pending-twice-expected.html (rev 0)
+++ trunk/LayoutTests/svg/custom/svg-pending-twice-expected.html 2012-12-07 00:21:57 UTC (rev 136906)
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<p id="description">Test that an animation with a pending resource can be changed to a second pending resource, then resolved correctly.</p>
+<svg id="svg" width="300" height="300">
+ <rect x="0" y="0" width="100" height="100" fill="green"/>
+</svg>
+</body>
+</html>
Added: trunk/LayoutTests/svg/custom/svg-pending-twice.html (0 => 136906)
--- trunk/LayoutTests/svg/custom/svg-pending-twice.html (rev 0)
+++ trunk/LayoutTests/svg/custom/svg-pending-twice.html 2012-12-07 00:21:57 UTC (rev 136906)
@@ -0,0 +1,34 @@
+<!DOCTYPE HTML>
+<html>
+<script>
+function runTest() {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ // The animation is already in a pending state, change it to another
+ // pending id (newId), then make the new id valid and check that
+ // the animation runs.
+ var rect = document.getElementById('rect');
+ var animate = document.getElementById('set');
+ animate.setAttributeNS("http://www.w3.org/1999/xlink", "href", "#newId");
+ rect.setAttribute('id', 'newId');
+
+ // Defer to allow animations to run.
+ setTimeout(function() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+}
+</script>
+<body _onload_="runTest()">
+<p id="description">Test that an animation with a pending resource can be changed to a second pending resource, then resolved correctly.</p>
+<svg id="svg" width="300" height="300">
+ <!-- This red square should be covered up if we pass. -->
+ <rect x="0" y="0" width="100" height="100" fill="red"/>
+
+ <!-- This green square should end up covering the red square if we pass. -->
+ <rect id="rect" x="-100" y="0" width="100" height="100" fill="green"/>
+ <set id="set" xlink:href="" attributeName="x" to="0" />
+</svg>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (136905 => 136906)
--- trunk/Source/WebCore/ChangeLog 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/ChangeLog 2012-12-07 00:21:57 UTC (rev 136906)
@@ -1,3 +1,95 @@
+2012-12-06 Philip Rogers <[email protected]>
+
+ Unify SVG's animation and target tracking systems.
+ https://bugs.webkit.org/show_bug.cgi?id=102655
+
+ Reviewed by Dirk Schulze.
+
+ This patch unifies our animation target tracking system and regular target tracking system.
+ This simplifies the code, fixes a bug, and cleans up a historically security-sensitive area.
+
+ Background: When <use>, <mpath>, <animate>, etc. reference another element using
+ xlink:href="" we need to track when #id changes to #otherId, when #id is removed, etc.
+ This bookkeeping of element -> target is done in SVGDocumentExtensions. Additionally, when
+ a target changes that causes layout (e.g., rect.x is changed), all dependent elements with
+ renderers are notified (<animate> has no renderer and will not use this).
+
+ Previously, xlink:href changes were lazily resolved when targetElement() was called, target
+ changes were tracked using the animation tracking framework, and pending targets did not
+ work (e.g., <animate xlink:href="" is now pending #p --><rect id="p"/>).
+
+ After this patch, we no longer lazily resolve targetElement() but instead change it when
+ our xlink:href attribute changes. Instead of using the animation tracking framework in
+ SVGDocumentExtensions, we now use the regular target tracking framework. Lastly, by using
+ the regular target tracking framework we are able to hook into the pending resource handling
+ which fixes a bug (see the test).
+
+ A test has been added to test that the order of animation elements does not matter. A second
+ test has been added to show we do not regress a pending-while-pending case.
+
+ Tests: svg/animations/svg-animation-order.html
+ svg/custom/svg-pending-twice.html
+
+ * svg/SVGAnimateElement.cpp:
+ (WebCore::SVGAnimateElement::setTargetElement):
+
+ setTargetElement and setAttributeName now work similarly. When the corresponding attribute
+ changes, we update our internal state (target or attributeName) and save it instead of
+ looking these values up on each iteration.
+
+ (WebCore::SVGAnimateElement::setAttributeName):
+ (WebCore):
+ (WebCore::SVGAnimateElement::resetAnimatedPropertyType):
+ * svg/SVGAnimateElement.h:
+ (SVGAnimateElement):
+ * svg/SVGAnimationElement.cpp:
+ (WebCore::SVGAnimationElement::setAttributeType):
+ (WebCore::SVGAnimationElement::setTargetElement):
+ (WebCore::SVGAnimationElement::setAttributeName):
+ * svg/SVGAnimationElement.h:
+ (SVGAnimationElement):
+ * svg/SVGDocumentExtensions.cpp:
+ (WebCore::SVGDocumentExtensions::~SVGDocumentExtensions):
+ (WebCore):
+ * svg/SVGDocumentExtensions.h:
+ (SVGDocumentExtensions):
+ * svg/SVGElement.cpp:
+ (WebCore::SVGElement::~SVGElement):
+ (WebCore::SVGElement::removedFrom):
+ (WebCore::SVGElement::attributeChanged):
+ * svg/SVGElementInstance.cpp:
+ (WebCore::SVGElementInstance::invalidateAllInstancesOfElement):
+
+ This can be removed after r131631 landed.
+
+ * svg/SVGMPathElement.cpp:
+ (WebCore::SVGMPathElement::buildPendingResource):
+ * svg/SVGTextPathElement.cpp:
+ (WebCore::SVGTextPathElement::buildPendingResource):
+
+ A bug was discovered in review with our resource tracking in a pending-while-pending
+ case. SVGMpathElement and SVGTextPathElement have been updated to fix this as well.
+
+ * svg/animation/SVGSMILElement.cpp:
+
+ The changes in SVGSMILElement should look very similar to SVGFEImageElement,
+ SVGMPathElement, etc. The idea is to build pending resources when added or
+ removed from the document, or when the href attribute changes.
+
+ (WebCore::SVGSMILElement::~SVGSMILElement):
+ (WebCore):
+ (WebCore::SVGSMILElement::clearResourceReferences):
+ (WebCore::SVGSMILElement::buildPendingResource):
+ (WebCore::SVGSMILElement::insertedInto):
+ (WebCore::SVGSMILElement::removedFrom):
+ (WebCore::SVGSMILElement::svgAttributeChanged):
+ (WebCore::SVGSMILElement::setAttributeName):
+ (WebCore::SVGSMILElement::setTargetElement):
+ * svg/animation/SVGSMILElement.h:
+ (WebCore):
+ (WebCore::SVGSMILElement::targetElement):
+ (SVGSMILElement):
+
2012-12-06 Jon Lee <[email protected]>
Retry snapshots if they are too empty
Modified: trunk/Source/WebCore/svg/SVGAnimateElement.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGAnimateElement.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGAnimateElement.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -396,16 +396,26 @@
return ensureAnimator()->calculateDistance(fromString, toString);
}
-void SVGAnimateElement::targetElementWillChange(SVGElement* currentTarget, SVGElement* newTarget)
+void SVGAnimateElement::setTargetElement(SVGElement* target)
{
- SVGAnimationElement::targetElementWillChange(currentTarget, newTarget);
+ SVGAnimationElement::setTargetElement(target);
+ resetAnimatedPropertyType();
+}
+void SVGAnimateElement::setAttributeName(const QualifiedName& attributeName)
+{
+ SVGAnimationElement::setAttributeName(attributeName);
+ resetAnimatedPropertyType();
+}
+
+void SVGAnimateElement::resetAnimatedPropertyType()
+{
ASSERT(!m_animatedType);
m_fromType.clear();
m_toType.clear();
m_toAtEndOfDurationType.clear();
m_animator.clear();
- m_animatedPropertyType = newTarget ? determineAnimatedPropertyType(newTarget) : AnimatedString;
+ m_animatedPropertyType = targetElement() ? determineAnimatedPropertyType(targetElement()) : AnimatedString;
}
SVGAnimatedTypeAnimator* SVGAnimateElement::ensureAnimator()
Modified: trunk/Source/WebCore/svg/SVGAnimateElement.h (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGAnimateElement.h 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGAnimateElement.h 2012-12-07 00:21:57 UTC (rev 136906)
@@ -45,6 +45,7 @@
virtual void resetAnimatedType();
virtual void clearAnimatedType(SVGElement* targetElement);
+
virtual bool calculateToAtEndOfDurationValue(const String& toAtEndOfDurationString);
virtual bool calculateFromAndToValues(const String& fromString, const String& toString);
virtual bool calculateFromAndByValues(const String& fromString, const String& byString);
@@ -53,11 +54,13 @@
virtual float calculateDistance(const String& fromString, const String& toString);
virtual bool isAdditive() const OVERRIDE;
- virtual void targetElementWillChange(SVGElement* currentTarget, SVGElement* oldTarget) OVERRIDE;
+ virtual void setTargetElement(SVGElement*) OVERRIDE;
+ virtual void setAttributeName(const QualifiedName&) OVERRIDE;
AnimatedPropertyType m_animatedPropertyType;
private:
+ void resetAnimatedPropertyType();
SVGAnimatedTypeAnimator* ensureAnimator();
virtual bool hasValidAttributeType();
Modified: trunk/Source/WebCore/svg/SVGAnimationElement.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGAnimationElement.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGAnimationElement.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -319,7 +319,7 @@
m_attributeType = AttributeTypeXML;
else
m_attributeType = AttributeTypeAuto;
- checkInvalidCSSAttributeType(targetElement(DoNotResolveNewTarget));
+ checkInvalidCSSAttributeType(targetElement());
}
String SVGAnimationElement::toValue() const
@@ -677,18 +677,16 @@
m_toPropertyValueType = InheritValue;
}
-void SVGAnimationElement::targetElementWillChange(SVGElement* currentTarget, SVGElement* newTarget)
+void SVGAnimationElement::setTargetElement(SVGElement* target)
{
- SVGSMILElement::targetElementWillChange(currentTarget, newTarget);
-
- checkInvalidCSSAttributeType(newTarget);
+ SVGSMILElement::setTargetElement(target);
+ checkInvalidCSSAttributeType(target);
}
void SVGAnimationElement::setAttributeName(const QualifiedName& attributeName)
{
SVGSMILElement::setAttributeName(attributeName);
-
- checkInvalidCSSAttributeType(targetElement(DoNotResolveNewTarget));
+ checkInvalidCSSAttributeType(targetElement());
}
void SVGAnimationElement::checkInvalidCSSAttributeType(SVGElement* target)
Modified: trunk/Source/WebCore/svg/SVGAnimationElement.h (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGAnimationElement.h 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGAnimationElement.h 2012-12-07 00:21:57 UTC (rev 136906)
@@ -197,7 +197,8 @@
AnimatedPropertyValueType m_fromPropertyValueType;
AnimatedPropertyValueType m_toPropertyValueType;
- virtual void targetElementWillChange(SVGElement* currentTarget, SVGElement* oldTarget) OVERRIDE;
+ virtual void setTargetElement(SVGElement*) OVERRIDE;
+ virtual void setAttributeName(const QualifiedName&) OVERRIDE;
bool hasInvalidCSSAttributeType() const { return m_hasInvalidCSSAttributeType; }
virtual void updateAnimationMode();
@@ -206,7 +207,6 @@
private:
virtual void animationAttributeChanged() OVERRIDE;
- virtual void setAttributeName(const QualifiedName&) OVERRIDE;
void setAttributeType(const AtomicString&);
void checkInvalidCSSAttributeType(SVGElement*);
Modified: trunk/Source/WebCore/svg/SVGDocumentExtensions.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGDocumentExtensions.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGDocumentExtensions.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -50,7 +50,6 @@
SVGDocumentExtensions::~SVGDocumentExtensions()
{
- deleteAllValues(m_animatedElements);
deleteAllValues(m_pendingResources);
deleteAllValues(m_pendingResourcesForRemoval);
}
@@ -133,58 +132,6 @@
}
}
-void SVGDocumentExtensions::addAnimationElementToTarget(SVGSMILElement* animationElement, SVGElement* targetElement)
-{
- ASSERT(targetElement);
- ASSERT(animationElement);
-
- if (HashSet<SVGSMILElement*>* animationElementsForTarget = m_animatedElements.get(targetElement)) {
- animationElementsForTarget->add(animationElement);
- return;
- }
-
- HashSet<SVGSMILElement*>* animationElementsForTarget = new HashSet<SVGSMILElement*>;
- animationElementsForTarget->add(animationElement);
- m_animatedElements.set(targetElement, animationElementsForTarget);
-}
-
-void SVGDocumentExtensions::removeAnimationElementFromTarget(SVGSMILElement* animationElement, SVGElement* targetElement)
-{
- ASSERT(targetElement);
- ASSERT(animationElement);
-
- HashMap<SVGElement*, HashSet<SVGSMILElement*>* >::iterator it = m_animatedElements.find(targetElement);
- ASSERT(it != m_animatedElements.end());
-
- HashSet<SVGSMILElement*>* animationElementsForTarget = it->value;
- ASSERT(!animationElementsForTarget->isEmpty());
-
- animationElementsForTarget->remove(animationElement);
- if (animationElementsForTarget->isEmpty()) {
- m_animatedElements.remove(it);
- delete animationElementsForTarget;
- }
-}
-
-void SVGDocumentExtensions::removeAllAnimationElementsFromTarget(SVGElement* targetElement)
-{
- ASSERT(targetElement);
- HashMap<SVGElement*, HashSet<SVGSMILElement*>* >::iterator it = m_animatedElements.find(targetElement);
- if (it == m_animatedElements.end())
- return;
-
- HashSet<SVGSMILElement*>* animationElementsForTarget = it->value;
- Vector<SVGSMILElement*> toBeReset;
-
- HashSet<SVGSMILElement*>::iterator end = animationElementsForTarget->end();
- for (HashSet<SVGSMILElement*>::iterator it = animationElementsForTarget->begin(); it != end; ++it)
- toBeReset.append(*it);
-
- Vector<SVGSMILElement*>::iterator vectorEnd = toBeReset.end();
- for (Vector<SVGSMILElement*>::iterator vectorIt = toBeReset.begin(); vectorIt != vectorEnd; ++vectorIt)
- (*vectorIt)->resetTargetElement();
-}
-
static void reportMessage(Document* document, MessageLevel level, const String& message)
{
if (document->frame())
Modified: trunk/Source/WebCore/svg/SVGDocumentExtensions.h (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGDocumentExtensions.h 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGDocumentExtensions.h 2012-12-07 00:21:57 UTC (rev 136906)
@@ -59,10 +59,6 @@
void pauseAnimations();
void unpauseAnimations();
void dispatchSVGLoadEventToOutermostSVGElements();
-
- void addAnimationElementToTarget(SVGSMILElement*, SVGElement*);
- void removeAnimationElementFromTarget(SVGSMILElement*, SVGElement*);
- void removeAllAnimationElementsFromTarget(SVGElement*);
void reportWarning(const String&);
void reportError(const String&);
@@ -83,7 +79,6 @@
private:
Document* m_document; // weak reference
HashSet<SVGSVGElement*> m_timeContainers; // For SVG 1.2 support this will need to be made more general.
- HashMap<SVGElement*, HashSet<SVGSMILElement*>* > m_animatedElements;
#if ENABLE(SVG_FONTS)
HashSet<SVGFontFaceElement*> m_svgFontFaceElements;
#endif
Modified: trunk/Source/WebCore/svg/SVGElement.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGElement.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGElement.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -83,7 +83,6 @@
rareDataMap.remove(it);
}
ASSERT(document());
- document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(this);
document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
}
@@ -183,7 +182,6 @@
StyledElement::removedFrom(rootParent);
if (wasInDocument) {
- document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(this);
document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
document()->accessSVGExtensions()->removeElementFromPendingResources(this);
}
@@ -537,10 +535,8 @@
{
StyledElement::attributeChanged(name, newValue);
- if (isIdAttributeName(name)) {
- document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(this);
+ if (isIdAttributeName(name))
document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
- }
// Changes to the style attribute are processed lazily (see Element::getAttribute() and related methods),
// so we don't want changes to the style attribute to result in extra work here.
Modified: trunk/Source/WebCore/svg/SVGElementInstance.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGElementInstance.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGElementInstance.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -140,14 +140,6 @@
ASSERT((*it)->correspondingElement() == element);
(*it)->shadowTreeElement()->setCorrespondingElement(0);
- // The shadow tree, which is eventually animated, is mutated. In order to keep the animVal
- // logic correct, we have to stop the animation, and restart it once the shadow tree has
- // recloned. Otherwise we miss to call stopAnimValAnimation() on the old shadow tree element
- // which leads to an assertion once garbage is collected, if the animVal bindings have been
- // accessed from JS. It would also assert on the next updateAnimations() call as the new
- // SVGAnimatedProperty object hasn't been initialized yet (using startAnimValAnimation).
- element->document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(element);
-
if (SVGUseElement* element = (*it)->correspondingUseElement()) {
ASSERT(element->inDocument());
element->invalidateShadowTree();
Modified: trunk/Source/WebCore/svg/SVGMPathElement.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGMPathElement.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGMPathElement.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -64,7 +64,8 @@
String id;
Element* target = SVGURIReference::targetElementFromIRIString(href(), document(), &id);
if (!target) {
- if (hasPendingResources())
+ // Do not register as pending if we are already pending this resource.
+ if (document()->accessSVGExtensions()->isElementPendingResource(this, id))
return;
if (!id.isEmpty()) {
Modified: trunk/Source/WebCore/svg/SVGTextPathElement.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/SVGTextPathElement.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/SVGTextPathElement.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -163,11 +163,14 @@
String id;
Element* target = SVGURIReference::targetElementFromIRIString(href(), document(), &id);
if (!target) {
- if (hasPendingResources() || id.isEmpty())
+ // Do not register as pending if we are already pending this resource.
+ if (document()->accessSVGExtensions()->isElementPendingResource(this, id))
return;
- document()->accessSVGExtensions()->addPendingResource(id, this);
- ASSERT(hasPendingResources());
+ if (!id.isEmpty()) {
+ document()->accessSVGExtensions()->addPendingResource(id, this);
+ ASSERT(hasPendingResources());
+ }
} else if (target->isSVGElement()) {
// Register us with the target in the dependencies map. Any change of hrefElement
// that leads to relayout/repainting now informs us, so we can react to it.
Modified: trunk/Source/WebCore/svg/animation/SVGSMILElement.cpp (136905 => 136906)
--- trunk/Source/WebCore/svg/animation/SVGSMILElement.cpp 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/animation/SVGSMILElement.cpp 2012-12-07 00:21:57 UTC (rev 136906)
@@ -139,13 +139,52 @@
SVGSMILElement::~SVGSMILElement()
{
+ clearResourceReferences();
disconnectConditions();
if (m_timeContainer && m_targetElement && hasValidAttributeName())
m_timeContainer->unschedule(this, m_targetElement, m_attributeName);
- if (m_targetElement)
- document()->accessSVGExtensions()->removeAnimationElementFromTarget(this, m_targetElement);
}
+void SVGSMILElement::clearResourceReferences()
+{
+ ASSERT(document());
+ document()->accessSVGExtensions()->removeAllTargetReferencesForElement(this);
+}
+
+void SVGSMILElement::buildPendingResource()
+{
+ clearResourceReferences();
+ if (!inDocument())
+ return;
+
+ String id;
+ String href = ""
+ Element* target;
+ if (href.isEmpty())
+ target = parentNode() && parentNode()->isElementNode() ? static_cast<Element*>(parentNode()) : 0;
+ else
+ target = SVGURIReference::targetElementFromIRIString(href, document(), &id);
+ SVGElement* svgTarget = target && target->isSVGElement() ? static_cast<SVGElement*>(target) : 0;
+
+ if (svgTarget != targetElement())
+ setTargetElement(svgTarget);
+
+ if (!svgTarget) {
+ // Do not register as pending if we are already pending this resource.
+ if (document()->accessSVGExtensions()->isElementPendingResource(this, id))
+ return;
+
+ if (!id.isEmpty()) {
+ document()->accessSVGExtensions()->addPendingResource(id, this);
+ ASSERT(hasPendingResources());
+ }
+ } else {
+ // Register us with the target in the dependencies map. Any change of hrefElement
+ // that leads to relayout/repainting now informs us, so we can react to it.
+ document()->accessSVGExtensions()->addElementReferencingTarget(this, svgTarget);
+ }
+}
+
static inline QualifiedName constructQualifiedName(const SVGElement* svgElement, const String& attributeName)
{
ASSERT(svgElement);
@@ -216,12 +255,11 @@
if (m_isWaitingForFirstInterval)
resolveFirstInterval();
- // Force resolution of target element
- if (!m_targetElement)
- targetElement();
if (m_timeContainer)
m_timeContainer->notifyIntervalsChanged();
+ buildPendingResource();
+
return InsertionDone;
}
@@ -238,11 +276,10 @@
RefPtr<SVGSMILElement> keepAlive(this);
disconnectConditions();
- // Clear target now, because disconnectConditions calls targetElement() which will recreate the target if we removed it sooner.
- if (m_targetElement)
- resetTargetElement(DoNotResolveNewTarget);
-
+ clearResourceReferences();
+ setTargetElement(0);
setAttributeName(anyQName());
+ animationAttributeChanged();
}
SVGElement::removedFrom(rootParent);
@@ -472,16 +509,11 @@
beginListChanged(elapsed());
else if (attrName == SVGNames::endAttr)
endListChanged(elapsed());
- else if (attrName == SVGNames::attributeNameAttr) {
+ else if (attrName == SVGNames::attributeNameAttr)
setAttributeName(constructQualifiedName(this, fastGetAttribute(SVGNames::attributeNameAttr)));
- if (m_targetElement) {
- resetTargetElement();
- return;
- }
- } else if (attrName.matches(XLinkNames::hrefAttr)) {
- // targetElement is resolved lazily but targetElement() will handle calling targetElementWillChange().
- if (SVGElement* targetElement = this->targetElement())
- document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(targetElement);
+ else if (attrName.matches(XLinkNames::hrefAttr)) {
+ SVGElementInstance::InvalidationGuard invalidationGuard(this);
+ buildPendingResource();
}
}
@@ -562,57 +594,30 @@
m_timeContainer->schedule(this, m_targetElement, m_attributeName);
} else
m_attributeName = attributeName;
-}
-SVGElement* SVGSMILElement::targetElement(ResolveTarget resolveTarget)
-{
+ // Only clear the animated type, if we had a target before.
if (m_targetElement)
- return m_targetElement;
-
- if (!inDocument() || resolveTarget == DoNotResolveNewTarget)
- return 0;
-
- String href = ""
- ContainerNode* target = href.isEmpty() ? parentNode() : SVGURIReference::targetElementFromIRIString(href, document());
- if (!target || !target->isSVGElement())
- return 0;
-
- SVGElement* targetElement = static_cast<SVGElement*>(target);
- targetElementWillChange(m_targetElement, targetElement);
- m_targetElement = targetElement;
- document()->accessSVGExtensions()->addAnimationElementToTarget(this, m_targetElement);
- return m_targetElement;
+ clearAnimatedType(m_targetElement);
}
-void SVGSMILElement::targetElementWillChange(SVGElement* currentTarget, SVGElement* newTarget)
+void SVGSMILElement::setTargetElement(SVGElement* target)
{
if (m_timeContainer && hasValidAttributeName()) {
- if (currentTarget)
- m_timeContainer->unschedule(this, currentTarget, m_attributeName);
- if (newTarget)
- m_timeContainer->schedule(this, newTarget, m_attributeName);
+ if (m_targetElement)
+ m_timeContainer->unschedule(this, m_targetElement, m_attributeName);
+ if (target)
+ m_timeContainer->schedule(this, target, m_attributeName);
}
// Only clear the animated type, if we had a target before.
- if (currentTarget)
- clearAnimatedType(currentTarget);
+ if (m_targetElement)
+ clearAnimatedType(m_targetElement);
// If the animation state is not Inactive, always reset to a clear state before leaving the old target element.
if (m_activeState != Inactive)
endedActiveInterval();
-}
-void SVGSMILElement::resetTargetElement(ResolveTarget resolveTarget)
-{
- document()->accessSVGExtensions()->removeAnimationElementFromTarget(this, m_targetElement);
- targetElementWillChange(m_targetElement, 0);
- m_targetElement = 0;
-
- // Immediately resolve the new targetElement (and call targetElementWillChange if needed) instead of doing it lazily.
- if (resolveTarget == ResolveNewTarget)
- targetElement();
-
- animationAttributeChanged();
+ m_targetElement = target;
}
SMILTime SVGSMILElement::elapsed() const
Modified: trunk/Source/WebCore/svg/animation/SVGSMILElement.h (136905 => 136906)
--- trunk/Source/WebCore/svg/animation/SVGSMILElement.h 2012-12-07 00:10:23 UTC (rev 136905)
+++ trunk/Source/WebCore/svg/animation/SVGSMILElement.h 2012-12-07 00:21:57 UTC (rev 136906)
@@ -33,8 +33,6 @@
namespace WebCore {
-enum ResolveTarget { DoNotResolveNewTarget, ResolveNewTarget };
-
class ConditionEventListener;
class SMILTimeContainer;
@@ -58,8 +56,7 @@
SMILTimeContainer* timeContainer() const { return m_timeContainer.get(); }
- SVGElement* targetElement(ResolveTarget = ResolveNewTarget);
- void resetTargetElement(ResolveTarget = ResolveNewTarget);
+ SVGElement* targetElement() const { return m_targetElement; }
const QualifiedName& attributeName() const { return m_attributeName; }
void beginByLinkActivation();
@@ -120,10 +117,13 @@
void setInactive() { m_activeState = Inactive; }
// Sub-classes may need to take action when the target is changed.
- virtual void targetElementWillChange(SVGElement* currentTarget, SVGElement* newTarget);
+ virtual void setTargetElement(SVGElement*);
virtual void setAttributeName(const QualifiedName&);
private:
+ void buildPendingResource();
+ void clearResourceReferences();
+
virtual void startedActiveInterval() = 0;
void endedActiveInterval();
virtual void updateAnimation(float percent, unsigned repeat, SVGSMILElement* resultElement) = 0;
