Animation: Add template for ListStyleInterpolation

Source/core/animation/ShadowStyleInterpolation.cpp

Index: Source/core/animation/ShadowStyleInterpolation.cpp
diff --git a/Source/core/animation/ShadowStyleInterpolation.cpp b/Source/core/animation/ShadowStyleInterpolation.cpp
index 1b7ab6fb7fb34085e34c0f2090f2f812e4e677c4..cf8d6f75ce23c056cdf65933128ad1b5ede2f8f6 100644
--- a/Source/core/animation/ShadowStyleInterpolation.cpp
+++ b/Source/core/animation/ShadowStyleInterpolation.cpp
@@ -13,60 +13,74 @@
 
 namespace blink {
 
-bool ShadowStyleInterpolation::canCreateFrom(const CSSValue& value)
+bool ShadowStyleInterpolation::canCreateFrom(const CSSValue& start, const CSSValue& end)
 {
-    return value.isShadowValue();
+    if (start.isShadowValue() || end.isShadowValue()) {

[samli, 2015/01/23 03:23:36]

Should be &&

[evemj (not active), 2015/01/28 00:41:14]

Done.

+        return toCSSShadowValue(start).style == toCSSShadowValue(end).style;
+    }
+    return false;

[samli, 2015/01/23 03:23:36]

Just simplify to "return start.isShadowValue() && end.isShadowValue() &&
...(style == style)...;"

[evemj (not active), 2015/01/28 00:41:14]

Done.

 }
-PassOwnPtrWillBeRawPtr<InterpolableValue> ShadowStyleInterpolation::shadowToInterpolableValue(const CSSValue& value)
+
+
+PassOwnPtrWillBeRawPtr<InterpolableValue> ShadowStyleInterpolation::toInterpolableValue(const CSSValue& value, NonInterpolableType& nonInterpolableData)
 {
-    OwnPtrWillBeRawPtr<InterpolableList> result = InterpolableList::create(6);
+    OwnPtrWillBeRawPtr<InterpolableList> result = InterpolableList::create(5);
     const CSSShadowValue* shadowValue = toCSSShadowValue(&value);
     ASSERT(shadowValue);
 
-    result->set(0, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->x));
-    result->set(1, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->y));
-    result->set(2, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->blur));
-    result->set(3, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->spread));
-    result->set(4, ColorStyleInterpolation::colorToInterpolableValue(*shadowValue->color));
+    if (shadowValue->x && LengthStyleInterpolation::canCreateFrom(*shadowValue->x))
+        result->set(0, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->x));
+    else
+        result->set(0, InterpolableBool::create(false));
+    if (shadowValue->y && LengthStyleInterpolation::canCreateFrom(*shadowValue->y))
+        result->set(1, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->y));
+    else
+        result->set(1, InterpolableBool::create(false));
+    if (shadowValue->blur && LengthStyleInterpolation::canCreateFrom(*shadowValue->blur))
+        result->set(2, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->blur));
+    else
+        result->set(2, InterpolableBool::create(false));
+    if (shadowValue->spread && LengthStyleInterpolation::canCreateFrom(*shadowValue->spread))
+        result->set(3, LengthStyleInterpolation::lengthToInterpolableValue(*shadowValue->spread));
+    else
+        result->set(3, InterpolableBool::create(false));
+    if (shadowValue->color && ColorStyleInterpolation::canCreateFrom(*shadowValue->color))
+        result->set(4, ColorStyleInterpolation::colorToInterpolableValue(*shadowValue->color));
+    else
+        result->set(4, InterpolableBool::create(false));

[samli, 2015/01/23 03:23:36]

This is messy and hard to read.

A possible solution is to create a static method in this file which takes a
length-type CSSPrimitiveValue and returns InterpolableValue.

Then for x, y, blur, spread we just have:
result->set(0, PrimitiveValueToInterpolableValue(shadowValue->x));
(maybe choose a better name).

Color will have to be separate, but that's fine.

[evemj (not active), 2015/01/28 00:41:14]

Done.

+    if (shadowValue->style)
+        nonInterpolableData = (shadowValue->style->getValueID() == CSSValueInset);
 
     return result.release();
 }
 
-PassRefPtrWillBeRawPtr<CSSValue> ShadowStyleInterpolation::interpolableValueToShadow(InterpolableValue* value, bool styleFlag)
+PassRefPtrWillBeRawPtr<CSSValue> ShadowStyleInterpolation::fromInterpolableValue(const InterpolableValue& value, NonInterpolableType nonInterpolableData, InterpolationRange range = RangeAll)
 {
-    InterpolableList* shadow = toInterpolableList(value);
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> x = LengthStyleInterpolation::interpolableValueToLength(shadow->get(0), RangeNonNegative);
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> y = LengthStyleInterpolation::interpolableValueToLength(shadow->get(1), RangeNonNegative);
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> blur = LengthStyleInterpolation::interpolableValueToLength(shadow->get(2), RangeNonNegative);
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> spread = LengthStyleInterpolation::interpolableValueToLength(shadow->get(3), RangeNonNegative);
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> color = ColorStyleInterpolation::interpolableValueToColor(*(shadow->get(4)));
-
-    RefPtrWillBeRawPtr<CSSPrimitiveValue> style = styleFlag ? CSSPrimitiveValue::createIdentifier(CSSValueInset) : CSSPrimitiveValue::createIdentifier(CSSValueNone);
+    const InterpolableList* shadow = toInterpolableList(&value);
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> x = (!shadow->get(0)->isBool()) ? LengthStyleInterpolation::interpolableValueToLength(*shadow->get(0),
+        RangeAll) : nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> y = (!shadow->get(1)->isBool()) ? LengthStyleInterpolation::interpolableValueToLength(*shadow->get(1),
+        RangeAll) : nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> blur = (!shadow->get(2)->isBool()) ? LengthStyleInterpolation::interpolableValueToLength(*shadow->get(2),
+        RangeNonNegative) : nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> spread = (!shadow->get(3)->isBool()) ? LengthStyleInterpolation::interpolableValueToLength(*shadow->get(3),
+        RangeAll) : nullptr;

[samli, 2015/01/23 03:23:36]

I think the same would work for these 4 values

[evemj (not active), 2015/01/28 00:41:14]

Done.

+    RefPtrWillBeRawPtr<CSSPrimitiveValue> color = (!shadow->get(4)->isBool()) ? ColorStyleInterpolation::interpolableValueToColor(*shadow->get(4)) : nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> style = nonInterpolableData ? CSSPrimitiveValue::createIdentifier(CSSValueInset) : CSSPrimitiveValue::createIdentifier(CSSValueNone);
 
     RefPtrWillBeRawPtr<CSSShadowValue> result = CSSShadowValue::create(x, y, blur, spread, style, color);
     return result.release();
 }
 
-PassRefPtrWillBeRawPtr<ShadowStyleInterpolation> ShadowStyleInterpolation::maybeCreateFromShadow(const CSSValue& start, const CSSValue& end, CSSPropertyID id)
+bool ShadowStyleInterpolation::usesDefaultStyleInterpolation(const CSSValue& start, const CSSValue& end)
 {
-    if (canCreateFrom(start) && canCreateFrom(end)) {
-        if (toCSSShadowValue(start).style == toCSSShadowValue(end).style) {
-            bool styleFlag = (toCSSShadowValue(start).style->getValueID() == CSSValueInset) ? true : false;
-            return ShadowStyleInterpolation::create(start, end, id, styleFlag);
+    if (start.isValueList() && end.isValueList()) {

[samli, 2015/01/23 03:23:36]

Do we need to check they are the same length as well? (I don't know the answer)

[evemj (not active), 2015/01/28 00:41:14]

Done.

+        for (size_t i = 0; i < toCSSValueList(start).length(); i++) {
+            if (canCreateFrom(*toCSSValueList(start).item(i), *toCSSValueList(end).item(i)))

[samli, 2015/01/23 03:23:36]

Shouldn't this be a "!canCreateFrom()"?

[evemj (not active), 2015/01/28 00:41:13]

Done.

+                return true;
+            }
         }
-    }
-    return nullptr;
+    return false;
 }
 
-
-void ShadowStyleInterpolation::apply(StyleResolverState& state) const
-{
-    StyleBuilder::applyProperty(m_id, state, interpolableValueToShadow(m_cachedValue.get(), m_styleFlag).get());
-}
-
-void ShadowStyleInterpolation::trace(Visitor* visitor)
-{
-    StyleInterpolation::trace(visitor);
-}
-
-}
+} // namespace blink

[jadeg, 2015/01/21 05:36:34]

Newline!!!