Changeset 259585 in webkit


Ignore:
Timestamp:
Apr 6, 2020 11:54:09 AM (4 years ago)
Author:
Antti Koivisto
Message:

'currentcolor' doesn't need setHasExplicitlyInheritedProperties marking anymore
https://bugs.webkit.org/show_bug.cgi?id=210017

Reviewed by Darin Adler.

Source/WebCore:

Removing this marking reveals problems in style update avoidance code in CSSComputedStyleDeclaration
that also need to be addressed. The problems are not specific to exlicit 'currentcolor', they also reproduce
with the initial value (thus the new test).

Test: fast/css/currentColor-initial-style-update.html

  • css/CSSComputedStyleDeclaration.cpp:

(WebCore::nonInheritedColorPropertyHasValueCurrentColor):

Add a helper.

(WebCore::hasValidStyleForProperty):

Treat a non-inherited color property as inherited if it is 'currentcolor'.

(WebCore::updateStyleIfNeededForProperty):

Expand shorthands so properties like border-color test correctly.

  • css/CSSProperty.cpp:

(WebCore::CSSProperty::isColorProperty):

Move here from CSSParserFastPaths.

  • css/CSSProperty.h:
  • css/parser/CSSParserFastPaths.cpp:

(WebCore::CSSParserFastPaths::maybeParseValue):
(WebCore::isColorPropertyID): Deleted.

  • rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::unresolvedColorForProperty const):

Factor into a helper.
Handle all color values.

(WebCore::RenderStyle::colorResolvingCurrentColor const):

Renamed for clarity and some cleanups.

(WebCore::RenderStyle::visitedDependentColor const):
(WebCore::RenderStyle::colorIncludingFallback const): Deleted.

  • rendering/style/RenderStyle.h:

(WebCore::RenderStyle::isCurrentColor):

  • style/StyleBuilderState.cpp:

(WebCore::Style::BuilderState::colorFromPrimitiveValue const):

Remove setHasExplicitlyInheritedProperties marking.

LayoutTests:

  • fast/css/currentColor-initial-style-update-expected.txt: Added.
  • fast/css/currentColor-initial-style-update.html: Added.
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r259578 r259585  
     12020-04-06  Antti Koivisto  <antti@apple.com>
     2
     3        'currentcolor' doesn't need setHasExplicitlyInheritedProperties marking anymore
     4        https://bugs.webkit.org/show_bug.cgi?id=210017
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/css/currentColor-initial-style-update-expected.txt: Added.
     9        * fast/css/currentColor-initial-style-update.html: Added.
     10
    1112020-04-06  Cathie Chen  <cathiechen@igalia.com>
    212
  • trunk/Source/WebCore/ChangeLog

    r259581 r259585  
     12020-04-06  Antti Koivisto  <antti@apple.com>
     2
     3        'currentcolor' doesn't need setHasExplicitlyInheritedProperties marking anymore
     4        https://bugs.webkit.org/show_bug.cgi?id=210017
     5
     6        Reviewed by Darin Adler.
     7
     8        Removing this marking reveals problems in style update avoidance code in CSSComputedStyleDeclaration
     9        that also need to be addressed. The problems are not specific to exlicit 'currentcolor', they also reproduce
     10        with the initial value (thus the new test).
     11
     12        Test: fast/css/currentColor-initial-style-update.html
     13
     14        * css/CSSComputedStyleDeclaration.cpp:
     15        (WebCore::nonInheritedColorPropertyHasValueCurrentColor):
     16
     17        Add a helper.
     18
     19        (WebCore::hasValidStyleForProperty):
     20
     21        Treat a non-inherited color property as inherited if it is 'currentcolor'.
     22
     23        (WebCore::updateStyleIfNeededForProperty):
     24
     25        Expand shorthands so properties like border-color test correctly.
     26
     27        * css/CSSProperty.cpp:
     28        (WebCore::CSSProperty::isColorProperty):
     29
     30        Move here from CSSParserFastPaths.
     31
     32        * css/CSSProperty.h:
     33        * css/parser/CSSParserFastPaths.cpp:
     34        (WebCore::CSSParserFastPaths::maybeParseValue):
     35        (WebCore::isColorPropertyID): Deleted.
     36        * rendering/style/RenderStyle.cpp:
     37        (WebCore::RenderStyle::unresolvedColorForProperty const):
     38
     39        Factor into a helper.
     40        Handle all color values.
     41
     42        (WebCore::RenderStyle::colorResolvingCurrentColor const):
     43
     44        Renamed for clarity and some cleanups.
     45
     46        (WebCore::RenderStyle::visitedDependentColor const):
     47        (WebCore::RenderStyle::colorIncludingFallback const): Deleted.
     48        * rendering/style/RenderStyle.h:
     49        (WebCore::RenderStyle::isCurrentColor):
     50        * style/StyleBuilderState.cpp:
     51        (WebCore::Style::BuilderState::colorFromPrimitiveValue const):
     52
     53        Remove setHasExplicitlyInheritedProperties marking.
     54
    1552020-04-06  Simon Fraser  <simon.fraser@apple.com>
    256
  • trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

    r259532 r259585  
    21202120}
    21212121
     2122static bool nonInheritedColorPropertyHasValueCurrentColor(CSSPropertyID propertyID, const RenderStyle* style)
     2123{
     2124    if (CSSProperty::isInheritedProperty(propertyID) || !CSSProperty::isColorProperty(propertyID))
     2125        return false;
     2126
     2127    if (!style)
     2128        return true;
     2129
     2130    return RenderStyle::isCurrentColor(style->unresolvedColorForProperty(propertyID));
     2131}
     2132
    21222133// In CSS 2.1 the returned object should actually contain the "used values"
    21232134// rather then the "computed values" (despite the name saying otherwise).
     
    21562167        if (maybeExplicitlyInherited) {
    21572168            auto* style = currentElement->renderStyle();
     2169            // While most color properties are not inherited, the value 'currentcolor' resolves to the value of the inherited 'color' property.
     2170            if (nonInheritedColorPropertyHasValueCurrentColor(propertyID, style))
     2171                isInherited = true;
     2172
    21582173            maybeExplicitlyInherited = !style || style->hasExplicitlyInheritedProperties();
    21592174        }
     
    21772192    document.styleScope().flushPendingUpdate();
    21782193
    2179     if (hasValidStyleForProperty(element, propertyID))
     2194    auto hasValidStyle = [&] {
     2195        auto shorthand = shorthandForProperty(propertyID);
     2196        if (shorthand.length()) {
     2197            for (size_t i = 0; i < shorthand.length(); ++i) {
     2198                if (!hasValidStyleForProperty(element, shorthand.properties()[i]))
     2199                    return false;
     2200            }
     2201            return true;
     2202        }
     2203        return hasValidStyleForProperty(element, propertyID);
     2204    }();
     2205
     2206    if (hasValidStyle)
    21802207        return false;
    21812208
  • trunk/Source/WebCore/css/CSSProperty.cpp

    r256477 r259585  
    213213}
    214214
     215bool CSSProperty::isColorProperty(CSSPropertyID propertyId)
     216{
     217    switch (propertyId) {
     218    case CSSPropertyColor:
     219    case CSSPropertyBackgroundColor:
     220    case CSSPropertyBorderBottomColor:
     221    case CSSPropertyBorderLeftColor:
     222    case CSSPropertyBorderRightColor:
     223    case CSSPropertyBorderTopColor:
     224    case CSSPropertyFill:
     225    case CSSPropertyFloodColor:
     226    case CSSPropertyLightingColor:
     227    case CSSPropertyOutlineColor:
     228    case CSSPropertyStopColor:
     229    case CSSPropertyStroke:
     230    case CSSPropertyStrokeColor:
     231    case CSSPropertyBorderBlockEndColor:
     232    case CSSPropertyBorderBlockStartColor:
     233    case CSSPropertyBorderInlineEndColor:
     234    case CSSPropertyBorderInlineStartColor:
     235    case CSSPropertyColumnRuleColor:
     236    case CSSPropertyWebkitTextEmphasisColor:
     237    case CSSPropertyWebkitTextFillColor:
     238    case CSSPropertyWebkitTextStrokeColor:
     239    case CSSPropertyTextDecorationColor:
     240    case CSSPropertyCaretColor:
     241        return true;
     242    default:
     243        return false;
     244    }
     245}
     246
    215247} // namespace WebCore
  • trunk/Source/WebCore/css/CSSProperty.h

    r236091 r259585  
    8282    static bool isDirectionAwareProperty(CSSPropertyID);
    8383    static bool isDescriptorOnly(CSSPropertyID);
     84    static bool isColorProperty(CSSPropertyID);
    8485
    8586    const StylePropertyMetadata& metadata() const { return m_metadata; }
  • trunk/Source/WebCore/css/parser/CSSParserFastPaths.cpp

    r259006 r259585  
    178178
    179179    return CSSPrimitiveValue::create(number, unit);
    180 }
    181 
    182 static inline bool isColorPropertyID(CSSPropertyID propertyId)
    183 {
    184     switch (propertyId) {
    185     case CSSPropertyColor:
    186     case CSSPropertyBackgroundColor:
    187     case CSSPropertyBorderBottomColor:
    188     case CSSPropertyBorderLeftColor:
    189     case CSSPropertyBorderRightColor:
    190     case CSSPropertyBorderTopColor:
    191     case CSSPropertyFill:
    192     case CSSPropertyFloodColor:
    193     case CSSPropertyLightingColor:
    194     case CSSPropertyOutlineColor:
    195     case CSSPropertyStopColor:
    196     case CSSPropertyStroke:
    197     case CSSPropertyStrokeColor:
    198     case CSSPropertyBorderBlockEndColor:
    199     case CSSPropertyBorderBlockStartColor:
    200     case CSSPropertyBorderInlineEndColor:
    201     case CSSPropertyBorderInlineStartColor:
    202     case CSSPropertyColumnRuleColor:
    203     case CSSPropertyWebkitTextEmphasisColor:
    204     case CSSPropertyWebkitTextFillColor:
    205     case CSSPropertyWebkitTextStrokeColor:
    206     case CSSPropertyTextDecorationColor:
    207         return true;
    208     default:
    209         return false;
    210     }
    211180}
    212181
     
    13121281    if (propertyID == CSSPropertyCaretColor)
    13131282        return parseCaretColor(string, context.mode);
    1314     if (isColorPropertyID(propertyID))
     1283    if (CSSProperty::isColorProperty(propertyID))
    13151284        return parseColor(string, context.mode, CSSValuePool::singleton());
    13161285    if (auto result = parseKeywordValue(propertyID, string, context))
  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r259562 r259585  
    19481948}
    19491949
    1950 Color RenderStyle::colorIncludingFallback(CSSPropertyID colorProperty, bool visitedLink) const
    1951 {
    1952     Color result;
    1953     BorderStyle borderStyle = BorderStyle::None;
     1950Color RenderStyle::unresolvedColorForProperty(CSSPropertyID colorProperty, bool visitedLink) const
     1951{
    19541952    switch (colorProperty) {
     1953    case CSSPropertyColor:
     1954        return visitedLink ? visitedLinkColor() : color();
    19551955    case CSSPropertyBackgroundColor:
    1956         result = visitedLink ? visitedLinkBackgroundColor() : backgroundColor();
    1957         break;
     1956        return visitedLink ? visitedLinkBackgroundColor() : backgroundColor();
     1957    case CSSPropertyBorderBottomColor:
     1958        return visitedLink ? visitedLinkBorderBottomColor() : borderBottomColor();
    19581959    case CSSPropertyBorderLeftColor:
    1959         result = visitedLink ? visitedLinkBorderLeftColor() : borderLeftColor();
    1960         borderStyle = borderLeftStyle();
    1961         break;
     1960        return visitedLink ? visitedLinkBorderLeftColor() : borderLeftColor();
    19621961    case CSSPropertyBorderRightColor:
    1963         result = visitedLink ? visitedLinkBorderRightColor() : borderRightColor();
    1964         borderStyle = borderRightStyle();
    1965         break;
     1962        return visitedLink ? visitedLinkBorderRightColor() : borderRightColor();
    19661963    case CSSPropertyBorderTopColor:
    1967         result = visitedLink ? visitedLinkBorderTopColor() : borderTopColor();
    1968         borderStyle = borderTopStyle();
    1969         break;
    1970     case CSSPropertyBorderBottomColor:
    1971         result = visitedLink ? visitedLinkBorderBottomColor() : borderBottomColor();
    1972         borderStyle = borderBottomStyle();
    1973         break;
     1964        return visitedLink ? visitedLinkBorderTopColor() : borderTopColor();
     1965    case CSSPropertyFill:
     1966        return fillPaintColor();
     1967    case CSSPropertyFloodColor:
     1968        return floodColor();
     1969    case CSSPropertyLightingColor:
     1970        return lightingColor();
     1971    case CSSPropertyOutlineColor:
     1972        return visitedLink ? visitedLinkOutlineColor() : outlineColor();
     1973    case CSSPropertyStopColor:
     1974        return stopColor();
     1975    case CSSPropertyStroke:
     1976        return strokePaintColor();
     1977    case CSSPropertyStrokeColor:
     1978        return visitedLink ? visitedLinkStrokeColor() : strokeColor();
     1979    case CSSPropertyBorderBlockEndColor:
     1980    case CSSPropertyBorderBlockStartColor:
     1981    case CSSPropertyBorderInlineEndColor:
     1982    case CSSPropertyBorderInlineStartColor:
     1983        return unresolvedColorForProperty(CSSProperty::resolveDirectionAwareProperty(colorProperty, direction(), writingMode()));
     1984    case CSSPropertyColumnRuleColor:
     1985        return visitedLink ? visitedLinkColumnRuleColor() : columnRuleColor();
     1986    case CSSPropertyWebkitTextEmphasisColor:
     1987        return visitedLink ? visitedLinkTextEmphasisColor() : textEmphasisColor();
     1988    case CSSPropertyWebkitTextFillColor:
     1989        return visitedLink ? visitedLinkTextFillColor() : textFillColor();
     1990    case CSSPropertyWebkitTextStrokeColor:
     1991        return visitedLink ? visitedLinkTextStrokeColor() : textStrokeColor();
     1992    case CSSPropertyTextDecorationColor:
     1993        return visitedLink ? visitedLinkTextDecorationColor() : textDecorationColor();
    19741994    case CSSPropertyCaretColor:
    1975         result = visitedLink ? visitedLinkCaretColor() : caretColor();
    1976         break;
    1977     case CSSPropertyColor:
    1978         result = visitedLink ? visitedLinkColor() : color();
    1979         break;
    1980     case CSSPropertyOutlineColor:
    1981         result = visitedLink ? visitedLinkOutlineColor() : outlineColor();
    1982         break;
    1983     case CSSPropertyColumnRuleColor:
    1984         result = visitedLink ? visitedLinkColumnRuleColor() : columnRuleColor();
    1985         break;
    1986     case CSSPropertyTextDecorationColor:
    1987         // Text decoration color fallback is handled in RenderObject::decorationColor.
    1988         return visitedLink ? visitedLinkTextDecorationColor() : textDecorationColor();
    1989     case CSSPropertyWebkitTextEmphasisColor:
    1990         result = visitedLink ? visitedLinkTextEmphasisColor() : textEmphasisColor();
    1991         break;
    1992     case CSSPropertyWebkitTextFillColor:
    1993         result = visitedLink ? visitedLinkTextFillColor() : textFillColor();
    1994         break;
    1995     case CSSPropertyWebkitTextStrokeColor:
    1996         result = visitedLink ? visitedLinkTextStrokeColor() : textStrokeColor();
    1997         break;
    1998     case CSSPropertyStrokeColor:
    1999         result = visitedLink ? visitedLinkStrokeColor() : strokeColor();
    2000         break;
     1995        return visitedLink ? visitedLinkCaretColor() : caretColor();
    20011996    default:
    20021997        ASSERT_NOT_REACHED();
     
    20041999    }
    20052000
    2006     if (!result.isValid()) {
     2001    return { };
     2002}
     2003
     2004Color RenderStyle::colorResolvingCurrentColor(CSSPropertyID colorProperty, bool visitedLink) const
     2005{
     2006    auto computeBorderStyle = [&] {
     2007        switch (colorProperty) {
     2008        case CSSPropertyBorderLeftColor:
     2009            return borderLeftStyle();
     2010        case CSSPropertyBorderRightColor:
     2011            return borderRightStyle();
     2012        case CSSPropertyBorderTopColor:
     2013            return borderTopStyle();
     2014        case CSSPropertyBorderBottomColor:
     2015            return borderBottomStyle();
     2016        default:
     2017            return BorderStyle::None;
     2018        }
     2019    };
     2020
     2021    auto result = unresolvedColorForProperty(colorProperty, visitedLink);
     2022
     2023    if (isCurrentColor(result)) {
     2024        auto borderStyle = computeBorderStyle();
    20072025        if (!visitedLink && (borderStyle == BorderStyle::Inset || borderStyle == BorderStyle::Outset || borderStyle == BorderStyle::Ridge || borderStyle == BorderStyle::Groove))
    2008             result = Color(238, 238, 238);
    2009         else
    2010             result = visitedLink ? visitedLinkColor() : color();
    2011     }
     2026            return Color(238, 238, 238);
     2027
     2028        return visitedLink ? visitedLinkColor() : color();
     2029    }
     2030
    20122031    return result;
    20132032}
     
    20152034Color RenderStyle::colorResolvingCurrentColor(const Color& color) const
    20162035{
    2017     if (color == currentColor())
     2036    if (isCurrentColor(color))
    20182037        return this->color();
    20192038
     
    20232042Color RenderStyle::visitedDependentColor(CSSPropertyID colorProperty) const
    20242043{
    2025     Color unvisitedColor = colorIncludingFallback(colorProperty, false);
     2044    Color unvisitedColor = colorResolvingCurrentColor(colorProperty, false);
    20262045    if (insideLink() != InsideLink::InsideVisited)
    20272046        return unvisitedColor;
    20282047
    2029     Color visitedColor = colorIncludingFallback(colorProperty, true);
     2048    Color visitedColor = colorResolvingCurrentColor(colorProperty, true);
    20302049
    20312050    // Text decoration color validity is preserved (checked in RenderObject::decorationColor).
  • trunk/Source/WebCore/rendering/style/RenderStyle.h

    r259532 r259585  
    14411441    void setLastChildState() { setUnique(); m_nonInheritedFlags.lastChildState = true; }
    14421442
     1443    Color unresolvedColorForProperty(CSSPropertyID colorProperty, bool visitedLink = false) const;
    14431444    Color colorResolvingCurrentColor(const Color&) const;
     1445
    14441446    WEBCORE_EXPORT Color visitedDependentColor(CSSPropertyID) const;
    14451447    WEBCORE_EXPORT Color visitedDependentColorWithColorFilter(CSSPropertyID) const;
     
    17351737    // In RenderStyle invalid color value is used to signify 'currentcolor' which resolves to color().
    17361738    static Color currentColor() { return { }; }
     1739    static bool isCurrentColor(const Color& color) { return !color.isValid(); }
     1740
    17371741    const Color& borderLeftColor() const { return m_surroundData->border.left().color(); }
    17381742    const Color& borderRightColor() const { return m_surroundData->border.right().color(); }
     
    18671871    static bool isDisplayFlexibleOrGridBox(DisplayType);
    18681872
    1869     Color colorIncludingFallback(CSSPropertyID colorProperty, bool visitedLink) const;
     1873    Color colorResolvingCurrentColor(CSSPropertyID colorProperty, bool visitedLink) const;
    18701874
    18711875    bool changeAffectsVisualOverflow(const RenderStyle&) const;
  • trunk/Source/WebCore/style/StyleBuilderState.cpp

    r259532 r259585  
    312312        return RenderTheme::singleton().focusRingColor(document().styleColorOptions(&m_style));
    313313    case CSSValueCurrentcolor:
    314         // Color is an inherited property so depending on it effectively makes the property inherited.
    315         m_style.setHasExplicitlyInheritedProperties();
    316314        return RenderStyle::currentColor();
    317315    default:
Note: See TracChangeset for help on using the changeset viewer.