Bug 1292001 - Move transition start value replacing behavior to the CSSTransition; r=boris

This is needed so that later we can make the effect of transitions replaceable.

Note that the test added in this patch will fail without the code changes in
this patch.

Differential Revision: https://phabricator.services.mozilla.com/D64519

--HG--
rename : layout/style/test/test_transitions_replacement_on_busy_frame.html => layout/style/test/test_transitions_replacement_with_setKeyframes.html
extra : moz-landing-system : lando

dom/animation/KeyframeEffect.cpp

@@ -253,6 +253,36 @@ void KeyframeEffect::SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
   }
 }
 
+void KeyframeEffect::ReplaceTransitionStartValue(AnimationValue&& aStartValue) {
+  if (!aStartValue.mServo) {
+    return;
+  }
+
+  // A typical transition should have a single property and a single segment.
+  //
+  // (And for atypical transitions, that is, those updated by script, we don't
+  // apply the replacing behavior.)
+  if (mProperties.Length() != 1 || mProperties[0].mSegments.Length() != 1) {
+    return;
+  }
+
+  // Likewise, check that the keyframes are of the expected shape.
+  if (mKeyframes.Length() != 2 || mKeyframes[0].mPropertyValues.Length() != 1) {
+    return;
+  }
+
+  // Check that the value we are about to substitute in is actually for the
+  // same property.
+  if (Servo_AnimationValue_GetPropertyId(aStartValue.mServo) !=
+      mProperties[0].mProperty) {
+    return;
+  }
+
+  mKeyframes[0].mPropertyValues[0].mServoDeclarationBlock =
+      Servo_AnimationValue_Uncompute(aStartValue.mServo).Consume();
+  mProperties[0].mSegments[0].mFromValue = std::move(aStartValue);
+}
+
 static bool IsEffectiveProperty(const EffectSet& aEffects,
                                 nsCSSPropertyID aProperty) {
   return !aEffects.PropertiesWithImportantRules().HasProperty(aProperty) ||

dom/animation/KeyframeEffect.h

@@ -187,6 +187,10 @@ class KeyframeEffect : public AnimationEffect {
   void SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
                     const ComputedStyle* aStyle);
 
+  // Replace the start value of the transition. This is used for updating
+  // transitions running on the compositor.
+  void ReplaceTransitionStartValue(AnimationValue&& aStartValue);
+
   // Returns the set of properties affected by this effect regardless of
   // whether any of these properties is overridden by an !important rule.
   nsCSSPropertyIDSet GetPropertySet() const;

layout/painting/nsDisplayList.cpp

@@ -435,13 +435,9 @@ static void AddAnimationForProperty(nsIFrame* aFrame,
   // since after generating the new transition other requestAnimationFrame
   // callbacks may run that introduce further lag between the main thread and
   // the compositor.
-  if (aAnimation->AsCSSTransition() && aAnimation->GetEffect() &&
-      aAnimation->GetEffect()->AsTransition()) {
-    // We update startValue from the replaced transition only if the effect is
-    // an ElementPropertyTransition.
-    aAnimation->GetEffect()
-        ->AsTransition()
-        ->UpdateStartValueFromReplacedTransition();
+  CSSTransition* cssTransition = aAnimation->AsCSSTransition();
+  if (cssTransition) {
+    cssTransition->UpdateStartValueFromReplacedTransition();
   }
 
   animation->originTime() =

layout/style/nsTransitionManager.cpp

@@ -69,54 +69,6 @@ double ElementPropertyTransition::CurrentValuePortion() const {
   return computedTiming.mProgress.Value();
 }
 
-void ElementPropertyTransition::UpdateStartValueFromReplacedTransition() {
-  if (!mReplacedTransition) {
-    return;
-  }
-  MOZ_ASSERT(nsCSSProps::PropHasFlags(TransitionProperty(),
-                                      CSSPropFlags::CanAnimateOnCompositor),
-             "The transition property should be able to be run on the "
-             "compositor");
-  MOZ_ASSERT(mTarget, "We should have a valid target at this moment");
-
-  dom::DocumentTimeline* timeline = mTarget.mElement->OwnerDoc()->Timeline();
-  ComputedTiming computedTiming = GetComputedTimingAt(
-      dom::CSSTransition::GetCurrentTimeAt(*timeline, TimeStamp::Now(),
-                                           mReplacedTransition->mStartTime,
-                                           mReplacedTransition->mPlaybackRate),
-      mReplacedTransition->mTiming, mReplacedTransition->mPlaybackRate);
-
-  if (!computedTiming.mProgress.IsNull()) {
-    double valuePosition = ComputedTimingFunction::GetPortion(
-        mReplacedTransition->mTimingFunction, computedTiming.mProgress.Value(),
-        computedTiming.mBeforeFlag);
-
-    MOZ_ASSERT(
-        mProperties.Length() == 1 && mProperties[0].mSegments.Length() == 1,
-        "The transition should have one property and one segment");
-    MOZ_ASSERT(mKeyframes.Length() == 2,
-               "Transitions should have exactly two animation keyframes");
-    MOZ_ASSERT(mKeyframes[0].mPropertyValues.Length() == 1,
-               "Transitions should have exactly one property in their first "
-               "frame");
-
-    const AnimationValue& replacedFrom = mReplacedTransition->mFromValue;
-    const AnimationValue& replacedTo = mReplacedTransition->mToValue;
-    AnimationValue startValue;
-    startValue.mServo =
-        Servo_AnimationValues_Interpolate(replacedFrom.mServo,
-                                          replacedTo.mServo, valuePosition)
-            .Consume();
-    if (startValue.mServo) {
-      mKeyframes[0].mPropertyValues[0].mServoDeclarationBlock =
-          Servo_AnimationValue_Uncompute(startValue.mServo).Consume();
-      mProperties[0].mSegments[0].mFromValue = std::move(startValue);
-    }
-  }
-
-  mReplacedTransition.reset();
-}
-
 ////////////////////////// CSSTransition ////////////////////////////
 
 JSObject* CSSTransition::WrapObject(JSContext* aCx,
@@ -341,7 +293,7 @@ bool CSSTransition::HasLowerCompositeOrderThan(
 
 /* static */
 Nullable<TimeDuration> CSSTransition::GetCurrentTimeAt(
-    const dom::DocumentTimeline& aTimeline, const TimeStamp& aBaseTime,
+    const AnimationTimeline& aTimeline, const TimeStamp& aBaseTime,
     const TimeDuration& aStartTime, double aPlaybackRate) {
   Nullable<TimeDuration> result;
 
@@ -354,6 +306,53 @@ Nullable<TimeDuration> CSSTransition::GetCurrentTimeAt(
   return result;
 }
 
+void CSSTransition::UpdateStartValueFromReplacedTransition() {
+  MOZ_ASSERT(nsCSSProps::PropHasFlags(mTransitionProperty,
+                                      CSSPropFlags::CanAnimateOnCompositor),
+             "The transition property should be able to be run on the "
+             "compositor");
+  MOZ_ASSERT(mTimeline,
+             "Should have a timeline if we are replacing transition start "
+             "values");
+
+  if (!mReplacedTransition) {
+    return;
+  }
+
+  if (!mEffect) {
+    return;
+  }
+
+  KeyframeEffect* keyframeEffect = mEffect->AsKeyframeEffect();
+  if (!keyframeEffect) {
+    return;
+  }
+
+  ComputedTiming computedTiming = AnimationEffect::GetComputedTimingAt(
+      CSSTransition::GetCurrentTimeAt(*mTimeline, TimeStamp::Now(),
+                                      mReplacedTransition->mStartTime,
+                                      mReplacedTransition->mPlaybackRate),
+      mReplacedTransition->mTiming, mReplacedTransition->mPlaybackRate);
+
+  if (!computedTiming.mProgress.IsNull()) {
+    double valuePosition = ComputedTimingFunction::GetPortion(
+        mReplacedTransition->mTimingFunction, computedTiming.mProgress.Value(),
+        computedTiming.mBeforeFlag);
+
+    const AnimationValue& replacedFrom = mReplacedTransition->mFromValue;
+    const AnimationValue& replacedTo = mReplacedTransition->mToValue;
+    AnimationValue startValue;
+    startValue.mServo =
+        Servo_AnimationValues_Interpolate(replacedFrom.mServo,
+                                          replacedTo.mServo, valuePosition)
+            .Consume();
+
+    keyframeEffect->ReplaceTransitionStartValue(std::move(startValue));
+  }
+
+  mReplacedTransition.reset();
+}
+
 void CSSTransition::SetEffectFromStyle(dom::AnimationEffect* aEffect) {
   Animation::SetEffectNoUpdate(aEffect);
 
@@ -562,6 +561,52 @@ static bool IsTransitionable(nsCSSPropertyID aProperty) {
   return Servo_Property_IsTransitionable(aProperty);
 }
 
+static Maybe<CSSTransition::ReplacedTransitionProperties>
+GetReplacedTransitionProperties(const CSSTransition* aTransition,
+                                const DocumentTimeline* aTimelineToMatch) {
+  Maybe<CSSTransition::ReplacedTransitionProperties> result;
+
+  // Transition needs to be currently running on the compositor to be
+  // replaceable.
+  if (!aTransition || !aTransition->HasCurrentEffect() ||
+      !aTransition->IsRunningOnCompositor() ||
+      aTransition->GetStartTime().IsNull()) {
+    return result;
+  }
+
+  // Transition needs to be running on the same timeline.
+  if (aTransition->GetTimeline() != aTimelineToMatch) {
+    return result;
+  }
+
+  // The transition needs to have a keyframe effect.
+  const KeyframeEffect* keyframeEffect =
+      aTransition->GetEffect() ? aTransition->GetEffect()->AsKeyframeEffect()
+                               : nullptr;
+  if (!keyframeEffect) {
+    return result;
+  }
+
+  // The keyframe effect needs to be a simple transition of the original
+  // transition property (i.e. not replaced with something else).
+  if (keyframeEffect->Properties().Length() != 1 ||
+      keyframeEffect->Properties()[0].mSegments.Length() != 1 ||
+      keyframeEffect->Properties()[0].mProperty !=
+          aTransition->TransitionProperty()) {
+    return result;
+  }
+
+  const AnimationPropertySegment& segment =
+      keyframeEffect->Properties()[0].mSegments[0];
+
+  result.emplace(CSSTransition::ReplacedTransitionProperties(
+      {aTransition->GetStartTime().Value(), aTransition->PlaybackRate(),
+       keyframeEffect->SpecifiedTiming(), segment.mTimingFunction,
+       segment.mFromValue, segment.mToValue}));
+
+  return result;
+}
+
 bool nsTransitionManager::ConsiderInitiatingTransition(
     nsCSSPropertyID aProperty, const nsStyleDisplay& aStyleDisplay,
     uint32_t transitionIdx, dom::Element* aElement, PseudoStyleType aPseudoType,
@@ -616,6 +661,7 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
 
   bool haveCurrentTransition = false;
   size_t currentIndex = nsTArray<ElementPropertyTransition>::NoIndex;
+  const CSSTransition* oldTransition = nullptr;
   const ElementPropertyTransition* oldPT = nullptr;
   if (aElementTransitions) {
     OwningCSSTransitionPtrArray& animations = aElementTransitions->mAnimations;
@@ -623,8 +669,9 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
       if (animations[i]->TransitionProperty() == aProperty) {
         haveCurrentTransition = true;
         currentIndex = i;
-        oldPT = animations[i]->GetEffect()
-                    ? animations[i]->GetEffect()->AsTransition()
+        oldTransition = animations[i];
+        oldPT = oldTransition->GetEffect()
+                    ? oldTransition->GetEffect()->AsTransition()
                     : nullptr;
         break;
       }
@@ -663,6 +710,7 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
           aElementTransitions->mAnimations;
       animations[currentIndex]->CancelFromStyle(PostRestyleMode::IfNeeded);
       oldPT = nullptr;  // Clear pointer so it doesn't dangle
+      oldTransition = nullptr;
       animations.RemoveElementAt(currentIndex);
       EffectSet* effectSet = EffectSet::GetEffectSet(aElement, aPseudoType);
       if (effectSet) {
@@ -783,19 +831,16 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
     // start value of the transition using TimeStamp::Now(). This allows us to
     // avoid a large jump when starting a new transition when the main thread
     // lags behind the compositor.
-    if (oldPT && oldPT->IsCurrent() && oldPT->IsRunningOnCompositor() &&
-        !oldPT->GetAnimation()->GetStartTime().IsNull() &&
-        timeline == oldPT->GetAnimation()->GetTimeline()) {
-      const AnimationPropertySegment& segment =
-          oldPT->Properties()[0].mSegments[0];
-      pt->mReplacedTransition.emplace(
-          ElementPropertyTransition::ReplacedTransitionProperties(
-              {oldPT->GetAnimation()->GetStartTime().Value(),
-               oldPT->GetAnimation()->PlaybackRate(), oldPT->SpecifiedTiming(),
-               segment.mTimingFunction, segment.mFromValue, segment.mToValue}));
+    auto replacedTransitionProperties =
+        GetReplacedTransitionProperties(oldTransition, timeline);
+    if (replacedTransitionProperties) {
+      animation->SetReplacedTransition(
+          std::move(replacedTransitionProperties.ref()));
     }
+
     animations[currentIndex]->CancelFromStyle(PostRestyleMode::IfNeeded);
     oldPT = nullptr;  // Clear pointer so it doesn't dangle
+    oldTransition = nullptr;
     animations[currentIndex] = animation;
   } else {
     if (!animations.AppendElement(animation)) {

layout/style/nsTransitionManager.h

@@ -93,20 +93,6 @@ struct ElementPropertyTransition : public dom::KeyframeEffect {
   // at the current time.  (The input to the transition timing function
   // has time units, the output has value units.)
   double CurrentValuePortion() const;
-
-  // For a new transition interrupting an existing transition on the
-  // compositor, update the start value to match the value of the replaced
-  // transitions at the current time.
-  void UpdateStartValueFromReplacedTransition();
-
-  struct ReplacedTransitionProperties {
-    TimeDuration mStartTime;
-    double mPlaybackRate;
-    TimingParams mTiming;
-    Maybe<ComputedTimingFunction> mTimingFunction;
-    AnimationValue mFromValue, mToValue;
-  };
-  Maybe<ReplacedTransitionProperties> mReplacedTransition;
 };
 
 namespace dom {
@@ -197,13 +183,30 @@ class CSSTransition final : public Animation {
   // because the animation on the compositor may be running ahead while
   // main-thread is busy.
   static Nullable<TimeDuration> GetCurrentTimeAt(
-      const DocumentTimeline& aTimeline, const TimeStamp& aBaseTime,
+      const AnimationTimeline& aTimeline, const TimeStamp& aBaseTime,
       const TimeDuration& aStartTime, double aPlaybackRate);
 
   void MaybeQueueCancelEvent(const StickyTimeDuration& aActiveTime) override {
     QueueEvents(aActiveTime);
   }
 
+  struct ReplacedTransitionProperties {
+    TimeDuration mStartTime;
+    double mPlaybackRate;
+    TimingParams mTiming;
+    Maybe<ComputedTimingFunction> mTimingFunction;
+    AnimationValue mFromValue, mToValue;
+  };
+  void SetReplacedTransition(
+      ReplacedTransitionProperties&& aReplacedTransition) {
+    mReplacedTransition.emplace(std::move(aReplacedTransition));
+  }
+
+  // For a new transition interrupting an existing transition on the
+  // compositor, update the start value to match the value of the replaced
+  // transitions at the current time.
+  void UpdateStartValueFromReplacedTransition();
+
  protected:
   virtual ~CSSTransition() {
     MOZ_ASSERT(!mOwningElement.IsSet(),
@@ -261,6 +264,8 @@ class CSSTransition final : public Animation {
   // using the Web Animations API.
   nsCSSPropertyID mTransitionProperty;
   AnimationValue mTransitionToValue;
+
+  Maybe<ReplacedTransitionProperties> mReplacedTransition;
 };
 
 }  // namespace dom

layout/style/test/mochitest.ini

@@ -346,6 +346,7 @@ support-files = file_specified_value_serialization_individual_transforms.html
 [test_transitions_dynamic_changes.html]
 [test_transitions_per_property.html]
 [test_transitions_replacement_on_busy_frame.html]
+[test_transitions_replacement_with_setKeyframes.html]
 [test_transitions_step_functions.html]
 [test_unclosed_parentheses.html]
 [test_unicode_range_loading.html]

layout/style/test/test_transitions_replacement_with_setKeyframes.html

@@ -0,0 +1,88 @@
+<!doctype html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1292001
+-->
+<head>
+  <meta charset=utf-8>
+  <title>Test for bug 1292001</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="/tests/SimpleTest/paint_listener.js"></script>
+  <script src="animation_utils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css">
+  <style>
+    #target {
+      height: 100px;
+      width: 100px;
+      background: green;
+      transition: transform 100s linear;
+    }
+  </style>
+</head>
+<body>
+<div id="target"></div>
+<script>
+'use strict';
+
+SimpleTest.waitForExplicitFinish();
+
+const OMTAPrefKey = 'layers.offmainthreadcomposition.async-animations';
+const omtaEnabled =
+  SpecialPowers.DOMWindowUtils.layerManagerRemote &&
+  SpecialPowers.getBoolPref(OMTAPrefKey);
+
+window.addEventListener('load', async function() {
+  if (!omtaEnabled) {
+    ok(true, 'Skipping the test since OMTA is disabled');
+    SimpleTest.finish();
+    return;
+  }
+
+  const div = document.getElementById('target');
+
+  // Start first transition
+  div.style.transform = 'translateX(300px)';
+  const firstTransition = div.getAnimations()[0];
+
+  // But then change its keyframes to something completely different.
+  firstTransition.effect.setKeyframes({ 'opacity': ['0', '1'] });
+
+  // Wait for the transition to start running on the main thread and
+  // compositor.
+  await firstTransition.ready;
+  await waitForPaints();
+  await new Promise(resolve => requestAnimationFrame(resolve));
+
+  // Start second transition
+  div.style.transform = 'translateX(600px)';
+  const secondTransition = div.getAnimations()[0];
+
+  const previousKeyframeValue = secondTransition.effect.getKeyframes()[0]
+    .transform;
+
+  // Tie up main thread for 300ms. In the meantime, the first transition
+  // will continue running on the compositor. If we don't update the start
+  // point of the second transition, it will appear to jump when it starts.
+  const startTime = performance.now();
+  while (performance.now() - startTime < 300);
+
+  // Ensure that our paint process has been done.
+  //
+  // (See explanation in test_transitions_replacement_on_busy_frame.html for
+  // why we don't use requestAnimationFrame here.)
+  await waitForPaints();
+
+  // Now check that the keyframes are NOT updated.
+  is(
+    secondTransition.effect.getKeyframes()[0].transform,
+    previousKeyframeValue,
+    'Keyframe value of transition is NOT updated since the moment when ' +
+      'it was generated'
+  );
+
+  SimpleTest.finish();
+});
+
+</script>
+</body>
+</html>