Refactored out property specific logic in ConsumeBorderImageSlice.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include <memory>
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSBasicShapeValues.h"
 #include "core/css/CSSBorderImage.h"
@@ skipping 822 matching lines @@
   CSSIdentifierValue* horizontal = ConsumeBorderImageRepeatKeyword(range);
   if (!horizontal)
     return nullptr;
   CSSIdentifierValue* vertical = ConsumeBorderImageRepeatKeyword(range);
   if (!vertical)
     vertical = horizontal;
   return CSSValuePair::Create(horizontal, vertical,
                               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};
 
   for (size_t index = 0; index < 4; ++index) {
     CSSPrimitiveValue* value = ConsumePercent(range, kValueRangeNonNegative);
     if (!value)
       value = ConsumeNumber(range, kValueRangeNonNegative);
     if (!value)
       break;
     slices[index] = value;
   }
   if (!slices[0])
     return nullptr;
   if (ConsumeIdent<CSSValueFill>(range)) {
     if (fill)
       return nullptr;
     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],
                            CSSQuadValue::kSerializeAsQuad),
       fill);
 }
 
 static CSSValue* ConsumeBorderImageOutset(CSSParserTokenRange& range) {
   CSSValue* outsets[4] = {0};
 
@@ skipping 29 matching lines @@
       break;
     widths[index] = value;
   }
   if (!widths[0])
     return nullptr;
   Complete4Sides(widths);
   return CSSQuadValue::Create(widths[0], widths[1], widths[2], widths[3],
                               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);
       if (source)
         continue;
     }
     if (!repeat) {
       repeat = ConsumeBorderImageRepeat(range);
       if (repeat)
         continue;
     }
     if (!slice) {
-      slice = ConsumeBorderImageSlice(property, range);
+      slice = ConsumeBorderImageSlice(range, default_fill);
       if (slice) {
         DCHECK(!width);
         DCHECK(!outset);
         if (ConsumeSlashIncludingWhitespace(range)) {
           width = ConsumeBorderImageWidth(range);
           if (ConsumeSlashIncludingWhitespace(range)) {
             outset = ConsumeBorderImageOutset(range);
             if (!outset)
               return false;
           } else if (!width) {
             return false;
           }
         }
       } else {
         return false;
       }
     } else {
       return false;
     }
   } while (!range.AtEnd());
   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;
 }
 
 static CSSValue* ConsumeReflect(CSSParserTokenRange& range,
                                 const CSSParserContext* context) {
   CSSIdentifierValue* direction =
       ConsumeIdent<CSSValueAbove, CSSValueBelow, CSSValueLeft, CSSValueRight>(
           range);
   if (!direction)
     return nullptr;
 
   CSSPrimitiveValue* offset = nullptr;
   if (range.AtEnd()) {
     offset = CSSPrimitiveValue::Create(0, CSSPrimitiveValue::UnitType::kPixels);
   } else {
     offset = ConsumeLengthOrPercent(range, context->Mode(), kValueRangeAll,
                                     UnitlessQuirk::kForbid);
     if (!offset)
       return nullptr;
   }
 
   CSSValue* mask = nullptr;
   if (!range.AtEnd()) {
-    mask =
-        ConsumeWebkitBorderImage(CSSPropertyWebkitBoxReflect, range, context);
+    mask = ConsumeWebkitBorderImage(range, context);
     if (!mask)
       return nullptr;
   }
   return CSSReflectValue::Create(direction, offset, mask);
 }
 
 static CSSValue* ConsumeBackgroundBlendMode(CSSParserTokenRange& range) {
   CSSValueID id = range.Peek().Id();
   if (id == CSSValueNormal || id == CSSValueOverlay ||
       (id >= CSSValueMultiply && id <= CSSValueLuminosity))
@@ skipping 777 matching lines @@
     case CSSPropertyPerspective:
       return ConsumePerspective(range_, context_, unresolved_property);
     case CSSPropertyScrollSnapPointsX:
     case CSSPropertyScrollSnapPointsY:
       return ConsumeScrollSnapPoints(range_, context_->Mode());
     case CSSPropertyBorderImageRepeat:
     case CSSPropertyWebkitMaskBoxImageRepeat:
       return ConsumeBorderImageRepeat(range_);
     case CSSPropertyBorderImageSlice:
     case CSSPropertyWebkitMaskBoxImageSlice:
-      return ConsumeBorderImageSlice(property, range_);
+      return ConsumeBorderImageSlice(range_, false);
     case CSSPropertyBorderImageOutset:
     case CSSPropertyWebkitMaskBoxImageOutset:
       return ConsumeBorderImageOutset(range_);
     case CSSPropertyBorderImageWidth:
     case CSSPropertyWebkitMaskBoxImageWidth:
       return ConsumeBorderImageWidth(range_);
     case CSSPropertyWebkitBorderImage:
-      return ConsumeWebkitBorderImage(property, range_, context_);
+      return ConsumeWebkitBorderImage(range_, context_);
     case CSSPropertyWebkitBoxReflect:
       return ConsumeReflect(range_, context_);
     case CSSPropertyBackgroundAttachment:
     case CSSPropertyBackgroundBlendMode:
     case CSSPropertyBackgroundClip:
     case CSSPropertyBackgroundImage:
     case CSSPropertyBackgroundOrigin:
     case CSSPropertyBackgroundPositionX:
     case CSSPropertyBackgroundPositionY:
     case CSSPropertyBackgroundSize:
@@ skipping 676 matching lines @@
     left = right;
 
   AddProperty(longhands[0], shorthand.id(), *top, important);
   AddProperty(longhands[1], shorthand.id(), *right, important);
   AddProperty(longhands[2], shorthand.id(), *bottom, important);
   AddProperty(longhands[3], shorthand.id(), *left, important);
 
   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,
                     CSSPropertyWebkitMaskBoxImage,
                     source ? *source : *CSSInitialValue::Create(), important);
         AddProperty(CSSPropertyWebkitMaskBoxImageSlice,
                     CSSPropertyWebkitMaskBoxImage,
                     slice ? *slice : *CSSInitialValue::Create(), important);
         AddProperty(CSSPropertyWebkitMaskBoxImageWidth,
                     CSSPropertyWebkitMaskBoxImage,
@@ skipping 818 matching lines @@
       return ConsumeShorthandGreedily(borderTopShorthand(), important);
     case CSSPropertyBorderRight:
       return ConsumeShorthandGreedily(borderRightShorthand(), important);
     case CSSPropertyBorderBottom:
       return ConsumeShorthandGreedily(borderBottomShorthand(), important);
     case CSSPropertyBorderLeft:
       return ConsumeShorthandGreedily(borderLeftShorthand(), important);
     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:
     case CSSPropertyWebkitColumnBreakAfter:
     case CSSPropertyWebkitColumnBreakBefore:
     case CSSPropertyWebkitColumnBreakInside:
       return ConsumeLegacyBreakProperty(property, important);
     case CSSPropertyWebkitMaskPosition:
     case CSSPropertyBackgroundPosition: {
       CSSValue* result_x = nullptr;
@@ skipping 65 matching lines @@
     case CSSPropertyPlaceItems:
       return ConsumePlaceItemsShorthand(important);
     case CSSPropertyPlaceSelf:
       return ConsumePlaceSelfShorthand(important);
     default:
       return false;
   }
 }
 
 }  // namespace blink