CSS: Fall back to cache-less cascade when encountering explicitly inherited value.

<https://webkit.org/b/125968>

When encountering an explicitly inherited value for a property that's not
"statically inherited", drop out of the matched properties cache path
immediately instead of waiting for some coincidence to trigger it later on.

Fixes 3 asserting table tests:

- fast/table/border-collapsing/cached-69296.html
- tables/mozilla/bugs/bug27038-3.html
- tables/mozilla_expected_failures/marvin/backgr_border-table-row-group.html

Reviewed by Antti Koivisto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@160820 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 931861610cbb3cac188ef03ff45c8fbe786e610e..c407c3599481ef9ac3f62ba4353f23c5f5e4a5cb 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2013-12-18  Andreas Kling  <akling@apple.com>
+
+        CSS: Fall back to cache-less cascade when encountering explicitly inherited value.
+        <https://webkit.org/b/125968>
+
+        When encountering an explicitly inherited value for a property that's not
+        "statically inherited", drop out of the matched properties cache path
+        immediately instead of waiting for some coincidence to trigger it later on.
+
+        Fixes 3 asserting table tests:
+
+        - fast/table/border-collapsing/cached-69296.html
+        - tables/mozilla/bugs/bug27038-3.html
+        - tables/mozilla_expected_failures/marvin/backgr_border-table-row-group.html
+
+        Reviewed by Antti Koivisto.
+
 2013-12-18  Ryosuke Niwa  <rniwa@webkit.org>
 
         Crash in WebCore::LogicalSelectionOffsetCaches::LogicalSelectionOffsetCaches

diff --git a/Source/WebCore/css/StyleResolver.cpp b/Source/WebCore/css/StyleResolver.cpp
index cf7f2b8457bdb25fe5b0b19ea2d91a9ed5f7b7f3..15f45dc3a65252f60efe2d3bad1a8ebe324e6d82 100644 (file)
--- a/Source/WebCore/css/StyleResolver.cpp
+++ b/Source/WebCore/css/StyleResolver.cpp
@@ -204,7 +204,7 @@ public:
     };
 
     Property& property(CSSPropertyID);
-    void addMatches(const MatchResult&, bool important, int startIndex, int endIndex, bool inheritedOnly);
+    bool addMatches(const MatchResult&, bool important, int startIndex, int endIndex, bool inheritedOnly);
 
     void set(CSSPropertyID, CSSValue&, unsigned linkMatchType);
     void setDeferred(CSSPropertyID, CSSValue&, unsigned linkMatchType);
@@ -212,7 +212,7 @@ public:
     void applyDeferredProperties(StyleResolver&);
 
 private:
-    void addStyleProperties(const StyleProperties&, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType, unsigned linkMatchType);
+    bool addStyleProperties(const StyleProperties&, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType, unsigned linkMatchType);
     static void setPropertyInternal(Property&, CSSPropertyID, CSSValue&, unsigned linkMatchType);
 
     Property m_properties[numCSSProperties + 1];
@@ -1830,8 +1830,9 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
         // can look at them later to figure out if this is a styled form control or not.
         state.setLineHeightValue(nullptr);
         CascadedProperties cascade(direction, writingMode);
-        cascade.addMatches(matchResult, false, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
-        cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
+        if (!cascade.addMatches(matchResult, false, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly)
+            || !cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly))
+            return applyMatchedProperties(matchResult, element, DoNotUseMatchedPropertiesCache);
 
         applyCascadedProperties(cascade, firstCSSProperty, CSSPropertyLineHeight);
         updateFont();
@@ -1841,10 +1842,11 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
     }
 
     CascadedProperties cascade(direction, writingMode);
-    cascade.addMatches(matchResult, false, 0, matchResult.matchedProperties.size() - 1, applyInheritedOnly);
-    cascade.addMatches(matchResult, true, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, applyInheritedOnly);
-    cascade.addMatches(matchResult, true, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly);
-    cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
+    if (!cascade.addMatches(matchResult, false, 0, matchResult.matchedProperties.size() - 1, applyInheritedOnly)
+        || !cascade.addMatches(matchResult, true, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, applyInheritedOnly)
+        || !cascade.addMatches(matchResult, true, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly)
+        || !cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly))
+        return applyMatchedProperties(matchResult, element, DoNotUseMatchedPropertiesCache);
 
     state.setLineHeightValue(nullptr);
 
@@ -4237,7 +4239,7 @@ void StyleResolver::CascadedProperties::setDeferred(CSSPropertyID id, CSSValue&
     m_deferredProperties.append(property);
 }
 
-void StyleResolver::CascadedProperties::addStyleProperties(const StyleProperties& properties, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType propertyWhitelistType, unsigned linkMatchType)
+bool StyleResolver::CascadedProperties::addStyleProperties(const StyleProperties& properties, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType propertyWhitelistType, unsigned linkMatchType)
 {
     for (unsigned i = 0, count = properties.propertyCount(); i < count; ++i) {
         auto current = properties.propertyAt(i);
@@ -4247,7 +4249,8 @@ void StyleResolver::CascadedProperties::addStyleProperties(const StyleProperties
             // If the property value is explicitly inherited, we need to apply further non-inherited properties
             // as they might override the value inherited here. For this reason we don't allow declarations with
             // explicitly inherited properties to be cached.
-            ASSERT(!current.value()->isInheritedValue());
+            if (current.value()->isInheritedValue())
+                return false;
             continue;
         }
         CSSPropertyID propertyID = current.id();
@@ -4264,17 +4267,20 @@ void StyleResolver::CascadedProperties::addStyleProperties(const StyleProperties
         else
             set(propertyID, *current.value(), linkMatchType);
     }
+    return true;
 }
 
-void StyleResolver::CascadedProperties::addMatches(const MatchResult& matchResult, bool important, int startIndex, int endIndex, bool inheritedOnly)
+bool StyleResolver::CascadedProperties::addMatches(const MatchResult& matchResult, bool important, int startIndex, int endIndex, bool inheritedOnly)
 {
     if (startIndex == -1)
-        return;
+        return true;
 
     for (int i = startIndex; i <= endIndex; ++i) {
         const MatchedProperties& matchedProperties = matchResult.matchedProperties[i];
-        addStyleProperties(*matchedProperties.properties, *matchResult.matchedRules[i], important, inheritedOnly, static_cast<PropertyWhitelistType>(matchedProperties.whitelistType), matchedProperties.linkMatchType);
+        if (!addStyleProperties(*matchedProperties.properties, *matchResult.matchedRules[i], important, inheritedOnly, static_cast<PropertyWhitelistType>(matchedProperties.whitelistType), matchedProperties.linkMatchType))
+            return false;
     }
+    return true;
 }
 
 void StyleResolver::CascadedProperties::applyDeferredProperties(StyleResolver& resolver)