Simplify the animated attribute interface in SVGElement

ResetAnimatedType() and ApplyResultsToTarget() are always called in
"sequence" by the one method
(SMILAnimationSandwich::ApplyAnimationValues), so we can move the call
to SetAnimatedAttribute() to the latter method.
Then we fold the calls to InvalidateAnimatedAttribute() into the
Set/ClearAnimatedAttribute calls.

Also simplify Set/ClearWebAnimatedAttributes() since they can now
trivially call into the modified versions of Set/ClearAnimatedAttribute().

For SVGAnimateMotionElement, move the invalidation of the target from
CalculateAnimatedValue() to ApplyResultsToTarget().

Bug: 1017723
Change-Id: I6ee237aa1be725db44b02212fe8a1bff1cd5e16b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1878771Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#709572}

third_party/blink/renderer/core/svg/svg_animate_element.cc

@@ -424,7 +424,6 @@ void SVGAnimateElement::ResetAnimatedType() {
     // SVG DOM animVal animation code-path.
     animated_value_ = target_property_->CreateAnimatedValue();
     DCHECK_EQ(animated_value_->GetType(), type_);
-    target_element->SetAnimatedAttribute(AttributeName(), animated_value_);
     return;
   }
   DCHECK(IsAnimatingCSSProperty());
@@ -448,8 +447,8 @@ void SVGAnimateElement::ClearAnimatedType() {
   }
 
   bool should_apply = ShouldApplyAnimation(*target_element);
+  // CSS properties animation code-path.
   if (IsAnimatingCSSProperty()) {
-    // CSS properties animation code-path.
     if (should_apply) {
       MutableCSSPropertyValueSet* property_set =
           target_element->EnsureAnimatedSMILStyleProperties();
@@ -460,12 +459,9 @@ void SVGAnimateElement::ClearAnimatedType() {
       }
     }
   }
-  if (IsAnimatingSVGDom()) {
-    // SVG DOM animVal animation code-path.
+  // SVG DOM animVal animation code-path.
+  if (IsAnimatingSVGDom())
     target_element->ClearAnimatedAttribute(AttributeName());
-    if (should_apply)
-      target_element->InvalidateAnimatedAttribute(AttributeName());
-  }
 
   animated_value_.Clear();
 }
@@ -484,8 +480,9 @@ void SVGAnimateElement::ApplyResultsToTarget() {
 
   // We do update the style and the animation property independent of each
   // other.
+
+  // CSS properties animation code-path.
   if (IsAnimatingCSSProperty()) {
-    // CSS properties animation code-path.
     // Convert the result of the animation to a String and apply it as CSS
     // property on the target_element.
     MutableCSSPropertyValueSet* properties =
@@ -502,12 +499,9 @@ void SVGAnimateElement::ApplyResultsToTarget() {
           StyleChangeReasonForTracing::Create(style_change_reason::kAnimation));
     }
   }
-  if (IsAnimatingSVGDom()) {
-    // SVG DOM animVal animation code-path.
-    // At this point the SVG DOM values are already changed, unlike for CSS.
-    // We only have to trigger update notifications here.
-    targetElement()->InvalidateAnimatedAttribute(AttributeName());
-  }
+  // SVG DOM animVal animation code-path.
+  if (IsAnimatingSVGDom())
+    target_element->SetAnimatedAttribute(AttributeName(), animated_value_);
 }
 
 bool SVGAnimateElement::AnimatedPropertyTypeSupportsAddition() const {

third_party/blink/renderer/core/svg/svg_animate_motion_element.cc

@@ -204,9 +204,6 @@ void SVGAnimateMotionElement::CalculateAnimatedValue(float percentage,
   if (!transform)
     return;
 
-  if (LayoutObject* target_layout_object = target_element->GetLayoutObject())
-    InvalidateForAnimateMotionTransformChange(*target_layout_object);
-
   if (!IsAdditive())
     transform->MakeIdentity();
 
@@ -264,6 +261,9 @@ void SVGAnimateMotionElement::ApplyResultsToTarget() {
   if (!target_transform)
     return;
 
+  if (LayoutObject* target_layout_object = target_element->GetLayoutObject())
+    InvalidateForAnimateMotionTransformChange(*target_layout_object);
+
   // ...except in case where we have additional instances in <use> trees.
   const auto& instances = target_element->InstancesForElement();
   for (SVGElement* shadow_tree_element : instances) {

third_party/blink/renderer/core/svg/svg_element.cc

@@ -232,13 +232,7 @@ static void ForSelfAndInstances(SVGElement* element, T callback) {
 
 void SVGElement::SetWebAnimatedAttribute(const QualifiedName& attribute,
                                          SVGPropertyBase* value) {
-  ForSelfAndInstances(this, [&attribute, &value](SVGElement* element) {
-    if (SVGAnimatedPropertyBase* animated_property =
-            element->PropertyFromAttribute(attribute)) {
-      animated_property->SetAnimatedValue(value);
-      NotifyAnimValChanged(element, attribute);
-    }
-  });
+  SetAnimatedAttribute(attribute, value);
   EnsureSVGRareData()->WebAnimatedAttributes().insert(&attribute);
 }
 
@@ -247,13 +241,7 @@ void SVGElement::ClearWebAnimatedAttributes() {
     return;
   for (const QualifiedName* attribute :
        SvgRareData()->WebAnimatedAttributes()) {
-    ForSelfAndInstances(this, [&attribute](SVGElement* element) {
-      if (SVGAnimatedPropertyBase* animated_property =
-              element->PropertyFromAttribute(*attribute)) {
-        animated_property->AnimationEnded();
-        NotifyAnimValChanged(element, *attribute);
-      }
-    });
+    ClearAnimatedAttribute(*attribute);
   }
   SvgRareData()->WebAnimatedAttributes().clear();
 }
@@ -262,22 +250,20 @@ void SVGElement::SetAnimatedAttribute(const QualifiedName& attribute,
                                       SVGPropertyBase* value) {
   ForSelfAndInstances(this, [&attribute, &value](SVGElement* element) {
     if (SVGAnimatedPropertyBase* animated_property =
-            element->PropertyFromAttribute(attribute))
+            element->PropertyFromAttribute(attribute)) {
       animated_property->SetAnimatedValue(value);
-  });
-}
-
-void SVGElement::InvalidateAnimatedAttribute(const QualifiedName& attribute) {
-  ForSelfAndInstances(this, [&attribute](SVGElement* element) {
-    NotifyAnimValChanged(element, attribute);
+      NotifyAnimValChanged(element, attribute);
+    }
   });
 }
 
 void SVGElement::ClearAnimatedAttribute(const QualifiedName& attribute) {
   ForSelfAndInstances(this, [&attribute](SVGElement* element) {
     if (SVGAnimatedPropertyBase* animated_property =
-            element->PropertyFromAttribute(attribute))
+            element->PropertyFromAttribute(attribute)) {
       animated_property->AnimationEnded();
+      NotifyAnimValChanged(element, attribute);
+    }
   });
 }
 

third_party/blink/renderer/core/svg/svg_element.h

@@ -103,7 +103,6 @@ class CORE_EXPORT SVGElement : public Element {
   void ClearWebAnimatedAttributes();
 
   void SetAnimatedAttribute(const QualifiedName&, SVGPropertyBase*);
-  void InvalidateAnimatedAttribute(const QualifiedName&);
   void ClearAnimatedAttribute(const QualifiedName&);
 
   SVGSVGElement* ownerSVGElement() const;