Bug 1033114 Part 4: Make mStartTime a nullable TimeDuration r=birtles
authorDavid Zbarsky <dzbarsky@gmail.com>
Sat, 30 Aug 2014 02:11:57 -0400
changeset 224207 9db3e43c19c1177fd9b07bac0f3326e18589cddd
parent 224206 f9448cb58a5a6bc0ff6ee336ff3daaa96ab72501
child 224208 f2412966d91d5e873f9bf0dde23de8c12ade7880
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1033114
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1033114 Part 4: Make mStartTime a nullable TimeDuration r=birtles
dom/animation/AnimationPlayer.cpp
dom/animation/AnimationPlayer.h
dom/animation/AnimationTimeline.h
layout/style/nsAnimationManager.cpp
--- a/dom/animation/AnimationPlayer.cpp
+++ b/dom/animation/AnimationPlayer.cpp
@@ -25,37 +25,17 @@ Nullable<double>
 AnimationPlayer::GetStartTime() const
 {
   return AnimationUtils::TimeDurationToDouble(mStartTime);
 }
 
 Nullable<double>
 AnimationPlayer::GetCurrentTime() const
 {
-  Nullable<double> result;
-  Nullable<TimeDuration> currentTime = GetCurrentTimeDuration();
-
-  // The current time is currently only going to be null when don't have a
-  // refresh driver (e.g. because we are in a display:none iframe).
-  //
-  // Web Animations says that in this case we should use a timeline time of
-  // 0 (the "effective timeline time") and calculate the current time from that.
-  // Doing that, however, requires storing the start time as an offset rather
-  // than a timestamp so for now we just return 0.
-  //
-  // FIXME: Store player start time and pause start as offsets rather than
-  // timestamps and return the appropriate current time when the timeline time
-  // is null.
-  if (currentTime.IsNull()) {
-    result.SetValue(0.0);
-  } else {
-    result.SetValue(currentTime.Value().ToMilliseconds());
-  }
-
-  return result;
+  return AnimationUtils::TimeDurationToDouble(GetCurrentTimeDuration());
 }
 
 void
 AnimationPlayer::SetSource(Animation* aSource)
 {
   if (mSource) {
     mSource->SetParentTime(Nullable<TimeDuration>());
   }
@@ -85,10 +65,25 @@ AnimationPlayer::IsRunning() const
 }
 
 bool
 AnimationPlayer::IsCurrent() const
 {
   return GetSource() && GetSource()->IsCurrent();
 }
 
+Nullable<TimeDuration>
+AnimationPlayer::GetCurrentTimeDuration() const
+{
+  Nullable<TimeDuration> result;
+  if (!mHoldTime.IsNull()) {
+    result = mHoldTime;
+  } else {
+    Nullable<TimeDuration> timelineTime = mTimeline->GetCurrentTimeDuration();
+    if (!timelineTime.IsNull() && !mStartTime.IsNull()) {
+      result.SetValue(timelineTime.Value() - mStartTime.Value());
+    }
+  }
+  return result;
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/AnimationPlayer.h
+++ b/dom/animation/AnimationPlayer.h
@@ -62,33 +62,22 @@ public:
   bool IsPaused() const {
     return mPlayState == NS_STYLE_ANIMATION_PLAY_STATE_PAUSED;
   }
 
   bool IsRunning() const;
   bool IsCurrent() const;
 
   // Return the duration since the start time of the player, taking into
-  // account the pause state.  May be negative.
-  // Returns a null value if the timeline associated with this object has a
-  // current timestamp that is null or if the start time of this object is
-  // null.
-  Nullable<TimeDuration> GetCurrentTimeDuration() const {
-    Nullable<TimeDuration> timelineTime = mTimeline->GetCurrentTimeDuration();
-    Nullable<TimeDuration> holdDuration = mTimeline->ToTimelineTime(mHoldTime);
-    Nullable<TimeDuration> result; // Initializes to null
-    if (!timelineTime.IsNull() && !mStartTime.IsNull()) {
-      result.SetValue((IsPaused() ? holdDuration.Value() : timelineTime.Value()) - mStartTime.Value());
-    }
-    return result;
-  }
+  // account the pause state.  May be negative or null.
+  Nullable<TimeDuration> GetCurrentTimeDuration() const;
 
   // The beginning of the delay period.
   Nullable<TimeDuration> mStartTime;
-  TimeStamp mHoldTime;
+  Nullable<TimeDuration> mHoldTime;
   uint8_t mPlayState;
   bool mIsRunningOnCompositor;
 
   nsRefPtr<AnimationTimeline> mTimeline;
   nsRefPtr<Animation> mSource;
 };
 
 } // namespace dom
--- a/dom/animation/AnimationTimeline.h
+++ b/dom/animation/AnimationTimeline.h
@@ -36,19 +36,19 @@ public:
   // WebIDL API
   Nullable<double> GetCurrentTime() const;
 
   Nullable<TimeDuration> GetCurrentTimeDuration() const;
 
   Nullable<TimeDuration> ToTimelineTime(const TimeStamp& aTimeStamp) const;
   TimeStamp ToTimeStamp(const TimeDuration& aTimelineTime) const;
 
+protected:
   TimeStamp GetCurrentTimeStamp() const;
 
-protected:
   virtual ~AnimationTimeline() { }
 
   nsCOMPtr<nsIDocument> mDocument;
 
   // Store the most recently returned value of current time. This is used
   // in cases where we don't have a refresh driver (e.g. because we are in
   // a display:none iframe).
   mutable TimeStamp mLastCurrentTime;
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -267,16 +267,19 @@ nsAnimationManager::CheckAnimationRule(n
       // This means that we honor dynamic changes, which isn't what the
       // spec says to do, but WebKit seems to honor at least some of
       // them.  See
       // http://lists.w3.org/Archives/Public/www-style/2011Apr/0079.html
       // In order to honor what the spec said, we'd copy more data over
       // (or potentially optimize BuildAnimations to avoid rebuilding it
       // in the first place).
       if (!collection->mPlayers.IsEmpty()) {
+
+        Nullable<TimeDuration> now = timeline->GetCurrentTimeDuration();
+
         for (size_t newIdx = newPlayers.Length(); newIdx-- != 0;) {
           AnimationPlayer* newPlayer = newPlayers[newIdx];
 
           // Find the matching animation with this name in the old list
           // of animations.  We iterate through both lists in a backwards
           // direction which means that if there are more animations in
           // the new list of animations with a given name than in the old
           // list, it will be the animations towards the of the beginning of
@@ -304,28 +307,25 @@ nsAnimationManager::CheckAnimationRule(n
           }
 
           // Reset compositor state so animation will be re-synchronized.
           oldPlayer->mIsRunningOnCompositor = false;
 
           // Handle changes in play state.
           if (!oldPlayer->IsPaused() && newPlayer->IsPaused()) {
             // Start pause at current time.
-            oldPlayer->mHoldTime = timeline->GetCurrentTimeStamp();
+            oldPlayer->mHoldTime = oldPlayer->GetCurrentTimeDuration();
           } else if (oldPlayer->IsPaused() && !newPlayer->IsPaused()) {
-            const TimeStamp& now = timeline->GetCurrentTimeStamp();
-            if (!now.IsNull()) {
-              // FIXME: Once we store the start time and pause start as
-              // offsets (not timestamps) we should be able to update the
-              // start time to something more appropriate when now IsNull.
-              // Handle change in pause state by adjusting start time to
-              // unpause.
-              oldPlayer->mStartTime.SetValue(now + oldPlayer->mStartTime.Value() - oldPlayer->mHoldTime);
+            if (now.IsNull()) {
+              oldPlayer->mStartTime.SetNull();
+            } else {
+              oldPlayer->mStartTime.SetValue(now.Value() -
+                                               oldPlayer->mHoldTime.Value());
             }
-            oldPlayer->mHoldTime = TimeStamp();
+            oldPlayer->mHoldTime.SetNull();
           }
           oldPlayer->mPlayState = newPlayer->mPlayState;
 
           // Replace new animation with the (updated) old one and remove the
           // old one from the array so we don't try to match it any more.
           //
           // Although we're doing this while iterating this is safe because
           // we're not changing the length of newPlayers and we've finished
@@ -414,17 +414,17 @@ nsAnimationManager::BuildAnimations(nsSt
                                     dom::AnimationTimeline* aTimeline,
                                     AnimationPlayerPtrArray& aPlayers)
 {
   NS_ABORT_IF_FALSE(aPlayers.IsEmpty(), "expect empty array");
 
   ResolvedStyleCache resolvedStyles;
 
   const nsStyleDisplay *disp = aStyleContext->StyleDisplay();
-  TimeStamp now = aTimeline->GetCurrentTimeStamp();
+  Nullable<TimeDuration> now = aTimeline->GetCurrentTimeDuration();
 
   for (size_t animIdx = 0, animEnd = disp->mAnimationNameCount;
        animIdx != animEnd; ++animIdx) {
     const StyleAnimation& src = disp->mAnimations[animIdx];
 
     // CSS Animations whose animation-name does not match a @keyframes rule do
     // not generate animation events. This includes when the animation-name is
     // "none" which is represented by an empty name in the StyleAnimation.
@@ -449,22 +449,20 @@ nsAnimationManager::BuildAnimations(nsSt
     timing.mIterationCount = src.GetIterationCount();
     timing.mDirection = src.GetDirection();
     timing.mFillMode = src.GetFillMode();
 
     nsRefPtr<Animation> destAnim =
       new Animation(mPresContext->Document(), timing, src.GetName());
     dest->SetSource(destAnim);
 
-    dest->mStartTime = aTimeline->GetCurrentTimeDuration();
+    dest->mStartTime = now;
     dest->mPlayState = src.GetPlayState();
     if (dest->IsPaused()) {
       dest->mHoldTime = now;
-    } else {
-      dest->mHoldTime = TimeStamp();
     }
 
     // While current drafts of css3-animations say that later keyframes
     // with the same key entirely replace earlier ones (no cascading),
     // this is a bad idea and contradictory to the rest of CSS.  So
     // we're going to keep all the keyframes for each key and then do
     // the replacement on a per-property basis rather than a per-rule
     // basis, just like everything else in CSS.