[CSS Shapes] inset args and radial args should serialize to the simplest form

Source/core/css/CSSBasicShapes.cpp

Index: Source/core/css/CSSBasicShapes.cpp
diff --git a/Source/core/css/CSSBasicShapes.cpp b/Source/core/css/CSSBasicShapes.cpp
index 72bfcef26d5af2c9cc68b41243562c6bc75579b9..9ec4ec96ef3281efef3b5df1098b187f5c8abcb5 100644
--- a/Source/core/css/CSSBasicShapes.cpp
+++ b/Source/core/css/CSSBasicShapes.cpp
@@ -296,43 +296,67 @@ static String buildInsetString(const String& top, const String& right, const Str
     StringBuilder result;
     result.appendLiteral(opening);
     result.append(top);
-    if (!right.isNull()) {
+    bool showLeftArg = !left.isNull() && left != right;
+    bool showBottomArg = !bottom.isNull() && (bottom != top || showLeftArg);
+    if (!right.isNull() && (right != top || showBottomArg)) {

[Bear Travis, 2014/04/07 21:59:13]

Personally, I think this should be stored in a 'showRightArg' since it's using
the same format as show[Left/Bottom]Arg.

         result.appendLiteral(separator);
         result.append(right);
     }
-    if (!bottom.isNull()) {
+    if (showBottomArg) {
         result.appendLiteral(separator);
         result.append(bottom);
     }
-    if (!left.isNull()) {
+    if (showLeftArg) {
         result.appendLiteral(separator);
         result.append(left);
     }
 
     if (!topLeftRadiusWidth.isNull() && !topLeftRadiusHeight.isNull()) {
-        result.appendLiteral(separator);
-        result.appendLiteral(cornersSeparator);
-        result.appendLiteral(separator);
+        bool showHorizontalBottomLeft = topRightRadiusWidth != bottomLeftRadiusWidth;

[Bear Travis, 2014/04/07 21:59:13]

This logic seems like it could be factored out and shared with the above inset
simplification. Something like: appendSimplifiedInsetSyntax(result, one, two,
three, four). (Since it's either top, right, bottom, left or topLeft, topRight,
bottomRight, bottomLeft).

+        bool showHorizontalBottomRight = showHorizontalBottomLeft || (bottomRightRadiusWidth != topLeftRadiusWidth);
+        bool showHorizontalTopRight = showHorizontalBottomRight || (topRightRadiusWidth != topLeftRadiusWidth);
+
+        Vector<String> horizontalRadii;

[Bear Travis, 2014/04/07 21:59:13]

Would it be possible to get around always creating these two vectors? I know
that the valueForBorderRadiusShorthand creates separate lists, but it seems a
bit expensive here.

+        horizontalRadii.append(topLeftRadiusWidth);
+        if (showHorizontalTopRight)
+            horizontalRadii.append(topRightRadiusWidth);
+        if (showHorizontalBottomRight)
+            horizontalRadii.append(bottomRightRadiusWidth);
+        if (showHorizontalBottomLeft)
+            horizontalRadii.append(bottomLeftRadiusWidth);
+
+        if (horizontalRadii.size() > 1 || horizontalRadii[0] != "0px") {

[Bear Travis, 2014/04/07 21:59:13]

This would not appear to catch the case inset(10px round 0px / 10px). I think it
should be: If any of the vertical radii are different than the horizontal radii,
append '/' and run the appendSimplifiedInsetSyntax method.

+            bool showVerticalBottomLeft = topRightRadiusHeight != bottomLeftRadiusHeight;
+            bool showVerticalBottomRight = showVerticalBottomLeft || (bottomRightRadiusHeight != topLeftRadiusHeight);
+            bool showVerticalTopRight = showVerticalBottomRight || (topRightRadiusHeight != topLeftRadiusHeight);
 
-        result.append(topLeftRadiusWidth);
-        result.appendLiteral(separator);
-        result.append(topRightRadiusWidth);
-        result.appendLiteral(separator);
-        result.append(bottomRightRadiusWidth);
-        result.appendLiteral(separator);
-        result.append(bottomLeftRadiusWidth);
-
-        result.appendLiteral(separator);
-        result.appendLiteral("/");
-        result.appendLiteral(separator);
-
-        result.append(topLeftRadiusHeight);
-        result.appendLiteral(separator);
-        result.append(topRightRadiusHeight);
-        result.appendLiteral(separator);
-        result.append(bottomRightRadiusHeight);
-        result.appendLiteral(separator);
-        result.append(bottomLeftRadiusHeight);
+            result.appendLiteral(separator);
+            result.appendLiteral(cornersSeparator);
+
+            Vector<String> verticalRadii;
+            verticalRadii.append(topLeftRadiusHeight);
+            if (showVerticalTopRight)
+                verticalRadii.append(topRightRadiusHeight);
+            if (showVerticalBottomRight)
+                verticalRadii.append(bottomRightRadiusHeight);
+            if (showVerticalBottomLeft)
+                verticalRadii.append(bottomLeftRadiusHeight);
+
+            for (size_t i = 0; i < horizontalRadii.size(); ++i) {
+                result.appendLiteral(separator);
+                result.append(horizontalRadii[i]);
+            }
+
+            if (horizontalRadii != verticalRadii) {
+                result.appendLiteral(separator);
+                result.appendLiteral("/");
+
+                for (size_t i = 0; i < verticalRadii.size(); ++i) {
+                    result.appendLiteral(separator);
+                    result.append(verticalRadii[i]);
+                }
+            }
+        }
     }
     result.append(')');