Animation: Implement DoubleStyleInterpolation in StringKeyframe

Source/core/animation/StringKeyframe.cpp

Index: Source/core/animation/StringKeyframe.cpp
diff --git a/Source/core/animation/StringKeyframe.cpp b/Source/core/animation/StringKeyframe.cpp
index d3ea1e2c6f4cbbe8c418bc3629fd7cdc80478ef1..766cbf4ec085653ddb4e487e9399a4233b89ca1e 100644
--- a/Source/core/animation/StringKeyframe.cpp
+++ b/Source/core/animation/StringKeyframe.cpp
@@ -97,17 +97,14 @@ PassRefPtrWillBeRawPtr<Interpolation> StringKeyframe::PropertySpecificKeyframe::
     // FIXME: Generate this giant switch statement.
     switch (property) {
     case CSSPropertyLineHeight:
-        if (LengthStyleInterpolation::canCreateFrom(*fromCSSValue) && LengthStyleInterpolation::canCreateFrom(*toCSSValue))
-            return LengthStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, RangeNonNegative);
-
         if (DoubleStyleInterpolation::canCreateFrom(*fromCSSValue) && DoubleStyleInterpolation::canCreateFrom(*toCSSValue))
             return DoubleStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, CSSPrimitiveValue::CSS_NUMBER, RangeNonNegative);
-
-        break;
+        // Fall through
     case CSSPropertyBorderBottomWidth:
     case CSSPropertyBorderLeftWidth:
     case CSSPropertyBorderRightWidth:
     case CSSPropertyBorderTopWidth:
+    case CSSPropertyFlexBasis:
     case CSSPropertyFontSize:
     case CSSPropertyHeight:
     case CSSPropertyMaxHeight:
@@ -122,6 +119,10 @@ PassRefPtrWillBeRawPtr<Interpolation> StringKeyframe::PropertySpecificKeyframe::
     case CSSPropertyPaddingTop:
     case CSSPropertyPerspective:
     case CSSPropertyShapeMargin:
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+    case CSSPropertyWebkitBorderVerticalSpacing:
+    case CSSPropertyWebkitColumnGap:
+    case CSSPropertyWebkitColumnWidth:

[alancutter (OOO until 2018), 2015/02/04 20:24:37]

Adding more LengthStyleInterpolation handling should be done in a separate
patch.

[jadeg, 2015/02/05 23:37:03]

Done.

     case CSSPropertyWidth:
         range = RangeNonNegative;
         // Fall through
@@ -143,7 +144,6 @@ PassRefPtrWillBeRawPtr<Interpolation> StringKeyframe::PropertySpecificKeyframe::
         // FIXME: Handle keywords e.g. 'none'.
         if (property == CSSPropertyPerspective)
             fallBackToLegacy = true;
-
         // FIXME: Handle keywords e.g. 'smaller', 'larger'.
         if (property == CSSPropertyFontSize)
             fallBackToLegacy = true;
@@ -155,14 +155,54 @@ PassRefPtrWillBeRawPtr<Interpolation> StringKeyframe::PropertySpecificKeyframe::
         // FIXME: Handle keywords e.g. 'thick'
         if (property == CSSPropertyOutlineWidth)
             fallBackToLegacy = true;
-
         break;
-    case CSSPropertyMotionRotation:
-        {
-            RefPtrWillBeRawPtr<Interpolation> interpolation = DoubleStyleInterpolation::maybeCreateFromMotionRotation(*fromCSSValue, *toCSSValue, property);
-            if (interpolation)
-                return interpolation.release();
+    case CSSPropertyWidows:
+    case CSSPropertyOrphans:
+        if (range == RangeAll)
+            range = RangeRound;
+        // Fall through
+    case CSSPropertyWebkitColumnCount:
+        if (range == RangeAll)

[dstockwell, 2015/02/04 11:09:28]

This fall through logic is a bit complicated, perhaps this should be extracted
into a separate function?

[jadeg, 2015/02/05 23:37:03]

Done.

+            range = RangeFloor;

[dstockwell, 2015/02/04 11:09:28]

AnimatedStyleBuilder rounds and then clamps greater than 1.

[jadeg, 2015/02/05 23:37:03]

Im confused if you mean that I should change the name of this or change the
clamping?

+        // Fall through
+    case CSSPropertyZIndex:
+        if (range == RangeAll)
+            range = RangeAbsoluteValueFloor;
+        // Fall through
+    case CSSPropertyShapeImageThreshold:

[alancutter (OOO until 2018), 2015/02/04 20:24:37]

This should clamp to [0, 1] instead of [0, 1).

[jadeg, 2015/02/05 23:37:03]

Done.

+    case CSSPropertyFillOpacity:
+    case CSSPropertyFloodOpacity:
+    case CSSPropertyOpacity:
+    case CSSPropertyStopOpacity:
+    case CSSPropertyStrokeOpacity:
+        if (range == RangeAll)
+            range = RangeFractions;

[dstockwell, 2015/02/04 11:09:28]

This seems different to AnimatedStyleBuidler. The clamping here seems to exclude
1?

[jadeg, 2015/02/05 23:37:03]

Yes now has been changed to RangeZeroToLessThanOne

+        // Fall through
+    case CSSPropertyStrokeMiterlimit:
+        if (range == RangeAll)
+            range = RangeGreaterThanOne;
+        // Fall through
+    case CSSPropertyZoom:
+        if (range == RangeAll)
+            range = RangeGreaterThanZero;
+        // Fall through
+    case CSSPropertyWebkitColumnRuleWidth:
+        if (DoubleStyleInterpolation::canCreateFrom(*fromCSSValue) && DoubleStyleInterpolation::canCreateFrom(*toCSSValue)) {
+            if (property == CSSPropertyOpacity) {
+                StringKeyframe::PropertySpecificKeyframe::ensureAnimatableValueCaches(property, end, element, fromCSSValue, toCSSValue);
+            }

[alancutter (OOO until 2018), 2015/02/04 20:24:37]

No need for braces around single line if body.

[jadeg, 2015/02/05 23:37:03]

Done.

+            return DoubleStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, toCSSPrimitiveValue(fromCSSValue)->primitiveType(), range);
+        }
+        break;
+    case CSSPropertyFlexGrow:

[Eric Willigers, 2015/02/04 06:16:27]

Can we use  RangeNonNegative  for flex-grow and flex-shrink?

[jadeg, 2015/02/05 23:37:02]

Done.

+    case CSSPropertyFlexShrink:
+        fallBackToLegacy = true;
+        break;
 
+    case CSSPropertyMotionRotation: {
+        RefPtrWillBeRawPtr<Interpolation> interpolation = DoubleStyleInterpolation::maybeCreateFromMotionRotation(*fromCSSValue, *toCSSValue, property);
+        if (interpolation)
+            return interpolation.release();
             break;
         }
     case CSSPropertyVisibility:
@@ -261,23 +301,15 @@ PassRefPtrWillBeRawPtr<Interpolation> StringKeyframe::PropertySpecificKeyframe::
             return DeferredLegacyStyleInterpolation::create(fromCSSValue, toCSSValue, property);
         }
 
-        // FIXME: Remove the use of AnimatableValues, RenderStyles and Elements here.
-        // FIXME: Remove this cache
-        ASSERT(element);
-        if (!m_animatableValueCache)
-            m_animatableValueCache = StyleResolver::createAnimatableValueSnapshot(*element, property, *fromCSSValue);
-
-        RefPtrWillBeRawPtr<AnimatableValue> to = StyleResolver::createAnimatableValueSnapshot(*element, property, *toCSSValue);
-        toStringPropertySpecificKeyframe(end).m_animatableValueCache = to;
+        StringKeyframe::PropertySpecificKeyframe::ensureAnimatableValueCaches(property, end, element, fromCSSValue, toCSSValue);
 
-        return LegacyStyleInterpolation::create(m_animatableValueCache.get(), to.release(), property);
+        return LegacyStyleInterpolation::create(m_animatableValueCache.get(), toStringPropertySpecificKeyframe(end).m_animatableValueCache.release(), property);
     }
 
     ASSERT(AnimatableValue::usesDefaultInterpolation(
         StyleResolver::createAnimatableValueSnapshot(*element, property, *fromCSSValue).get(),
         StyleResolver::createAnimatableValueSnapshot(*element, property, *toCSSValue).get()));
 
-
     // FIXME: Remove this once TimingFunction partitioning is implemented for all types.
     if (!RuntimeEnabledFeatures::webAnimationsAPITimingFunctionPartitioningEnabled())
         return DefaultStyleInterpolation::create(fromCSSValue, toCSSValue, property);
@@ -289,6 +321,18 @@ PassRefPtrWillBeRawPtr<Interpolation> StringKeyframe::PropertySpecificKeyframe::
 
 }
 
+// FIXME: Remove the use of AnimatableValues, RenderStyles and Elements here.
+// FIXME: Remove this cache
+void StringKeyframe::PropertySpecificKeyframe::ensureAnimatableValueCaches(CSSPropertyID property, Keyframe::PropertySpecificKeyframe& end, Element* element, CSSValue* fromCSSValue, CSSValue* toCSSValue) const

[alancutter (OOO until 2018), 2015/02/04 20:24:37]

element, from and to should be references.

[jadeg, 2015/02/05 23:37:03]

Done.

+{
+    ASSERT(element);
+    if (!m_animatableValueCache)
+        m_animatableValueCache = StyleResolver::createAnimatableValueSnapshot(*element, property, *fromCSSValue);
+    RefPtrWillBeRawPtr<AnimatableValue> to = StyleResolver::createAnimatableValueSnapshot(*element, property, *toCSSValue);
+    toStringPropertySpecificKeyframe(end).m_animatableValueCache = to;
+}
+
+
 PassOwnPtrWillBeRawPtr<Keyframe::PropertySpecificKeyframe> StringKeyframe::PropertySpecificKeyframe::neutralKeyframe(double offset, PassRefPtr<TimingFunction> easing) const
 {
     return adoptPtrWillBeNoop(new PropertySpecificKeyframe(offset, easing, 0, AnimationEffect::CompositeAdd));