Commit fe3a12df authored by fs's avatar fs Committed by Commit bot

Tighten SVGAnimationElement::shouldApplyAnimation

Fold the targetIsUsable(...) helper from SVGAnimateElement into said
method, and then replace the uses of the former with the corresponding
'should apply' predicate.

BUG=640676

Review-Url: https://codereview.chromium.org/2287983002
Cr-Commit-Position: refs/heads/master@{#415253}
parent 860564e6
...@@ -178,7 +178,6 @@ void SVGAnimateElement::resetAnimatedType() ...@@ -178,7 +178,6 @@ void SVGAnimateElement::resetAnimatedType()
m_animator.reset(targetElement); m_animator.reset(targetElement);
ShouldApplyAnimationType shouldApply = shouldApplyAnimation(targetElement, attributeName); ShouldApplyAnimationType shouldApply = shouldApplyAnimation(targetElement, attributeName);
if (shouldApply == DontApplyAnimation) if (shouldApply == DontApplyAnimation)
return; return;
if (shouldApply == ApplyXMLAnimation || shouldApply == ApplyXMLandCSSAnimation) { if (shouldApply == ApplyXMLAnimation || shouldApply == ApplyXMLandCSSAnimation) {
...@@ -206,12 +205,6 @@ void SVGAnimateElement::resetAnimatedType() ...@@ -206,12 +205,6 @@ void SVGAnimateElement::resetAnimatedType()
m_animatedProperty = m_animator.constructFromString(baseValue); m_animatedProperty = m_animator.constructFromString(baseValue);
} }
static bool targetIsUsable(SVGElement* targetElement, const QualifiedName& attribute)
{
return attribute != anyQName()
&& targetElement->isConnected() && targetElement->parentNode();
}
void SVGAnimateElement::clearAnimatedType() void SVGAnimateElement::clearAnimatedType()
{ {
if (!m_animatedProperty) if (!m_animatedProperty)
...@@ -231,7 +224,7 @@ void SVGAnimateElement::clearAnimatedType() ...@@ -231,7 +224,7 @@ void SVGAnimateElement::clearAnimatedType()
ShouldApplyAnimationType shouldApply = shouldApplyAnimation(targetElement, attributeName()); ShouldApplyAnimationType shouldApply = shouldApplyAnimation(targetElement, attributeName());
if (shouldApply == ApplyXMLandCSSAnimation || m_animator.isAnimatingCSSProperty()) { if (shouldApply == ApplyXMLandCSSAnimation || m_animator.isAnimatingCSSProperty()) {
// CSS properties animation code-path. // CSS properties animation code-path.
if (targetIsUsable(targetElement, attributeName())) { if (shouldApply != DontApplyAnimation) {
CSSPropertyID id = cssPropertyID(attributeName().localName()); CSSPropertyID id = cssPropertyID(attributeName().localName());
targetElement->ensureAnimatedSMILStyleProperties()->removeProperty(id); targetElement->ensureAnimatedSMILStyleProperties()->removeProperty(id);
targetElement->setNeedsStyleRecalc(LocalStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::Animation)); targetElement->setNeedsStyleRecalc(LocalStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::Animation));
...@@ -240,7 +233,7 @@ void SVGAnimateElement::clearAnimatedType() ...@@ -240,7 +233,7 @@ void SVGAnimateElement::clearAnimatedType()
if (shouldApply == ApplyXMLandCSSAnimation || m_animator.isAnimatingSVGDom()) { if (shouldApply == ApplyXMLandCSSAnimation || m_animator.isAnimatingSVGDom()) {
// SVG DOM animVal animation code-path. // SVG DOM animVal animation code-path.
m_animator.stopAnimValAnimation(); m_animator.stopAnimValAnimation();
if (targetIsUsable(targetElement, attributeName())) if (shouldApply != DontApplyAnimation)
targetElement->invalidateAnimatedAttribute(attributeName()); targetElement->invalidateAnimatedAttribute(attributeName());
} }
...@@ -259,8 +252,7 @@ void SVGAnimateElement::applyResultsToTarget() ...@@ -259,8 +252,7 @@ void SVGAnimateElement::applyResultsToTarget()
// We do update the style and the animation property independent of each other. // We do update the style and the animation property independent of each other.
ShouldApplyAnimationType shouldApply = shouldApplyAnimation(targetElement(), attributeName()); ShouldApplyAnimationType shouldApply = shouldApplyAnimation(targetElement(), attributeName());
DCHECK(targetElement()); if (shouldApply == DontApplyAnimation)
if (!targetIsUsable(targetElement(), attributeName()))
return; return;
if (shouldApply == ApplyXMLandCSSAnimation || m_animator.isAnimatingCSSProperty()) { if (shouldApply == ApplyXMLandCSSAnimation || m_animator.isAnimatingCSSProperty()) {
// CSS properties animation code-path. // CSS properties animation code-path.
......
...@@ -376,7 +376,11 @@ bool SVGAnimationElement::isTargetAttributeCSSProperty(SVGElement* targetElement ...@@ -376,7 +376,11 @@ bool SVGAnimationElement::isTargetAttributeCSSProperty(SVGElement* targetElement
SVGAnimationElement::ShouldApplyAnimationType SVGAnimationElement::shouldApplyAnimation(SVGElement* targetElement, const QualifiedName& attributeName) SVGAnimationElement::ShouldApplyAnimationType SVGAnimationElement::shouldApplyAnimation(SVGElement* targetElement, const QualifiedName& attributeName)
{ {
if (!hasValidAttributeType() || !targetElement || attributeName == anyQName() || !targetElement->inActiveDocument()) if (!hasValidAttributeType()
|| attributeName == anyQName()
|| !targetElement
|| !targetElement->inActiveDocument()
|| !targetElement->parentNode())
return DontApplyAnimation; return DontApplyAnimation;
// Always animate CSS properties, using the ApplyCSSAnimation code path, regardless of the attributeType value. // Always animate CSS properties, using the ApplyCSSAnimation code path, regardless of the attributeType value.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment