[webkit-changes] [285837] trunk/Source/WebCore

Mon, 15 Nov 2021 14:42:17 -0800

Title: [285837] trunk/Source/WebCore
Revision
285837
Author
[email protected]
Date
2021-11-15 14:41:10 -0800 (Mon, 15 Nov 2021)

Log Message

Add helper to add CSS property with implicit default
https://bugs.webkit.org/show_bug.cgi?id=233108

Patch by Kiet Ho <[email protected]> on 2021-11-15
Reviewed by Myles C. Maxfield.

Add a helper in CSSPropertyParser to add a property with implicit default. This helper
accepts a RefPtr<> value and a Ref<> default. If the value is a null pointer, the
default is added, and the implicit flag is set. Otherwise, the value is added. This is
useful for parsing shorthands, because the CSS spec requires that missing value be
set to its default value, and WebCore additionally requires the implicit flag to be set.
This helper handles both of them.

No functional changes; no tests needed.

* css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::addPropertyWithImplicitDefault):
(WebCore::CSSPropertyParser::consumeTransformOrigin):
(WebCore::CSSPropertyParser::consumeFont):
(WebCore::CSSPropertyParser::consumeFontVariantShorthand):
(WebCore::CSSPropertyParser::consumeColumns):
(WebCore::CSSPropertyParser::consumeShorthandGreedily):
(WebCore::CSSPropertyParser::consumeBorderImage):
* css/parser/CSSPropertyParser.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (285836 => 285837)


--- trunk/Source/WebCore/ChangeLog	2021-11-15 22:25:17 UTC (rev 285836)
+++ trunk/Source/WebCore/ChangeLog	2021-11-15 22:41:10 UTC (rev 285837)
@@ -1,3 +1,29 @@
+2021-11-15  Kiet Ho  <[email protected]>
+
+        Add helper to add CSS property with implicit default
+        https://bugs.webkit.org/show_bug.cgi?id=233108
+
+        Reviewed by Myles C. Maxfield.
+
+        Add a helper in CSSPropertyParser to add a property with implicit default. This helper
+        accepts a RefPtr<> value and a Ref<> default. If the value is a null pointer, the
+        default is added, and the implicit flag is set. Otherwise, the value is added. This is
+        useful for parsing shorthands, because the CSS spec requires that missing value be
+        set to its default value, and WebCore additionally requires the implicit flag to be set.
+        This helper handles both of them.
+
+        No functional changes; no tests needed.
+
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::CSSPropertyParser::addPropertyWithImplicitDefault):
+        (WebCore::CSSPropertyParser::consumeTransformOrigin):
+        (WebCore::CSSPropertyParser::consumeFont):
+        (WebCore::CSSPropertyParser::consumeFontVariantShorthand):
+        (WebCore::CSSPropertyParser::consumeColumns):
+        (WebCore::CSSPropertyParser::consumeShorthandGreedily):
+        (WebCore::CSSPropertyParser::consumeBorderImage):
+        * css/parser/CSSPropertyParser.h:
+
 2021-11-15  Antti Koivisto  <[email protected]>
 
         Revert r285235

Modified: trunk/Source/WebCore/css/parser/CSSPropertyParser.cpp (285836 => 285837)


--- trunk/Source/WebCore/css/parser/CSSPropertyParser.cpp	2021-11-15 22:25:17 UTC (rev 285836)
+++ trunk/Source/WebCore/css/parser/CSSPropertyParser.cpp	2021-11-15 22:41:10 UTC (rev 285837)
@@ -224,6 +224,14 @@
     m_parsedProperties->append(CSSProperty(property, WTFMove(value), important, setFromShorthand, shorthandIndex, implicit));
 }
 
+void CSSPropertyParser::addPropertyWithImplicitDefault(CSSPropertyID property, CSSPropertyID currentShorthand, RefPtr<CSSValue>&& value, Ref<CSSValue>&& implicitDefault, bool important)
+{
+    if (value)
+        addProperty(property, currentShorthand, value.releaseNonNull(), important, false);
+    else
+        addProperty(property, currentShorthand, WTFMove(implicitDefault), important, true);
+}
+
 void CSSPropertyParser::addExpandedPropertyForValue(CSSPropertyID property, Ref<CSSValue>&& value, bool important)
 {
     for (auto longhand : shorthandForProperty(property))
@@ -355,12 +363,11 @@
         m_range.consumeWhitespace();
         bool atEnd = m_range.atEnd();
         auto resultZ = consumeLength(m_range, m_context.mode, ValueRange::All);
-        bool hasZ = resultZ;
-        if ((!hasZ && !atEnd) || !m_range.atEnd())
+        if ((!resultZ && !atEnd) || !m_range.atEnd())
             return false;
         addProperty(CSSPropertyTransformOriginX, CSSPropertyTransformOrigin, WTFMove(resultXY->x), important);
         addProperty(CSSPropertyTransformOriginY, CSSPropertyTransformOrigin, WTFMove(resultXY->y), important);
-        addProperty(CSSPropertyTransformOriginZ, CSSPropertyTransformOrigin, resultZ ? resultZ.releaseNonNull() : CSSValuePool::singleton().createValue(0, CSSUnitType::CSS_PX), important, !hasZ);
+        addPropertyWithImplicitDefault(CSSPropertyTransformOriginZ, CSSPropertyTransformOrigin, resultZ, CSSValuePool::singleton().createValue(0, CSSUnitType::CSS_PX), important);
         
         return true;
     }
@@ -5021,6 +5028,7 @@
         if (id == CSSValueInherit || id == CSSValueInitial)
             return false;
     }
+
     // Optional font-style, font-variant, font-stretch and font-weight.
     RefPtr<CSSFontStyleValue> fontStyle;
     RefPtr<CSSPrimitiveValue> fontVariantCaps;
@@ -5057,16 +5065,10 @@
     if (m_range.atEnd())
         return false;
 
-    bool hasStyle = fontStyle;
-    bool hasVariant = fontVariantCaps;
-    bool hasWeight = fontWeight;
-    bool hasStretch = fontStretch;
+    auto& valuePool = CSSValuePool::singleton();
 
-    if (!fontStyle)
-        fontStyle = CSSFontStyleValue::create(CSSValuePool::singleton().createIdentifierValue(CSSValueNormal));
-    
-    addProperty(CSSPropertyFontStyle, CSSPropertyFont, fontStyle.releaseNonNull(), important, !hasStyle);
-    addProperty(CSSPropertyFontVariantCaps, CSSPropertyFont, fontVariantCaps ? fontVariantCaps.releaseNonNull() : CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, !hasVariant);
+    addPropertyWithImplicitDefault(CSSPropertyFontStyle, CSSPropertyFont, fontStyle, CSSFontStyleValue::create(valuePool.createIdentifierValue(CSSValueNormal)), important);
+    addPropertyWithImplicitDefault(CSSPropertyFontVariantCaps, CSSPropertyFont, fontVariantCaps, valuePool.createIdentifierValue(CSSValueNormal), important);
 /*  
     // FIXME-NEWPARSER: What do we do with these? They aren't part of our fontShorthand().
     addProperty(CSSPropertyFontVariantLigatures, CSSPropertyFont, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
@@ -5073,8 +5075,8 @@
     addProperty(CSSPropertyFontVariantNumeric, CSSPropertyFont, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
 */
 
-    addProperty(CSSPropertyFontWeight, CSSPropertyFont, fontWeight ? fontWeight.releaseNonNull() : CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, !hasWeight);
-    addProperty(CSSPropertyFontStretch, CSSPropertyFont, fontStretch ? fontStretch.releaseNonNull() : CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, !hasStretch);
+    addPropertyWithImplicitDefault(CSSPropertyFontWeight, CSSPropertyFont, fontWeight, valuePool.createIdentifierValue(CSSValueNormal), important);
+    addPropertyWithImplicitDefault(CSSPropertyFontStretch, CSSPropertyFont, fontStretch, valuePool.createIdentifierValue(CSSValueNormal), important);
 
     // Now a font size _must_ come.
     RefPtr<CSSValue> fontSize = consumeFontSize(m_range, m_context.mode);
@@ -5081,15 +5083,15 @@
     if (!fontSize || m_range.atEnd())
         return false;
 
-    addProperty(CSSPropertyFontSize, CSSPropertyFont, *fontSize, important);
+    addProperty(CSSPropertyFontSize, CSSPropertyFont, fontSize.releaseNonNull(), important);
 
+    RefPtr<CSSPrimitiveValue> lineHeight;
     if (consumeSlashIncludingWhitespace(m_range)) {
-        RefPtr<CSSPrimitiveValue> lineHeight = consumeLineHeight(m_range, m_context.mode);
+        lineHeight = consumeLineHeight(m_range, m_context.mode);
         if (!lineHeight)
             return false;
-        addProperty(CSSPropertyLineHeight, CSSPropertyFont, lineHeight.releaseNonNull(), important);
-    } else
-        addProperty(CSSPropertyLineHeight, CSSPropertyFont, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
+    }
+    addPropertyWithImplicitDefault(CSSPropertyLineHeight, CSSPropertyFont, lineHeight, valuePool.createIdentifierValue(CSSValueNormal), important);
 
     // Font family must come now.
     RefPtr<CSSValue> parsedFamilyValue = consumeFontFamily(m_range);
@@ -5167,18 +5169,13 @@
 
     addProperty(CSSPropertyFontVariantLigatures, CSSPropertyFontVariant, ligaturesParser.finalizeValue().releaseNonNull(), important, implicitLigatures);
     addProperty(CSSPropertyFontVariantNumeric, CSSPropertyFontVariant, numericParser.finalizeValue().releaseNonNull(), important, implicitNumeric);
-    bool implicitCaps = !capsValue;
-    addProperty(CSSPropertyFontVariantCaps, CSSPropertyFontVariant, capsValue ? capsValue.releaseNonNull() : CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, implicitCaps);
-    bool implicitAlternates = !alternatesValue;
-    addProperty(CSSPropertyFontVariantAlternates, CSSPropertyFontVariant, alternatesValue ? alternatesValue.releaseNonNull() : CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, implicitAlternates);
-    bool implicitPosition = !positionValue;
-    addProperty(CSSPropertyFontVariantPosition, CSSPropertyFontVariant, positionValue ? positionValue.releaseNonNull() : CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, implicitPosition);
 
-    bool implicitEastAsian = !eastAsianValue;
-    if (!eastAsianValue)
-        eastAsianValue = CSSValuePool::singleton().createIdentifierValue(CSSValueNormal);
-    addProperty(CSSPropertyFontVariantEastAsian, CSSPropertyFontVariant, eastAsianValue.releaseNonNull(), important, implicitEastAsian);
-    
+    auto& valuePool = CSSValuePool::singleton();
+    addPropertyWithImplicitDefault(CSSPropertyFontVariantCaps, CSSPropertyFontVariant, capsValue, valuePool.createIdentifierValue(CSSValueNormal), important);
+    addPropertyWithImplicitDefault(CSSPropertyFontVariantAlternates, CSSPropertyFontVariant, alternatesValue, valuePool.createIdentifierValue(CSSValueNormal), important);
+    addPropertyWithImplicitDefault(CSSPropertyFontVariantPosition, CSSPropertyFontVariant, positionValue, valuePool.createIdentifierValue(CSSValueNormal), important);
+    addPropertyWithImplicitDefault(CSSPropertyFontVariantEastAsian, CSSPropertyFontVariant, WTFMove(eastAsianValue), valuePool.createIdentifierValue(CSSValueNormal), important);
+
     return true;
 }
 
@@ -5225,7 +5222,7 @@
     
     if (!m_range.atEnd())
         return false;
-    
+
     // Any unassigned property at this point will become implicit 'auto'.
     if (columnWidth)
         addProperty(CSSPropertyColumnWidth, CSSPropertyInvalid, columnWidth.releaseNonNull(), important);
@@ -5261,10 +5258,7 @@
     } while (!m_range.atEnd());
 
     for (size_t i = 0; i < shorthand.length(); ++i) {
-        if (longhands[i])
-            addProperty(shorthandProperties[i], shorthand.id(), longhands[i].releaseNonNull(), important);
-        else
-            addProperty(shorthandProperties[i], shorthand.id(), CSSValuePool::singleton().createImplicitInitialValue(), important);
+        addPropertyWithImplicitDefault(shorthandProperties[i], shorthand.id(), WTFMove(longhands[i]), CSSValuePool::singleton().createImplicitInitialValue(), important);
     }
     return true;
 }
@@ -5424,30 +5418,21 @@
     RefPtr<CSSValue> repeat;
     
     if (consumeBorderImageComponents(property, m_range, m_context, source, slice, width, outset, repeat)) {
-        if (!source)
-            source = CSSValuePool::singleton().createImplicitInitialValue();
-        if (!slice)
-            slice = CSSValuePool::singleton().createImplicitInitialValue();
-        if (!width)
-            width = CSSValuePool::singleton().createImplicitInitialValue();
-        if (!outset)
-            outset = CSSValuePool::singleton().createImplicitInitialValue();
-        if (!repeat)
-            repeat = CSSValuePool::singleton().createImplicitInitialValue();
+        auto& valuePool = CSSValuePool::singleton();
         switch (property) {
         case CSSPropertyWebkitMaskBoxImage:
-            addProperty(CSSPropertyWebkitMaskBoxImageSource, CSSPropertyWebkitMaskBoxImage, source.releaseNonNull(), important);
-            addProperty(CSSPropertyWebkitMaskBoxImageSlice, CSSPropertyWebkitMaskBoxImage, slice.releaseNonNull(), important);
-            addProperty(CSSPropertyWebkitMaskBoxImageWidth, CSSPropertyWebkitMaskBoxImage, width.releaseNonNull(), important);
-            addProperty(CSSPropertyWebkitMaskBoxImageOutset, CSSPropertyWebkitMaskBoxImage, outset.releaseNonNull(), important);
-            addProperty(CSSPropertyWebkitMaskBoxImageRepeat, CSSPropertyWebkitMaskBoxImage, repeat.releaseNonNull(), important);
+            addPropertyWithImplicitDefault(CSSPropertyWebkitMaskBoxImageSource, CSSPropertyWebkitMaskBoxImage, WTFMove(source), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyWebkitMaskBoxImageSlice, CSSPropertyWebkitMaskBoxImage, WTFMove(slice), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyWebkitMaskBoxImageWidth, CSSPropertyWebkitMaskBoxImage, WTFMove(width), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyWebkitMaskBoxImageOutset, CSSPropertyWebkitMaskBoxImage, WTFMove(outset), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyWebkitMaskBoxImageRepeat, CSSPropertyWebkitMaskBoxImage, WTFMove(repeat), valuePool.createImplicitInitialValue(), important);
             return true;
         case CSSPropertyBorderImage:
-            addProperty(CSSPropertyBorderImageSource, CSSPropertyBorderImage, source.releaseNonNull(), important);
-            addProperty(CSSPropertyBorderImageSlice, CSSPropertyBorderImage, slice.releaseNonNull(), important);
-            addProperty(CSSPropertyBorderImageWidth, CSSPropertyBorderImage, width.releaseNonNull() , important);
-            addProperty(CSSPropertyBorderImageOutset, CSSPropertyBorderImage, outset.releaseNonNull(), important);
-            addProperty(CSSPropertyBorderImageRepeat, CSSPropertyBorderImage, repeat.releaseNonNull(), important);
+            addPropertyWithImplicitDefault(CSSPropertyBorderImageSource, CSSPropertyBorderImage, WTFMove(source), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyBorderImageSlice, CSSPropertyBorderImage, WTFMove(slice), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyBorderImageWidth, CSSPropertyBorderImage, WTFMove(width), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyBorderImageOutset, CSSPropertyBorderImage, WTFMove(outset), valuePool.createImplicitInitialValue(), important);
+            addPropertyWithImplicitDefault(CSSPropertyBorderImageRepeat, CSSPropertyBorderImage, WTFMove(repeat), valuePool.createImplicitInitialValue(), important);
             return true;
         default:
             ASSERT_NOT_REACHED();

Modified: trunk/Source/WebCore/css/parser/CSSPropertyParser.h (285836 => 285837)


--- trunk/Source/WebCore/css/parser/CSSPropertyParser.h	2021-11-15 22:25:17 UTC (rev 285836)
+++ trunk/Source/WebCore/css/parser/CSSPropertyParser.h	2021-11-15 22:41:10 UTC (rev 285837)
@@ -77,6 +77,7 @@
     bool parseCounterStyleDescriptor(CSSPropertyID, const CSSParserContext&);
 
     void addProperty(CSSPropertyID, CSSPropertyID, Ref<CSSValue>&&, bool important, bool implicit = false);
+    void addPropertyWithImplicitDefault(CSSPropertyID, CSSPropertyID, RefPtr<CSSValue>&&, Ref<CSSValue>&& implicitDefault, bool important);
     void addExpandedPropertyForValue(CSSPropertyID propId, Ref<CSSValue>&&, bool);
 
     bool consumeBorder(RefPtr<CSSValue>& width, RefPtr<CSSValue>& style, RefPtr<CSSValue>& color);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to