Cleanup invalidation of SMIL timing/animation parameters

The main gist is that timing and animation should be essentially
independent - i.e the former only feeds into the latter by means of the
progress state. Based on this we can perform some cleanups:

 * Any change to the animation target will also invalidate the
   animation parameter state via WillChangeAnimationTarget(), so any
   such change (SetTargetElement, SetAttributeType and SetAttributeName)
   needn't be followed by a call to AnimationAttributeChanged(), so
   remove those.

 * Any change to timing state (such as 'dur', 'repeatDur',
   'repeatCount', 'min', 'max', 'begin' and 'end') will only influence
   the timing and not animation state, so we can drop calls to
   AnimationAttributeChanged() in those cases as well.

 * Since changing animation state should not affect timing, drop the
   call to SetInactive() from AnimationAttributeChanged() - this will be
   handled by timing updates when needed. This also means that
   SetInactive() is no longer used so it can be removed.

After the above changes AnimationAttributeChanged() no longer needs to
be virtual, and can reside in SVGAnimationElement alone.
Also fold InvalidatedValuesCache() into AnimationAttributeChanged()
since it is the only caller.
Take this opportunity to move some state updates from the
SvgAttributeChanged() override (which is for attributes which have an
SVG DOM representation - and 'href' attributes because of how the
dependency notifications work) to ParseAttribute().

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

third_party/blink/renderer/core/svg/animation/svg_smil_element.cc

@@ -298,7 +298,6 @@ void SVGSMILElement::RemovedFrom(ContainerNode& root_parent) {
     ClearResourceAndEventBaseReferences();
     ClearConditions();
     SetTargetElement(nullptr);
-    AnimationAttributeChanged();
     time_container_ = nullptr;
   }
 
@@ -483,7 +482,6 @@ void SVGSMILElement::ParseAttribute(const AttributeModificationParams& params) {
         time_container_->ScheduleIntervalUpdate();
       }
     }
-    AnimationAttributeChanged();
   } else if (name == svg_names::kEndAttr) {
     if (!conditions_.IsEmpty()) {
       ClearConditions();
@@ -499,7 +497,6 @@ void SVGSMILElement::ParseAttribute(const AttributeModificationParams& params) {
         time_container_->ScheduleIntervalUpdate();
       }
     }
-    AnimationAttributeChanged();
   } else if (name == svg_names::kOnbeginAttr) {
     SetAttributeEventListener(event_type_names::kBeginEvent,
                               CreateAttributeEventListener(this, name, value));
@@ -518,34 +515,31 @@ void SVGSMILElement::ParseAttribute(const AttributeModificationParams& params) {
       restart_ = kRestartAlways;
   } else if (name == svg_names::kFillAttr) {
     fill_ = value == "freeze" ? kFillFreeze : kFillRemove;
+  } else if (name == svg_names::kDurAttr) {
+    cached_dur_ = kInvalidCachedTime;
+  } else if (name == svg_names::kRepeatDurAttr) {
+    cached_repeat_dur_ = kInvalidCachedTime;
+  } else if (name == svg_names::kRepeatCountAttr) {
+    cached_repeat_count_ = SMILRepeatCount::Invalid();
+  } else if (name == svg_names::kMinAttr) {
+    cached_min_ = kInvalidCachedTime;
+  } else if (name == svg_names::kMaxAttr) {
+    cached_max_ = kInvalidCachedTime;
   } else {
     SVGElement::ParseAttribute(params);
   }
 }
 
 void SVGSMILElement::SvgAttributeChanged(const QualifiedName& attr_name) {
-  if (attr_name == svg_names::kDurAttr) {
-    cached_dur_ = kInvalidCachedTime;
-  } else if (attr_name == svg_names::kRepeatDurAttr) {
-    cached_repeat_dur_ = kInvalidCachedTime;
-  } else if (attr_name == svg_names::kRepeatCountAttr) {
-    cached_repeat_count_ = SMILRepeatCount::Invalid();
-  } else if (attr_name == svg_names::kMinAttr) {
-    cached_min_ = kInvalidCachedTime;
-  } else if (attr_name == svg_names::kMaxAttr) {
-    cached_max_ = kInvalidCachedTime;
-  } else if (attr_name.Matches(svg_names::kHrefAttr) ||
-             attr_name.Matches(xlink_names::kHrefAttr)) {
+  if (attr_name.Matches(svg_names::kHrefAttr) ||
+      attr_name.Matches(xlink_names::kHrefAttr)) {
     // TODO(fs): Could be smarter here when 'href' is specified and 'xlink:href'
     // is changed.
     SVGElement::InvalidationGuard invalidation_guard(this);
     BuildPendingResource();
-  } else {
-    SVGElement::SvgAttributeChanged(attr_name);
     return;
   }
-
-  AnimationAttributeChanged();
+  SVGElement::SvgAttributeChanged(attr_name);
 }
 
 void SVGSMILElement::ConnectConditions() {

third_party/blink/renderer/core/svg/animation/svg_smil_element.h

@@ -56,7 +56,6 @@ class CORE_EXPORT SVGSMILElement : public SVGElement, public SVGTests {
   void RemovedFrom(ContainerNode&) override;
 
   virtual bool HasValidTarget() const;
-  virtual void AnimationAttributeChanged() = 0;
 
   SMILTimeContainer* TimeContainer() const { return time_container_.Get(); }
 
@@ -131,8 +130,6 @@ class CORE_EXPORT SVGSMILElement : public SVGElement, public SVGTests {
 
   void AddInstanceTimeAndUpdate(BeginOrEnd, SMILTime, SMILTimeOrigin);
 
-  void SetInactive() { active_state_ = kInactive; }
-
   void SetTargetElement(SVGElement*);
 
   // Sub-classes may need to take action when the target is changed.

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

@@ -155,12 +155,10 @@ void SVGAnimateElement::ParseAttribute(
     const AttributeModificationParams& params) {
   if (params.name == svg_names::kAttributeTypeAttr) {
     SetAttributeType(params.new_value);
-    AnimationAttributeChanged();
     return;
   }
   if (params.name == svg_names::kAttributeNameAttr) {
     SetAttributeName(ConstructQualifiedName(*this, params.new_value));
-    AnimationAttributeChanged();
     return;
   }
   SVGAnimationElement::ParseAttribute(params);

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

@@ -171,6 +171,7 @@ void SVGAnimationElement::ParseAttribute(
       return;
     }
     UpdateAnimationMode();
+    AnimationAttributeChanged();
     return;
   }
 
@@ -179,6 +180,7 @@ void SVGAnimationElement::ParseAttribute(
       ReportAttributeParsingError(SVGParseStatus::kParsingFailed, name,
                                   params.new_value);
     }
+    AnimationAttributeChanged();
     return;
   }
 
@@ -191,6 +193,7 @@ void SVGAnimationElement::ParseAttribute(
                                     params.new_value);
       }
     }
+    AnimationAttributeChanged();
     return;
   }
 
@@ -199,53 +202,36 @@ void SVGAnimationElement::ParseAttribute(
       ReportAttributeParsingError(SVGParseStatus::kParsingFailed, name,
                                   params.new_value);
     }
+    AnimationAttributeChanged();
     return;
   }
 
   if (name == svg_names::kCalcModeAttr) {
     SetCalcMode(params.new_value);
+    AnimationAttributeChanged();
     return;
   }
 
   if (name == svg_names::kFromAttr || name == svg_names::kToAttr ||
       name == svg_names::kByAttr) {
     UpdateAnimationMode();
-    return;
-  }
-
-  SVGSMILElement::ParseAttribute(params);
-}
-
-void SVGAnimationElement::SvgAttributeChanged(const QualifiedName& attr_name) {
-  if (attr_name == svg_names::kValuesAttr || attr_name == svg_names::kByAttr ||
-      attr_name == svg_names::kFromAttr || attr_name == svg_names::kToAttr ||
-      attr_name == svg_names::kCalcModeAttr ||
-      attr_name == svg_names::kKeySplinesAttr ||
-      attr_name == svg_names::kKeyPointsAttr ||
-      attr_name == svg_names::kKeyTimesAttr) {
     AnimationAttributeChanged();
     return;
   }
 
-  SVGSMILElement::SvgAttributeChanged(attr_name);
-}
-
-void SVGAnimationElement::InvalidatedValuesCache() {
-  last_values_animation_from_ = String();
-  last_values_animation_to_ = String();
+  SVGSMILElement::ParseAttribute(params);
 }
 
 void SVGAnimationElement::AnimationAttributeChanged() {
   // Assumptions may not hold after an attribute change.
   animation_valid_ = AnimationValidity::kUnknown;
-  InvalidatedValuesCache();
-  SetInactive();
+  last_values_animation_from_ = String();
+  last_values_animation_to_ = String();
 }
 
 void SVGAnimationElement::WillChangeAnimationTarget() {
   SVGSMILElement::WillChangeAnimationTarget();
-  animation_valid_ = AnimationValidity::kUnknown;
-  InvalidatedValuesCache();
+  AnimationAttributeChanged();
 }
 
 float SVGAnimationElement::getStartTime(ExceptionState& exception_state) const {

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

@@ -114,7 +114,6 @@ class CORE_EXPORT SVGAnimationElement : public SVGSMILElement {
   SVGAnimationElement(const QualifiedName&, Document&);
 
   void ParseAttribute(const AttributeModificationParams&) override;
-  void SvgAttributeChanged(const QualifiedName&) override;
 
   String ToValue() const;
   String ByValue() const;
@@ -141,13 +140,12 @@ class CORE_EXPORT SVGAnimationElement : public SVGSMILElement {
   // http://www.w3.org/TR/SVG/animate.html#ValuesAttribute .
   static bool ParseValues(const String&, Vector<String>& result);
 
-  void InvalidatedValuesCache();
-  void AnimationAttributeChanged() override;
   void WillChangeAnimationTarget() override;
 
  private:
   bool IsValid() const final { return SVGTests::IsValid(); }
 
+  void AnimationAttributeChanged();
   bool CheckAnimationParameters();
   virtual bool CalculateToAtEndOfDurationValue(
       const String& to_at_end_of_duration_string) = 0;

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

@@ -47,7 +47,6 @@ class SVGDiscardElement final : public SVGSMILElement {
   void ResetAnimatedType() override {}
   void ClearAnimatedType() override {}
   void ApplyResultsToTarget() override {}
-  void AnimationAttributeChanged() override {}
 
   bool OverwritesUnderlyingAnimationValue() const override { return false; }