animation: Move global NullValue and IsNull into Timing class

In third_party/blink/renderer/core/animation/timing.h,
there are static inline functions defined, which are harmful
from a PoV of namespace contamination.  (Static inline functions
in *.h are no good in the first place.)

Moves IsNull and NullValue into Timing struct, and removes
ValueOrUnresolved.

Change-Id: I777740c4ce020dc78350fb2b5c9322db10660705
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106988Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751391}

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

@@ -396,7 +396,7 @@ double Animation::currentTimeForBinding(bool& is_null) {
 }
 
 double Animation::currentTime() const {
-  return currentTimeForBinding().value_or(NullValue());
+  return currentTimeForBinding().value_or(Timing::NullValue());
 }
 
 double Animation::currentTime(bool& is_null) {
@@ -572,7 +572,7 @@ void Animation::NotifyReady(double ready_time) {
 // Refer to Step 8.3 'pending play task' in
 // https://drafts.csswg.org/web-animations/#playing-an-animation-section.
 void Animation::CommitPendingPlay(double ready_time) {
-  DCHECK(!IsNull(ready_time));
+  DCHECK(!Timing::IsNull(ready_time));
   DCHECK(start_time_ || hold_time_);
   DCHECK(pending_play_);
   pending_play_ = false;
@@ -1366,12 +1366,12 @@ void Animation::updatePlaybackRate(double playback_rate,
           timeline_ ? timeline_->CurrentTimeSeconds() : base::nullopt;
       if (playback_rate) {
         if (timeline_time) {
-          start_time_ =
-              (timeline_time && unconstrained_current_time)
-                  ? ValueOrUnresolved((timeline_time.value() -
-                                       unconstrained_current_time.value()) /
-                                      playback_rate)
-                  : base::nullopt;
+          start_time_ = (timeline_time && unconstrained_current_time)
+                            ? base::make_optional<double>(
+                                  (timeline_time.value() -
+                                   unconstrained_current_time.value()) /
+                                  playback_rate)
+                            : base::nullopt;
         }
       } else {
         start_time_ = timeline_time;
@@ -1487,7 +1487,7 @@ void Animation::setPlaybackRate(double playback_rate,
   pending_playback_rate_ = base::nullopt;
   double previous_current_time = currentTime();
   playback_rate_ = playback_rate;
-  if (!IsNull(previous_current_time)) {
+  if (!Timing::IsNull(previous_current_time)) {
     setCurrentTime(previous_current_time, false, exception_state);
   }
 
@@ -1625,7 +1625,7 @@ void Animation::StartAnimationOnCompositor(
     time_offset = time_offset / fabs(EffectivePlaybackRate());
   }
 
-  DCHECK(!start_time || !IsNull(start_time.value()));
+  DCHECK(!start_time || !Timing::IsNull(start_time.value()));
   DCHECK_NE(compositor_group_, 0);
   DCHECK(To<KeyframeEffect>(content_.Get()));
   DCHECK(std::isfinite(time_offset));

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

@@ -98,7 +98,7 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable {
     return EnsureCalculated().time_to_reverse_effect_change;
   }
   double LocalTime() const {
-    return EnsureCalculated().local_time.value_or(NullValue());
+    return EnsureCalculated().local_time.value_or(Timing::NullValue());
   }
 
   const Timing& SpecifiedTiming() const { return timing_; }

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

@@ -98,7 +98,7 @@ class TestAnimationEffect : public AnimationEffect {
       bool forwards,
       base::Optional<double> local_time,
       AnimationTimeDelta time_to_next_iteration) const override {
-    DCHECK(!local_time || !IsNull(local_time.value()));
+    DCHECK(!local_time || !Timing::IsNull(local_time.value()));
     local_time_ = local_time;
     time_to_next_iteration_ = time_to_next_iteration;
     return AnimationTimeDelta::FromSecondsD(

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

@@ -16,7 +16,7 @@ Keyframe::PropertySpecificKeyframe::PropertySpecificKeyframe(
     scoped_refptr<TimingFunction> easing,
     EffectModel::CompositeOperation composite)
     : offset_(offset), easing_(std::move(easing)), composite_(composite) {
-  DCHECK(!IsNull(offset));
+  DCHECK(!Timing::IsNull(offset));
   if (!easing_)
     easing_ = LinearTimingFunction::Shared();
 }

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

@@ -45,24 +45,6 @@ namespace blink {
 class EffectTiming;
 class ComputedEffectTiming;
 
-static inline bool IsNull(double value) {
-  return std::isnan(value);
-}
-
-static inline double NullValue() {
-  return std::numeric_limits<double>::quiet_NaN();
-}
-
-static inline base::Optional<double> ValueOrUnresolved(double a) {
-  return IsNull(a) ? base::nullopt : base::Optional<double>(a);
-}
-
-// TODO(crbug.com/1060971): Remove both ValueOrUnresolved().
-static inline base::Optional<double> ValueOrUnresolved(
-    const base::Optional<double>& value) {
-  return value;
-}
-
 struct CORE_EXPORT Timing {
   USING_FAST_MALLOC(Timing);
 
@@ -84,6 +66,9 @@ struct CORE_EXPORT Timing {
   using FillMode = CompositorKeyframeModel::FillMode;
   using PlaybackDirection = CompositorKeyframeModel::Direction;
 
+  static bool IsNull(double value) { return std::isnan(value); }
+  static double NullValue() { return std::numeric_limits<double>::quiet_NaN(); }
+
   static String FillModeString(FillMode);
   static FillMode StringToFillMode(const String&);
   static String PlaybackDirectionString(PlaybackDirection);

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

@@ -55,14 +55,14 @@ inline bool EndsOnIterationBoundary(double iteration_count,
 }  // namespace
 
 static inline double MultiplyZeroAlwaysGivesZero(double x, double y) {
-  DCHECK(!IsNull(x));
-  DCHECK(!IsNull(y));
+  DCHECK(!Timing::IsNull(x));
+  DCHECK(!Timing::IsNull(y));
   return x && y ? x * y : 0;
 }
 
 static inline double MultiplyZeroAlwaysGivesZero(AnimationTimeDelta x,
                                                  double y) {
-  DCHECK(!IsNull(y));
+  DCHECK(!Timing::IsNull(y));
   return x.is_zero() || y == 0 ? 0 : (x * y).InSecondsF();
 }
 

third_party/blink/renderer/core/events/animation_playback_event.cc

@@ -25,10 +25,11 @@ AnimationPlaybackEvent::AnimationPlaybackEvent(
     const AtomicString& type,
     const AnimationPlaybackEventInit* initializer)
     : Event(type, initializer) {
-  if (initializer->hasCurrentTime())
-    current_time_ = ValueOrUnresolved(initializer->currentTime());
-  if (initializer->hasTimelineTime())
-    timeline_time_ = ValueOrUnresolved(initializer->timelineTime());
+  if (initializer->hasCurrentTime() && !std::isnan(initializer->currentTime()))
+    current_time_ = initializer->currentTime();
+  if (initializer->hasTimelineTime() &&
+      !std::isnan(initializer->timelineTime()))
+    timeline_time_ = initializer->timelineTime();
   DCHECK(!current_time_ || !std::isnan(current_time_.value()));
   DCHECK(!timeline_time_ || !std::isnan(timeline_time_.value()));
 }

third_party/blink/renderer/core/inspector/inspector_animation_agent.cc

@@ -230,10 +230,10 @@ Response InspectorAnimationAgent::getCurrentTime(const String& id,
     // Use startTime where possible since currentTime is limited.
     base::Optional<double> timeline_time = animation->timeline()->CurrentTime();
     // TODO(crbug.com/916117): Handle NaN values for scroll linked animations.
-    *current_time = timeline_time
-                        ? timeline_time.value() -
-                              animation->startTime().value_or(NullValue())
-                        : NullValue();
+    *current_time =
+        timeline_time ? timeline_time.value() -
+                            animation->startTime().value_or(Timing::NullValue())
+                      : Timing::NullValue();
   }
   return Response::OK();
 }
@@ -257,10 +257,10 @@ Response InspectorAnimationAgent::setPaused(
       } else {
         base::Optional<double> timeline_time = clone->timeline()->CurrentTime();
         // TODO(crbug.com/916117): Handle NaN values.
-        current_time = timeline_time
-                           ? timeline_time.value() -
-                                 clone->startTime().value_or(NullValue())
-                           : NullValue();
+        current_time =
+            timeline_time ? timeline_time.value() -
+                                clone->startTime().value_or(Timing::NullValue())
+                          : Timing::NullValue();
       }
       clone->pause();
       clone->setCurrentTime(current_time, false);
@@ -310,7 +310,8 @@ blink::Animation* InspectorAnimationAgent::AnimationClone(
     id_to_animation_clone_.Set(id, clone);
     id_to_animation_.Set(String::Number(clone->SequenceNumber()), clone);
     clone->play();
-    clone->setStartTime(animation->startTime().value_or(NullValue()), false);
+    clone->setStartTime(animation->startTime().value_or(Timing::NullValue()),
+                        false);
 
     animation->SetEffectSuppressed(true);
   }
@@ -509,7 +510,7 @@ DocumentTimeline& InspectorAnimationAgent::ReferenceTimeline() {
 
 double InspectorAnimationAgent::NormalizedStartTime(
     blink::Animation& animation) {
-  double time_ms = animation.startTime().value_or(NullValue());
+  double time_ms = animation.startTime().value_or(Timing::NullValue());
   auto* document_timeline = DynamicTo<DocumentTimeline>(animation.timeline());
   if (document_timeline) {
     if (ReferenceTimeline().PlaybackRate() == 0) {