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

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

 {% from 'macros.tmpl' import license %}
 {#
     This file is for property handlers which use the templating engine to
     reduce (handwritten) code duplication.
 
     The `properties' dict can be used to access a property's parameters in
     jinja2 templates (i.e. setter, getter, initial, type_name)
 #}
 #include "config.h"
 #include "StyleBuilderFunctions.h"
@@ skipping 165 matching lines @@
 }
 
 static bool lengthTypeAndValueMatch(const BorderImageLengthBox& borderImageLengthBox, LengthType type, float value)
 {
     return (lengthTypeAndValueMatch(borderImageLengthBox.left(), type, value)
         && lengthTypeAndValueMatch(borderImageLengthBox.right(), type, value)
         && lengthTypeAndValueMatch(borderImageLengthBox.top(), type, value)
         && 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' %}
 {% set setter = 'setMaskBoxImage' if is_mask_box else 'setBorderImage' %}
 {{ declare_initial_function(property_id) }}
 {
     const NinePieceImage& currentImage = state.style()->{{getter}}();
     {# Check for equality in case we can bail out before creating a new NinePieceImage. #}
     {% if modifier_type == 'Outset' %}
     if (lengthTypeAndValueMatch(currentImage.outset(), Fixed, 0))
         return;
     {% elif modifier_type == 'Repeat' %}
     if (currentImage.horizontalRule() == StretchImageRule && currentImage.verticalRule() == StretchImageRule)
         return;
     {% elif modifier_type == 'Slice' and is_mask_box %}
     // Masks have a different initial value for slices. Preserve the value of 0 for backwards compatibility.
     if (currentImage.fill() == true && lengthTypeAndValueMatch(currentImage.imageSlices(), Fixed, 0))
         return;
     {% elif modifier_type == 'Slice' and not is_mask_box %}
     if (currentImage.fill() == false && lengthTypeAndValueMatch(currentImage.imageSlices(), Percent, 100))
         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 %}
 
     NinePieceImage image(currentImage);
     {% if modifier_type == 'Outset' %}
     image.setOutset(Length(0, Fixed));
     {% elif modifier_type == 'Repeat' %}
     image.setHorizontalRule(StretchImageRule);
     image.setVerticalRule(StretchImageRule);
     {% elif modifier_type == 'Slice' and is_mask_box %}
@@ skipping 334 matching lines @@
         }
         {{set_value(property)}}(ptype, c, url,
             state.applyPropertyToRegularStyle(),
             state.applyPropertyToVisitedLinkStyle());
     }
 }
 {% endmacro %}
 {{apply_svg_paint('CSSPropertyFill', 'FillPaint')}}
 {{apply_svg_paint('CSSPropertyStroke', 'StrokePaint')}}
 } // namespace blink