Avoid visually transitioning on length CSS properties when zoom changes

third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp

Index: third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp
diff --git a/third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp b/third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp
index cc0489fb1b0fc077f1ad2646accb2f9734ab632a..c1e639ed821eaffd98e53f13d459279ec88e5c58 100644
--- a/third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp
+++ b/third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp
@@ -31,6 +31,7 @@
 #include "core/animation/css/CSSAnimatableValueFactory.h"
 
 #include "core/CSSValueKeywords.h"
+#include "core/animation/LengthPropertyFunctions.h"
 #include "core/animation/animatable/AnimatableClipPathOperation.h"
 #include "core/animation/animatable/AnimatableColor.h"
 #include "core/animation/animatable/AnimatableDouble.h"
@@ -96,6 +97,15 @@ static PassRefPtr<AnimatableValue> createFromLength(
   return createFromLengthWithZoom(length, style.effectiveZoom());
 }
 
+static PassRefPtr<AnimatableValue> createFromPropertyLength(
+    CSSPropertyID property,
+    const ComputedStyle& style) {
+  Length length;
+  bool success = LengthPropertyFunctions::getLength(property, style, length);
+  DCHECK(success);
+  return createFromLength(length, style);
+}
+
 static PassRefPtr<AnimatableValue> createFromUnzoomedLength(
     const UnzoomedLength& unzoomedLength) {
   return createFromLengthWithZoom(unzoomedLength.length(), 1);
@@ -175,7 +185,7 @@ inline static PassRefPtr<AnimatableValue> createFromTransformOrigin(
   return AnimatableLengthPoint3D::create(
       createFromLength(transformOrigin.x(), style),
       createFromLength(transformOrigin.y(), style),
-      createFromDouble(transformOrigin.z()));
+      createFromLength(Length(transformOrigin.z(), Fixed), style));
 }
 
 inline static PassRefPtr<AnimatableValue> createFromLengthSize(
@@ -370,8 +380,6 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
       return createFromLengthSize(style.borderBottomLeftRadius(), style);
     case CSSPropertyBorderBottomRightRadius:
       return createFromLengthSize(style.borderBottomRightRadius(), style);
-    case CSSPropertyBorderBottomWidth:
-      return createFromDouble(style.borderBottomWidth());
     case CSSPropertyBorderImageOutset:
       return createFromBorderImageLengthBox(style.borderImageOutset(), style);
     case CSSPropertyBorderImageSlice:
@@ -383,22 +391,14 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
       return createFromBorderImageLengthBox(style.borderImageWidth(), style);
     case CSSPropertyBorderLeftColor:
       return createFromColor(property, style);
-    case CSSPropertyBorderLeftWidth:
-      return createFromDouble(style.borderLeftWidth());
     case CSSPropertyBorderRightColor:
       return createFromColor(property, style);
-    case CSSPropertyBorderRightWidth:
-      return createFromDouble(style.borderRightWidth());
     case CSSPropertyBorderTopColor:
       return createFromColor(property, style);
     case CSSPropertyBorderTopLeftRadius:
       return createFromLengthSize(style.borderTopLeftRadius(), style);
     case CSSPropertyBorderTopRightRadius:
       return createFromLengthSize(style.borderTopRightRadius(), style);
-    case CSSPropertyBorderTopWidth:
-      return createFromDouble(style.borderTopWidth());
-    case CSSPropertyBottom:
-      return createFromLength(style.bottom(), style);
     case CSSPropertyBoxShadow:
       return AnimatableShadow::create(style.boxShadow(), style.color());
     case CSSPropertyClip:
@@ -423,8 +423,6 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
       return createFromDouble(style.flexGrow());
     case CSSPropertyFlexShrink:
       return createFromDouble(style.flexShrink());
-    case CSSPropertyFlexBasis:
-      return createFromLength(style.flexBasis(), style);
     case CSSPropertyFloodColor:
       return createFromColor(property, style);
     case CSSPropertyFloodOpacity:
@@ -445,34 +443,12 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
       return createFromFontStretch(style.fontStretch());
     case CSSPropertyFontWeight:
       return createFromFontWeight(style.fontWeight());
-    case CSSPropertyHeight:
-      return createFromLength(style.height(), style);
     case CSSPropertyLightingColor:
       return createFromColor(property, style);
     case CSSPropertyListStyleImage:
       return createFromStyleImage(style.listStyleImage());
-    case CSSPropertyLeft:
-      return createFromLength(style.left(), style);
-    case CSSPropertyLetterSpacing:
-      return createFromDouble(style.letterSpacing());
     case CSSPropertyLineHeight:
       return createFromLineHeight(style.specifiedLineHeight(), style);
-    case CSSPropertyMarginBottom:
-      return createFromLength(style.marginBottom(), style);
-    case CSSPropertyMarginLeft:
-      return createFromLength(style.marginLeft(), style);
-    case CSSPropertyMarginRight:
-      return createFromLength(style.marginRight(), style);
-    case CSSPropertyMarginTop:
-      return createFromLength(style.marginTop(), style);
-    case CSSPropertyMaxHeight:
-      return createFromLength(style.maxHeight(), style);
-    case CSSPropertyMaxWidth:
-      return createFromLength(style.maxWidth(), style);
-    case CSSPropertyMinHeight:
-      return createFromLength(style.minHeight(), style);
-    case CSSPropertyMinWidth:
-      return createFromLength(style.minWidth(), style);
     case CSSPropertyObjectPosition:
       return createFromLengthPoint(style.objectPosition(), style);
     case CSSPropertyOpacity:
@@ -481,20 +457,6 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
       return createFromDouble(style.orphans());
     case CSSPropertyOutlineColor:
       return createFromColor(property, style);
-    case CSSPropertyOutlineOffset:
-      return createFromDouble(style.outlineOffset());
-    case CSSPropertyOutlineWidth:
-      return createFromDouble(style.outlineWidth());
-    case CSSPropertyPaddingBottom:
-      return createFromLength(style.paddingBottom(), style);
-    case CSSPropertyPaddingLeft:
-      return createFromLength(style.paddingLeft(), style);
-    case CSSPropertyPaddingRight:
-      return createFromLength(style.paddingRight(), style);
-    case CSSPropertyPaddingTop:
-      return createFromLength(style.paddingTop(), style);
-    case CSSPropertyRight:
-      return createFromLength(style.right(), style);
     case CSSPropertyStrokeWidth:
       return createFromUnzoomedLength(style.strokeWidth());
     case CSSPropertyStopColor:
@@ -504,8 +466,6 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
     case CSSPropertyStrokeDasharray:
       return AnimatableStrokeDasharrayList::create(style.strokeDashArray(),
                                                    style.effectiveZoom());
-    case CSSPropertyStrokeDashoffset:
-      return createFromLength(style.strokeDashOffset(), style);
     case CSSPropertyStrokeMiterlimit:
       return createFromDouble(style.strokeMiterLimit());
     case CSSPropertyStrokeOpacity:
@@ -520,16 +480,8 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
           style.svgStyle().visitedLinkStrokePaintUri());
     case CSSPropertyTextDecorationColor:
       return createFromColor(property, style);
-    case CSSPropertyTextIndent:
-      return createFromLength(style.textIndent(), style);
     case CSSPropertyTextShadow:
       return AnimatableShadow::create(style.textShadow(), style.color());
-    case CSSPropertyTop:
-      return createFromLength(style.top(), style);
-    case CSSPropertyWebkitBorderHorizontalSpacing:
-      return createFromDouble(style.horizontalBorderSpacing());
-    case CSSPropertyWebkitBorderVerticalSpacing:
-      return createFromDouble(style.verticalBorderSpacing());
     case CSSPropertyClipPath:
       if (ClipPathOperation* operation = style.clipPath())
         return AnimatableClipPathOperation::create(operation);
@@ -538,16 +490,12 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
       if (style.hasAutoColumnCount())
         return AnimatableUnknown::create(CSSValueAuto);
       return createFromDouble(style.columnCount());
-    case CSSPropertyColumnGap:
-      return createFromDouble(style.columnGap());
     case CSSPropertyColumnRuleColor:
       return createFromColor(property, style);
-    case CSSPropertyColumnRuleWidth:
-      return createFromDouble(style.columnRuleWidth());
     case CSSPropertyColumnWidth:
       if (style.hasAutoColumnWidth())
         return AnimatableUnknown::create(CSSValueAuto);
-      return createFromDouble(style.columnWidth());
+      return createFromPropertyLength(property, style);
     case CSSPropertyFilter:
       return AnimatableFilterOperations::create(style.filter());
     case CSSPropertyBackdropFilter:
@@ -578,13 +526,11 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
         return AnimatableUnknown::create(
             CSSIdentifierValue::create(CSSValueNone));
       }
-      return createFromDouble(style.perspective());
+      return createFromPropertyLength(property, style);
     case CSSPropertyPerspectiveOrigin:
       return createFromLengthPoint(style.perspectiveOrigin(), style);
     case CSSPropertyShapeOutside:
       return createFromShapeValue(style.shapeOutside());
-    case CSSPropertyShapeMargin:
-      return createFromLength(style.shapeMargin(), style);
     case CSSPropertyShapeImageThreshold:
       return createFromDouble(style.shapeImageThreshold());
     case CSSPropertyWebkitTextStrokeColor:
@@ -616,8 +562,6 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
     }
     case CSSPropertyOffsetAnchor:
       return createFromLengthPoint(style.offsetAnchor(), style);
-    case CSSPropertyOffsetDistance:
-      return createFromLength(style.offsetDistance(), style);
     case CSSPropertyOffsetPosition:
       return createFromLengthPoint(style.offsetPosition(), style);
     case CSSPropertyOffsetRotate:
@@ -627,49 +571,69 @@ PassRefPtr<AnimatableValue> CSSAnimatableValueFactory::create(
           style.offsetRotation().type == OffsetRotationAuto, style);
     case CSSPropertyTransformOrigin:
       return createFromTransformOrigin(style.transformOrigin(), style);
-    case CSSPropertyWebkitPerspectiveOriginX:
-      return createFromLength(style.perspectiveOriginX(), style);
-    case CSSPropertyWebkitPerspectiveOriginY:
-      return createFromLength(style.perspectiveOriginY(), style);
-    case CSSPropertyWebkitTransformOriginX:
-      return createFromLength(style.transformOriginX(), style);
-    case CSSPropertyWebkitTransformOriginY:
-      return createFromLength(style.transformOriginY(), style);
-    case CSSPropertyWebkitTransformOriginZ:
-      return createFromDouble(style.transformOriginZ());
     case CSSPropertyWidows:
       return createFromDouble(style.widows());
-    case CSSPropertyWidth:
-      return createFromLength(style.width(), style);
-    case CSSPropertyWordSpacing:
-      return createFromDouble(style.wordSpacing());
     case CSSPropertyVerticalAlign:
       if (style.verticalAlign() == VerticalAlignLength)
-        return createFromLength(style.getVerticalAlignLength(), style);
+        return createFromPropertyLength(property, style);
       return AnimatableUnknown::create(
           CSSIdentifierValue::create(style.verticalAlign()));
     case CSSPropertyVisibility:
       return AnimatableVisibility::create(style.visibility());
     case CSSPropertyD:
       return createFromPath(style.svgStyle().d());
+    case CSSPropertyZIndex:
+      if (style.hasAutoZIndex())
+        return AnimatableUnknown::create(CSSValueAuto);
+      return createFromDouble(style.zIndex());
+    case CSSPropertyBottom:
     case CSSPropertyCx:
-      return createFromLength(style.svgStyle().cx(), style);
     case CSSPropertyCy:
-      return createFromLength(style.svgStyle().cy(), style);
-    case CSSPropertyX:
-      return createFromLength(style.svgStyle().x(), style);
-    case CSSPropertyY:
-      return createFromLength(style.svgStyle().y(), style);
+    case CSSPropertyFlexBasis:
+    case CSSPropertyHeight:
+    case CSSPropertyLeft:
+    case CSSPropertyMarginBottom:
+    case CSSPropertyMarginLeft:
+    case CSSPropertyMarginRight:
+    case CSSPropertyMarginTop:
+    case CSSPropertyMaxHeight:
+    case CSSPropertyMaxWidth:
+    case CSSPropertyMinHeight:
+    case CSSPropertyMinWidth:
+    case CSSPropertyOffsetDistance:
+    case CSSPropertyPaddingBottom:
+    case CSSPropertyPaddingLeft:
+    case CSSPropertyPaddingRight:
+    case CSSPropertyPaddingTop:
     case CSSPropertyR:
-      return createFromLength(style.svgStyle().r(), style);
+    case CSSPropertyRight:
     case CSSPropertyRx:
-      return createFromLength(style.svgStyle().rx(), style);
     case CSSPropertyRy:
-      return createFromLength(style.svgStyle().ry(), style);
-    case CSSPropertyZIndex:
-      if (style.hasAutoZIndex())
-        return AnimatableUnknown::create(CSSValueAuto);
-      return createFromDouble(style.zIndex());
+    case CSSPropertyShapeMargin:
+    case CSSPropertyStrokeDashoffset:
+    case CSSPropertyTextIndent:
+    case CSSPropertyTop:
+    case CSSPropertyWebkitPerspectiveOriginX:
+    case CSSPropertyWebkitPerspectiveOriginY:
+    case CSSPropertyWebkitTransformOriginX:
+    case CSSPropertyWebkitTransformOriginY:
+    case CSSPropertyWidth:
+    case CSSPropertyX:
+    case CSSPropertyY:
+    case CSSPropertyBorderBottomWidth:
+    case CSSPropertyBorderLeftWidth:
+    case CSSPropertyBorderRightWidth:
+    case CSSPropertyBorderTopWidth:
+    case CSSPropertyLetterSpacing:
+    case CSSPropertyOutlineOffset:
+    case CSSPropertyOutlineWidth:
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+    case CSSPropertyWebkitBorderVerticalSpacing:
+    case CSSPropertyColumnGap:
+    case CSSPropertyColumnRuleWidth:
+    case CSSPropertyWebkitTransformOriginZ:
+    case CSSPropertyWordSpacing:
+      return createFromPropertyLength(property, style);

[suzyh_UTC10 (ex-contributor), 2016/12/12 04:30:13]

I can see why we might want to group all of these together, but I'd prefer not
to reorder the case lines here. It creates extra churn and obscures the actual
change of this patch. Please leave the ordering of cases alone in this patch.

I'd be happy to see a separate patch that changes the case ordering without
changing the behaviour (either before or after this change), if that still seems
desirable. I would probably go for strict alphabetical order of the cases, even
if that involves duplicating the action for each case. However, I'd also be fine
with a different grouping to avoid duplicating the action as long as (a) the
properties within each group are in alphabetical order, and (b) blank
lines/comments/etc are used to make it easy to see how and why the groups are
formed.

[alancutter (OOO until 2018), 2016/12/13 02:36:34]

Done.

     default:
       NOTREACHED();
       return nullptr;