Bug 1181392 part 9 - Remove use of IsFinishedTransition from nsTransitionManager::FlushTransitions; r=dbaron

This patch updates the logic in nsTransitionManager::FlushTransitions that deals
with detecting newly-started (i.e. when the delay phase has just finished) or
newly-finished animations. The existing logic had the following behavior:

* Calling SetIsFinishedTransition for newly-finished transitions.
  This is no longer needed since by this point all consumers of
  IsFinishedTransition have been updated to use alternative means for testing
  if an animation is relevant.

* Setting transitionStartedOrEnded to true so that we would post an animation
  restyle.
  We can achieve this same effect by simplying updating the canThrottleTick
  flag using the result of calling Animation::CanThrottle on each animation.
  Animation::CanThrottle will return false for a newly-started or
  newly-finished animation (it returns false for a newly-started animation
  since the animation's mIsRunningOnCompositor flag won't yet be true at that
  point.)

* Updating the animation generation for newly-started and newly-finished
  animations in order to trigger a layer update.
  At least that appears to be the intention of this code. However, it currently
  has no effect since the animation generation on the pres context is not
  incremented in any context I could find where a newly started/finished
  animation might be detected.

For this third case, this patch adds tests to ensure that transitions are
correctly added and removed from the compositor when they start and finish.
This is because most of the existing tests in test_animations_omta.html cover
only CSS animations.

As noted in the comments in the test, if a tick lands at the exact beginning of
a transition we will typically not send it to the layer until the following tick
because the RestyleManager will filter out the seemingly redundant change (i.e.
no change to the computed style). Presumably updating the animation generation
was intended to avoid this.

This same behavior is true of CSS animations (and the same kind of comment
appears elsewhere in test_animations_omta.html). Long-term this is probably
worth fixing so that when a transition is triggered we get it to the compositor
one tick earlier which should improve responsiveness when the main thread is
busy and the interval between ticks is long. Furthermore, we should do the same
for both all type of animations, not just transitions.

Currently, however, this patch preserves the existing behavior and helps narrow
the difference between transitions and animations so we can apply this
optimization to both.

layout/style/nsTransitionManager.cpp

@@ -937,38 +937,13 @@ nsTransitionManager::FlushTransitions(FlushFlags aFlags)
                  "Element::UnbindFromTree should have "
                  "destroyed the element transitions object");
 
-      size_t i = collection->mAnimations.Length();
-      MOZ_ASSERT(i != 0, "empty transitions list?");
-      bool transitionStartedOrEnded = false;
-      do {
-        --i;
-        Animation* anim = collection->mAnimations[i];
-        if (!anim->GetEffect()->IsFinishedTransition()) {
-          MOZ_ASSERT(anim->GetEffect(), "Transitions should have an effect");
-          ComputedTiming computedTiming =
-            anim->GetEffect()->GetComputedTiming();
-          if (computedTiming.mPhase == ComputedTiming::AnimationPhase_After) {
-            // Leave this transition in the list for one more refresh
-            // cycle, since we haven't yet processed its style change, and
-            // if we also have (already, or will have from processing
-            // transitionend events or other refresh driver notifications)
-            // a non-animation style change that would affect it, we need
-            // to know not to start a new transition for the transition
-            // from the almost-completed value to the final value.
-            anim->GetEffect()->SetIsFinishedTransition(true);
-            collection->UpdateAnimationGeneration(mPresContext);
-            transitionStartedOrEnded = true;
-          } else if ((computedTiming.mPhase ==
-                      ComputedTiming::AnimationPhase_Active) &&
-                     canThrottleTick &&
-                     !anim->IsRunningOnCompositor()) {
-            // Start a transition with a delay where we should start the
-            // transition proper.
-            collection->UpdateAnimationGeneration(mPresContext);
-            transitionStartedOrEnded = true;
-          }
-        }
-      } while (i != 0);
+      // Look for any transitions that can't be throttled because they
+      // may be starting or stopping
+      for (auto iter = collection->mAnimations.cbegin();
+           canThrottleTick && iter != collection->mAnimations.cend();
+           ++iter) {
+        canThrottleTick &= (*iter)->CanThrottle();
+      }
 
       // We need to restyle even if the transition rule no longer
       // applies (in which case we just made it not apply).
@@ -979,7 +954,7 @@ nsTransitionManager::FlushTransitions(FlushFlags aFlags)
                  collection->mElementProperty ==
                    nsGkAtoms::transitionsOfAfterProperty,
                  "Unexpected element property; might restyle too much");
-      if (!canThrottleTick || transitionStartedOrEnded) {
+      if (!canThrottleTick) {
         collection->PostRestyleForAnimation(mPresContext);
       } else {
         didThrottle = true;

layout/style/test/test_animations_omta.html

@@ -2345,5 +2345,38 @@ addAsyncAnimTest(function *() {
   done_div();
 });
 
+addAsyncAnimTest(function *() {
+  new_div("transform: translate(100px); transition: transform 10s 5s linear");
+  yield waitForPaintsFlushed();
+  gDiv.style.transform = "translate(200px)";
+  yield waitForPaintsFlushed();
+  omta_is("transform", { tx: 100 }, RunningOn.MainThread,
+          "transition runs on main thread during delay");
+  // At the *very* start of the transition the start value of the transition
+  // will match the underlying transform value. Various optimizations in
+  // RestyleManager may recognize this a "no change" and filter out the
+  // transition meaning that the animation doesn't get added to the compositor
+  // thread until the first time the value changes. As a result, we fast-forward
+  // a little past the beginning and then wait for the animation to be sent
+  // to the compositor.
+  advance_clock(5100);
+  yield waitForPaints();
+  omta_is("transform", { tx: 101 }, RunningOn.Compositor,
+          "transition runs on compositor at start of active interval");
+  advance_clock(4900);
+  omta_is("transform", { tx: 150 }, RunningOn.Compositor,
+          "transition runs on compositor at during active interval");
+  advance_clock(5000);
+  // Currently the compositor will apply a forwards fill until it gets told by
+  // the main thread to clear the animation. As a result we should wait for
+  // paints before checking that the animated value does *not* appear on the
+  // compositor thread.
+  yield waitForPaints();
+  omta_is("transform", { tx: 200 }, RunningOn.MainThread,
+          "transition runs on main thread at end of active interval");
+
+  done_div();
+});
+
 </script>
 </html>