Use lossless TimeDelta in DocumentTimeline::CurrentTimeInternal.

Early conversion to double resulted in DocumentTimeline returning a
different value than the requestAnimationFrame timestamp. By preserving
the full precision time delta we can use the exact same conversion as
the requestAnimationFrame timestamp.

Bug: 986974
Change-Id: I508330a168bf6523b1ec3912efb24159b876f6e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1719853
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681296}

third_party/blink/renderer/core/animation/animation_test.cc

@@ -209,7 +209,7 @@ TEST_F(AnimationAnimationTestNoCompositing, InitialState) {
 
   StartTimeline();
   EXPECT_EQ(Animation::kFinished, animation->PlayStateInternal());
-  EXPECT_EQ(0, timeline->CurrentTimeInternal());
+  EXPECT_EQ(0, timeline->CurrentTimeInternal()->InSecondsF());
   EXPECT_EQ(0, animation->CurrentTimeInternal());
   EXPECT_FALSE(animation->Paused());
   EXPECT_EQ(1, animation->playbackRate());

third_party/blink/renderer/core/animation/css/css_animations.cc

@@ -714,7 +714,8 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
     double inherited_time = old_start_time.has_value()
                                 ? state.animating_element->GetDocument()
                                           .Timeline()
-                                          .CurrentTimeInternal() -
+                                          .CurrentTimeInternal()
+                                          ->InSecondsF() -
                                       old_start_time.value()
                                 : 0;
     std::unique_ptr<TypedInterpolationValue> retargeted_start = SampleAnimation(

third_party/blink/renderer/core/animation/document_timeline.cc

@@ -98,8 +98,7 @@ DocumentTimeline::DocumentTimeline(Document* document,
       zero_time_(base::TimeTicks() + origin_time_),
       zero_time_initialized_(false),
       outdated_animation_count_(0),
-      playback_rate_(1),
-      last_current_time_internal_(0) {
+      playback_rate_(1) {
   if (!timing)
     timing_ = MakeGarbageCollected<DocumentTimelineTiming>(this);
   else
@@ -177,9 +176,7 @@ void DocumentTimeline::ServiceAnimations(TimingUpdateReason reason) {
   }
 
   DCHECK_EQ(outdated_animation_count_, 0U);
-  DCHECK(last_current_time_internal_ == CurrentTimeInternal() ||
-         (std::isnan(CurrentTimeInternal()) &&
-          std::isnan(last_current_time_internal_)));
+  DCHECK(last_current_time_internal_ == CurrentTimeInternal());
 
 #if DCHECK_IS_ON()
   for (const auto& animation : animations_needing_update_)
@@ -233,7 +230,7 @@ void DocumentTimeline::ResetForTesting() {
   zero_time_ = base::TimeTicks() + origin_time_;
   zero_time_initialized_ = true;
   playback_rate_ = 1;
-  last_current_time_internal_ = 0;
+  last_current_time_internal_.reset();
 }
 
 void DocumentTimeline::SetTimingForTesting(PlatformTiming* timing) {
@@ -241,39 +238,33 @@ void DocumentTimeline::SetTimingForTesting(PlatformTiming* timing) {
 }
 
 double DocumentTimeline::currentTime(bool& is_null) {
-  return CurrentTimeInternal(is_null) * 1000;
+  base::Optional<base::TimeDelta> result = CurrentTimeInternal();
+
+  is_null = !result.has_value();
+  return result.has_value() ? result->InMillisecondsF()
+                            : std::numeric_limits<double>::quiet_NaN();
 }
 
-// TODO(npm): change the return type to base::Optional<base::TimeTicks>.
-double DocumentTimeline::CurrentTimeInternal(bool& is_null) {
+base::Optional<base::TimeDelta> DocumentTimeline::CurrentTimeInternal() {
   if (!IsActive()) {
-    is_null = true;
-    return std::numeric_limits<double>::quiet_NaN();
+    return base::nullopt;
   }
 
-  double result =
+  base::Optional<base::TimeDelta> result =
       playback_rate_ == 0
-          ? ZeroTime().since_origin().InSecondsF()
-          : (CurrentAnimationTime(GetDocument()) - ZeroTime()).InSecondsF() *
-                playback_rate_;
-  is_null = std::isnan(result);
-  // This looks like it could never be NaN here.
-  DCHECK(!is_null);
+          ? ZeroTime().since_origin()
+          : (CurrentAnimationTime(GetDocument()) - ZeroTime()) * playback_rate_;
   return result;
 }
 
 double DocumentTimeline::currentTime() {
-  return CurrentTimeInternal() * 1000;
-}
-
-double DocumentTimeline::CurrentTimeInternal() {
-  bool is_null;
-  return CurrentTimeInternal(is_null);
+  base::Optional<base::TimeDelta> result = CurrentTimeInternal();
+  return result.has_value() ? result->InMillisecondsF()
+                            : std::numeric_limits<double>::quiet_NaN();
 }
 
 double DocumentTimeline::EffectiveTime() {
-  double time = CurrentTimeInternal();
-  return std::isnan(time) ? 0 : time;
+  return CurrentTimeInternal().value_or(base::TimeDelta()).InSecondsF();
 }
 
 void DocumentTimeline::PauseAnimationsForTesting(double pause_time) {
@@ -286,10 +277,6 @@ bool DocumentTimeline::NeedsAnimationTimingUpdate() {
   if (CurrentTimeInternal() == last_current_time_internal_)
     return false;
 
-  if (std::isnan(CurrentTimeInternal()) &&
-      std::isnan(last_current_time_internal_))
-    return false;
-
   // We allow |last_current_time_internal_| to advance here when there
   // are no animations to allow animations spawned during style
   // recalc to not invalidate this flag.
@@ -315,13 +302,11 @@ void DocumentTimeline::SetOutdatedAnimation(Animation* animation) {
 void DocumentTimeline::SetPlaybackRate(double playback_rate) {
   if (!IsActive())
     return;
-  double current_time = CurrentTimeInternal();
+  base::TimeDelta current_time = CurrentTimeInternal().value();
   playback_rate_ = playback_rate;
-  zero_time_ =
-      playback_rate == 0
-          ? base::TimeTicks() + base::TimeDelta::FromSecondsD(current_time)
-          : CurrentAnimationTime(GetDocument()) -
-                base::TimeDelta::FromSecondsD(current_time / playback_rate);
+  zero_time_ = playback_rate == 0 ? base::TimeTicks() + current_time
+                                  : CurrentAnimationTime(GetDocument()) -
+                                        current_time / playback_rate;
   zero_time_initialized_ = true;
 
   // Corresponding compositor animation may need to be restarted to pick up

third_party/blink/renderer/core/animation/document_timeline.h

@@ -103,8 +103,7 @@ class CORE_EXPORT DocumentTimeline : public AnimationTimeline {
   base::TimeTicks ZeroTime();
   double currentTime(bool& is_null) override;
   double currentTime();
-  double CurrentTimeInternal(bool& is_null);
-  double CurrentTimeInternal();
+  base::Optional<base::TimeDelta> CurrentTimeInternal();
   double EffectiveTime();
   void PauseAnimationsForTesting(double);
 
@@ -152,7 +151,7 @@ class CORE_EXPORT DocumentTimeline : public AnimationTimeline {
   static const double kMinimumDelay;
 
   Member<PlatformTiming> timing_;
-  double last_current_time_internal_;
+  base::Optional<base::TimeDelta> last_current_time_internal_;
 
   std::unique_ptr<CompositorAnimationTimeline> compositor_timeline_;
 

third_party/blink/renderer/core/svg/animation/smil_time_container.cc

@@ -151,12 +151,20 @@ double SMILTimeContainer::Elapsed() const {
   if (IsPaused())
     return presentation_time_;
 
-  return presentation_time_ +
-         (GetDocument().Timeline().CurrentTimeInternal() - reference_time_);
+  return presentation_time_ + (GetDocument()
+                                   .Timeline()
+                                   .CurrentTimeInternal()
+                                   .value_or(base::TimeDelta())
+                                   .InSecondsF() -
+                               reference_time_);
 }
 
 void SMILTimeContainer::SynchronizeToDocumentTimeline() {
-  reference_time_ = GetDocument().Timeline().CurrentTimeInternal();
+  reference_time_ = GetDocument()
+                        .Timeline()
+                        .CurrentTimeInternal()
+                        .value_or(base::TimeDelta())
+                        .InSecondsF();
 }
 
 bool SMILTimeContainer::IsPaused() const {