Always update intervals using the "latest update time"

For interval updates performed "outside" of the regular flow in
SMILTimeContainer, we would use the "previous" presentation - i.e the
time just before the time we latest updated timing to. This could cause
elements to resolve intervals in the past - for example when the latest
update time was at the end of an interval it could discard the current
interval and resolve an interval in the past, which could then be
propagated to its dependents. When these elements were later updated by
the main update loop they would again discard the current (in the past)
interval and resolve a new one, notifying its dependents. Repeat.

Instead always use the "latest update time" (the same is used by
SMILTimeContainer::UpdateIntervals) as the argument to Updateinterval(),
and only use the "previous" presentation time when checking if the
active state may have changed in a relevant way and for rescheduling.

Bug: 1021630, 1028839, 1029327
Change-Id: Ibe691ae336df51a36626fdd800e5ddc2303aee5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939788Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#720669}

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

@@ -169,7 +169,7 @@ void SMILTimeContainer::ResetDocumentTime() {
   SynchronizeToDocumentTimeline();
 }
 
-SMILTime SMILTimeContainer::CurrentDocumentTime() const {
+SMILTime SMILTimeContainer::LatestUpdatePresentationTime() const {
   return latest_update_time_;
 }
 

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

@@ -50,10 +50,11 @@ class SMILTimeContainer final : public GarbageCollected<SMILTimeContainer> {
   void Reschedule(SVGSMILElement*, SMILTime interval_time);
   void Unschedule(SVGSMILElement*);
 
-  // Returns the time we are currently updating.
+  // Returns the current animation time.
   SMILTime Elapsed() const;
-  // Returns the current time in the document.
-  SMILTime CurrentDocumentTime() const;
+  // Returns the time that we last updated timed elements to. This differs from
+  // the above in that it only moves during animation update steps.
+  SMILTime LatestUpdatePresentationTime() const;
 
   bool IsPaused() const;
   bool IsStarted() const;

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

@@ -827,15 +827,17 @@ SMILTime SVGSMILElement::ComputeNextIntervalTime(
 
 void SVGSMILElement::InstanceListChanged() {
   DCHECK(instance_lists_have_changed_);
-  // Update the interval to the time just before the current presentation
-  // time. This means that the next animation update will take of updating the
-  // active state and send events as needed.
-  SMILTime previous_presentation_time =
-      time_container_ ? time_container_->CurrentDocumentTime() : SMILTime();
-  previous_presentation_time = previous_presentation_time - SMILTime::Epsilon();
-  DCHECK(!previous_presentation_time.IsUnresolved());
+  SMILTime current_presentation_time =
+      time_container_ ? time_container_->LatestUpdatePresentationTime()
+                      : SMILTime();
+  DCHECK(!current_presentation_time.IsUnresolved());
   const bool was_active = GetActiveState() == kActive;
-  UpdateInterval(previous_presentation_time);
+  UpdateInterval(current_presentation_time);
+  // Check active state and reschedule using the time just before the current
+  // presentation time. This means that the next animation update will take
+  // care of updating the active state and send events as needed.
+  SMILTime previous_presentation_time =
+      current_presentation_time - SMILTime::Epsilon();
   if (was_active && interval_.BeginsAfter(previous_presentation_time)) {
     active_state_ = DetermineActiveState(previous_presentation_time);
     if (GetActiveState() != kActive)
@@ -920,12 +922,14 @@ void SVGSMILElement::UpdateInterval(SMILTime presentation_time) {
 
 void SVGSMILElement::AddedToTimeContainer() {
   DCHECK(time_container_);
-  // Update the interval to the time just before the current presentation
-  // time. This means that the next animation update will take of updating the
-  // active state and send events as needed.
+  SMILTime current_presentation_time =
+      time_container_->LatestUpdatePresentationTime();
+  UpdateInterval(current_presentation_time);
+  // Check active state and reschedule using the time just before the current
+  // presentation time. This means that the next animation update will take
+  // care of updating the active state and send events as needed.
   SMILTime previous_presentation_time =
-      time_container_->CurrentDocumentTime() - SMILTime::Epsilon();
-  UpdateInterval(previous_presentation_time);
+      current_presentation_time - SMILTime::Epsilon();
   active_state_ = DetermineActiveState(previous_presentation_time);
   time_container_->Reschedule(
       this, ComputeNextIntervalTime(previous_presentation_time));

third_party/blink/web_tests/external/wpt/svg/animations/begin-attribute-mutation.html

+<!DOCTYPE html>
+<title>Mutating the 'begin' attribute after the element has started</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<svg>
+  <rect width="100" height="100" fill="blue">
+    <animate id="anim" attributeName="x" values="100; 0"
+             begin="0s" dur="50ms" fill="freeze"/>
+  </rect>
+</svg>
+<script>
+  async_test(function(t) {
+    let anim = document.getElementById("anim");
+    anim.endEventsReceived = 0;
+    anim.addEventListener('endEvent', t.step_func(function() {
+      anim.endEventsReceived++;
+      if (anim.endEventsReceived)
+        t.done();
+    }));
+    onload = function() {
+      // Allow some time to pass before mutating 'begin'. This should ensure
+      // that the element has started.
+      requestAnimationFrame(function() {
+        anim.ownerSVGElement.setCurrentTime(0);
+        anim.setAttribute("begin", "50ms");
+      });
+    };
+  });
+</script>

third_party/blink/web_tests/external/wpt/svg/animations/cyclic-syncbase-2.html

+<!DOCTYPE html>
+<title>Cyclic syncbase dependency with syncbase trigger</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<svg>
+  <rect width="100" height="100" fill="blue">
+    <animate attributeName="fill" from="yellow" to="blue" id="a"
+             begin="c.end; b.begin" dur="10ms"/>
+  </rect>
+  <rect width="100" height="100" x="100" fill="blue">
+    <animate attributeName="fill" from="yellow" to="blue" id="b"
+             begin="c.end; a.begin" dur="10ms"/>
+  </rect>
+  <rect width="100" height="100" x="200" fill="blue">
+    <animate attributeName="fill" from="yellow" to="blue" id="c"
+             begin="0; 15ms" dur="10ms"/>
+  </rect>
+</svg>
+<script>
+  async_test(function(t) {
+    let a = document.getElementById('a');
+    a.ended = 0;
+    a.addEventListener('endEvent', t.step_func(function() {
+      a.ended++;
+      if (a.ended === 2)
+        t.done();
+    }));
+  });
+</script>