Bug 1202333 part 1 - Remove excessive animationiteration event. r?birtles draft
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Tue, 20 Dec 2016 15:57:13 +0900
changeset 451382 afe11470934216131c48c3b8e7b597ef6aa784e9
parent 450805 2824deb82146bba070995a762cfc98ddb2d1decb
child 451383 0edcac465e2ccd1ddf1746c15967bc85959789c3
push id39145
push usermantaroh@gmail.com
push dateTue, 20 Dec 2016 06:59:18 +0000
reviewersbirtles
bugs1202333
milestone53.0a1
Bug 1202333 part 1 - Remove excessive animationiteration event. r?birtles The Firefox fired excessive animationiteration event. But We fixed specification in order to prevent firing the animationiteration when animation is start. For detail, See https://github.com/w3c/csswg-drafts/issues/68 MozReview-Commit-ID: 391DRxSpq86
dom/events/test/test_legacy_event.html
layout/reftests/transform-3d/animate-backface-hidden.html
layout/reftests/transform-3d/animate-preserve3d-parent.html
layout/style/nsAnimationManager.cpp
layout/style/test/test_animations.html
layout/style/test/test_animations_omta.html
--- a/dom/events/test/test_legacy_event.html
+++ b/dom/events/test/test_legacy_event.html
@@ -68,32 +68,25 @@ function triggerShortTransition(node) {
 }
 
 // This function triggers a very short (1ms long) animation, which will cause
 // events to fire for the animation beginning & ending.
 function triggerShortAnimation(node) {
   node.style.animation = "anim1 1ms linear";
 }
 
-// This function triggers a long animation with two iterations, which is
-// *nearly* at the end of its first iteration.  It will hit the end of that
-// iteration (firing an event) almost immediately, 1ms in the future.
+// This function triggers a very short (10ms long) animation with many
+// iterations, which will cause a start event followed by an iteration event
+// on each subsequent tick, to fire.
 //
-// NOTE: It's important that this animation have a *long* duration.  If it were
-// short (e.g. 1ms duration), then we might jump past all its iterations in
-// a single refresh-driver tick. And if that were to happens, we'd *never* fire
-// any animationiteration events -- the CSS Animations spec says this event
-// must not be fired "...when an animationend event would fire at the same time"
-// (which would be the case in this example with a 1ms duration). So, to make
-// sure our event does fire, we use a long duration and a nearly-as-long
-// negative delay. This ensures we hit the end of the first iteration right
-// away, and that we don't risk hitting the end of the second iteration at the
-// same time.
+// NOTE: We need the many iterations since if an animation frame coincides
+// with the animation starting or ending we dispatch only the start or end
+// event and not the iteration event.
 function triggerAnimationIteration(node) {
-  node.style.animation = "anim1 300s -299.999s linear 2";
+  node.style.animation = "anim1 10ms linear 20000";
 }
 
 // GENERAL UTILITY FUNCTIONS
 // -------------------------
 // Creates a new div and appends it as a child of the specified parentNode, or
 // (if no parent is specified) as a child of the element with ID 'display'.
 function createChildDiv(parentNode) {
   if (!parentNode) {
--- a/layout/reftests/transform-3d/animate-backface-hidden.html
+++ b/layout/reftests/transform-3d/animate-backface-hidden.html
@@ -12,27 +12,33 @@ body { padding: 50px }
   90%, 100% { transform: rotateX(10deg); }
 }
 
 #test {
   background: blue;
   height: 200px; width: 200px;
   backface-visibility: hidden;
   /* use a -99.9s delay to start at 99.9% and then move to 0% */
-  animation: flip 100s -99.9s linear 2;
+  animation: flip 100s -99.9s linear 2 paused;
 }
 
 </style>
 
 <div id="test">
 </div>
 
 <script>
 
-document.getElementById("test").addEventListener("animationiteration", IterationListener, false);
+document.getElementById("test").addEventListener("animationstart", StartListener, false);
+
+function StartListener(event) {
+  var test = document.getElementById("test");
+  test.style.animationPlayState = 'running';
+  test.addEventListener("animationiteration", IterationListener, false);
+}
 
 function IterationListener(event) {
   setTimeout(RemoveReftestWait, 0);
 }
 
 function RemoveReftestWait() {
   document.documentElement.classList.remove("reftest-wait");
 }
--- a/layout/reftests/transform-3d/animate-preserve3d-parent.html
+++ b/layout/reftests/transform-3d/animate-preserve3d-parent.html
@@ -13,17 +13,17 @@ body { padding: 50px }
 }
 
 #parent {
   background: blue;
   height: 200px; width: 200px;
   border: 1px solid black;
   transform-style: preserve-3d;
   /* use a -99.9s delay to start at 99.9% and then move to 0% */
-  animation: spin 100s -99.9s linear 2;
+  animation: spin 100s -99.9s linear 2 paused;
 }
 
 #child {
   transform: translateZ(15px);
   height: 100px; width: 100px; margin: 50px;
   background: yellow;
   box-shadow: 3px 3px olive;
 }
@@ -34,17 +34,23 @@ body { padding: 50px }
   <div id="parent">
     <div id="child">
     </div>
   </div>
 </div>
 
 <script>
 
-document.getElementById("parent").addEventListener("animationiteration", IterationListener, false);
+document.getElementById("parent").addEventListener("animationstart", StartListener, false);
+
+function StartListener(event) {
+  var test = document.getElementById("parent");
+  test.style.animationPlayState = 'running';
+  test.addEventListener("animationiteration", IterationListener, false);
+}
 
 function IterationListener(event) {
   setTimeout(RemoveReftestWait, 0);
 }
 
 function RemoveReftestWait() {
   document.documentElement.classList.remove("reftest-wait");
 }
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -213,20 +213,16 @@ CSSAnimation::QueueEvents()
     computedTiming.mPhase == ComputedTiming::AnimationPhase::Active;
   bool isSameIteration =
          computedTiming.mCurrentIteration == mPreviousPhaseOrIteration;
   bool skippedActivePhase =
     (mPreviousPhaseOrIteration == PREVIOUS_PHASE_BEFORE &&
      computedTiming.mPhase == ComputedTiming::AnimationPhase::After) ||
     (mPreviousPhaseOrIteration == PREVIOUS_PHASE_AFTER &&
      computedTiming.mPhase == ComputedTiming::AnimationPhase::Before);
-  bool skippedFirstIteration =
-    isActive &&
-    mPreviousPhaseOrIteration == PREVIOUS_PHASE_BEFORE &&
-    computedTiming.mCurrentIteration > 0;
 
   MOZ_ASSERT(!skippedActivePhase || (!isActive && !wasActive),
              "skippedActivePhase only makes sense if we were & are inactive");
 
   if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Before) {
     mPreviousPhaseOrIteration = PREVIOUS_PHASE_BEFORE;
   } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Active) {
     mPreviousPhaseOrIteration = computedTiming.mCurrentIteration;
@@ -235,22 +231,17 @@ CSSAnimation::QueueEvents()
   }
 
   AutoTArray<EventPair, 2> events;
   StickyTimeDuration initialAdvance = StickyTimeDuration(InitialAdvance());
   StickyTimeDuration iterationStart = computedTiming.mDuration *
                                       computedTiming.mCurrentIteration;
   const StickyTimeDuration& activeDuration = computedTiming.mActiveDuration;
 
-  if (skippedFirstIteration) {
-    // Notify animationstart and animationiteration in same tick.
-    events.AppendElement(EventPair(eAnimationStart, initialAdvance));
-    events.AppendElement(EventPair(eAnimationIteration,
-                                   std::max(iterationStart, initialAdvance)));
-  } else if (!wasActive && isActive) {
+  if (!wasActive && isActive) {
     events.AppendElement(EventPair(eAnimationStart, initialAdvance));
   } else if (wasActive && !isActive) {
     events.AppendElement(EventPair(eAnimationEnd, activeDuration));
   } else if (wasActive && isActive && !isSameIteration) {
     events.AppendElement(EventPair(eAnimationIteration, iterationStart));
   } else if (skippedActivePhase) {
     events.AppendElement(EventPair(eAnimationStart,
                                    std::min(initialAdvance, activeDuration)));
--- a/layout/style/test/test_animations.html
+++ b/layout/style/test/test_animations.html
@@ -1222,19 +1222,16 @@ done_div();
 new_div("animation: anim2 1s -3.6s ease-in 5 alternate forwards");
 listen(); // rely on no flush having happened yet
 cs.animationName; // flush styles so animation is created
 advance_clock(0); // complete pending animation start
 is_approx(px_to_num(cs.marginRight), 100 * gTF.ease_in(0.4), 0.01,
           "large negative delay test at 0ms");
 check_events([{ type: 'animationstart', target: div,
                 animationName: 'anim2', elapsedTime: 3.6,
-                pseudoElement: "" },
-              { type: 'animationiteration', target: div,
-                animationName: 'anim2', elapsedTime: 3.6,
                 pseudoElement: "" }],
              "right after start in large negative delay test");
 advance_clock(380);
 is_approx(px_to_num(cs.marginRight), 100 * gTF.ease_in(0.02), 0.01,
           "large negative delay test at 380ms");
 check_events([]);
 advance_clock(20);
 is(cs.marginRight, "0px", "large negative delay test at 400ms");
--- a/layout/style/test/test_animations_omta.html
+++ b/layout/style/test/test_animations_omta.html
@@ -1403,19 +1403,16 @@ addAsyncAnimTest(function *() {
   // in the fourth iteration
   new_div("animation: anim2 1s -3.6s ease-in 5 alternate forwards");
   listen();
   yield waitForPaintsFlushed();
   omta_is_approx("opacity", gTF.ease_in(0.4), 0.01, RunningOn.Compositor,
                  "large negative delay test at 0ms");
   check_events([{ type: 'animationstart', target: gDiv,
                   animationName: 'anim2', elapsedTime: 3.6,
-                  pseudoElement: "" },
-                { type: 'animationiteration', target: gDiv,
-                  animationName: 'anim2', elapsedTime: 3.6,
                   pseudoElement: "" }],
                "right after start in large negative delay test");
   advance_clock(380);
   omta_is_approx("opacity", gTF.ease_in(0.02), 0.01, RunningOn.Compositor,
                  "large negative delay test at 380ms");
   check_events([]);
   advance_clock(20);
   omta_is("opacity", 0, RunningOn.Compositor,