Bug 1923208 - Fallback to compare the order of event targets for CSSTransition if they have the same time. r=layout-reviewers,firefox-animation-reviewers,emilio,birtles

Since the cancelled transition has no owning element and its transition
generation is out-of-date, so we have to compare the event targets and
the transition generations in `AnimationEventInfo`. We still follow the
composite order defined in the spec but use the event target and transition
generation before cancelling this transition, to get the correct event order
if we have the same schedule time.

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

dom/animation/Animation.cpp

@@ -1192,7 +1192,9 @@ void Animation::Remove() {
   QueuePlaybackEvent(nsGkAtoms::onremove, GetTimelineCurrentTimeAsTimeStamp());
 }
 
-bool Animation::HasLowerCompositeOrderThan(const Animation& aOther) const {
+bool Animation::HasLowerCompositeOrderThan(
+    const Maybe<EventContext>& aContext, const Animation& aOther,
+    const Maybe<EventContext>& aOtherContext) const {
   // 0. Object-equality case
   if (&aOther == this) {
     return false;
@@ -1201,14 +1203,20 @@ bool Animation::HasLowerCompositeOrderThan(const Animation& aOther) const {
   // 1. CSS Transitions sort lowest
   {
     auto asCSSTransitionForSorting =
-        [](const Animation& anim) -> const CSSTransition* {
+        [](const Animation& anim,
+           const Maybe<EventContext>& aContext) -> const CSSTransition* {
       const CSSTransition* transition = anim.AsCSSTransition();
-      return transition && transition->IsTiedToMarkup() ? transition : nullptr;
+      return transition && (aContext || transition->IsTiedToMarkup())
+                 ? transition
+                 : nullptr;
     };
-    auto thisTransition = asCSSTransitionForSorting(*this);
-    auto otherTransition = asCSSTransitionForSorting(aOther);
+    const auto* const thisTransition =
+        asCSSTransitionForSorting(*this, aContext);
+    const auto* const otherTransition =
+        asCSSTransitionForSorting(aOther, aOtherContext);
     if (thisTransition && otherTransition) {
-      return thisTransition->HasLowerCompositeOrderThan(*otherTransition);
+      return thisTransition->HasLowerCompositeOrderThan(
+          aContext, *otherTransition, aOtherContext);
     }
     if (thisTransition || otherTransition) {
       // Cancelled transitions no longer have an owning element. To be strictly

dom/animation/Animation.h

@@ -300,7 +300,18 @@ class Animation : public DOMEventTargetHelper,
   /**
    * Returns true if this Animation has a lower composite order than aOther.
    */
-  bool HasLowerCompositeOrderThan(const Animation& aOther) const;
+  struct EventContext {
+    NonOwningAnimationTarget mTarget;
+    uint64_t mIndex;
+  };
+  // Note: we provide |aContext|/|aOtherContext| only when it is a cancelled
+  // transition or animation (for overridding the target and animation index).
+  bool HasLowerCompositeOrderThan(
+      const Maybe<EventContext>& aContext, const Animation& aOther,
+      const Maybe<EventContext>& aOtherContext) const;
+  bool HasLowerCompositeOrderThan(const Animation& aOther) const {
+    return HasLowerCompositeOrderThan(Nothing(), aOther, Nothing());
+  }
 
   /**
    * Returns the level at which the effect(s) associated with this Animation

dom/animation/AnimationEventDispatcher.h

@@ -73,6 +73,30 @@ struct AnimationEventInfo {
     return nullptr;
   }
 
+  // Return the event context if the event is animationcancel or
+  // transitioncancel.
+  Maybe<dom::Animation::EventContext> GetEventContext() const {
+    if (mData.is<CssAnimationData>()) {
+      const auto& data = mData.as<CssAnimationData>();
+      return data.mMessage == eAnimationCancel
+                 ? Some(dom::Animation::EventContext{
+                       NonOwningAnimationTarget(data.mTarget),
+                       data.mAnimationIndex})
+                 : Nothing();
+    }
+
+    if (mData.is<CssTransitionData>()) {
+      const auto& data = mData.as<CssTransitionData>();
+      return data.mMessage == eTransitionCancel
+                 ? Some(dom::Animation::EventContext{
+                       NonOwningAnimationTarget(data.mTarget),
+                       data.mAnimationIndex})
+                 : Nothing();
+    }
+
+    return Nothing();
+  }
+
   void MaybeAddMarker() const;
 
   // For CSS animation events
@@ -143,8 +167,8 @@ struct AnimationEventInfo {
       return this->IsWebAnimationEvent();
     }
 
-    AnimationPtrComparator<RefPtr<dom::Animation>> comparator;
-    return comparator.LessThan(this->mAnimation, aOther.mAnimation);
+    return mAnimation->HasLowerCompositeOrderThan(
+        GetEventContext(), *aOther.mAnimation, aOther.GetEventContext());
   }
 
   bool IsWebAnimationEvent() const { return mData.is<WebAnimationData>(); }

dom/animation/CSSTransition.cpp

@@ -206,10 +206,13 @@ AnimationValue CSSTransition::ToValue() const {
 }
 
 bool CSSTransition::HasLowerCompositeOrderThan(
-    const CSSTransition& aOther) const {
-  MOZ_ASSERT(IsTiedToMarkup() && aOther.IsTiedToMarkup(),
+    const Maybe<EventContext>& aContext, const CSSTransition& aOther,
+    const Maybe<EventContext>& aOtherContext) const {
+  MOZ_ASSERT((IsTiedToMarkup() || aContext) &&
+                 (aOther.IsTiedToMarkup() || aOtherContext),
              "Should only be called for CSS transitions that are sorted "
-             "as CSS transitions (i.e. tied to CSS markup)");
+             "as CSS transitions (i.e. tied to CSS markup) or with overridden "
+             "target and animation index");
 
   // 0. Object-equality case
   if (&aOther == this) {
@@ -217,16 +220,23 @@ bool CSSTransition::HasLowerCompositeOrderThan(
   }
 
   // 1. Sort by document order
-  if (!mOwningElement.Equals(aOther.mOwningElement)) {
-    return mOwningElement.LessThan(
-        const_cast<CSSTransition*>(this)->CachedChildIndexRef(),
-        aOther.mOwningElement,
+  const OwningElementRef& owningElement1 =
+      aContext ? OwningElementRef(aContext->mTarget) : mOwningElement;
+  const OwningElementRef& owningElement2 =
+      aOtherContext ? OwningElementRef(aOtherContext->mTarget)
+                    : aOther.mOwningElement;
+  if (!owningElement1.Equals(owningElement2)) {
+    return owningElement1.LessThan(
+        const_cast<CSSTransition*>(this)->CachedChildIndexRef(), owningElement2,
         const_cast<CSSTransition*>(&aOther)->CachedChildIndexRef());
   }
 
   // 2. (Same element and pseudo): Sort by transition generation
-  if (mAnimationIndex != aOther.mAnimationIndex) {
-    return mAnimationIndex < aOther.mAnimationIndex;
+  const uint64_t& index1 = aContext ? aContext->mIndex : mAnimationIndex;
+  const uint64_t& index2 =
+      aOtherContext ? aOtherContext->mIndex : aOther.mAnimationIndex;
+  if (index1 != index2) {
+    return index1 < index2;
   }
 
   // 3. (Same transition generation): Sort by transition property

dom/animation/CSSTransition.h

@@ -81,7 +81,9 @@ class CSSTransition final : public Animation {
   const AnimatedPropertyID& TransitionProperty() const;
   AnimationValue ToValue() const;
 
-  bool HasLowerCompositeOrderThan(const CSSTransition& aOther) const;
+  bool HasLowerCompositeOrderThan(
+      const Maybe<EventContext>& aContext, const CSSTransition& aOther,
+      const Maybe<EventContext>& aOtherContext) const;
   EffectCompositor::CascadeLevel CascadeLevel() const override {
     return IsTiedToMarkup() ? EffectCompositor::CascadeLevel::Transitions
                             : EffectCompositor::CascadeLevel::Animations;

testing/web-platform/tests/css/css-transitions/event-dispatch.tentative.html

@@ -432,4 +432,57 @@ promise_test(async t => {
   await waitForAnimationFrames(2);
 }, 'Cancel the transition after it finishes');
 
+promise_test(async t => {
+  const { transition, watcher, div } = setupTransition(t, 'margin-left 100s');
+
+  transition.currentTime = 50 * MS_PER_SEC;
+  await watcher.wait_for(['transitionrun', 'transitionstart']);
+
+  // Replace the running transition.
+  div.style.marginLeft = '200px';
+
+  // transitioncancel event should be fired before transitionrun because we
+  // expect to cancel the running transition first.
+  await watcher.wait_for(
+    ['transitioncancel', 'transitionrun', 'transitionstart']
+  );
+
+  // Then wait a couple of frames and check that no event was dispatched
+  await waitForAnimationFrames(2);
+}, 'Replacing a running transition should get transitioncancel earlier than ' +
+   'transitionrun and transitionstart');
+
+promise_test(async t => {
+  const div =
+    addDiv(t, { style: 'transition: margin-left 100s, margin-top 100s' });
+  const watcher = new EventWatcher(t, div, [ 'transitionrun',
+                                             'transitioncancel' ],
+                                   transitionEventsTimeout);
+  getComputedStyle(div).marginLeft;
+
+  div.style.marginLeft = '100px';
+  div.style.marginTop = '100px';
+  const transitions = div.getAnimations();
+  transitions[0].currentTime = 50 * MS_PER_SEC;
+  transitions[1].currentTime = 50 * MS_PER_SEC;
+
+  await watcher.wait_for(['transitionrun', 'transitionrun']);
+
+  // Replace both running transitions.
+  div.style.marginLeft = '200px';
+  div.style.marginTop = '200px';
+
+  await watcher.wait_for([
+    // Cancel events show first because their transition generations are
+    // smaller than the new ones.
+    'transitioncancel', 'transitioncancel',
+    'transitionrun', 'transitionrun'
+  ]);
+
+  // Then wait a couple of frames and check that no event was dispatched
+  await waitForAnimationFrames(2);
+}, 'Replacing two running transitions on the same target should get two ' +
+   'transitioncancel events earlier than two transitionrun events, per ' +
+   'transition generation');
+
 </script>