Migrate the remaining CSS properties interpolable as Lengths to LengthInterpolationType

Source/core/animation/LengthPropertyFunctions.cpp

Index: Source/core/animation/LengthPropertyFunctions.cpp
diff --git a/Source/core/animation/LengthPropertyFunctions.cpp b/Source/core/animation/LengthPropertyFunctions.cpp
index 64d133fc49981883a37d3538c6b4642ae0973772..aae1e421ec8e8f058dadfc9a8b39f0a014aec77e 100644
--- a/Source/core/animation/LengthPropertyFunctions.cpp
+++ b/Source/core/animation/LengthPropertyFunctions.cpp
@@ -13,26 +13,426 @@ namespace blink {
 
 ValueRange LengthPropertyFunctions::valueRange(CSSPropertyID property)
 {
-    ASSERT(property == CSSPropertyLeft);
-    return ValueRangeAll;
+    switch (property) {
+    case CSSPropertyBorderBottomWidth:
+    case CSSPropertyBorderLeftWidth:
+    case CSSPropertyBorderRightWidth:
+    case CSSPropertyBorderTopWidth:
+    case CSSPropertyFlexBasis:
+    case CSSPropertyHeight:
+    case CSSPropertyLineHeight:
+    case CSSPropertyMaxHeight:
+    case CSSPropertyMaxWidth:
+    case CSSPropertyMinHeight:
+    case CSSPropertyMinWidth:
+    case CSSPropertyOutlineWidth:
+    case CSSPropertyPaddingBottom:
+    case CSSPropertyPaddingLeft:
+    case CSSPropertyPaddingRight:
+    case CSSPropertyPaddingTop:
+    case CSSPropertyPerspective:
+    case CSSPropertyR:
+    case CSSPropertyRx:
+    case CSSPropertyRy:
+    case CSSPropertyShapeMargin:
+    case CSSPropertyStrokeWidth:
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+    case CSSPropertyWebkitBorderVerticalSpacing:
+    case CSSPropertyWebkitColumnGap:
+    case CSSPropertyWebkitColumnWidth:
+    case CSSPropertyWidth:
+        return ValueRangeNonNegative;
+    default:
+        return ValueRangeAll;
+    }
+}
+
+bool LengthPropertyFunctions::hasZoomedLength(CSSPropertyID property)
+{
+    return property != CSSPropertyStrokeWidth;
 }
 
 bool LengthPropertyFunctions::getPixelsForKeyword(CSSPropertyID property, CSSValueID valueID, double& result)

[dstockwell, 2015/08/31 05:33:39]

Do we usually say 'pixels' or 'px'?

Why do we only support pixels for keywords? Is it just that all the existing
keywords are in pixels?

[alancutter (OOO until 2018), 2015/08/31 07:15:30]

All the existing keywords are in pixels.

 {
-    ASSERT(property == CSSPropertyLeft);
-    return false;
+    switch (property) {
+    case CSSPropertyBaselineShift:
+        if (valueID == CSSValueBaseline) {

[dstockwell, 2015/08/31 05:33:39]

This looks painful to maintain, how can we keep it in sync?

Maybe we should write a guide on animation considerations for changes in css?

[alancutter (OOO until 2018), 2015/08/31 07:15:29]

We could expose this function to StyleBuilderConverter to avoid duplication of
logic, the only issue is where the code lives might be awkward. I suspect the
Compartmentalisation of CSS Properties will provide adequate housing for logic
like this.

+            result = 0;
+            return true;
+        }
+        return false;
+    case CSSPropertyBorderBottomWidth:
+    case CSSPropertyBorderLeftWidth:
+    case CSSPropertyBorderRightWidth:
+    case CSSPropertyBorderTopWidth:
+    case CSSPropertyWebkitColumnRuleWidth:
+    case CSSPropertyOutlineWidth:
+        if (valueID == CSSValueThin) {
+            result = 1;
+            return true;
+        }
+        if (valueID == CSSValueMedium) {
+            result = 3;
+            return true;
+        }
+        if (valueID == CSSValueThick) {
+            result = 5;
+            return true;
+        }
+        return false;
+    case CSSPropertyLetterSpacing:
+    case CSSPropertyWordSpacing:
+        if (valueID == CSSValueNormal) {
+            result = 0;
+            return true;
+        }
+        return false;
+    default:
+        return false;
+    }
 }
 
-bool LengthPropertyFunctions::getLength(CSSPropertyID property, const ComputedStyle& style, Length& result)
+static Length lengthFromUnsigned(unsigned short value)
 {
-    ASSERT(property == CSSPropertyLeft);
-    result = style.left();
-    return true;
+    return Length(static_cast<float>(value), Fixed);
 }
 
 bool LengthPropertyFunctions::getInitialLength(CSSPropertyID property, Length& result)
 {
-    return getLength(property, *ComputedStyle::initialStyle(), result);
+    switch (property) {
+    // The computed value of "initial" for the following properties is 0px if the associated *-style property resolves to "none" or "hidden".
+    // border-width: https://drafts.csswg.org/css-backgrounds-3/#the-border-width
+    // outline-width: https://drafts.csswg.org/css-ui-3/#outline-width
+    // -webkit-column-rule-width: https://drafts.csswg.org/css-multicol-1/#crw
+    // We ignore this value adjustment for animations use the wrong value for hidden widths.
+    // This is acceptable since animations on hidden widths are unobservable to the user, even via getComputedStyle().
+    case CSSPropertyBorderBottomWidth:

[dstockwell, 2015/08/31 05:33:39]

Why do we need to treat these differently?

[alancutter (OOO until 2018), 2015/08/31 07:15:30]

See above comment.

[alancutter (OOO until 2018), 2015/08/31 07:20:51]

Updated comment to not have grammatical error.

+    case CSSPropertyBorderLeftWidth:
+    case CSSPropertyBorderRightWidth:
+    case CSSPropertyBorderTopWidth:
+        result = lengthFromUnsigned(ComputedStyle::initialBorderWidth());
+        return true;
+    case CSSPropertyOutlineWidth:
+        result = lengthFromUnsigned(ComputedStyle::initialOutlineWidth());
+        return true;
+    case CSSPropertyWebkitColumnRuleWidth:
+        result = lengthFromUnsigned(ComputedStyle::initialColumnRuleWidth());
+        return true;
+
+    default:
+        return getLength(property, *ComputedStyle::initialStyle(), result);
+    }
+}
+
+bool LengthPropertyFunctions::getLength(CSSPropertyID property, const ComputedStyle& style, Length& result)
+{
+    switch (property) {
+    case CSSPropertyBottom:
+        result = style.bottom();
+        return true;
+    case CSSPropertyCx:
+        result = style.svgStyle().cx();
+        return true;
+    case CSSPropertyCy:
+        result = style.svgStyle().cy();
+        return true;
+    case CSSPropertyFlexBasis:
+        result = style.flexBasis();
+        return true;
+    case CSSPropertyHeight:
+        result = style.height();
+        return true;
+    case CSSPropertyLeft:
+        result = style.left();
+        return true;
+    case CSSPropertyMarginBottom:
+        result = style.marginBottom();
+        return true;
+    case CSSPropertyMarginLeft:
+        result = style.marginLeft();
+        return true;
+    case CSSPropertyMarginRight:
+        result = style.marginRight();
+        return true;
+    case CSSPropertyMarginTop:
+        result = style.marginTop();
+        return true;
+    case CSSPropertyMaxHeight:
+        result = style.maxHeight();
+        return true;
+    case CSSPropertyMaxWidth:
+        result = style.maxWidth();
+        return true;
+    case CSSPropertyMinHeight:
+        result = style.minHeight();
+        return true;
+    case CSSPropertyMinWidth:
+        result = style.minWidth();
+        return true;
+    case CSSPropertyMotionOffset:
+        result = style.motionOffset();
+        return true;
+    case CSSPropertyPaddingBottom:
+        result = style.paddingBottom();
+        return true;
+    case CSSPropertyPaddingLeft:
+        result = style.paddingLeft();
+        return true;
+    case CSSPropertyPaddingRight:
+        result = style.paddingRight();
+        return true;
+    case CSSPropertyPaddingTop:
+        result = style.paddingTop();
+        return true;
+    case CSSPropertyR:
+        result = style.svgStyle().r();
+        return true;
+    case CSSPropertyRight:
+        result = style.right();
+        return true;
+    case CSSPropertyRx:
+        result = style.svgStyle().rx();
+        return true;
+    case CSSPropertyRy:
+        result = style.svgStyle().ry();
+        return true;
+    case CSSPropertyShapeMargin:
+        result = style.shapeMargin();
+        return true;
+    case CSSPropertyStrokeDashoffset:
+        result = style.strokeDashOffset();
+        return true;
+    case CSSPropertyTextIndent:
+        result = style.textIndent();
+        return true;
+    case CSSPropertyTop:
+        result = style.top();
+        return true;
+    case CSSPropertyWebkitPerspectiveOriginX:
+        result = style.perspectiveOriginX();
+        return true;
+    case CSSPropertyWebkitPerspectiveOriginY:
+        result = style.perspectiveOriginY();
+        return true;
+    case CSSPropertyWebkitTransformOriginX:
+        result = style.transformOriginX();
+        return true;
+    case CSSPropertyWebkitTransformOriginY:
+        result = style.transformOriginY();
+        return true;
+    case CSSPropertyWidth:
+        result = style.width();
+        return true;
+    case CSSPropertyX:
+        result = style.svgStyle().x();
+        return true;
+    case CSSPropertyY:
+        result = style.svgStyle().y();
+        return true;
+
+    case CSSPropertyBorderBottomWidth:
+        result = Length(style.borderBottomWidth(), Fixed);
+        return true;
+    case CSSPropertyBorderLeftWidth:
+        result = Length(style.borderLeftWidth(), Fixed);
+        return true;
+    case CSSPropertyBorderRightWidth:
+        result = Length(style.borderRightWidth(), Fixed);
+        return true;
+    case CSSPropertyBorderTopWidth:
+        result = Length(style.borderTopWidth(), Fixed);
+        return true;
+    case CSSPropertyLetterSpacing:
+        result = Length(style.letterSpacing(), Fixed);
+        return true;
+    case CSSPropertyOutlineOffset:
+        result = Length(style.outlineOffset(), Fixed);
+        return true;
+    case CSSPropertyOutlineWidth:
+        result = Length(style.outlineWidth(), Fixed);
+        return true;
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+        result = Length(style.horizontalBorderSpacing(), Fixed);
+        return true;
+    case CSSPropertyWebkitBorderVerticalSpacing:
+        result = Length(style.verticalBorderSpacing(), Fixed);
+        return true;
+    case CSSPropertyWebkitColumnGap:
+        result = Length(style.columnGap(), Fixed);
+        return true;
+    case CSSPropertyWebkitColumnRuleWidth:
+        result = Length(style.columnRuleWidth(), Fixed);
+        return true;
+    case CSSPropertyWebkitTransformOriginZ:
+        result = Length(style.transformOriginZ(), Fixed);
+        return true;
+    case CSSPropertyWordSpacing:
+        result = Length(style.wordSpacing(), Fixed);
+        return true;
+
+    case CSSPropertyBaselineShift:
+        if (style.baselineShift() != BS_LENGTH)
+            return false;
+        result = style.baselineShiftValue();
+        return true;
+    case CSSPropertyPerspective:
+        // 0 represents the "none" keyword.

[dstockwell, 2015/08/31 05:33:39]

We should add a style.hasPerspective and assert in perspective() that !0
(separately).

[alancutter (OOO until 2018), 2015/08/31 07:15:30]

Turns out hasPerspective() already exists. Adding the assert in perspective
would be a separate patch.

+        if (style.perspective() == 0)
+            return false;
+        result = Length(style.perspective(), Fixed);
+        return true;
+    case CSSPropertyStrokeWidth:
+        ASSERT(!hasZoomedLength(CSSPropertyStrokeWidth));

[dstockwell, 2015/08/31 05:33:39]

Why do we assert this?

[alancutter (OOO until 2018), 2015/08/31 07:15:29]

style.strokeWidth() returns an UnzoomedLength from which we pull out a Length.
The assert is to justify dropping the Unzoomed part since we account for the
non-zoomed nature of the value in other parts of the API.

+        result = style.strokeWidth().length();
+        return true;
+    case CSSPropertyVerticalAlign:
+        if (style.verticalAlign() != LENGTH)
+            return false;
+        result = style.verticalAlignLength();
+        return true;
+    case CSSPropertyWebkitColumnWidth:
+        if (style.hasAutoColumnWidth())
+            return false;
+        result = Length(style.columnWidth(), Fixed);
+        return true;
+    default:
+        return false;
+    }
+}
+
+bool LengthPropertyFunctions::setLength(CSSPropertyID property, ComputedStyle& style, const Length& value)

[dstockwell, 2015/08/31 05:33:39]

Why did we move away from the setter based approach?

[alancutter (OOO until 2018), 2015/08/31 07:15:30]

Windows was getting crazy pants runtime crashes that seemed to be a result of
using function pointers across compilation units. I didn't fully investigate it
because I can't use function pointers for line-height anyway (later patch).

+{
+    switch (property) {
+    // Setters that take a Length value.
+    case CSSPropertyBaselineShift:
+        style.setBaselineShiftValue(value);
+        return true;
+    case CSSPropertyBottom:
+        style.setBottom(value);
+        return true;
+    case CSSPropertyCx:
+        style.setCx(value);
+        return true;
+    case CSSPropertyCy:
+        style.setCy(value);
+        return true;
+    case CSSPropertyFlexBasis:
+        style.setFlexBasis(value);
+        return true;
+    case CSSPropertyHeight:
+        style.setHeight(value);
+        return true;
+    case CSSPropertyLeft:
+        style.setLeft(value);
+        return true;
+    case CSSPropertyLineHeight:
+        style.setLineHeight(value);
+        return true;
+    case CSSPropertyMarginBottom:
+        style.setMarginBottom(value);
+        return true;
+    case CSSPropertyMarginLeft:
+        style.setMarginLeft(value);
+        return true;
+    case CSSPropertyMarginRight:
+        style.setMarginRight(value);
+        return true;
+    case CSSPropertyMarginTop:
+        style.setMarginTop(value);
+        return true;
+    case CSSPropertyMaxHeight:
+        style.setMaxHeight(value);
+        return true;
+    case CSSPropertyMaxWidth:
+        style.setMaxWidth(value);
+        return true;
+    case CSSPropertyMinHeight:
+        style.setMinHeight(value);
+        return true;
+    case CSSPropertyMinWidth:
+        style.setMinWidth(value);
+        return true;
+    case CSSPropertyMotionOffset:
+        style.setMotionOffset(value);
+        return true;
+    case CSSPropertyPaddingBottom:
+        style.setPaddingBottom(value);
+        return true;
+    case CSSPropertyPaddingLeft:
+        style.setPaddingLeft(value);
+        return true;
+    case CSSPropertyPaddingRight:
+        style.setPaddingRight(value);
+        return true;
+    case CSSPropertyPaddingTop:
+        style.setPaddingTop(value);
+        return true;
+    case CSSPropertyR:
+        style.setR(value);
+        return true;
+    case CSSPropertyRx:
+        style.setRx(value);
+        return true;
+    case CSSPropertyRy:
+        style.setRy(value);
+        return true;
+    case CSSPropertyRight:
+        style.setRight(value);
+        return true;
+    case CSSPropertyShapeMargin:
+        style.setShapeMargin(value);
+        return true;
+    case CSSPropertyStrokeDashoffset:
+        style.setStrokeDashOffset(value);
+        return true;
+    case CSSPropertyTop:
+        style.setTop(value);
+        return true;
+    case CSSPropertyWidth:
+        style.setWidth(value);
+        return true;
+    case CSSPropertyWebkitPerspectiveOriginX:
+        style.setPerspectiveOriginX(value);
+        return true;
+    case CSSPropertyWebkitPerspectiveOriginY:
+        style.setPerspectiveOriginY(value);
+        return true;
+    case CSSPropertyWebkitTransformOriginX:
+        style.setTransformOriginX(value);
+        return true;
+    case CSSPropertyWebkitTransformOriginY:
+        style.setTransformOriginY(value);
+        return true;
+    case CSSPropertyX:
+        style.setX(value);
+        return true;
+    case CSSPropertyY:
+        style.setY(value);
+        return true;
+
+    // TODO(alancutter): Support setters that take a numeric value (need to resolve percentages).
+    case CSSPropertyBorderBottomWidth:
+    case CSSPropertyBorderLeftWidth:
+    case CSSPropertyBorderRightWidth:
+    case CSSPropertyBorderTopWidth:
+    case CSSPropertyLetterSpacing:
+    case CSSPropertyOutlineOffset:
+    case CSSPropertyOutlineWidth:
+    case CSSPropertyPerspective:
+    case CSSPropertyStrokeWidth:
+    case CSSPropertyVerticalAlign:
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+    case CSSPropertyWebkitBorderVerticalSpacing:
+    case CSSPropertyWebkitColumnGap:
+    case CSSPropertyWebkitColumnRuleWidth:
+    case CSSPropertyWebkitColumnWidth:
+    case CSSPropertyWebkitTransformOriginZ:
+    case CSSPropertyWordSpacing:
+        return false;
+
+    default:
+        return false;
+    }
 }
 
 } // namespace blink