Web Animations: Fix CSS events to handle animations with very short durations

Source/core/animation/TimedItem.cpp

Index: Source/core/animation/TimedItem.cpp
diff --git a/Source/core/animation/TimedItem.cpp b/Source/core/animation/TimedItem.cpp
index ee6b47cfb0023a884f710f04476876720dd6c592..05b431d719b700c973dbb7a2e9f55c466e9bf681 100644
--- a/Source/core/animation/TimedItem.cpp
+++ b/Source/core/animation/TimedItem.cpp
@@ -41,6 +41,7 @@ TimedItem::TimedItem(const Timing& timing, PassOwnPtr<TimedItemEventDelegate> ev
     , m_specified(timing)
     , m_calculated()
     , m_eventDelegate(eventDelegate)
+    , m_isFirstSample(true)
 {
     timing.assertValid();
 }
@@ -86,35 +87,34 @@ void TimedItem::updateInheritedTime(double inheritedTime) const
         currentIteration = calculateCurrentIteration(iterationDuration, iterationTime, scaledActiveTime, m_specified);
         timeFraction = calculateTransformedTime(currentIteration, iterationDuration, iterationTime, m_specified);
     }
-
-    const double lastIteration = m_calculated.currentIteration;
-    m_calculated.currentIteration = currentIteration;
     m_calculated.activeDuration = activeDuration;
     m_calculated.timeFraction = timeFraction;
 
+    const bool newIsInPlay = phase == PhaseActive && (!m_parent || m_parent->isInPlay());
+    const bool newIsCurrent = phase == PhaseBefore || newIsInPlay || (m_parent && m_parent->isCurrent());
+
     const bool wasInEffect = m_calculated.isInEffect;
-    const bool wasInPlay = m_calculated.isInPlay;
     m_calculated.isInEffect = !isNull(activeTime);
-    m_calculated.isInPlay = phase == PhaseActive && (!m_parent || m_parent->isInPlay());
-    m_calculated.isCurrent = phase == PhaseBefore || isInPlay() || (m_parent && m_parent->isCurrent());
 
     // This logic is specific to CSS events and assumes that all animations
     // start after the DocumentTimeline has started.
-    if (m_eventDelegate && (isInPlay() != wasInPlay || (isInPlay() && lastIteration != currentIteration)))
-        m_eventDelegate->onEventCondition(wasInPlay, isInPlay(), lastIteration, currentIteration);
+
+    // If this is the first sample, fake the values for the previous sample to
+    // avoid spurious events.
+    const double lastIteration = m_isFirstSample ? currentIteration : m_calculated.currentIteration;
+    const bool wasInPlay = m_isFirstSample ? newIsInPlay : m_calculated.isInPlay;
+    const bool wasCurrent = m_isFirstSample ? newIsCurrent : m_calculated.isCurrent;
+    m_isFirstSample = false;
+
+    m_calculated.currentIteration = currentIteration;
+    m_calculated.isInPlay = newIsInPlay;
+    m_calculated.isCurrent = newIsCurrent;
+
+    if (m_eventDelegate && (isInPlay() != wasInPlay || isCurrent() != wasCurrent || (wasInPlay && isInPlay() && lastIteration != currentIteration)))

[dstockwell, 2013/08/21 02:29:44]

This is getting complicated. I wonder if instead of looking at a change to
isCurrent (and perhaps even isInPlay) whether we can get by just looking at
changes to the phase (and iteration)? This might also avoid needing that first
forced sample.

I think None -> !None = started. !After -> After = finished.

[dstockwell, 2013/08/21 04:37:36]

Oops, I guess that's: None -> (Active | After) = started

+        m_eventDelegate->onEventCondition(wasInPlay, isInPlay(), wasCurrent, isCurrent(), lastIteration, currentIteration);
 
     // FIXME: This probably shouldn't be recursive.
     updateChildrenAndEffects(wasInEffect);
 }
 
-TimedItem::CalculatedTiming::CalculatedTiming()
-    : activeDuration(nullValue())
-    , currentIteration(nullValue())
-    , timeFraction(nullValue())
-    , isCurrent(false)
-    , isInEffect(false)
-    , isInPlay(false)
-{
-}
-
 } // namespace WebCore