Avoid mutating NonInterpolableValue to set additive flags

Instead of having an AdditiveKeyframeHook function which "patches" the
InterpolationValue after it's created, this CL introduces the MakeAdditive
function, which takes an InterpolationValue as input, and returns a new
InterpolationValue with necessary flags set.

This means we will need to copy the NonInterpolableValue in some cases,
which *is* more expensive than poking the existing value. However, I
think it's worth it if we can make NonInterpolableValues immutable
(which will happen in a subsequent CL).

Long term we might want to create the correct NonInterpolableValue in the
first place, by passing the state we need to the converters.

Bug: 981024
Change-Id: I897e176d38346a1c724e79dea028701bd33f922d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724510
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682639}

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

@@ -141,7 +141,7 @@ InterpolationValue CSSInterpolationType::MaybeConvertSingle(
       keyframe, environment, underlying, conversion_checkers);
   if (result && keyframe.Composite() !=
                     EffectModel::CompositeOperation::kCompositeReplace) {
-    AdditiveKeyframeHook(result);
+    return MakeAdditive(std::move(result));
   }
   return result;
 }

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

@@ -63,7 +63,9 @@ class CORE_EXPORT CSSInterpolationType : public InterpolationType {
                                         const InterpolationValue& underlying,
                                         ConversionCheckers&) const final;
 
-  virtual void AdditiveKeyframeHook(InterpolationValue&) const {}
+  virtual InterpolationValue MakeAdditive(InterpolationValue value) const {
+    return value;
+  }
 
   InterpolationValue MaybeConvertUnderlyingValue(
       const InterpolationEnvironment&) const final;

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

@@ -67,6 +67,15 @@ class CSSRotateNonInterpolableValue : public NonInterpolableValue {
         start.IsAdditive(), end.IsAdditive()));
   }
 
+  static scoped_refptr<CSSRotateNonInterpolableValue> CreateAdditive(
+      const CSSRotateNonInterpolableValue& other) {
+    DCHECK(other.is_single_);
+    const bool is_single = true;
+    const bool is_additive = true;
+    return base::AdoptRef(new CSSRotateNonInterpolableValue(
+        is_single, other.start_, other.end_, is_additive, is_additive));
+  }
+
   scoped_refptr<CSSRotateNonInterpolableValue> Composite(
       const CSSRotateNonInterpolableValue& other,
       double other_progress) {
@@ -90,11 +99,6 @@ class CSSRotateNonInterpolableValue : public NonInterpolableValue {
     return Create(OptionalRotation::Slerp(start, end, other_progress));
   }
 
-  void SetSingleAdditive() {
-    DCHECK(is_single_);
-    is_start_additive_ = true;
-  }
-
   OptionalRotation SlerpedRotation(double progress) const {
     DCHECK(!is_start_additive_ && !is_end_additive_);
     DCHECK(!is_single_ || progress == 0);
@@ -208,10 +212,11 @@ InterpolationValue CSSRotateInterpolationType::MaybeConvertValue(
       OptionalRotation(StyleBuilderConverter::ConvertRotation(value)));
 }
 
-void CSSRotateInterpolationType::AdditiveKeyframeHook(
-    InterpolationValue& value) const {
-  ToCSSRotateNonInterpolableValue(*value.non_interpolable_value)
-      .SetSingleAdditive();
+InterpolationValue CSSRotateInterpolationType::MakeAdditive(
+    InterpolationValue value) const {
+  value.non_interpolable_value = CSSRotateNonInterpolableValue::CreateAdditive(
+      ToCSSRotateNonInterpolableValue(*value.non_interpolable_value));
+  return value;
 }
 
 PairwiseInterpolationValue CSSRotateInterpolationType::MaybeMergeSingles(

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

@@ -39,7 +39,7 @@ class CSSRotateInterpolationType : public CSSInterpolationType {
   InterpolationValue MaybeConvertValue(const CSSValue&,
                                        const StyleResolverState*,
                                        ConversionCheckers&) const final;
-  void AdditiveKeyframeHook(InterpolationValue&) const final;
+  InterpolationValue MakeAdditive(InterpolationValue) const final;
 };
 
 }  // namespace blink

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

@@ -91,6 +91,13 @@ class CSSScaleNonInterpolableValue : public NonInterpolableValue {
         new CSSScaleNonInterpolableValue(scale, scale, false, false));
   }
 
+  static scoped_refptr<CSSScaleNonInterpolableValue> CreateAdditive(
+      const CSSScaleNonInterpolableValue& other) {
+    const bool is_additive = true;
+    return base::AdoptRef(new CSSScaleNonInterpolableValue(
+        other.start_, other.end_, is_additive, is_additive));
+  }
+
   static scoped_refptr<CSSScaleNonInterpolableValue> Merge(
       const CSSScaleNonInterpolableValue& start,
       const CSSScaleNonInterpolableValue& end) {
@@ -104,11 +111,6 @@ class CSSScaleNonInterpolableValue : public NonInterpolableValue {
   bool IsStartAdditive() const { return is_start_additive_; }
   bool IsEndAdditive() const { return is_end_additive_; }
 
-  void SetIsAdditive() {
-    is_start_additive_ = true;
-    is_end_additive_ = true;
-  }
-
   DECLARE_NON_INTERPOLABLE_VALUE_TYPE();
 
  private:
@@ -191,9 +193,11 @@ InterpolationValue CSSScaleInterpolationType::MaybeConvertValue(
   }
 }
 
-void CSSScaleInterpolationType::AdditiveKeyframeHook(
-    InterpolationValue& value) const {
-  ToCSSScaleNonInterpolableValue(*value.non_interpolable_value).SetIsAdditive();
+InterpolationValue CSSScaleInterpolationType::MakeAdditive(
+    InterpolationValue value) const {
+  value.non_interpolable_value = CSSScaleNonInterpolableValue::CreateAdditive(
+      ToCSSScaleNonInterpolableValue(*value.non_interpolable_value));
+  return value;
 }
 
 PairwiseInterpolationValue CSSScaleInterpolationType::MaybeMergeSingles(

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

@@ -36,7 +36,7 @@ class CSSScaleInterpolationType : public CSSInterpolationType {
   InterpolationValue MaybeConvertValue(const CSSValue&,
                                        const StyleResolverState*,
                                        ConversionCheckers&) const final;
-  void AdditiveKeyframeHook(InterpolationValue&) const final;
+  InterpolationValue MakeAdditive(InterpolationValue) const final;
 
   PairwiseInterpolationValue MaybeMergeSingles(
       InterpolationValue&&,

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

@@ -28,6 +28,16 @@ class CSSTransformNonInterpolableValue : public NonInterpolableValue {
         true, std::move(transform), EmptyTransformOperations(), false, false));
   }
 
+  static scoped_refptr<CSSTransformNonInterpolableValue> CreateAdditive(
+      const CSSTransformNonInterpolableValue& other) {
+    DCHECK(other.is_single_);
+    const bool is_single = true;
+    const bool is_additive = true;
+    return base::AdoptRef(new CSSTransformNonInterpolableValue(
+        is_single, TransformOperations(other.start_),
+        TransformOperations(other.end_), is_additive, is_additive));
+  }
+
   static scoped_refptr<CSSTransformNonInterpolableValue> Create(
       CSSTransformNonInterpolableValue&& start,
       double start_fraction,
@@ -62,12 +72,6 @@ class CSSTransformNonInterpolableValue : public NonInterpolableValue {
     return Create(end.Blend(start, other_progress));
   }
 
-  void SetSingleAdditive() {
-    DCHECK(is_single_);
-    is_start_additive_ = true;
-    is_end_additive_ = true;
-  }
-
   TransformOperations GetInterpolatedTransform(double progress) const {
     if (progress == 0)
       return start_;
@@ -206,10 +210,12 @@ InterpolationValue CSSTransformInterpolationType::MaybeConvertValue(
   return ConvertTransform(std::move(transform));
 }
 
-void CSSTransformInterpolationType::AdditiveKeyframeHook(
-    InterpolationValue& value) const {
-  ToCSSTransformNonInterpolableValue(*value.non_interpolable_value)
-      .SetSingleAdditive();
+InterpolationValue CSSTransformInterpolationType::MakeAdditive(
+    InterpolationValue value) const {
+  value.non_interpolable_value =
+      CSSTransformNonInterpolableValue::CreateAdditive(
+          ToCSSTransformNonInterpolableValue(*value.non_interpolable_value));
+  return value;
 }
 
 PairwiseInterpolationValue CSSTransformInterpolationType::MaybeMergeSingles(

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

@@ -39,7 +39,7 @@ class CSSTransformInterpolationType : public CSSInterpolationType {
   InterpolationValue MaybeConvertValue(const CSSValue&,
                                        const StyleResolverState*,
                                        ConversionCheckers&) const final;
-  void AdditiveKeyframeHook(InterpolationValue&) const final;
+  InterpolationValue MakeAdditive(InterpolationValue) const final;
 };
 
 }  // namespace blink