Fix float-cast overflow in SVGNumberInterpolationType

InterpolableNumber can represent doubles, but SVGNumber can only
represent floats. Previously this conversion was implicit and resulted
in +/- inf for non-representable doubles; this CL makes it explicit and
clamps the result to max/min float instead.

Clamping rather than infinity was chosen arbitrarily just based on
existing WTF math helpers; it is unlikely that it results in much visual
difference (since max-float is a very big number already...).

Bug: 961859
Change-Id: I675471c552b2bd0419698a64d3a1c68c66d8a3ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610465
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659198}

third_party/blink/renderer/core/animation/BUILD.gn

@@ -264,6 +264,7 @@ blink_core_tests("unit_tests") {
     "property_handle_test.cc",
     "scroll_timeline_test.cc",
     "scroll_timeline_util_test.cc",
+    "svg_number_interpolation_type_test.cc",
     "timing_calculations_test.cc",
     "timing_input_test.cc",
   ]

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

@@ -14,6 +14,7 @@
 #include "third_party/blink/renderer/core/animation/primitive_interpolation.h"
 #include "third_party/blink/renderer/core/animation/property_handle.h"
 #include "third_party/blink/renderer/core/animation/underlying_value_owner.h"
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/wtf/allocator.h"
 
@@ -28,7 +29,7 @@ class InterpolationEnvironment;
 // - Convert the target Element's property value to an InterpolationValue:
 // maybeConvertUnderlyingValue()
 // - Apply an InterpolationValue to a target Element's property: apply().
-class InterpolationType {
+class CORE_EXPORT InterpolationType {
   USING_FAST_MALLOC(InterpolationType);
 
  public:

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

@@ -7,11 +7,13 @@
 
 #include "third_party/blink/renderer/core/animation/interpolation_type.h"
 
+#include "third_party/blink/renderer/core/core_export.h"
+
 namespace blink {
 
 class SVGPropertyBase;
 
-class SVGInterpolationType : public InterpolationType {
+class CORE_EXPORT SVGInterpolationType : public InterpolationType {
  protected:
   SVGInterpolationType(const QualifiedName& attribute)
       : InterpolationType(PropertyHandle(attribute)) {}

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

@@ -11,9 +11,16 @@
 #include "third_party/blink/renderer/core/svg/properties/svg_animated_property.h"
 #include "third_party/blink/renderer/core/svg/svg_number.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/wtf/math_extras.h"
 
 namespace blink {
 
+SVGPropertyBase* SVGNumberInterpolationType::AppliedSVGValueForTesting(
+    const InterpolableValue& interpolable_value,
+    const NonInterpolableValue* non_interpolable_value) const {
+  return AppliedSVGValue(interpolable_value, non_interpolable_value);
+}
+
 InterpolationValue SVGNumberInterpolationType::MaybeConvertNeutral(
     const InterpolationValue&,
     ConversionCheckers&) const {
@@ -31,7 +38,8 @@ InterpolationValue SVGNumberInterpolationType::MaybeConvertSVGValue(
 SVGPropertyBase* SVGNumberInterpolationType::AppliedSVGValue(
     const InterpolableValue& interpolable_value,
     const NonInterpolableValue*) const {
-  double value = ToInterpolableNumber(interpolable_value).Value();
+  float value =
+      clampTo<float>(ToInterpolableNumber(interpolable_value).Value());
   return MakeGarbageCollected<SVGNumber>(is_non_negative_ && value < 0 ? 0
                                                                        : value);
 }

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

@@ -6,16 +6,20 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_ANIMATION_SVG_NUMBER_INTERPOLATION_TYPE_H_
 
 #include "third_party/blink/renderer/core/animation/svg_interpolation_type.h"
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/svg_names.h"
 
 namespace blink {
 
-class SVGNumberInterpolationType : public SVGInterpolationType {
+class CORE_EXPORT SVGNumberInterpolationType : public SVGInterpolationType {
  public:
   SVGNumberInterpolationType(const QualifiedName& attribute)
       : SVGInterpolationType(attribute),
         is_non_negative_(attribute == svg_names::kPathLengthAttr) {}
 
+  SVGPropertyBase* AppliedSVGValueForTesting(const InterpolableValue&,
+                                             const NonInterpolableValue*) const;
+
  private:
   InterpolationValue MaybeConvertNeutral(const InterpolationValue& underlying,
                                          ConversionCheckers&) const final;

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/animation/svg_number_interpolation_type.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/svg/svg_number.h"
+
+namespace blink {
+
+TEST(SVGNumberInterpolationTypeTest, NonNegativeSVGNumber) {
+  // kPathLengthAttr implies non-negative.
+  SVGNumberInterpolationType interpolation_type(svg_names::kPathLengthAttr);
+
+  SVGNumber* svg_number =
+      static_cast<SVGNumber*>(interpolation_type.AppliedSVGValueForTesting(
+          InterpolableNumber(5), nullptr));
+  EXPECT_EQ(svg_number->Value(), 5);
+
+  svg_number =
+      static_cast<SVGNumber*>(interpolation_type.AppliedSVGValueForTesting(
+          InterpolableNumber(-5), nullptr));
+  EXPECT_EQ(svg_number->Value(), 0);
+}
+
+TEST(SVGNumberInterpolationTypeTest, NegativeSVGNumber) {
+  // kOffsetAttr can be negative.
+  SVGNumberInterpolationType interpolation_type(svg_names::kOffsetAttr);
+
+  SVGNumber* svg_number =
+      static_cast<SVGNumber*>(interpolation_type.AppliedSVGValueForTesting(
+          InterpolableNumber(5), nullptr));
+  EXPECT_EQ(svg_number->Value(), 5);
+
+  svg_number =
+      static_cast<SVGNumber*>(interpolation_type.AppliedSVGValueForTesting(
+          InterpolableNumber(-5), nullptr));
+  EXPECT_EQ(svg_number->Value(), -5);
+}
+
+// This is a regression test for https://crbug.com/961859. InterpolableNumber
+// can represent a double, but SVGNumber is created from a float, so we must
+// make sure to clamp it.
+TEST(SVGNumberInterpolationTypeTest, InterpolableNumberOutOfRange) {
+  SVGNumberInterpolationType interpolation_type(svg_names::kOffsetAttr);
+
+  double too_large = std::numeric_limits<float>::max() * 2;
+  SVGNumber* svg_number =
+      static_cast<SVGNumber*>(interpolation_type.AppliedSVGValueForTesting(
+          InterpolableNumber(too_large), nullptr));
+  EXPECT_EQ(svg_number->Value(), std::numeric_limits<float>::max());
+}
+
+}  // namespace blink