Fast path when setting border-image-width to initial always fails

Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl

Index: Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl
diff --git a/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl b/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl
index da95da69b3f99c926d7968be98e19f9dd816f8fe..871bbd4de90eb2837361af2076dd467234c41a09 100644
--- a/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl
+++ b/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl
@@ -183,6 +183,19 @@ static bool lengthTypeAndValueMatch(const BorderImageLengthBox& borderImageLengt
         && lengthTypeAndValueMatch(borderImageLengthBox.bottom(), type, value));
 }
 
+static bool numberTypeAndValueMatch(const BorderImageLength& borderImageLength, float value)
+{
+    return borderImageLength.isNumber() && (borderImageLength.number() == value);
+}
+
+static bool numberTypeAndValueMatch(const BorderImageLengthBox& borderImageLengthBox, float value)

[Timothy Loh, 2015/05/29 02:10:15]

Naming is weird, lengthTypeAndValueMatch is checking the Length's type and
value, but here we're only checking the number value. It might be cleaner to
just pass in a BorderImageLength here and use its operator==, and not need
separate paths for lengths/numbers.

[Sunil Ratnu, 2015/05/29 13:56:01]

Made the changes. Now passing BorderImageLength as the parameter here but for
that we need to construct the BorderImageLength object at the caller site. This
does avoid writing multiple functions but at the same time makes us to create an
additional object. 

+{
+    return (numberTypeAndValueMatch(borderImageLengthBox.left(), value)
+        && numberTypeAndValueMatch(borderImageLengthBox.right(), value)
+        && numberTypeAndValueMatch(borderImageLengthBox.top(), value)
+        && numberTypeAndValueMatch(borderImageLengthBox.bottom(), value));
+}
+
 {% macro apply_border_image_modifier(property_id, modifier_type) %}
 {% set is_mask_box = 'MaskBox' in property_id %}
 {% set getter = 'maskBoxImage' if is_mask_box else 'borderImage' %}
@@ -206,10 +219,10 @@ static bool lengthTypeAndValueMatch(const BorderImageLengthBox& borderImageLengt
         return;
     {% elif modifier_type == 'Width' and is_mask_box %}
     // Masks have a different initial value for widths. Preserve the value of 'auto' for backwards compatibility.
-    if (lengthTypeAndValueMatch(currentImage.borderSlices(), Auto, 0))
+    if (numberTypeAndValueMatch(currentImage.borderSlices(), 0))

[Timothy Loh, 2015/05/29 02:10:15]

I think this is wrong. The default for -webkit-mask-box-image-width is auto (see
NinePieceImage::setMaskDefaults), not 0.

[Sunil Ratnu, 2015/05/29 13:56:01]

Done.

         return;
     {% elif modifier_type == 'Width' and not is_mask_box %}
-    if (lengthTypeAndValueMatch(currentImage.borderSlices(), Fixed, 1))
+    if (numberTypeAndValueMatch(currentImage.borderSlices(), 1.0))

[Timothy Loh, 2015/05/29 02:10:15]

This is fixing a bug, right? We could add a test here even though it's not
something people are likely to run into. In the example below, the div should
have a value of 1 even though it'll have one of 1px.

<style>
#d { border-image-width: 1px; }
div { border-image-width: initial; }
</style>
<div id=d></div>

[Sunil Ratnu, 2015/05/29 13:56:01]

Not sure if its fixing a bug for which we can have the tests. But as per your
suggestion, I will try to write out a test if possible.

         return;
     {% endif %}