Move background related shorthands into CSSPropertyParser

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 "core/StylePropertyShorthand.h"
 #include "core/css/CSSBasicShapeValues.h"
 #include "core/css/CSSBorderImage.h"
 #include "core/css/CSSCalculationValue.h"
@@ skipping 4504 matching lines @@
         ASSERT_NOT_REACHED();
     }
     if (value == CSSValueInvalid)
         return false;
 
     CSSPropertyID genericBreakProperty = mapFromLegacyBreakProperty(property);
     addProperty(genericBreakProperty, cssValuePool().createIdentifierValue(value), important);
     return true;
 }
 
+static bool consumeBackgroundPosition(CSSParserTokenRange& range, const CSSParserContext& context, UnitlessQuirk unitless, RefPtrWillBeRawPtr<CSSValue>& resultX, RefPtrWillBeRawPtr<CSSValue>& resultY)
+{
+    do {
+        RefPtrWillBeRawPtr<CSSValue> positionX = nullptr;
+        RefPtrWillBeRawPtr<CSSValue> positionY = nullptr;
+        if (!consumePosition(range, context.mode(), unitless, positionX, positionY))
+            return false;
+        addBackgroundValue(resultX, positionX);
+        if (positionY)

[Timothy Loh, 2016/02/23 05:53:15]

I think both always get set if consumePosition returns true

[rwlbuis, 2016/02/23 20:16:42]

Done.

+            addBackgroundValue(resultY, positionY);
+    } while (consumeCommaIncludingWhitespace(range));
+    return resultX;

[Timothy Loh, 2016/02/23 05:53:14]

return true?

[rwlbuis, 2016/02/23 20:16:42]

Done, also in consumeRepeatStyle.

+}
+
+static bool consumeRepeatStyleComponent(CSSParserTokenRange& range, RefPtrWillBeRawPtr<CSSValue>& value1, RefPtrWillBeRawPtr<CSSValue>& value2, bool& implicit)
+{
+    if (consumeIdent<CSSValueRepeatX>(range)) {
+        value1 = cssValuePool().createIdentifierValue(CSSValueRepeat);
+        value2 = cssValuePool().createIdentifierValue(CSSValueNoRepeat);
+        implicit = true;
+        return true;
+    }
+    if (consumeIdent<CSSValueRepeatY>(range)) {
+        value1 = cssValuePool().createIdentifierValue(CSSValueNoRepeat);
+        value2 = cssValuePool().createIdentifierValue(CSSValueRepeat);
+        implicit = true;
+        return true;
+    }
+    CSSValueID id = range.peek().id();
+    value1 = consumeIdent<CSSValueRepeat, CSSValueNoRepeat, CSSValueRound, CSSValueSpace>(range);
+    if (!value1)
+        return false;
+
+    value2 = consumeIdent<CSSValueRepeat, CSSValueNoRepeat, CSSValueRound, CSSValueSpace>(range);
+    if (!value2) {
+        value2 = cssValuePool().createIdentifierValue(id);

[Timothy Loh, 2016/02/23 05:53:14]

isn't this just value1?

[rwlbuis, 2016/02/23 20:16:41]

True, I changed it.

+        implicit = true;
+    }
+    return true;
+}
+
+static bool consumeRepeatStyle(CSSParserTokenRange& range, RefPtrWillBeRawPtr<CSSValue>& resultX, RefPtrWillBeRawPtr<CSSValue>& resultY, bool& implicit)
+{
+    do {
+        RefPtrWillBeRawPtr<CSSValue> repeatX = nullptr;
+        RefPtrWillBeRawPtr<CSSValue> repeatY = nullptr;
+        if (!consumeRepeatStyleComponent(range, repeatX, repeatY, implicit))
+            break;

[Timothy Loh, 2016/02/23 05:53:15]

Any reason for this to be different than consumeBackgroundPosition?

[rwlbuis, 2016/02/23 20:16:42]

Maybe at some point, but probably not anymore, fixed.

+        addBackgroundValue(resultX, repeatX);
+        addBackgroundValue(resultY, repeatY);
+    } while (consumeCommaIncludingWhitespace(range));
+    return resultX;
+}
+
 bool CSSPropertyParser::parseShorthand(CSSPropertyID unresolvedProperty, bool important)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
 
     CSSPropertyID oldShorthand = m_currentShorthand;
     // TODO(rob.buis): Remove this when the legacy property parser is gone
     m_currentShorthand = property;
     switch (property) {
     case CSSPropertyWebkitMarginCollapse: {
         CSSValueID id = m_range.consumeIncludingWhitespace().id();
@@ skipping 121 matching lines @@
     case CSSPropertyBorderImage:
     case CSSPropertyWebkitMaskBoxImage:
         return consumeBorderImage(property, important);
     case CSSPropertyPageBreakAfter:
     case CSSPropertyPageBreakBefore:
     case CSSPropertyPageBreakInside:
     case CSSPropertyWebkitColumnBreakAfter:
     case CSSPropertyWebkitColumnBreakBefore:
     case CSSPropertyWebkitColumnBreakInside:
         return consumeLegacyBreakProperty(property, important);
+    case CSSPropertyWebkitMaskPosition:
+    case CSSPropertyBackgroundPosition: {
+        RefPtrWillBeRawPtr<CSSValue> resultX = nullptr;
+        RefPtrWillBeRawPtr<CSSValue> resultY = nullptr;
+        CSSPropertyID positionX = property == CSSPropertyBackgroundPosition ? CSSPropertyBackgroundPositionX : CSSPropertyWebkitMaskPositionX;
+        CSSPropertyID positionY = property == CSSPropertyBackgroundPosition ? CSSPropertyBackgroundPositionY : CSSPropertyWebkitMaskPositionY;
+        if (!consumeBackgroundPosition(m_range, m_context, UnitlessQuirk::Allow, resultX, resultY) || !m_range.atEnd())
+            return false;
+        addProperty(positionX, resultX.release(), important);
+        if (resultY)

[Timothy Loh, 2016/02/23 05:53:14]

There shouldn't be any case where a shorthand only sets some of its longhands
(except for that one quirk that we have, see bug 277157). Same below.

[rwlbuis, 2016/02/23 20:16:42]

Good point! Fixed.

+            addProperty(positionY, resultY.release(), important);
+        return true;
+    }
+    case CSSPropertyBackgroundRepeat:
+    case CSSPropertyWebkitMaskRepeat: {
+        RefPtrWillBeRawPtr<CSSValue> resultX = nullptr;
+        RefPtrWillBeRawPtr<CSSValue> resultY = nullptr;
+        CSSPropertyID repeatX = property == CSSPropertyBackgroundRepeat ? CSSPropertyBackgroundRepeatX : CSSPropertyWebkitMaskRepeatX;
+        CSSPropertyID repeatY = property == CSSPropertyBackgroundRepeat ? CSSPropertyBackgroundRepeatY : CSSPropertyWebkitMaskRepeatY;
+        bool implicit = false;
+        if (!consumeRepeatStyle(m_range, resultX, resultY, implicit) || !m_range.atEnd())
+            return false;
+        addProperty(repeatX, resultX.release(), important, implicit);
+        if (resultY)
+            addProperty(repeatY, resultY.release(), important, implicit);
+        return true;
+    }
     default:
         m_currentShorthand = oldShorthand;
         CSSParserValueList valueList(m_range);
         if (!valueList.size())
             return false;
         m_valueList = &valueList;
         return legacyParseShorthand(unresolvedProperty, important);
     }
 }
 
 } // namespace blink