`currentColor` computes to the same colour on all elements, even if 'color' is inheri...
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Mar 2015 03:10:48 +0000 (03:10 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Mar 2015 03:10:48 +0000 (03:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133420

Reviewed by Darin Adler.

Source/WebCore:

When resolving a style with the help of the property cache, we were
completely ignoring currentColor.

Since you can set currentColor on properties that are not inherited,
those properties would just be copied from the cached style, which
may have a completely different inherited color.

This pacth fixes the issue by preventing any MatchResult from hitting
the cache if it contains any non-inherited property that would require
resolution by the cache:
-Using the inherit value.
-Using the currentColor value.

Tests: fast/css/currentColor-on-before-after-pseudo-elements.html
       fast/css/currentColor-style-update-reftest.html
       fast/css/currentColor-value-style-update.html

* css/ElementRuleCollector.cpp:
(WebCore::ElementRuleCollector::addElementStyleProperties):
(WebCore::ElementRuleCollector::matchAuthorRules):
(WebCore::ElementRuleCollector::matchUserRules):
(WebCore::ElementRuleCollector::matchUARules):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::MatchResult::addMatchedProperties):
(WebCore::StyleResolver::styleForKeyframe):
(WebCore::StyleResolver::pseudoStyleForElement):
(WebCore::StyleResolver::styleForPage):
(WebCore::StyleResolver::findFromMatchedPropertiesCache):
(WebCore::StyleResolver::addToMatchedPropertiesCache):
(WebCore::extractDirectionAndWritingMode):
(WebCore::StyleResolver::applyMatchedProperties):
(WebCore::StyleResolver::CascadedProperties::addStyleProperties):
(WebCore::StyleResolver::CascadedProperties::addMatches):
* css/StyleResolver.h:
(WebCore::StyleResolver::MatchResult::matchedProperties):

LayoutTests:

* fast/css/currentColor-on-before-after-pseudo-elements-expected.html: Added.
* fast/css/currentColor-on-before-after-pseudo-elements.html: Added.
* fast/css/currentColor-style-update-reftest-expected.html: Added.
* fast/css/currentColor-style-update-reftest.html: Added.
* fast/css/currentColor-value-style-update-expected.txt: Added.
* fast/css/currentColor-value-style-update.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/currentColor-on-before-after-pseudo-elements-expected.html [new file with mode: 0644]
LayoutTests/fast/css/currentColor-on-before-after-pseudo-elements.html [new file with mode: 0644]
LayoutTests/fast/css/currentColor-style-update-reftest-expected.html [new file with mode: 0644]
LayoutTests/fast/css/currentColor-style-update-reftest.html [new file with mode: 0644]
LayoutTests/fast/css/currentColor-value-style-update-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/currentColor-value-style-update.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/ElementRuleCollector.cpp
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h

index 6dfd6db..8ba7331 100644 (file)
@@ -1,3 +1,17 @@
+2015-03-29  Benjamin Poulain  <benjamin@webkit.org>
+
+        `currentColor` computes to the same colour on all elements, even if 'color' is inherited differently
+        https://bugs.webkit.org/show_bug.cgi?id=133420
+
+        Reviewed by Darin Adler.
+
+        * fast/css/currentColor-on-before-after-pseudo-elements-expected.html: Added.
+        * fast/css/currentColor-on-before-after-pseudo-elements.html: Added.
+        * fast/css/currentColor-style-update-reftest-expected.html: Added.
+        * fast/css/currentColor-style-update-reftest.html: Added.
+        * fast/css/currentColor-value-style-update-expected.txt: Added.
+        * fast/css/currentColor-value-style-update.html: Added.
+
 2015-03-29  Darin Adler  <darin@apple.com>
 
         HTMLCollection caches incorrect length if item(0) is called before length on an empty collection
diff --git a/LayoutTests/fast/css/currentColor-on-before-after-pseudo-elements-expected.html b/LayoutTests/fast/css/currentColor-on-before-after-pseudo-elements-expected.html
new file mode 100644 (file)
index 0000000..79b373f
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<style>
+    #first::before, #first::after { content: 'FAIL'; background: white; }
+    #second::before, #second::after { content: 'FAIL'; background: navy; }
+</style>
+<p>There should be a blue block below:
+<p><span id="first" style="color:white"></span> <span id="second" style="color:navy"></span>
\ No newline at end of file
diff --git a/LayoutTests/fast/css/currentColor-on-before-after-pseudo-elements.html b/LayoutTests/fast/css/currentColor-on-before-after-pseudo-elements.html
new file mode 100644 (file)
index 0000000..38371be
--- /dev/null
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<style>span:matches(::before, ::after) { content: 'FAIL'; background: currentColor; }</style>
+<p>There should be a blue block below:
+<p><span style="color:white"></span> <span style="color:navy"></span>
\ No newline at end of file
diff --git a/LayoutTests/fast/css/currentColor-style-update-reftest-expected.html b/LayoutTests/fast/css/currentColor-style-update-reftest-expected.html
new file mode 100644 (file)
index 0000000..ad392ee
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .container1 {
+                margin: 4px;
+                color: blue;
+            }
+            .container1 > span {
+                border: 1px solid blue;
+            }
+            .container2 {
+                margin: 4px;
+                color: red;
+            }
+            .container2 > span {
+                border: 1px solid red;
+            }
+        </style>
+    </head>
+    <body>
+        <p>This test verifies that the value of currentColor is updated correctly when the style is updated. If the text is successful, each of the line should be styled as described by its text.</p>
+
+        <div class="container1">
+            <span>This text and border should be blue.</span>
+        </div>
+        <div class="container2">
+            <span>This text and border should be red.</span>
+        </div>
+        <div class="container2">
+            <span>This text and border should be red.</span>
+        </div>
+        <div class="container1">
+            <span>This text and border should be blue.</span>
+        </div>
+        <div class="container2">
+            <span>This text and border should be red.</span>
+        </div>
+        <div class="container1">
+            <span>This text and border should be blue.</span>
+        </div>
+        <div class="container2">
+            <span>This text and border should be red.</span>
+        </div>
+        <div class="container1">
+            <span>This text and border should be blue.</span>
+        </div>
+    </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/css/currentColor-style-update-reftest.html b/LayoutTests/fast/css/currentColor-style-update-reftest.html
new file mode 100644 (file)
index 0000000..9a6ad76
--- /dev/null
@@ -0,0 +1,86 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script>
+            function forceLayout() {
+                // We must make sure to hit the cache.
+                var firstParagraph = document.querySelector("p");
+                var originalOffset = firstParagraph.offsetLeft;
+                firstParagraph.style.marginLeft = '42px';
+                var newOffset = firstParagraph.offsetLeft;
+                if (originalOffset == newOffset)
+                    document.write("Something horribly wrong happened to the layout.");
+                firstParagraph.style.marginLeft = '0';
+            }
+
+            document.addEventListener("DOMContentLoaded", forceLayout);
+
+
+            function runTest() {
+                forceLayout();
+
+                var target1 = document.getElementById("target1");
+                target1.classList.add("container2");
+
+                var target2 = document.getElementById("target2");
+                target2.setAttribute("class", "container1");
+
+                var target3 = document.getElementById("target3");
+                target3.style.color = "red";
+
+                var target4 = document.getElementById("target4");
+                target4.style.color = "blue";
+
+                var target5 = document.getElementById("target5");
+                target5.setAttribute("style", "color: red");
+
+                var target6 = document.getElementById("target6");
+                target6.setAttribute("style", "color: blue");
+            }
+
+            window.addEventListener("load", runTest);
+        </script>
+        <style>
+            .container1 {
+                color: blue;
+            }
+            .container2 {
+                color: red;
+            }
+            div {
+                margin: 4px;
+            }
+            div > span {
+                border: 1px solid currentColor;
+            }
+        </style>
+    </head>
+    <body>
+        <p>This test verifies that the value of currentColor is updated correctly when the style is updated. If the text is successful, each of the line should be styled as described by its text.</p>
+
+        <div class="container1">
+            <span>This text and border should be blue.</span>
+        </div>
+        <div class="container2">
+            <span>This text and border should be red.</span>
+        </div>
+        <div id="target1">
+            <span>This text and border should be red.</span>
+        </div>
+        <div id="target2">
+            <span>This text and border should be blue.</span>
+        </div>
+        <div id="target3">
+            <span>This text and border should be red.</span>
+        </div>
+        <div id="target4">
+            <span>This text and border should be blue.</span>
+        </div>
+        <div id="target5">
+            <span>This text and border should be red.</span>
+        </div>
+        <div id="target6">
+            <span>This text and border should be blue.</span>
+        </div>
+    </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/css/currentColor-value-style-update-expected.txt b/LayoutTests/fast/css/currentColor-value-style-update-expected.txt
new file mode 100644 (file)
index 0000000..f0bbde3
--- /dev/null
@@ -0,0 +1,59 @@
+Test that the properties that use the CSS Value "currentcolor" to define the color are updated correctly when the inherited color changes.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+Initial state.
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).backgroundColor is "rgb(4, 5, 6)"
+
+Let's override the style of the wrapper through their style object.
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).borderColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).backgroundColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).borderColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).backgroundColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).borderColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).backgroundColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).borderColor is "rgb(7, 8, 9)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).backgroundColor is "rgb(7, 8, 9)"
+
+Let's remove the style attribute on the wrapper.
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).backgroundColor is "rgb(4, 5, 6)"
+
+Let's remove class on the wrappers.
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).borderColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).backgroundColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).borderColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).backgroundColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).borderColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).backgroundColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).borderColor is "rgb(1, 2, 3)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).backgroundColor is "rgb(1, 2, 3)"
+
+Then add it back.
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[0]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[1]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[2]).backgroundColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).borderColor is "rgb(4, 5, 6)"
+PASS getComputedStyle(document.querySelectorAll(".test-case >> target")[3]).backgroundColor is "rgb(4, 5, 6)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css/currentColor-value-style-update.html b/LayoutTests/fast/css/currentColor-value-style-update.html
new file mode 100644 (file)
index 0000000..f5ae0a0
--- /dev/null
@@ -0,0 +1,87 @@
+<!doctype html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<style>
+    .test-case {
+        color: rgb(1, 2, 3);
+    }
+    .wrapper {
+        color: rgb(4, 5, 6);
+    }
+    target {
+        border-color: currentcolor;
+        background-color: currentcolor;
+    }
+</style>
+<script>
+    jsTestIsAsync = true;
+    function testColor(expectedColor) {
+        var allTargets = document.querySelectorAll(".test-case >> target");
+        for (var i = 0; i < allTargets.length; ++i) {
+            shouldBeEqualToString('getComputedStyle(document.querySelectorAll(".test-case >> target")[' + i + ']).borderColor', expectedColor);
+            shouldBeEqualToString('getComputedStyle(document.querySelectorAll(".test-case >> target")[' + i + ']).backgroundColor', expectedColor);
+        }
+    }
+
+    function runTest() {
+        description('Test that the properties that use the CSS Value "currentcolor" to define the color are updated correctly when the inherited color changes.');
+
+        debug("");
+        debug("Initial state.");
+        testColor("rgb(4, 5, 6)");
+
+        debug("");
+        debug("Let's override the style of the wrapper through their style object.");
+        var allWrappers = document.querySelectorAll(".wrapper");
+        for (var i = 0; i < allWrappers.length; ++i) {
+            allWrappers[i].style.color = "rgb(7, 8, 9)";
+        }
+        testColor("rgb(7, 8, 9)");
+
+        debug("");
+        debug("Let's remove the style attribute on the wrapper.");
+        for (var i = 0; i < allWrappers.length; ++i) {
+            allWrappers[i].removeAttribute("style");
+        }
+        testColor("rgb(4, 5, 6)");
+
+        debug("");
+        debug("Let's remove class on the wrappers.");
+        for (var i = 0; i < allWrappers.length; ++i) {
+            allWrappers[i].classList.remove("wrapper");
+        }
+        testColor("rgb(1, 2, 3)");
+
+        debug("");
+        debug("Then add it back.");
+        for (var i = 0; i < allWrappers.length; ++i) {
+            allWrappers[i].classList.add("wrapper");
+        }
+        testColor("rgb(4, 5, 6)");
+
+        finishJSTest();
+    }
+    window.addEventListener("load", runTest);
+</script>
+</head>
+<body>
+    <div class="test-case">
+        <div class="wrapper">
+            <target></target>
+        </div>
+        <div class="wrapper">
+            <target></target>
+        </div>
+    </div>
+    <div class="test-case" style="display:none;">
+        <div class="wrapper">
+            <target></target>
+        </div>
+        <div class="wrapper">
+            <target></target>
+        </div>
+    </div>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
index 5893fb3..67cf37c 100644 (file)
@@ -1,5 +1,48 @@
 2015-03-29  Benjamin Poulain  <benjamin@webkit.org>
 
+        `currentColor` computes to the same colour on all elements, even if 'color' is inherited differently
+        https://bugs.webkit.org/show_bug.cgi?id=133420
+
+        Reviewed by Darin Adler.
+
+        When resolving a style with the help of the property cache, we were
+        completely ignoring currentColor.
+
+        Since you can set currentColor on properties that are not inherited,
+        those properties would just be copied from the cached style, which
+        may have a completely different inherited color.
+
+        This pacth fixes the issue by preventing any MatchResult from hitting
+        the cache if it contains any non-inherited property that would require
+        resolution by the cache:
+        -Using the inherit value.
+        -Using the currentColor value.
+
+        Tests: fast/css/currentColor-on-before-after-pseudo-elements.html
+               fast/css/currentColor-style-update-reftest.html
+               fast/css/currentColor-value-style-update.html
+
+        * css/ElementRuleCollector.cpp:
+        (WebCore::ElementRuleCollector::addElementStyleProperties):
+        (WebCore::ElementRuleCollector::matchAuthorRules):
+        (WebCore::ElementRuleCollector::matchUserRules):
+        (WebCore::ElementRuleCollector::matchUARules):
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::MatchResult::addMatchedProperties):
+        (WebCore::StyleResolver::styleForKeyframe):
+        (WebCore::StyleResolver::pseudoStyleForElement):
+        (WebCore::StyleResolver::styleForPage):
+        (WebCore::StyleResolver::findFromMatchedPropertiesCache):
+        (WebCore::StyleResolver::addToMatchedPropertiesCache):
+        (WebCore::extractDirectionAndWritingMode):
+        (WebCore::StyleResolver::applyMatchedProperties):
+        (WebCore::StyleResolver::CascadedProperties::addStyleProperties):
+        (WebCore::StyleResolver::CascadedProperties::addMatches):
+        * css/StyleResolver.h:
+        (WebCore::StyleResolver::MatchResult::matchedProperties):
+
+2015-03-29  Benjamin Poulain  <benjamin@webkit.org>
+
         Enable :any-link by default
         https://bugs.webkit.org/show_bug.cgi?id=143201
 
index 1b216a0..6e7e0d1 100644 (file)
@@ -104,7 +104,7 @@ inline void ElementRuleCollector::addElementStyleProperties(const StylePropertie
 {
     if (!propertySet)
         return;
-    m_result.ranges.lastAuthorRule = m_result.matchedProperties.size();
+    m_result.ranges.lastAuthorRule = m_result.matchedProperties().size();
     if (m_result.ranges.firstAuthorRule == -1)
         m_result.ranges.firstAuthorRule = m_result.ranges.lastAuthorRule;
     m_result.addMatchedProperties(*propertySet);
@@ -220,7 +220,7 @@ void ElementRuleCollector::sortAndTransferMatchedRules()
 void ElementRuleCollector::matchAuthorRules(bool includeEmptyRules)
 {
     clearMatchedRules();
-    m_result.ranges.lastAuthorRule = m_result.matchedProperties.size() - 1;
+    m_result.ranges.lastAuthorRule = m_result.matchedProperties().size() - 1;
 
     // Match global author rules.
     MatchRequest matchRequest(m_ruleSets.authorStyle(), includeEmptyRules);
@@ -238,7 +238,7 @@ void ElementRuleCollector::matchUserRules(bool includeEmptyRules)
     
     clearMatchedRules();
 
-    m_result.ranges.lastUserRule = m_result.matchedProperties.size() - 1;
+    m_result.ranges.lastUserRule = m_result.matchedProperties().size() - 1;
     MatchRequest matchRequest(m_ruleSets.userStyle(), includeEmptyRules);
     StyleResolver::RuleRange ruleRange = m_result.ranges.userRuleRange();
     collectMatchingRules(matchRequest, ruleRange);
@@ -267,7 +267,7 @@ void ElementRuleCollector::matchUARules(RuleSet* rules)
 {
     clearMatchedRules();
     
-    m_result.ranges.lastUARule = m_result.matchedProperties.size() - 1;
+    m_result.ranges.lastUARule = m_result.matchedProperties().size() - 1;
     StyleResolver::RuleRange ruleRange = m_result.ranges.UARuleRange();
     collectMatchingRules(MatchRequest(rules), ruleRange);
 
index c181eaa..d65acaf 100644 (file)
@@ -184,7 +184,7 @@ public:
 
     bool hasProperty(CSSPropertyID id) const;
     Property& property(CSSPropertyID);
-    bool addMatches(const MatchResult&, bool important, int startIndex, int endIndex, bool inheritedOnly = false);
+    void addMatches(const MatchResult&, bool important, int startIndex, int endIndex, bool inheritedOnly = false);
 
     void set(CSSPropertyID, CSSValue&, unsigned linkMatchType);
     void setDeferred(CSSPropertyID, CSSValue&, unsigned linkMatchType);
@@ -192,7 +192,7 @@ public:
     void applyDeferredProperties(StyleResolver&);
 
 private:
-    bool addStyleProperties(const StyleProperties&, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType, unsigned linkMatchType);
+    void addStyleProperties(const StyleProperties&, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType, unsigned linkMatchType);
     static void setPropertyInternal(Property&, CSSPropertyID, CSSValue&, unsigned linkMatchType);
 
     Property m_properties[numCSSProperties + 1];
@@ -245,12 +245,41 @@ inline void StyleResolver::State::clear()
 
 void StyleResolver::MatchResult::addMatchedProperties(const StyleProperties& properties, StyleRule* rule, unsigned linkMatchType, PropertyWhitelistType propertyWhitelistType)
 {
-    matchedProperties.grow(matchedProperties.size() + 1);
-    StyleResolver::MatchedProperties& newProperties = matchedProperties.last();
+    m_matchedProperties.grow(m_matchedProperties.size() + 1);
+    StyleResolver::MatchedProperties& newProperties = m_matchedProperties.last();
     newProperties.properties = const_cast<StyleProperties*>(&properties);
     newProperties.linkMatchType = linkMatchType;
     newProperties.whitelistType = propertyWhitelistType;
     matchedRules.append(rule);
+
+    if (isCacheable) {
+        for (unsigned i = 0, count = properties.propertyCount(); i < count; ++i) {
+            // Currently the property cache only copy the non-inherited values and resolve
+            // the inherited ones.
+            // Here we define some exception were we have to resolve some properties that are not inherited
+            // by default. If those exceptions become too common on the web, it should be possible
+            // to build a list of exception to resolve instead of completely disabling the cache.
+
+            StyleProperties::PropertyReference current = properties.propertyAt(i);
+            if (!current.isInherited()) {
+                // 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.
+                const CSSValue& value = *current.value();
+                if (value.isInheritedValue()) {
+                    isCacheable = false;
+                    break;
+                }
+
+                // The value currentColor has implicitely the same side effect. It depends on the value of color,
+                // which is an inherited value, making the non-inherited property implicitely inherited.
+                if (is<CSSPrimitiveValue>(value) && downcast<CSSPrimitiveValue>(value).getValueID() == CSSValueCurrentcolor) {
+                    isCacheable = false;
+                    break;
+                }
+            }
+        }
+    }
 }
 
 StyleResolver::StyleResolver(Document& document, bool matchAuthorAndUserStyles)
@@ -835,7 +864,7 @@ Ref<RenderStyle> StyleResolver::styleForKeyframe(const RenderStyle* elementStyle
     // We don't need to bother with !important. Since there is only ever one
     // decl, there's nothing to override. So just add the first properties.
     CascadedProperties cascade(direction, writingMode);
-    cascade.addMatches(result, false, 0, result.matchedProperties.size() - 1);
+    cascade.addMatches(result, false, 0, result.matchedProperties().size() - 1);
 
     applyCascadedProperties(cascade, firstCSSProperty, lastHighPriorityProperty);
 
@@ -962,7 +991,7 @@ PassRefPtr<RenderStyle> StyleResolver::pseudoStyleForElement(Element* element, c
         collector.matchAuthorRules(false);
     }
 
-    if (collector.matchedResult().matchedProperties.isEmpty())
+    if (collector.matchedResult().matchedProperties().isEmpty())
         return 0;
 
     state.style()->setStyleType(pseudoStyleRequest.pseudoId);
@@ -1001,7 +1030,7 @@ Ref<RenderStyle> StyleResolver::styleForPage(int pageIndex)
     extractDirectionAndWritingMode(*m_state.style(), result, direction, writingMode);
 
     CascadedProperties cascade(direction, writingMode);
-    cascade.addMatches(result, false, 0, result.matchedProperties.size() - 1);
+    cascade.addMatches(result, false, 0, result.matchedProperties().size() - 1);
 
     applyCascadedProperties(cascade, firstCSSProperty, lastHighPriorityProperty);
 
@@ -1610,11 +1639,11 @@ const StyleResolver::MatchedPropertiesCacheItem* StyleResolver::findFromMatchedP
         return 0;
     MatchedPropertiesCacheItem& cacheItem = it->value;
 
-    size_t size = matchResult.matchedProperties.size();
+    size_t size = matchResult.matchedProperties().size();
     if (size != cacheItem.matchedProperties.size())
         return 0;
     for (size_t i = 0; i < size; ++i) {
-        if (matchResult.matchedProperties[i] != cacheItem.matchedProperties[i])
+        if (matchResult.matchedProperties()[i] != cacheItem.matchedProperties[i])
             return 0;
     }
     if (cacheItem.ranges != matchResult.ranges)
@@ -1633,7 +1662,7 @@ void StyleResolver::addToMatchedPropertiesCache(const RenderStyle* style, const
 
     ASSERT(hash);
     MatchedPropertiesCacheItem cacheItem;
-    cacheItem.matchedProperties.appendVector(matchResult.matchedProperties);
+    cacheItem.matchedProperties.appendVector(matchResult.matchedProperties());
     cacheItem.ranges = matchResult.ranges;
     // Note that we don't cache the original RenderStyle instance. It may be further modified.
     // The RenderStyle in the cache is really just a holder for the substructures and never used as-is.
@@ -1685,7 +1714,7 @@ void extractDirectionAndWritingMode(const RenderStyle& style, const StyleResolve
     bool hadImportantWebkitWritingMode = false;
     bool hadImportantDirection = false;
 
-    for (auto& matchedProperties : matchResult.matchedProperties) {
+    for (const auto& matchedProperties : matchResult.matchedProperties()) {
         for (unsigned i = 0, count = matchedProperties.properties->propertyCount(); i < count; ++i) {
             auto property = matchedProperties.properties->propertyAt(i);
             if (!property.value()->isPrimitiveValue())
@@ -1714,7 +1743,7 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
 {
     ASSERT(element);
     State& state = m_state;
-    unsigned cacheHash = shouldUseMatchedPropertiesCache && matchResult.isCacheable ? computeMatchedPropertiesHash(matchResult.matchedProperties.data(), matchResult.matchedProperties.size()) : 0;
+    unsigned cacheHash = shouldUseMatchedPropertiesCache && matchResult.isCacheable ? computeMatchedPropertiesHash(matchResult.matchedProperties().data(), matchResult.matchedProperties().size()) : 0;
     bool applyInheritedOnly = false;
     const MatchedPropertiesCacheItem* cacheItem = 0;
     if (cacheHash && (cacheItem = findFromMatchedPropertiesCache(cacheHash, matchResult))
@@ -1748,9 +1777,8 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
         // If so, we cache the border and background styles so that RenderTheme::adjustStyle()
         // can look at them later to figure out if this is a styled form control or not.
         CascadedProperties cascade(direction, writingMode);
-        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);
+        cascade.addMatches(matchResult, false, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
+        cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
 
         applyCascadedProperties(cascade, CSSPropertyWebkitRubyPosition, CSSPropertyWebkitRubyPosition);
         adjustStyleForInterCharacterRuby();
@@ -1765,11 +1793,10 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
     }
 
     CascadedProperties cascade(direction, writingMode);
-    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);
+    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);
 
     applyCascadedProperties(cascade, CSSPropertyWebkitRubyPosition, CSSPropertyWebkitRubyPosition);
     
@@ -2623,18 +2650,16 @@ void StyleResolver::CascadedProperties::setDeferred(CSSPropertyID id, CSSValue&
     m_deferredProperties.append(property);
 }
 
-bool StyleResolver::CascadedProperties::addStyleProperties(const StyleProperties& properties, StyleRule&, bool isImportant, bool inheritedOnly, PropertyWhitelistType propertyWhitelistType, unsigned linkMatchType)
+void 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);
         if (isImportant != current.isImportant())
             continue;
         if (inheritedOnly && !current.isInherited()) {
-            // 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.
-            if (current.value()->isInheritedValue())
-                return false;
+            // We apply the inherited properties only when using the property cache.
+            // A match with a value that is explicitely inherited should never have been cached.
+            ASSERT(!current.value()->isInheritedValue());
             continue;
         }
         CSSPropertyID propertyID = current.id();
@@ -2651,20 +2676,17 @@ bool StyleResolver::CascadedProperties::addStyleProperties(const StyleProperties
         else
             set(propertyID, *current.value(), linkMatchType);
     }
-    return true;
 }
 
-bool StyleResolver::CascadedProperties::addMatches(const MatchResult& matchResult, bool important, int startIndex, int endIndex, bool inheritedOnly)
+void StyleResolver::CascadedProperties::addMatches(const MatchResult& matchResult, bool important, int startIndex, int endIndex, bool inheritedOnly)
 {
     if (startIndex == -1)
-        return true;
+        return;
 
     for (int i = startIndex; i <= endIndex; ++i) {
-        const MatchedProperties& matchedProperties = matchResult.matchedProperties[i];
-        if (!addStyleProperties(*matchedProperties.properties, *matchResult.matchedRules[i], important, inheritedOnly, static_cast<PropertyWhitelistType>(matchedProperties.whitelistType), matchedProperties.linkMatchType))
-            return false;
+        const MatchedProperties& matchedProperties = matchResult.matchedProperties()[i];
+        addStyleProperties(*matchedProperties.properties, *matchResult.matchedRules[i], important, inheritedOnly, static_cast<PropertyWhitelistType>(matchedProperties.whitelistType), matchedProperties.linkMatchType);
     }
-    return true;
 }
 
 void StyleResolver::CascadedProperties::applyDeferredProperties(StyleResolver& resolver)
index 97275d8..e4422e0 100644 (file)
@@ -277,12 +277,15 @@ public:
 
     struct MatchResult {
         MatchResult() : isCacheable(true) { }
-        Vector<MatchedProperties, 64> matchedProperties;
         Vector<StyleRule*, 64> matchedRules;
         MatchRanges ranges;
         bool isCacheable;
 
-        void addMatchedProperties(const StyleProperties&, StyleRule* = 0, unsigned linkMatchType = SelectorChecker::MatchAll, PropertyWhitelistType = PropertyWhitelistNone);
+        const Vector<MatchedProperties, 64>& matchedProperties() const { return m_matchedProperties; }
+
+        void addMatchedProperties(const StyleProperties&, StyleRule* = nullptr, unsigned linkMatchType = SelectorChecker::MatchAll, PropertyWhitelistType = PropertyWhitelistNone);
+    private:
+        Vector<MatchedProperties, 64> m_matchedProperties;
     };
 
 private: