Remove NaN from AnimationPlaybackEvent

NaN is used to represent null values for double types. Replace them by Optional<double> so that the base::nullopt represents the null value. This will ease the transition to an free of doubles animation engine. Bug: 994811 Change-Id: I4562999e05faeb284661878b86820c794dbfe5f2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984311 Reviewed-by: Kevin Ellis <kevers@chromium.org> Commit-Queue: Sergio Villar <svillar@igalia.com> Cr-Commit-Position: refs/heads/master@{#728178}
marcosholgado · Jan 3, 2020 · 79b637e · 79b637e
1 parent 8c5da8e
commit 79b637e
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 20 deletions.
diff --git a/third_party/blink/renderer/core/animation/animation.cc b/third_party/blink/renderer/core/animation/animation.cc
@@ -228,10 +228,8 @@ Document* Animation::GetDocument() {
   return document_;
 }
 
-double Animation::TimelineTime() const {
-  if (timeline_)
-    return timeline_->CurrentTime().value_or(NullValue());
-  return NullValue();
+base::Optional<double> Animation::TimelineTime() const {
+  return timeline_ ? timeline_->CurrentTime() : base::nullopt;
 }
 
 // https://drafts.csswg.org/web-animations/#setting-the-current-time-of-an-animation.
@@ -1734,8 +1732,9 @@ bool Animation::Update(TimingUpdateReason reason) {
 void Animation::QueueFinishedEvent() {
   const AtomicString& event_type = event_type_names::kFinish;
   if (GetExecutionContext() && HasEventListeners(event_type)) {
-    double event_current_time =
-        SecondsToMilliseconds(CurrentTimeInternal().value_or(NullValue()));
+    base::Optional<double> event_current_time = CurrentTimeInternal();
+    if (event_current_time)
+      event_current_time = SecondsToMilliseconds(event_current_time.value());
     // TODO(crbug.com/916117): Handle NaN values for scroll-linked animations.
     pending_finished_event_ = MakeGarbageCollected<AnimationPlaybackEvent>(
         event_type, event_current_time, TimelineTime());
@@ -1815,7 +1814,7 @@ void Animation::cancel() {
 
     const AtomicString& event_type = event_type_names::kCancel;
     if (GetExecutionContext() && HasEventListeners(event_type)) {
-      double event_current_time = NullValue();
+      base::Optional<double> event_current_time = base::nullopt;
       // TODO(crbug.com/916117): Handle NaN values for scroll-linked
       // animations.
       pending_cancelled_event_ = MakeGarbageCollected<AnimationPlaybackEvent>(

diff --git a/third_party/blink/renderer/core/animation/animation.h b/third_party/blink/renderer/core/animation/animation.h
@@ -324,7 +324,7 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   void PlayInternal(AutoRewind auto_rewind, ExceptionState& exception_state);
 
   void ResetPendingTasks();
-  double TimelineTime() const;
+  base::Optional<double> TimelineTime() const;
 
   void ScheduleAsyncFinish();
   void AsyncFinishMicrotask();

diff --git a/third_party/blink/renderer/core/events/animation_playback_event.cc b/third_party/blink/renderer/core/events/animation_playback_event.cc
@@ -4,28 +4,32 @@
 
 #include "third_party/blink/renderer/core/events/animation_playback_event.h"
 
+#include "third_party/blink/renderer/core/animation/timing.h"
 #include "third_party/blink/renderer/core/event_interface_names.h"
 
 namespace blink {
 
-AnimationPlaybackEvent::AnimationPlaybackEvent(const AtomicString& type,
-                                               double current_time,
-                                               double timeline_time)
-    : Event(type, Bubbles::kNo, Cancelable::kNo) {
-  if (!std::isnan(current_time))
-    current_time_ = current_time;
-  if (!std::isnan(timeline_time))
-    timeline_time_ = timeline_time;
+AnimationPlaybackEvent::AnimationPlaybackEvent(
+    const AtomicString& type,
+    base::Optional<double> current_time,
+    base::Optional<double> timeline_time)
+    : Event(type, Bubbles::kNo, Cancelable::kNo),
+      current_time_(current_time),
+      timeline_time_(timeline_time) {
+  DCHECK(!current_time_ || !std::isnan(current_time_.value()));
+  DCHECK(!timeline_time_ || !std::isnan(timeline_time_.value()));
 }
 
 AnimationPlaybackEvent::AnimationPlaybackEvent(
     const AtomicString& type,
     const AnimationPlaybackEventInit* initializer)
     : Event(type, initializer) {
   if (initializer->hasCurrentTime())
-    current_time_ = initializer->currentTime();
+    current_time_ = ValueOrUnresolved(initializer->currentTime());
   if (initializer->hasTimelineTime())
-    timeline_time_ = initializer->timelineTime();
+    timeline_time_ = ValueOrUnresolved(initializer->timelineTime());
+  DCHECK(!current_time_ || !std::isnan(current_time_.value()));
+  DCHECK(!timeline_time_ || !std::isnan(timeline_time_.value()));
 }
 
 AnimationPlaybackEvent::~AnimationPlaybackEvent() = default;

diff --git a/third_party/blink/renderer/core/events/animation_playback_event.h b/third_party/blink/renderer/core/events/animation_playback_event.h
@@ -22,8 +22,8 @@ class AnimationPlaybackEvent final : public Event {
   }
 
   AnimationPlaybackEvent(const AtomicString& type,
-                         double current_time,
-                         double timeline_time);
+                         base::Optional<double> current_time,
+                         base::Optional<double> timeline_time);
   AnimationPlaybackEvent(const AtomicString&,
                          const AnimationPlaybackEventInit*);
   ~AnimationPlaybackEvent() override;