Bug 1208938 part 3 - Update pending finishing handling; r=heycam
authorBrian Birtles <birtles@gmail.com>
Wed, 07 Oct 2015 14:30:28 +0900
changeset 266441 649b90ba93630a3b541adb93397617314a9a6679
parent 266440 05f79b5c5ef9565c5ab44d222c5f3e45ae286cfb
child 266442 8ba8e24a84d3766b0c5442b1f0a08b94ea63d15e
push id66189
push userbbirtles@mozilla.com
push dateWed, 07 Oct 2015 05:32:19 +0000
treeherdermozilla-inbound@649b90ba9363 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1208938, 1195180
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1208938 part 3 - Update pending finishing handling; r=heycam Animation::Tick contains special handling to cope with pending ready times that are in the future. This was originally introduced to cope with the situation where we are called multiple times per refresh-driver tick. As of bug 1195180, Animation::Tick should no longer be called multiple times per refresh driver tick. It would seem, therefore, that we no longer need to check for a future time. However, since introducing this check, the vsync refresh driver timer has been added which means that we can still have a recorded time from TimeStamp::Now that is ahead of the vsync time used to update the refresh driver. In that case, however, rather than waiting for the next tick, we should simply clamp that pending ready time to the refresh driver time and finish pending immediately. This patch also updates one of the tests for reversing. With this updated behavior we can sometimes arrive at a situation where when an Animation starts and its ready promise resolves, its currentTime is still 0. If we call reverse() at this point on an animation with an infinite active duration it should throw an InvalidStateError. To avoid this situation, this test makes sure we wait an extra frame before calling reverse().
dom/animation/Animation.cpp
dom/animation/test/css-animations/file_animation-reverse.html
dom/animation/test/testcommon.js
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -355,25 +355,28 @@ Animation::SetCurrentTimeAsDouble(const 
   return SetCurrentTime(TimeDuration::FromMilliseconds(aCurrentTime.Value()));
 }
 
 // ---------------------------------------------------------------------------
 
 void
 Animation::Tick()
 {
-  // Since we are not guaranteed to get only one call per refresh driver tick,
-  // it's possible that mPendingReadyTime is set to a time in the future.
-  // In that case, we should wait until the next refresh driver tick before
-  // resuming.
+  // Finish pending if we have a pending ready time, but only if we also
+  // have an active timeline.
   if (mPendingState != PendingState::NotPending &&
       !mPendingReadyTime.IsNull() &&
       mTimeline &&
-      !mTimeline->GetCurrentTime().IsNull() &&
-      mPendingReadyTime.Value() <= mTimeline->GetCurrentTime().Value()) {
+      !mTimeline->GetCurrentTime().IsNull()) {
+    // Even though mPendingReadyTime is initialized using TimeStamp::Now()
+    // during the *previous* tick of the refresh driver, it can still be
+    // ahead of the *current* timeline time when we are using the
+    // vsync timer so we need to clamp it to the timeline time.
+    mPendingReadyTime.SetValue(std::min(mTimeline->GetCurrentTime().Value(),
+                                        mPendingReadyTime.Value()));
     FinishPendingAt(mPendingReadyTime.Value());
     mPendingReadyTime.SetNull();
   }
 
   if (IsPossiblyOrphanedPendingAnimation()) {
     MOZ_ASSERT(mTimeline && !mTimeline->GetCurrentTime().IsNull(),
                "Orphaned pending animtaions should have an active timeline");
     FinishPendingAt(mTimeline->GetCurrentTime().Value());
--- a/dom/animation/test/css-animations/file_animation-reverse.html
+++ b/dom/animation/test/css-animations/file_animation-reverse.html
@@ -9,17 +9,21 @@
 <body>
 <script>
 'use strict';
 
 async_test(function(t) {
   var div = addDiv(t, { style: 'animation: anim 100s infinite' });
   var animation = div.getAnimations()[0];
 
-  animation.ready.then(t.step_func_done(function() {
+  // Wait a frame because if currentTime is still 0 when we call
+  // reverse(), it will throw (per spec).
+  animation.ready.then(waitForFrame).then(t.step_func_done(function() {
+    assert_greater_than(animation.currentTime, 0,
+      'currentTime expected to be greater than 0, one frame after starting');
     var previousPlaybackRate = animation.playbackRate;
     animation.reverse();
     assert_equals(animation.playbackRate, -previousPlaybackRate,
       'playbackRate should be invetrted');
   }));
 }, 'reverse() inverts playbackRate');
 
 async_test(function(t) {
--- a/dom/animation/test/testcommon.js
+++ b/dom/animation/test/testcommon.js
@@ -87,17 +87,18 @@ function waitForAllAnimations(animations
 function flushComputedStyle(elem) {
   var cs = window.getComputedStyle(elem);
   cs.marginLeft;
 }
 
 if (opener) {
   for (var funcName of ["async_test", "assert_not_equals", "assert_equals",
                         "assert_approx_equals", "assert_less_than",
-                        "assert_less_than_equal", "assert_between_inclusive",
+                        "assert_less_than_equal", "assert_greater_than",
+                        "assert_between_inclusive",
                         "assert_true", "assert_false",
                         "assert_class_string", "assert_throws",
                         "assert_unreached", "test"]) {
     window[funcName] = opener[funcName].bind(opener);
   }
 
   window.EventWatcher = opener.EventWatcher;