Bug 1472900 - Use timestamp associated with the timeline for animation cancel events. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Thu, 05 Jul 2018 06:19:12 +0900
changeset 816381 61e9670b462ff2bd4b766453d3a718fd1247063e
parent 816380 a49daf7ac9a83d9347b4ac5a4a0938ee79d1c2dc
push id115823
push userhikezoe@mozilla.com
push dateWed, 11 Jul 2018 04:16:55 +0000
reviewersbirtles
bugs1472900
milestone63.0a1
Bug 1472900 - Use timestamp associated with the timeline for animation cancel events. r?birtles Before this change, the test in this commit fails. The received events order is; 1) cancel 2) transitioncancel 3) transitionstart 4) finish MozReview-Commit-ID: 8liTFXime6e
dom/animation/Animation.cpp
dom/animation/Animation.h
dom/animation/AnimationTimeline.h
layout/style/nsAnimationManager.cpp
layout/style/nsTransitionManager.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/web-animations/timing-model/timelines/update-and-send-events.html
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -869,18 +869,18 @@ Animation::CancelNoUpdate()
   if (PlayState() != AnimationPlayState::Idle) {
     ResetPendingTasks();
 
     if (mFinished) {
       mFinished->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
     }
     ResetFinishedPromise();
 
-    // FIXME: Bug 1472900 - Use the timestamp associated with the timeline.
-    QueuePlaybackEvent(NS_LITERAL_STRING("cancel"), TimeStamp());
+    QueuePlaybackEvent(NS_LITERAL_STRING("cancel"),
+                       GetTimelineCurrentTimeAsTimeStamp());
   }
 
   StickyTimeDuration activeTime = mEffect
                                   ? mEffect->GetComputedTiming().mActiveTime
                                   : StickyTimeDuration();
 
   mHoldTime.SetNull();
   mStartTime.SetNull();
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -517,16 +517,21 @@ protected:
 
     static constexpr StickyTimeDuration zeroDuration = StickyTimeDuration();
     return std::max(
       std::min((EffectEnd() - mEffect->SpecifiedTiming().Delay()),
                aActiveDuration),
       zeroDuration);
   }
 
+  TimeStamp GetTimelineCurrentTimeAsTimeStamp() const
+  {
+    return mTimeline ? mTimeline->GetCurrentTimeAsTimeStamp() : TimeStamp();
+  }
+
   nsIDocument* GetRenderedDocument() const;
   nsIDocument* GetTimelineDocument() const;
 
   RefPtr<AnimationTimeline> mTimeline;
   RefPtr<AnimationEffect> mEffect;
   // The beginning of the delay period.
   Nullable<TimeDuration> mStartTime; // Timeline timescale
   Nullable<TimeDuration> mHoldTime;  // Animation timescale
--- a/dom/animation/AnimationTimeline.h
+++ b/dom/animation/AnimationTimeline.h
@@ -57,16 +57,23 @@ public:
   virtual Nullable<TimeDuration> GetCurrentTime() const = 0;
 
   // Wrapper functions for AnimationTimeline DOM methods when called from
   // script.
   Nullable<double> GetCurrentTimeAsDouble() const {
     return AnimationUtils::TimeDurationToDouble(GetCurrentTime());
   }
 
+  TimeStamp GetCurrentTimeAsTimeStamp() const {
+    Nullable<TimeDuration> currentTime = GetCurrentTime();
+    return !currentTime.IsNull()
+      ? ToTimeStamp(currentTime.Value())
+      : TimeStamp();
+  }
+
   /**
    * Returns true if the times returned by GetCurrentTime() are convertible
    * to and from wallclock-based TimeStamp (e.g. from TimeStamp::Now()) values
    * using ToTimelineTime() and ToTimeStamp().
    *
    * Typically this is true, but it will be false in the case when this
    * timeline has no refresh driver or is tied to a refresh driver under test
    * control.
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -249,18 +249,19 @@ CSSAnimation::QueueEvents(const StickyTi
                                             aScheduledEventTimeStamp,
                                             this));
   };
 
   // Handle cancel event first
   if ((mPreviousPhase != AnimationPhase::Idle &&
        mPreviousPhase != AnimationPhase::After) &&
       currentPhase == AnimationPhase::Idle) {
-    TimeStamp activeTimeStamp = ElapsedTimeToTimeStamp(aActiveTime);
-    appendAnimationEvent(eAnimationCancel, aActiveTime, activeTimeStamp);
+    appendAnimationEvent(eAnimationCancel,
+                         aActiveTime,
+                         GetTimelineCurrentTimeAsTimeStamp());
   }
 
   switch (mPreviousPhase) {
     case AnimationPhase::Idle:
     case AnimationPhase::Before:
       if (currentPhase == AnimationPhase::Active) {
         appendAnimationEvent(eAnimationStart,
                              intervalStartTime,
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -250,18 +250,19 @@ CSSTransition::QueueEvents(const StickyT
                                             aScheduledEventTimeStamp,
                                             this));
   };
 
   // Handle cancel events first
   if ((mPreviousTransitionPhase != TransitionPhase::Idle &&
        mPreviousTransitionPhase != TransitionPhase::After) &&
       currentPhase == TransitionPhase::Idle) {
-    TimeStamp activeTimeStamp = ElapsedTimeToTimeStamp(aActiveTime);
-    appendTransitionEvent(eTransitionCancel, aActiveTime, activeTimeStamp);
+    appendTransitionEvent(eTransitionCancel,
+                          aActiveTime,
+                          GetTimelineCurrentTimeAsTimeStamp());
   }
 
   // All other events
   switch (mPreviousTransitionPhase) {
     case TransitionPhase::Idle:
       if (currentPhase == TransitionPhase::Pending ||
           currentPhase == TransitionPhase::Before) {
         appendTransitionEvent(eTransitionRun, intervalStartTime, zeroTimeStamp);
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -619233,17 +619233,17 @@
    "d0fcb390c19c9ede7288278dc11ea5b3d33671cb",
    "testharness"
   ],
   "web-animations/timing-model/timelines/timelines.html": [
    "29d7fe91c355fc22f563ca17315d2ab493dc0566",
    "testharness"
   ],
   "web-animations/timing-model/timelines/update-and-send-events.html": [
-   "6ef855775c8fbb7220b0fd7f909b23cc8a64aebe",
+   "22947e732cd1b879118b0379302132c097960970",
    "testharness"
   ],
   "web-nfc/OWNERS": [
    "d42f3f15d00686bf5a5c7c69169ef5cf2554bd7b",
    "support"
   ],
   "web-nfc/idlharness.https.html": [
    "4e939e8328c0fa1ffde6a0e5a259fc790db84551",
--- a/testing/web-platform/tests/web-animations/timing-model/timelines/update-and-send-events.html
+++ b/testing/web-platform/tests/web-animations/timing-model/timelines/update-and-send-events.html
@@ -173,9 +173,37 @@ promise_test(async t => {
   assert_array_equals(receivedEvents, [],
     'The queued cancel event shouldn\'t be dispatched in the same frame');
 
   await waitForAnimationFrames(1);
   assert_array_equals(receivedEvents.map(event => event.type), ['cancel'],
     'The cancel event should be dispatched in a later frame');
 }, 'Queues a cancel event in transitionstart event callback');
 
+promise_test(async t => {
+  const div = createDiv(t);
+  getComputedStyle(div).marginLeft;
+  div.style = 'transition: margin-left 100s; margin-left: 100px;';
+  const anim = div.getAnimations()[0];
+
+  let receivedEvents = [];
+  anim.oncancel = event => receivedEvents.push(event);
+  anim.onfinish = event => receivedEvents.push(event);
+  div.ontransitionstart = event => receivedEvents.push(event);
+  div.ontransitioncancel = event => receivedEvents.push(event);
+
+  await anim.ready;
+
+  anim.finish();
+  anim.cancel();
+
+  await waitForAnimationFrames(1);
+
+  assert_array_equals(receivedEvents.map(event => event.type),
+    [ 'transitionstart',
+      'finish',
+      'cancel',
+      'transitioncancel' ],
+    'Playback and CSS events for the same transition should be sorted by ' +
+    'schedule event time and composite order');
+}, 'Sorts events for the same transition');
+
 </script>