Flush pending SMIL synchronizations on frame callbacks

Previously a synchronization could be pending when we got a frame
callback, meaning that the animations would not be updated after the
frame callback had run, but still require the synchronization timer to
fire.

Instead flush any pending synchronizations if we get a frame callback
before the synchronization timer fires. This gives stronger guarantees
about when to expect animation values to be updated.

This could address the flakiness observed in the referenced bug.

Bug: 1099657
Change-Id: Ieb90a4cd2b5bb5a3b3d1f0a0d86383f08ce0d3fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292309Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#787819}

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

@@ -445,6 +445,12 @@ void SMILTimeContainer::ServiceOnNextFrame() {
 }
 
 void SMILTimeContainer::ServiceAnimations() {
+  // If a synchronization is pending, we can flush it now.
+  if (frame_scheduling_state_ == kSynchronizeAnimations) {
+    DCHECK(wakeup_timer_.IsActive());
+    wakeup_timer_.Stop();
+    frame_scheduling_state_ = kAnimationFrame;
+  }
   if (frame_scheduling_state_ != kAnimationFrame)
     return;
   frame_scheduling_state_ = kIdle;

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

@@ -14,6 +14,7 @@
 #include "third_party/blink/renderer/core/svg/svg_length.h"
 #include "third_party/blink/renderer/core/svg/svg_length_context.h"
 #include "third_party/blink/renderer/core/svg/svg_rect_element.h"
+#include "third_party/blink/renderer/core/svg/svg_set_element.h"
 #include "third_party/blink/renderer/core/svg/svg_svg_element.h"
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
 #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
@@ -22,6 +23,71 @@
 namespace blink {
 namespace {
 
+class SMILTimeContainerTest : public PageTestBase {
+ public:
+  void SetUp() override {
+    EnablePlatform();
+    platform()->SetAutoAdvanceNowToPendingTasks(false);
+    PageTestBase::SetUp();
+  }
+
+  void Load(base::span<const char> data) {
+    auto params = WebNavigationParams::CreateWithHTMLString(
+        data, KURL("http://example.com"));
+    GetFrame().Loader().CommitNavigation(std::move(params),
+                                         nullptr /* extra_data */);
+    GetAnimationClock().ResetTimeForTesting();
+    GetAnimationClock().SetAllowedToDynamicallyUpdateTime(false);
+    GetDocument().Timeline().ResetForTesting();
+  }
+
+  void StepTime(base::TimeDelta delta) {
+    platform()->RunForPeriod(delta);
+    current_time_ += delta;
+    GetAnimationClock().UpdateTime(current_time_);
+  }
+
+ private:
+  base::TimeTicks current_time_;
+};
+
+TEST_F(SMILTimeContainerTest, ServiceAnimationsFlushesPendingSynchronizations) {
+  Load(R"HTML(
+    <svg id="container">
+      <rect width="100" height="0" fill="green"/>
+    </svg>
+  )HTML");
+  platform()->RunUntilIdle();
+
+  auto* svg_root = To<SVGSVGElement>(GetElementById("container"));
+  ASSERT_TRUE(svg_root);
+  auto* rect = Traversal<SVGRectElement>::FirstChild(*svg_root);
+  ASSERT_TRUE(rect);
+  SVGLengthContext length_context(rect);
+
+  SMILTimeContainer* time_container = svg_root->TimeContainer();
+  EXPECT_TRUE(time_container->IsStarted());
+  EXPECT_FALSE(time_container->IsPaused());
+  EXPECT_EQ(0, rect->height()->CurrentValue()->Value(length_context));
+
+  // Insert an animation: <set attributeName="height" to="100"/> of the <rect>.
+  auto* animation = MakeGarbageCollected<SVGSetElement>(GetDocument());
+  animation->setAttribute(svg_names::kAttributeTypeAttr, "XML");
+  animation->setAttribute(svg_names::kAttributeNameAttr, "height");
+  animation->setAttribute(svg_names::kToAttr, "100");
+  rect->appendChild(animation);
+
+  // Frame callback before the synchronization timer fires.
+  SVGDocumentExtensions::ServiceOnAnimationFrame(GetDocument());
+
+  // The frame callback should have flushed any pending updates.
+  EXPECT_EQ(100, rect->height()->CurrentValue()->Value(length_context));
+
+  StepTime(base::TimeDelta::FromMilliseconds(500));
+  EXPECT_EQ(100, rect->height()->CurrentValue()->Value(length_context));
+  EXPECT_EQ(SMILTime::FromSecondsD(0.5), time_container->Elapsed());
+}
+
 class ContentLoadedEventListener final : public NativeEventListener {
  public:
   using CallbackType = base::OnceCallback<void(Document&)>;

third_party/blink/web_tests/TestExpectations

@@ -6863,7 +6863,7 @@ crbug.com/1099689 [ Linux ] external/wpt/html/browsers/windows/nested-browsing-c
 crbug.com/1010170 media/video-played-reset.html [ Pass Failure ]
 
 #Sheriff 2020-06-30
-crbug.com/1100774 [ Mac10.13 ] virtual/threaded-no-composited-antialiasing/animations/svg-attribute-interpolation/svg-d-interpolation.html [ Pass Failure ]
+crbug.com/1099657 [ Mac10.13 ] virtual/threaded-no-composited-antialiasing/animations/svg-attribute-interpolation/svg-d-interpolation.html [ Pass Failure ]
 crbug.com/1100786 [ Mac10.10 ] fast/scroll-snap/snaps-after-touchpad-scrolling.html [ Failure ]
 crbug.com/1100786 [ Mac10.10 ] storage/indexeddb/transaction-active-flag.html [ Timeout ]