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

Source/core/css/resolver/CascadedValues.cpp

Index: Source/core/css/resolver/CascadedValues.cpp
diff --git a/Source/core/css/resolver/CascadedValues.cpp b/Source/core/css/resolver/CascadedValues.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..78e36c0eddb51b5ab74487852a919447c11433b6
--- /dev/null
+++ b/Source/core/css/resolver/CascadedValues.cpp
@@ -0,0 +1,387 @@
+// 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 blink {
+
+namespace {
+
+bool isValidCueStyleProperty(CSSPropertyID property)

[esprehn, 2014/07/21 18:50:34]

static, get rid of the anonymous namespace.

[Timothy Loh, 2014/07/22 14:32:48]

Done.

+{
+    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)

[esprehn, 2014/07/21 18:50:34]

static

[Timothy Loh, 2014/07/22 14:32:48]

Done.

+{
+    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)

[esprehn, 2014/07/21 18:50:34]

static

[Timothy Loh, 2014/07/22 14:32:48]

Done.

+{
+    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));
+
+    addMatchResultInternal(matchResult, false, 0, matchResult.matchedProperties.size() - 1, CSSPropertyDirection, CSSPropertyWebkitWritingMode);

[esprehn, 2014/07/21 18:50:34]

Can we get rid of this boolean argument?

[Timothy Loh, 2014/07/22 14:32:48]

The MatchResult stores declarations in the order (UA Rules -> Author rules), but
we want the order (UA not important -> author not important -> author important
-> UA important), so there's not really much we can do here. I've simplfiied the
interface a bit though, so now the ctor here will add the rest of the values in
the MatchResult.

+    addMatchResultInternal(matchResult, true, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, CSSPropertyDirection, CSSPropertyWebkitWritingMode);
+    addMatchResultInternal(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, CSSPropertyDirection, CSSPropertyWebkitWritingMode);
+    applyValues(CSSPropertyDirection, CSSPropertyWebkitWritingMode);
+    direction = state.style()->direction();
+    writingMode = state.style()->writingMode();
+}
+
+void CascadedValues::addMatchResult(const MatchResult& matchResult, bool isImportant, int firstIndex, int lastIndex)
+{
+    COMPILE_ASSERT(CSSPropertyColor == CSSPropertyWebkitWritingMode + 1, CSS_color_is_after_super_high_priority);
+    addMatchResultInternal(matchResult, isImportant, firstIndex, lastIndex, CSSPropertyColor, convertToCSSPropertyID(lastCSSProperty));
+}
+
+void CascadedValues::addMatchResultInternal(const MatchResult& matchResult, bool isImportant, int firstIndex, int lastIndex, CSSPropertyID firstId, CSSPropertyID lastId)
+{
+    if (firstIndex == -1)
+        return;
+
+    for (int i = firstIndex; i <= lastIndex; ++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);
+        addStylePropertySet(matchedProperties.properties.get(), matchResult.matchedRules[i], isImportant, firstId, lastId, static_cast<PropertyWhitelistType>(matchedProperties.m_types.whitelistType), linkMatchType);
+    }
+}
+
+void CascadedValues::addStylePropertySet(const StylePropertySet* properties, const StyleRule* rule, bool isImportant, CSSPropertyID first, CSSPropertyID last, 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();
+        if (property < first || property > last)
+            continue;
+
+        // 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::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;
+    }
+
+    // Shorthands that are not expanded in the parser need special handling here.
+    // The shorthand property occurs earlier in CSSPropertyNames.in than its
+    // longhands so we can cascade properly. If a longhand value is explicitly
+    // set, with higher priority than the shorthand, then the value computed
+    // from the shorthand will be overriden.
+
+    // FIXME: font should always be expanded by the parser; crbug.com/353932
+    if (property == CSSPropertyFont) {
+        values[CSSPropertyLineHeight - firstCSSProperty] = nullptr;

[esprehn, 2014/07/21 18:50:34]

Why are you always subtracting backwards? I think you want to make this either a
macro or a function with a comment.

values[INDEX(CSSPropertyLineHeight)] = nullptr

[Timothy Loh, 2014/07/22 14:32:48]

Done.

+        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 sometimes also sets border-*-width, but properly
+    // cascading this requires too much effort
+    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::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 = convertToCSSPropertyID(i);
+        if (CSSProperty::isAffectedByAllProperty(property))
+            addValue(property, value, nullptr, linkMatchType);
+    }
+}
+
+void CascadedValues::applyValues(CSSPropertyID first, CSSPropertyID last, bool inheritedOnly)
+{
+    for (int i = first - firstCSSProperty; i <= last - firstCSSProperty; i++) {
+        CSSPropertyID property = convertToCSSPropertyID(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 blink