Refactored out property specific logic in ConsumeBorderImageSlice.

third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp

Index: third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
index b0c84bf2a96c0df47427e4ad828ef9807f8e8d44..c0460f008ca3336c6a3d256211a6aa064291c349 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -840,8 +840,8 @@ static CSSValue* ConsumeBorderImageRepeat(CSSParserTokenRange& range) {
                               CSSValuePair::kDropIdenticalValues);
 }
 
-static CSSValue* ConsumeBorderImageSlice(CSSPropertyID property,
-                                         CSSParserTokenRange& range) {
+static CSSValue* ConsumeBorderImageSlice(CSSParserTokenRange& range,
+                                         bool default_fill) {
   bool fill = ConsumeIdent<CSSValueFill>(range);
   CSSValue* slices[4] = {0};
 
@@ -861,14 +861,7 @@ static CSSValue* ConsumeBorderImageSlice(CSSPropertyID property,
     fill = true;
   }
   Complete4Sides(slices);
-  // FIXME: For backwards compatibility, -webkit-border-image,
-  // -webkit-mask-box-image and -webkit-box-reflect have to do a fill by
-  // default.
-  // FIXME: What do we do with -webkit-box-reflect and -webkit-mask-box-image?
-  // Probably just have to leave them filling...
-  if (property == CSSPropertyWebkitBorderImage ||
-      property == CSSPropertyWebkitMaskBoxImage ||
-      property == CSSPropertyWebkitBoxReflect)
+  if (default_fill)
     fill = true;
   return CSSBorderImageSliceValue::Create(
       CSSQuadValue::Create(slices[0], slices[1], slices[2], slices[3],
@@ -918,14 +911,14 @@ static CSSValue* ConsumeBorderImageWidth(CSSParserTokenRange& range) {
                               CSSQuadValue::kSerializeAsQuad);
 }
 
-static bool ConsumeBorderImageComponents(CSSPropertyID property,
-                                         CSSParserTokenRange& range,
+static bool ConsumeBorderImageComponents(CSSParserTokenRange& range,
                                          const CSSParserContext* context,
                                          CSSValue*& source,
                                          CSSValue*& slice,
                                          CSSValue*& width,
                                          CSSValue*& outset,
-                                         CSSValue*& repeat) {
+                                         CSSValue*& repeat,
+                                         bool default_fill) {
   do {
     if (!source) {
       source = ConsumeImageOrNone(range, context);
@@ -938,7 +931,7 @@ static bool ConsumeBorderImageComponents(CSSPropertyID property,
         continue;
     }
     if (!slice) {
-      slice = ConsumeBorderImageSlice(property, range);
+      slice = ConsumeBorderImageSlice(range, default_fill);
       if (slice) {
         DCHECK(!width);
         DCHECK(!outset);
@@ -962,16 +955,15 @@ static bool ConsumeBorderImageComponents(CSSPropertyID property,
   return true;
 }
 
-static CSSValue* ConsumeWebkitBorderImage(CSSPropertyID property,
-                                          CSSParserTokenRange& range,
+static CSSValue* ConsumeWebkitBorderImage(CSSParserTokenRange& range,
                                           const CSSParserContext* context) {
   CSSValue* source = nullptr;
   CSSValue* slice = nullptr;
   CSSValue* width = nullptr;
   CSSValue* outset = nullptr;
   CSSValue* repeat = nullptr;
-  if (ConsumeBorderImageComponents(property, range, context, source, slice,
-                                   width, outset, repeat))
+  if (ConsumeBorderImageComponents(range, context, source, slice, width, outset,
+                                   repeat, true))

[suzyh_UTC10 (ex-contributor), 2017/05/05 05:47:12]

I'd suggest following this sort of protocol here:

...   repeat, true /* default_fill */))

Or use a named constant instead of literal "true".

Likewise for the other literal bools.

[Bugs Nash, 2017/05/05 06:23:55]

Done

     return CreateBorderImageValue(source, slice, width, outset, repeat);
   return nullptr;
 }
@@ -996,8 +988,7 @@ static CSSValue* ConsumeReflect(CSSParserTokenRange& range,
 
   CSSValue* mask = nullptr;
   if (!range.AtEnd()) {
-    mask =
-        ConsumeWebkitBorderImage(CSSPropertyWebkitBoxReflect, range, context);
+    mask = ConsumeWebkitBorderImage(range, context);
     if (!mask)
       return nullptr;
   }
@@ -1795,7 +1786,7 @@ const CSSValue* CSSPropertyParser::ParseSingleValue(
       return ConsumeBorderImageRepeat(range_);
     case CSSPropertyBorderImageSlice:
     case CSSPropertyWebkitMaskBoxImageSlice:
-      return ConsumeBorderImageSlice(property, range_);
+      return ConsumeBorderImageSlice(range_, false);
     case CSSPropertyBorderImageOutset:
     case CSSPropertyWebkitMaskBoxImageOutset:
       return ConsumeBorderImageOutset(range_);
@@ -1803,7 +1794,7 @@ const CSSValue* CSSPropertyParser::ParseSingleValue(
     case CSSPropertyWebkitMaskBoxImageWidth:
       return ConsumeBorderImageWidth(range_);
     case CSSPropertyWebkitBorderImage:
-      return ConsumeWebkitBorderImage(property, range_, context_);
+      return ConsumeWebkitBorderImage(range_, context_);
     case CSSPropertyWebkitBoxReflect:
       return ConsumeReflect(range_, context_);
     case CSSPropertyBackgroundAttachment:
@@ -2500,15 +2491,18 @@ bool CSSPropertyParser::Consume4Values(const StylePropertyShorthand& shorthand,
   return range_.AtEnd();
 }
 
+// TODO(bugsnash): refactor out property specific logic from this method

[suzyh_UTC10 (ex-contributor), 2017/05/05 05:47:12]

Consider referring to a particular bug in the TODO.

[Bugs Nash, 2017/05/05 06:23:55]

Done

+// and remove CSSPropetyID argument
 bool CSSPropertyParser::ConsumeBorderImage(CSSPropertyID property,
+                                           bool default_fill,
                                            bool important) {
   CSSValue* source = nullptr;
   CSSValue* slice = nullptr;
   CSSValue* width = nullptr;
   CSSValue* outset = nullptr;
   CSSValue* repeat = nullptr;
-  if (ConsumeBorderImageComponents(property, range_, context_, source, slice,
-                                   width, outset, repeat)) {
+  if (ConsumeBorderImageComponents(range_, context_, source, slice, width,
+                                   outset, repeat, default_fill)) {
     switch (property) {
       case CSSPropertyWebkitMaskBoxImage:
         AddProperty(CSSPropertyWebkitMaskBoxImageSource,
@@ -3347,8 +3341,9 @@ bool CSSPropertyParser::ParseShorthand(CSSPropertyID unresolved_property,
     case CSSPropertyBorder:
       return ConsumeBorder(important);
     case CSSPropertyBorderImage:
+      return ConsumeBorderImage(property, important, false);
     case CSSPropertyWebkitMaskBoxImage:
-      return ConsumeBorderImage(property, important);
+      return ConsumeBorderImage(property, important, true);
     case CSSPropertyPageBreakAfter:
     case CSSPropertyPageBreakBefore:
     case CSSPropertyPageBreakInside: