From 34ef9d4683b3e81b8df1be1a9c38eae331e8c398 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Wed, 14 Mar 2018 09:16:03 +0100 Subject: Bug 1264125: Call the queueing events when canceling transition via Style or Script Issue #55 --- dom/animation/Animation.cpp | 23 +++++++++++++++++++++++ dom/animation/Animation.h | 10 ++++++++++ layout/style/nsTransitionManager.cpp | 9 +++------ layout/style/nsTransitionManager.h | 18 +++++++++++++++++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp index 6dd583ed1..cefdbb76d 100644 --- a/dom/animation/Animation.cpp +++ b/dom/animation/Animation.cpp @@ -230,6 +230,10 @@ Animation::SetTimelineNoUpdate(AnimationTimeline* aTimeline) return; } + StickyTimeDuration activeTime = mEffect + ? mEffect->GetComputedTiming().mActiveTime + : StickyTimeDuration(); + RefPtr oldTimeline = mTimeline; if (oldTimeline) { oldTimeline->RemoveAnimation(this); @@ -240,6 +244,9 @@ Animation::SetTimelineNoUpdate(AnimationTimeline* aTimeline) mHoldTime.SetNull(); } + if (!aTimeline) { + MaybeQueueCancelEvent(activeTime); + } UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async); } @@ -771,6 +778,10 @@ Animation::CancelNoUpdate() DispatchPlaybackEvent(NS_LITERAL_STRING("cancel")); + StickyTimeDuration activeTime = mEffect + ? mEffect->GetComputedTiming().mActiveTime + : StickyTimeDuration(); + mHoldTime.SetNull(); mStartTime.SetNull(); @@ -779,6 +790,7 @@ Animation::CancelNoUpdate() if (mTimeline) { mTimeline->RemoveAnimation(this); } + MaybeQueueCancelEvent(activeTime); } void @@ -819,6 +831,17 @@ Animation::HasLowerCompositeOrderThan(const Animation& aOther) const return thisTransition->HasLowerCompositeOrderThan(*otherTransition); } if (thisTransition || otherTransition) { + // Cancelled transitions no longer have an owning element. To be strictly + // correct we should store a strong reference to the owning element + // so that if we arrive here while sorting cancel events, we can sort + // them in the correct order. + // + // However, given that cancel events are almost always queued + // synchronously in some deterministic manner, we can be fairly sure + // that cancel events will be dispatched in a deterministic order + // (which is our only hard requirement until specs say otherwise). + // Furthermore, we only reach here when we have events with equal + // timestamps so this is an edge case we can probably ignore for now. return thisTransition; } } diff --git a/dom/animation/Animation.h b/dom/animation/Animation.h index c59d7d6ce..3263b30c4 100644 --- a/dom/animation/Animation.h +++ b/dom/animation/Animation.h @@ -326,6 +326,16 @@ public: void NotifyEffectTimingUpdated(); + /** + * Used by subclasses to synchronously queue a cancel event in situations + * where the Animation may have been cancelled. + * + * We need to do this synchronously because after a CSS animation/transition + * is canceled, it will be released by its owning element and may not still + * exist when we would normally go to queue events on the next tick. + */ + virtual void MaybeQueueCancelEvent(StickyTimeDuration aActiveTime) {}; + protected: void SilentlySetCurrentTime(const TimeDuration& aNewCurrentTime); void SilentlySetPlaybackRate(double aPlaybackRate); diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp index 4a5ecdef6..da12a0ecd 100644 --- a/layout/style/nsTransitionManager.cpp +++ b/layout/style/nsTransitionManager.cpp @@ -180,7 +180,7 @@ CSSTransition::UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag) } void -CSSTransition::QueueEvents() +CSSTransition::QueueEvents(StickyTimeDuration aActiveTime) { if (!mEffect || !mOwningElement.IsSet()) { @@ -230,12 +230,9 @@ CSSTransition::QueueEvents() // Handle cancel events firts if (mPreviousTransitionPhase != TransitionPhase::Idle && currentPhase == TransitionPhase::Idle) { - // FIXME: bug 1264125: We will need to get active time when cancelling - // the transition. - StickyTimeDuration activeTime(0); - TimeStamp activeTimeStamp = ElapsedTimeToTimeStamp(activeTime); + TimeStamp activeTimeStamp = ElapsedTimeToTimeStamp(aActiveTime); events.AppendElement(TransitionEventParams{ eTransitionCancel, - activeTime, + aActiveTime, activeTimeStamp }); } diff --git a/layout/style/nsTransitionManager.h b/layout/style/nsTransitionManager.h index 56ec61572..80042adcd 100644 --- a/layout/style/nsTransitionManager.h +++ b/layout/style/nsTransitionManager.h @@ -162,6 +162,18 @@ public: Animation::CancelFromStyle(); + // The above call to Animation::CancelFromStyle may cause a transitioncancel + // event to be queued. However, it will also remove the transition from its + // timeline. If this transition was the last animation attached to + // the timeline, the timeline will stop observing the refresh driver and + // there may be no subsequent tick fro dispatching animation events. + // + // To ensure the cancel event is dispatched we tell the timeline it needs to + // observe the refresh driver for at least one more tick. + if (mTimeline) { + mTimeline->NotifyAnimationUpdated(*this); + } + // It is important we do this *after* calling CancelFromStyle(). // This is because CancelFromStyle() will end up posting a restyle and // that restyle should target the *transitions* level of the cascade. @@ -214,6 +226,10 @@ public: const TimeDuration& aStartTime, double aPlaybackRate); + void MaybeQueueCancelEvent(StickyTimeDuration aActiveTime) override { + QueueEvents(aActiveTime); + } + protected: virtual ~CSSTransition() { @@ -225,7 +241,7 @@ protected: void UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag) override; - void QueueEvents(); + void QueueEvents(StickyTimeDuration activeTime = StickyTimeDuration()); // The (pseudo-)element whose computed transition-property refers to this // transition (if any). -- cgit v1.2.3