Cascade declared property values instead of applying values on top of each other

Source/core/css/resolver/CascadedValues.cpp

+// Copyright 2014 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 "config.h"
+#include "core/css/resolver/CascadedValues.h"
+
+#include "core/css/CSSValuePool.h"
+#include "core/css/StylePropertySet.h"
+#include "core/css/resolver/MatchResult.h"
+#include "core/css/resolver/StyleBuilder.h"
+#include "core/css/resolver/StyleResolverState.h"
+
+namespace WebCore {
+
+namespace {
+
+bool isValidCueStyleProperty(CSSPropertyID property)
+{
+    switch (property) {
+    case CSSPropertyBackground:
+    case CSSPropertyBackgroundAttachment:
+    case CSSPropertyBackgroundClip:
+    case CSSPropertyBackgroundColor:
+    case CSSPropertyBackgroundImage:
+    case CSSPropertyBackgroundOrigin:
+    case CSSPropertyBackgroundPosition:
+    case CSSPropertyBackgroundPositionX:
+    case CSSPropertyBackgroundPositionY:
+    case CSSPropertyBackgroundRepeat:
+    case CSSPropertyBackgroundRepeatX:
+    case CSSPropertyBackgroundRepeatY:
+    case CSSPropertyBackgroundSize:
+    case CSSPropertyColor:
+    case CSSPropertyFont:
+    case CSSPropertyFontFamily:
+    case CSSPropertyFontSize:
+    case CSSPropertyFontStyle:
+    case CSSPropertyFontVariant:
+    case CSSPropertyFontWeight:
+    case CSSPropertyLineHeight:
+    case CSSPropertyOpacity:
+    case CSSPropertyOutline:
+    case CSSPropertyOutlineColor:
+    case CSSPropertyOutlineOffset:
+    case CSSPropertyOutlineStyle:
+    case CSSPropertyOutlineWidth:
+    case CSSPropertyVisibility:
+    case CSSPropertyWhiteSpace:
+    // FIXME: 'text-decoration' shorthand to be handled when available.
+    // See https://chromiumcodereview.appspot.com/19516002 for details.
+    case CSSPropertyTextDecoration:
+    case CSSPropertyTextShadow:
+    case CSSPropertyBorderStyle:
+        return true;
+    case CSSPropertyTextDecorationLine:
+    case CSSPropertyTextDecorationStyle:
+    case CSSPropertyTextDecorationColor:
+        return RuntimeEnabledFeatures::css3TextDecorationsEnabled();
+    default:
+        break;
+    }
+    return false;
+}
+
+bool isValidFirstLetterStyleProperty(CSSPropertyID property)
+{
+    switch (property) {
+    // Valid ::first-letter properties listed in spec:
+    // http://www.w3.org/TR/css3-selectors/#application-in-css
+    case CSSPropertyBackgroundAttachment:
+    case CSSPropertyBackgroundBlendMode:
+    case CSSPropertyBackgroundClip:
+    case CSSPropertyBackgroundColor:
+    case CSSPropertyBackgroundImage:
+    case CSSPropertyBackgroundOrigin:
+    case CSSPropertyBackgroundPosition:
+    case CSSPropertyBackgroundPositionX:
+    case CSSPropertyBackgroundPositionY:
+    case CSSPropertyBackgroundRepeat:
+    case CSSPropertyBackgroundRepeatX:
+    case CSSPropertyBackgroundRepeatY:
+    case CSSPropertyBackgroundSize:
+    case CSSPropertyBorderBottomColor:
+    case CSSPropertyBorderBottomLeftRadius:
+    case CSSPropertyBorderBottomRightRadius:
+    case CSSPropertyBorderBottomStyle:
+    case CSSPropertyBorderBottomWidth:
+    case CSSPropertyBorderImageOutset:
+    case CSSPropertyBorderImageRepeat:
+    case CSSPropertyBorderImageSlice:
+    case CSSPropertyBorderImageSource:
+    case CSSPropertyBorderImageWidth:
+    case CSSPropertyBorderLeftColor:
+    case CSSPropertyBorderLeftStyle:
+    case CSSPropertyBorderLeftWidth:
+    case CSSPropertyBorderRightColor:
+    case CSSPropertyBorderRightStyle:
+    case CSSPropertyBorderRightWidth:
+    case CSSPropertyBorderTopColor:
+    case CSSPropertyBorderTopLeftRadius:
+    case CSSPropertyBorderTopRightRadius:
+    case CSSPropertyBorderTopStyle:
+    case CSSPropertyBorderTopWidth:
+    case CSSPropertyColor:
+    case CSSPropertyFloat:
+    case CSSPropertyFont:
+    case CSSPropertyFontFamily:
+    case CSSPropertyFontKerning:
+    case CSSPropertyFontSize:
+    case CSSPropertyFontStretch:
+    case CSSPropertyFontStyle:
+    case CSSPropertyFontVariant:
+    case CSSPropertyFontVariantLigatures:
+    case CSSPropertyFontWeight:
+    case CSSPropertyLetterSpacing:
+    case CSSPropertyLineHeight:
+    case CSSPropertyMarginBottom:
+    case CSSPropertyMarginLeft:
+    case CSSPropertyMarginRight:
+    case CSSPropertyMarginTop:
+    case CSSPropertyPaddingBottom:
+    case CSSPropertyPaddingLeft:
+    case CSSPropertyPaddingRight:
+    case CSSPropertyPaddingTop:
+    case CSSPropertyTextTransform:
+    case CSSPropertyVerticalAlign:
+    case CSSPropertyWebkitBackgroundClip:
+    case CSSPropertyWebkitBackgroundComposite:
+    case CSSPropertyWebkitBackgroundOrigin:
+    case CSSPropertyWebkitBackgroundSize:
+    case CSSPropertyWebkitBorderAfter:
+    case CSSPropertyWebkitBorderAfterColor:
+    case CSSPropertyWebkitBorderAfterStyle:
+    case CSSPropertyWebkitBorderAfterWidth:
+    case CSSPropertyWebkitBorderBefore:
+    case CSSPropertyWebkitBorderBeforeColor:
+    case CSSPropertyWebkitBorderBeforeStyle:
+    case CSSPropertyWebkitBorderBeforeWidth:
+    case CSSPropertyWebkitBorderEnd:
+    case CSSPropertyWebkitBorderEndColor:
+    case CSSPropertyWebkitBorderEndStyle:
+    case CSSPropertyWebkitBorderEndWidth:
+    case CSSPropertyWebkitBorderFit:
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+    case CSSPropertyWebkitBorderImage:
+    case CSSPropertyWebkitBorderRadius:
+    case CSSPropertyWebkitBorderStart:
+    case CSSPropertyWebkitBorderStartColor:
+    case CSSPropertyWebkitBorderStartStyle:
+    case CSSPropertyWebkitBorderStartWidth:
+    case CSSPropertyWebkitBorderVerticalSpacing:
+    case CSSPropertyWebkitFontSmoothing:
+    case CSSPropertyWebkitMarginAfter:
+    case CSSPropertyWebkitMarginAfterCollapse:
+    case CSSPropertyWebkitMarginBefore:
+    case CSSPropertyWebkitMarginBeforeCollapse:
+    case CSSPropertyWebkitMarginBottomCollapse:
+    case CSSPropertyWebkitMarginCollapse:
+    case CSSPropertyWebkitMarginEnd:
+    case CSSPropertyWebkitMarginStart:
+    case CSSPropertyWebkitMarginTopCollapse:
+    case CSSPropertyWordSpacing:
+        return true;
+    case CSSPropertyTextDecorationColor:
+    case CSSPropertyTextDecorationLine:
+    case CSSPropertyTextDecorationStyle:
+        return RuntimeEnabledFeatures::css3TextDecorationsEnabled();
+
+    // text-shadow added in text decoration spec:
+    // http://www.w3.org/TR/css-text-decor-3/#text-shadow-property
+    case CSSPropertyTextShadow:
+    // box-shadox added in CSS3 backgrounds spec:
+    // http://www.w3.org/TR/css3-background/#placement
+    case CSSPropertyBoxShadow:
+    case CSSPropertyWebkitBoxShadow:
+    // Properties that we currently support outside of spec.
+    case CSSPropertyWebkitLineBoxContain:
+    case CSSPropertyVisibility:
+        return true;
+
+    default:
+        return false;
+    }
+}
+
+// We want properties in their prefixed and non-prefixed variants to cascade
+// together, so we map them to the same property before adding to the cascade.
+CSSPropertyID resolveProperty(CSSPropertyID property, TextDirection direction, WritingMode writingMode)
+{
+    switch (property) {
+    case CSSPropertyWebkitBorderEndColor:
+    case CSSPropertyWebkitBorderBeforeStyle:
+    case CSSPropertyWebkitBorderEndStyle:
+    case CSSPropertyWebkitPaddingStart:
+    case CSSPropertyWebkitBorderStartWidth:
+    case CSSPropertyWebkitMaxLogicalWidth:
+    case CSSPropertyWebkitLogicalHeight:
+    case CSSPropertyWebkitMinLogicalWidth:
+    case CSSPropertyWebkitBorderBeforeWidth:
+    case CSSPropertyWebkitPaddingBefore:
+    case CSSPropertyWebkitBorderBeforeColor:
+    case CSSPropertyWebkitMarginEnd:
+    case CSSPropertyWebkitBorderAfterWidth:
+    case CSSPropertyWebkitMinLogicalHeight:
+    case CSSPropertyWebkitBorderEndWidth:
+    case CSSPropertyWebkitPaddingEnd:
+    case CSSPropertyWebkitLogicalWidth:
+    case CSSPropertyWebkitBorderAfterColor:
+    case CSSPropertyWebkitMaxLogicalHeight:
+    case CSSPropertyWebkitBorderStartColor:
+    case CSSPropertyWebkitBorderAfterStyle:
+    case CSSPropertyWebkitPaddingAfter:
+    case CSSPropertyWebkitBorderStartStyle:
+    case CSSPropertyWebkitMarginBefore:
+    case CSSPropertyWebkitMarginStart:
+    case CSSPropertyWebkitMarginAfter:
+        return CSSProperty::resolveDirectionAwareProperty(property, direction, writingMode);
+    case CSSPropertyWebkitBackfaceVisibility:
+        return CSSPropertyBackfaceVisibility;
+    case CSSPropertyWebkitBackgroundClip:
+        return CSSPropertyBackgroundClip;
+    case CSSPropertyWebkitBackgroundOrigin:
+        return CSSPropertyBackgroundOrigin;
+    case CSSPropertyWebkitBackgroundSize:
+        return CSSPropertyBackgroundSize;
+    case CSSPropertyWebkitBoxShadow:
+        return CSSPropertyBoxShadow;
+    case CSSPropertyWebkitTransform:
+        return CSSPropertyTransform;
+    case CSSPropertyWebkitTransformStyle:
+        return CSSPropertyTransformStyle;
+    case CSSPropertyWebkitPerspective:
+        return CSSPropertyPerspective;
+    case CSSPropertyWebkitPerspectiveOrigin:
+        return CSSPropertyPerspectiveOrigin;
+    // Used by editing when the css3 text decorations flag is off
+    case CSSPropertyTextDecoration:
+        ASSERT(!RuntimeEnabledFeatures::css3TextDecorationsEnabled());
+        return CSSPropertyTextDecorationLine;
+    // As per css3-text, word-wrap is an alias for overflow-wrap
+    case CSSPropertyWordWrap:
+        return CSSPropertyOverflowWrap;
+    default:
+        return property;
+    }
+}
+
+} // namespace
+
+CascadedValues::CascadedValues(StyleResolverState& state, const MatchResult& matchResult)
+    : state(state)
+{
+    memset(values, 0, sizeof(values));
+    memset(visitedLinkValues, 0, sizeof(visitedLinkValues));
+
+    bool seenImportantDirection = false;
+    bool seenImportantWritingMode = false;
+    for (size_t i = 0; i < matchResult.matchedProperties.size(); ++i) {

[andersr, 2014/07/17 12:15:38]

This code *seems* annoyingly redundant, since it does approximately a subset of
what CascadedValues does as a whole.

Would it be possible to move '-webkit-writing-mode' next to 'direction' in
CSSPropertyNames.in (to create an ultra-high priority segment), then extend some
'addValues' method with a valid range of property names to add? Then you could
perhaps apply this more "normally", e.g.:

CascadedValues::CascadedValues(...) {
   // memset ...
   addValues(matchResult, ... , CSSPropertyDirection,
CSSPropertyWebkitWritingMode);
   applyValues(CSSPropertyDirection, CSSPropertyWebkitWritingMode, false);
   direction = ...;
   writingMode = ...;
}

Although the !important handling complicates things a bit here ...

[Timothy Loh, 2014/07/21 06:05:59]

Done.

+        const StylePropertySet& properties = *matchResult.matchedProperties[i].properties;
+        unsigned propertyCount = properties.propertyCount();
+        for (size_t j = 0; j < propertyCount; ++j) {
+            StylePropertySet::PropertyReference current = properties.propertyAt(j);
+            if (current.id() == CSSPropertyDirection) {
+                if (seenImportantDirection && !current.isImportant())
+                    continue;
+                seenImportantDirection = current.isImportant();
+            } else if (current.id() == CSSPropertyWebkitWritingMode) {
+                if (seenImportantWritingMode && !current.isImportant())
+                    continue;
+                seenImportantWritingMode = current.isImportant();
+            } else {
+                continue;
+            }
+            StyleBuilder::applyProperty(current.id(), state, current.value());
+        }
+    }
+
+    direction = state.style()->direction();
+    writingMode = state.style()->writingMode();
+}
+
+void CascadedValues::addAllProperty(CSSValue* value, SelectorChecker::LinkMatchMask linkMatchType)
+{
+    if (!value->isInitialValue() && !value->isInheritedValue()) {
+        ASSERT(value->isPrimitiveValue() && toCSSPrimitiveValue(value)->getValueID() == CSSValueUnset);
+        value = nullptr;
+    }
+
+    for (int i = firstCSSProperty; i <= lastCSSProperty; ++i) {
+        CSSPropertyID property = static_cast<CSSPropertyID>(i);
+        if (CSSProperty::isAffectedByAllProperty(property))
+            addValue(property, value, nullptr, linkMatchType);
+    }
+}
+
+void CascadedValues::addValue(CSSPropertyID property, CSSValue* value, const StyleRule* rule, SelectorChecker::LinkMatchMask linkMatchType)
+{
+    property = resolveProperty(property, direction, writingMode);
+
+    if (property == CSSPropertyInternalCallback) {
+        ASSERT(value->isPrimitiveValue() && toCSSPrimitiveValue(value)->getValueID() == CSSValueInternalPresence);
+        state.style()->addCallbackSelector(rule->selectorList().selectorsText());
+        return;
+    }
+
+    // FIXME: font is a shorthand but system fonts are not currently expanded (crbug.com/353932).
+    // For now, we explicitly clear the properties for which font is a shorthand
+    // and rely on font having a lower CSSPropertyID than these other properties.
+    if (property == CSSPropertyFont) {
+        values[CSSPropertyLineHeight - firstCSSProperty] = nullptr;
+        values[CSSPropertyFontStyle - firstCSSProperty] = nullptr;
+        values[CSSPropertyFontFamily - firstCSSProperty] = nullptr;
+        values[CSSPropertyFontVariant - firstCSSProperty] = nullptr;
+        values[CSSPropertyFontWeight - firstCSSProperty] = nullptr;
+        values[CSSPropertyFontStyle - firstCSSProperty] = nullptr;
+        values[CSSPropertyFontSize - firstCSSProperty] = nullptr;
+    }
+
+    // -webkit-border-image is a shorthand for the below properties but not
+    // expanded in the parser. It sometimes also sets border-*-width, but
+    // we'll not try too hard to support legacy properties
+    if (property == CSSPropertyWebkitBorderImage) {
+        values[CSSPropertyBorderImageSource - firstCSSProperty] = nullptr;
+        values[CSSPropertyBorderImageSlice - firstCSSProperty] = nullptr;
+        values[CSSPropertyBorderImageWidth - firstCSSProperty] = nullptr;
+        values[CSSPropertyBorderImageOutset - firstCSSProperty] = nullptr;
+        values[CSSPropertyBorderImageRepeat - firstCSSProperty] = nullptr;
+    }
+
+    if (linkMatchType & SelectorChecker::MatchLink)
+        values[property - firstCSSProperty] = value;
+    if (linkMatchType & SelectorChecker::MatchVisited)
+        visitedLinkValues[property - firstCSSProperty] = value;
+}
+
+void CascadedValues::addValues(const MatchResult& matchResult, bool isImportant, int startIndex, int endIndex)
+{
+    if (startIndex == -1)
+        return;
+
+    for (int i = startIndex; i <= endIndex; ++i) {
+        const MatchedProperties& matchedProperties = matchResult.matchedProperties[i];
+        SelectorChecker::LinkMatchMask linkMatchType = SelectorChecker::MatchLink;
+        if (state.style()->insideLink())
+            linkMatchType = static_cast<SelectorChecker::LinkMatchMask>(matchedProperties.m_types.linkMatchType);
+        addValues(matchedProperties.properties.get(), matchResult.matchedRules[i], isImportant, static_cast<PropertyWhitelistType>(matchedProperties.m_types.whitelistType), linkMatchType);
+    }
+}
+
+void CascadedValues::addValues(const StylePropertySet* properties, const StyleRule* rule, bool isImportant, PropertyWhitelistType propertyWhitelistType, SelectorChecker::LinkMatchMask linkMatchType)
+{
+    unsigned propertyCount = properties->propertyCount();
+    for (unsigned i = 0; i < propertyCount; ++i) {
+        StylePropertySet::PropertyReference current = properties->propertyAt(i);
+        if (isImportant != current.isImportant())
+            continue;
+
+        CSSPropertyID property = current.id();
+
+        // FIXME: We should check the whitelist, otherwise we might run into
+        // problems with e.g. foo:first-letter { all: inherit; }
+        if (property == CSSPropertyAll) {
+            addAllProperty(current.value(), linkMatchType);
+            continue;
+        }
+
+        if (propertyWhitelistType == PropertyWhitelistCue && !isValidCueStyleProperty(property))
+            continue;
+        if (propertyWhitelistType == PropertyWhitelistFirstLetter && !isValidFirstLetterStyleProperty(property))
+            continue;
+        addValue(current.id(), current.value(), rule, linkMatchType);
+    }
+}
+
+void CascadedValues::applyValues(CSSPropertyID start, CSSPropertyID end, bool inheritedOnly)

[andersr, 2014/07/17 12:15:38]

Is first/last more appropriate than start/end?

   applyValues(CSSPropertyID first, CSSPropertyID last, bool inheritedOnly)

In STL (FWIW), and in wtf/Vector, 'end()' means past-the-last-element, whereas
here it means last-element.

[Timothy Loh, 2014/07/21 06:05:59]

Good idea; changed to first/last.

+{
+    for (int i = start - firstCSSProperty; i <= end - firstCSSProperty; i++) {
+        CSSPropertyID property = static_cast<CSSPropertyID>(i + firstCSSProperty);
+        if (inheritedOnly && !CSSProperty::isInheritedProperty(property))
+            continue;
+        if (values[i])
+            StyleBuilder::applyProperty(property, state, values[i]);
+        if (visitedLinkValues[i]) {
+            state.setApplyPropertyToRegularStyle(false);
+            state.setApplyPropertyToVisitedLinkStyle(true);
+            StyleBuilder::applyProperty(property, state, visitedLinkValues[i]);
+            state.setApplyPropertyToRegularStyle(true);
+            state.setApplyPropertyToVisitedLinkStyle(false);
+        }
+    }
+}
+
+} // namespace WebCore