Bug 1049975 - Part 3: Handle removed/replaced effect for CSS Transition. draft
authorBoris Chiou <boris.chiou@gmail.com>
Thu, 28 Jul 2016 11:20:13 +0800
changeset 405231 c892d73b7fc6454602a515e3422122c983a80f1b
parent 405230 0a6fb0c0d6eac014233cbce43f26dc61c0aa7aff
child 405232 e78fe8ab037b42a14658ac1851a247316d8ffd21
push id27442
push userbmo:boris.chiou@gmail.com
push dateThu, 25 Aug 2016 04:26:27 +0000
bugs1049975
milestone51.0a1
Bug 1049975 - Part 3: Handle removed/replaced effect for CSS Transition. Add mTransitionProperty and mTransitionToValue into CSSTransition, so we can retrieve the original property and ToValue after setting a different effect. MozReview-Commit-ID: 6sBGHkPAhGX
dom/animation/Animation.h
layout/base/nsDisplayList.cpp
layout/style/nsTransitionManager.cpp
layout/style/nsTransitionManager.h
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -97,17 +97,17 @@ public:
               ErrorResult& aRv);
   void GetId(nsAString& aResult) const { aResult = mId; }
   void SetId(const nsAString& aId);
   KeyframeEffectReadOnly* GetEffect() const { return mEffect; }
   void SetEffect(AnimationEffectReadOnly* aEffect)
   {
     // TODO: Merged with SetEffect(KeyframeEffectReadOnly*) in the next patch.
   }
-  void SetEffect(KeyframeEffectReadOnly* aEffect);
+  virtual void SetEffect(KeyframeEffectReadOnly* aEffect);
   AnimationTimeline* GetTimeline() const { return mTimeline; }
   void SetTimeline(AnimationTimeline* aTimeline);
   Nullable<TimeDuration> GetStartTime() const { return mStartTime; }
   void SetStartTime(const Nullable<TimeDuration>& aNewStartTime);
   Nullable<TimeDuration> GetCurrentTime() const;
   void SetCurrentTime(const TimeDuration& aNewCurrentTime);
   double PlaybackRate() const { return mPlaybackRate; }
   void SetPlaybackRate(double aPlaybackRate);
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -400,20 +400,20 @@ AddAnimationForProperty(nsIFrame* aFrame
   // re-calculate the starting point of the new transition by applying the
   // current TimeStamp to the parameters of the replaced transition.
   //
   // We need to do this here, rather than when we generate the new transition,
   // 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()) {
-    MOZ_ASSERT(aAnimation->GetEffect()->AsTransition(),
-               "CSSTransition' effect should be an ElementPropertyTransition "
-               "until we fix bug 1049975");
+      aAnimation->GetEffect() &&
+      aAnimation->GetEffect()->AsTransition()) {
+    // We update startValue from the replaced transition only if the effect is
+    // an ElementPropertyTransition.
     aAnimation->GetEffect()->AsTransition()->
       UpdateStartValueFromReplacedTransition();
   }
 
   const ComputedTiming computedTiming =
     aAnimation->GetEffect()->GetComputedTiming();
   Nullable<TimeDuration> startTime = aAnimation->GetCurrentOrPendingStartTime();
   animation->startTime() = startTime.IsNull()
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -132,23 +132,20 @@ JSObject*
 CSSTransition::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return dom::CSSTransitionBinding::Wrap(aCx, this, aGivenProto);
 }
 
 void
 CSSTransition::GetTransitionProperty(nsString& aRetVal) const
 {
-  // Once we make the effect property settable (bug 1049975) we will need
-  // to store the transition property on the CSSTransition itself but for
-  // now we can just query the effect.
-  MOZ_ASSERT(mEffect && mEffect->AsTransition(),
-             "Transitions should have a transition effect");
-  nsCSSPropertyID prop = mEffect->AsTransition()->TransitionProperty();
-  aRetVal = NS_ConvertUTF8toUTF16(nsCSSProps::GetStringValue(prop));
+  MOZ_ASSERT(eCSSProperty_UNKNOWN != mTransitionProperty,
+             "Transition Property should be initialized");
+  aRetVal =
+    NS_ConvertUTF8toUTF16(nsCSSProps::GetStringValue(mTransitionProperty));
 }
 
 AnimationPlayState
 CSSTransition::PlayStateFromJS() const
 {
   FlushStyle();
   return Animation::PlayStateFromJS();
 }
@@ -208,34 +205,27 @@ CSSTransition::Tick()
 {
   Animation::Tick();
   QueueEvents();
 }
 
 nsCSSPropertyID
 CSSTransition::TransitionProperty() const
 {
-  // FIXME: Once we support replacing/removing the effect (bug 1049975)
-  // we'll need to store the original transition property so we keep
-  // returning the same value in that case.
-  dom::KeyframeEffectReadOnly* effect = GetEffect();
-  MOZ_ASSERT(effect && effect->AsTransition(),
-             "Transition should have a transition effect");
-  return effect->AsTransition()->TransitionProperty();
+  MOZ_ASSERT(eCSSProperty_UNKNOWN != mTransitionProperty,
+             "Transition property should be initialized");
+  return mTransitionProperty;
 }
 
 StyleAnimationValue
 CSSTransition::ToValue() const
 {
-  // FIXME: Once we support replacing/removing the effect (bug 1049975)
-  // the following assertion will no longer hold.
-  dom::KeyframeEffectReadOnly* effect = GetEffect();
-  MOZ_ASSERT(effect && effect->AsTransition(),
-             "Transition should have a transition effect");
-  return effect->AsTransition()->ToValue();
+  MOZ_ASSERT(!mTransitionToValue.IsNull(),
+             "Transition ToValue should be initialized");
+  return mTransitionToValue;
 }
 
 bool
 CSSTransition::HasLowerCompositeOrderThan(const CSSTransition& aOther) const
 {
   MOZ_ASSERT(IsTiedToMarkup() && aOther.IsTiedToMarkup(),
              "Should only be called for CSS transitions that are sorted "
              "as CSS transitions (i.e. tied to CSS markup)");
@@ -272,16 +262,29 @@ CSSTransition::GetCurrentTimeAt(const Do
   if (!timelineTime.IsNull()) {
     result.SetValue((timelineTime.Value() - aStartTime)
                       .MultDouble(aPlaybackRate));
   }
 
   return result;
 }
 
+void
+CSSTransition::SetEffect(KeyframeEffectReadOnly* aEffect)
+{
+  Animation::SetEffect(aEffect);
+
+  // Initialize transition property.
+  ElementPropertyTransition* pt = aEffect ? aEffect->AsTransition() : nullptr;
+  if (eCSSProperty_UNKNOWN == mTransitionProperty && pt) {
+    mTransitionProperty = pt->TransitionProperty();
+    mTransitionToValue = pt->ToValue();
+  }
+}
+
 ////////////////////////// nsTransitionManager ////////////////////////////
 
 NS_IMPL_CYCLE_COLLECTION(nsTransitionManager, mEventDispatcher)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsTransitionManager, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsTransitionManager, Release)
 
 static inline bool
@@ -645,22 +648,22 @@ nsTransitionManager::ConsiderStartingTra
                                      0.5, dummyValue);
 
   bool haveCurrentTransition = false;
   size_t currentIndex = nsTArray<ElementPropertyTransition>::NoIndex;
   const ElementPropertyTransition *oldPT = nullptr;
   if (aElementTransitions) {
     OwningCSSTransitionPtrArray& animations = aElementTransitions->mAnimations;
     for (size_t i = 0, i_end = animations.Length(); i < i_end; ++i) {
-      const ElementPropertyTransition *iPt =
-        animations[i]->GetEffect()->AsTransition();
-      if (iPt->TransitionProperty() == aProperty) {
+      if (animations[i]->TransitionProperty() == aProperty) {
         haveCurrentTransition = true;
         currentIndex = i;
-        oldPT = iPt;
+        oldPT = animations[i]->GetEffect()
+                ? animations[i]->GetEffect()->AsTransition()
+                : nullptr;
         break;
       }
     }
   }
 
   // If we got a style change that changed the value to the endpoint
   // of the currently running transition, we don't want to interrupt
   // its timing function.
@@ -670,17 +673,18 @@ nsTransitionManager::ConsiderStartingTra
   // this case, we'll end up with shouldAnimate as false (because
   // there's no value change), but we need to return early here rather
   // than cancel the running transition because shouldAnimate is false!
   //
   // Likewise, if we got a style change that changed the value to the
   // endpoint of our finished transition, we also don't want to start
   // a new transition for the reasons described in
   // https://lists.w3.org/Archives/Public/www-style/2015Jan/0444.html .
-  if (haveCurrentTransition && haveValues && oldPT->ToValue() == endValue) {
+  if (haveCurrentTransition && haveValues &&
+      aElementTransitions->mAnimations[currentIndex]->ToValue() == endValue) {
     // GetAnimationRule already called RestyleForAnimation.
     return;
   }
 
   if (!shouldAnimate) {
     if (haveCurrentTransition) {
       // We're in the middle of a transition, and just got a non-transition
       // style change to something that we can't animate.  This might happen
@@ -717,18 +721,22 @@ nsTransitionManager::ConsiderStartingTra
     duration = 0.0;
   }
 
   StyleAnimationValue startForReversingTest = startValue;
   double reversePortion = 1.0;
 
   // If the new transition reverses an existing one, we'll need to
   // handle the timing differently.
+  // FIXME: Move mStartForReversingTest, mReversePortion to CSSTransition,
+  //        and set the timing function on transitions as an effect-level
+  //        easing (rather than keyframe-level easing). (Bug 1292001)
   if (haveCurrentTransition &&
       aElementTransitions->mAnimations[currentIndex]->HasCurrentEffect() &&
+      oldPT &&
       oldPT->mStartForReversingTest == endValue) {
     // Compute the appropriate negative transition-delay such that right
     // now we'd end up at the current position.
     double valuePortion =
       oldPT->CurrentValuePortion() * oldPT->mReversePortion +
       (1.0 - oldPT->mReversePortion);
     // A timing function with negative y1 (or y2!) might make
     // valuePortion negative.  In this case, we still want to apply our
@@ -812,31 +820,29 @@ nsTransitionManager::ConsiderStartingTra
       AddElementCollection(aElementTransitions);
     }
   }
 
   OwningCSSTransitionPtrArray& animations = aElementTransitions->mAnimations;
 #ifdef DEBUG
   for (size_t i = 0, i_end = animations.Length(); i < i_end; ++i) {
     MOZ_ASSERT(
-      i == currentIndex ||
-      (animations[i]->GetEffect() &&
-       animations[i]->GetEffect()->AsTransition()->TransitionProperty()
-         != aProperty),
+      i == currentIndex || animations[i]->TransitionProperty() != aProperty,
       "duplicate transitions for property");
   }
 #endif
   if (haveCurrentTransition) {
     // If this new transition is replacing an existing transition that is running
     // on the compositor, we store select parameters from the replaced transition
     // so that later, once all scripts have run, we can update the 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->IsCurrent() &&
+    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(),
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -116,16 +116,17 @@ namespace dom {
 
 class CSSTransition final : public Animation
 {
 public:
  explicit CSSTransition(nsIGlobalObject* aGlobal)
     : dom::Animation(aGlobal)
     , mWasFinishedOnLastTick(false)
     , mNeedsNewAnimationIndexWhenRun(false)
+    , mTransitionProperty(eCSSProperty_UNKNOWN)
   {
   }
 
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
   CSSTransition* AsCSSTransition() override { return this; }
   const CSSTransition* AsCSSTransition() const override { return this; }
@@ -205,16 +206,18 @@ public:
   // 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 TimeDuration& aStartTime,
       double aPlaybackRate);
 
+  void SetEffect(KeyframeEffectReadOnly* aEffect) override;
+
 protected:
   virtual ~CSSTransition()
   {
     MOZ_ASSERT(!mOwningElement.IsSet(), "Owning element should be cleared "
                                         "before a CSS transition is destroyed");
   }
 
   // Animation overrides
@@ -242,16 +245,24 @@ protected:
   // For (b) and (c) the owning element will return !IsSet().
   OwningElementRef mOwningElement;
 
   bool mWasFinishedOnLastTick;
 
   // When true, indicates that when this transition next leaves the idle state,
   // its animation index should be updated.
   bool mNeedsNewAnimationIndexWhenRun;
+
+  // Store the transition property and to-value here since we need that
+  // information in order to determine if there is an existing transition
+  // for a given style change. We can't store that information on the
+  // ElementPropertyTransition (effect) however since it can be replaced
+  // using the Web Animations API.
+  nsCSSPropertyID mTransitionProperty;
+  StyleAnimationValue mTransitionToValue;
 };
 
 } // namespace dom
 
 template <>
 struct AnimationTypeTraits<dom::CSSTransition>
 {
   static nsIAtom* ElementPropertyAtom()