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

Wed, 18 Dec 2013 19:10:54 -0800

Title: [160820] trunk/Source/WebCore
Revision
160820
Author
[email protected]
Date
2013-12-18 19:13:18 -0800 (Wed, 18 Dec 2013)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (160819 => 160820)


--- trunk/Source/WebCore/ChangeLog	2013-12-19 03:09:56 UTC (rev 160819)
+++ trunk/Source/WebCore/ChangeLog	2013-12-19 03:13:18 UTC (rev 160820)
@@ -1,3 +1,20 @@
+2013-12-18  Andreas Kling  <[email protected]>
+
+        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  <[email protected]>
 
         Crash in WebCore::LogicalSelectionOffsetCaches::LogicalSelectionOffsetCaches

Modified: trunk/Source/WebCore/css/StyleResolver.cpp (160819 => 160820)


--- trunk/Source/WebCore/css/StyleResolver.cpp	2013-12-19 03:09:56 UTC (rev 160819)
+++ trunk/Source/WebCore/css/StyleResolver.cpp	2013-12-19 03:13:18 UTC (rev 160820)
@@ -204,7 +204,7 @@
     };
 
     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 @@
     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 @@
         // 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 @@
     }
 
     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 @@
     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 @@
             // 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 @@
         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)

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

Reply via email to