Fix style sharing with the "type" and "readonly" attributes
https://bugs.webkit.org/show_bug.cgi?id=139283
Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-12-05
Reviewed by Antti Koivisto.
Source/WebCore:
There are two bugs adressed with this patch:
1) The attributes "type" and "readonly" where only handled correctly
for input elements. For everything else, they could incorrectly
be ignored for style sharing.
2) The handling of attributes was incorrect for selector lists, leading
to various bugs (incorrect style sharing in some cases, disabling
style sharing on valid cases).
For [1], the problem was that attribute checking had been limited to
StyleResolver::canShareStyleWithControl(). That function is for handling
the special states of input element. For any other element, the attributes
were simply ignored.
For [2], there were a bunch of small problems. First, containsUncommonAttributeSelector()
was not recursive, which caused it to ignored any nested selector list. This used to be
correct but since we have advanced selectors we can no longer assumed selectors are not nested.
A second issue was that any attribute in a selector list was causing us to fall back
to the slow case. Now that we have the fast :matches(), we really don't want that.
The function containsUncommonAttributeSelector() was transformed into a recursive function
tracking where we are in the selector.
At the entry point, we start with the flag "startsOnRightmostElement" set to true. The flag is then
updated on the stack of each recursive call.
For example, "webkit > is:matches(freaking > awesome)". We evalute "is" with the flag to true, then recurse
into evaluating "freaking > awesome" with the flag still set to true. When we evalute ">", the flag
is set to false to evaluate any following selectors.
After evaluating "freaking > awesome", we go back to our previous stack frame, and the flag
is back to true and we can continue evaluating with the curren top level state.
From some logging, I discovered that the attribute handling is way too aggressive.
This is not a regression and I cannot fix that easily so I left a fixme.
Tests: fast/css/data-attribute-style-sharing-1.html
fast/css/data-attribute-style-sharing-2.html
fast/css/data-attribute-style-sharing-3.html
fast/css/data-attribute-style-sharing-4.html
fast/css/data-attribute-style-sharing-5.html
fast/css/data-attribute-style-sharing-6.html
fast/css/data-attribute-style-sharing-7.html
fast/css/readonly-attribute-style-sharing-1.html
fast/css/readonly-attribute-style-sharing-2.html
fast/css/readonly-attribute-style-sharing-3.html
fast/css/readonly-attribute-style-sharing-4.html
fast/css/readonly-attribute-style-sharing-5.html
fast/css/readonly-attribute-style-sharing-6.html
fast/css/readonly-attribute-style-sharing-7.html
fast/css/type-attribute-style-sharing-1.html
fast/css/type-attribute-style-sharing-2.html
fast/css/type-attribute-style-sharing-3.html
fast/css/type-attribute-style-sharing-4.html
fast/css/type-attribute-style-sharing-5.html
fast/css/type-attribute-style-sharing-6.html
fast/css/type-attribute-style-sharing-7.html
* css/RuleSet.cpp:
(WebCore::containsUncommonAttributeSelector):
(WebCore::RuleData::RuleData):
(WebCore::selectorListContainsAttributeSelector): Deleted.
* css/StyleResolver.cpp:
(WebCore::StyleResolver::canShareStyleWithControl):
(WebCore::StyleResolver::canShareStyleWithElement):
LayoutTests:
* fast/css/data-attribute-style-sharing-1-expected.html: Added.
* fast/css/data-attribute-style-sharing-1.html: Added.
* fast/css/data-attribute-style-sharing-2-expected.html: Added.
* fast/css/data-attribute-style-sharing-2.html: Added.
* fast/css/data-attribute-style-sharing-3-expected.html: Added.
* fast/css/data-attribute-style-sharing-3.html: Added.
* fast/css/data-attribute-style-sharing-4-expected.html: Added.
* fast/css/data-attribute-style-sharing-4.html: Added.
* fast/css/data-attribute-style-sharing-5-expected.html: Added.
* fast/css/data-attribute-style-sharing-5.html: Added.
* fast/css/data-attribute-style-sharing-6-expected.html: Added.
* fast/css/data-attribute-style-sharing-6.html: Added.
* fast/css/data-attribute-style-sharing-7-expected.html: Added.
* fast/css/data-attribute-style-sharing-7.html: Added.
* fast/css/readonly-attribute-style-sharing-1-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-1.html: Added.
* fast/css/readonly-attribute-style-sharing-2-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-2.html: Added.
* fast/css/readonly-attribute-style-sharing-3-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-3.html: Added.
* fast/css/readonly-attribute-style-sharing-4-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-4.html: Added.
* fast/css/readonly-attribute-style-sharing-5-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-5.html: Added.
* fast/css/readonly-attribute-style-sharing-6-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-6.html: Added.
* fast/css/readonly-attribute-style-sharing-7-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-7.html: Added.
* fast/css/type-attribute-style-sharing-1-expected.html: Added.
* fast/css/type-attribute-style-sharing-1.html: Added.
* fast/css/type-attribute-style-sharing-2-expected.html: Added.
* fast/css/type-attribute-style-sharing-2.html: Added.
* fast/css/type-attribute-style-sharing-3-expected.html: Added.
* fast/css/type-attribute-style-sharing-3.html: Added.
* fast/css/type-attribute-style-sharing-4-expected.html: Added.
* fast/css/type-attribute-style-sharing-4.html: Added.
* fast/css/type-attribute-style-sharing-5-expected.html: Added.
* fast/css/type-attribute-style-sharing-5.html: Added.
* fast/css/type-attribute-style-sharing-6-expected.html: Added.
* fast/css/type-attribute-style-sharing-6.html: Added.
* fast/css/type-attribute-style-sharing-7-expected.html: Added.
* fast/css/type-attribute-style-sharing-7.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@176864 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/css/RuleSet.cpp b/Source/WebCore/css/RuleSet.cpp
index 148ff60..1ff9c4d 100644
--- a/Source/WebCore/css/RuleSet.cpp
+++ b/Source/WebCore/css/RuleSet.cpp
@@ -96,49 +96,44 @@
return false;
}
-static inline bool selectorListContainsAttributeSelector(const CSSSelector* selector)
-{
- const CSSSelectorList* selectorList = selector->selectorList();
- if (!selectorList)
- return false;
- for (const CSSSelector* selector = selectorList->first(); selector; selector = CSSSelectorList::next(selector)) {
- for (const CSSSelector* component = selector; component; component = component->tagHistory()) {
- if (component->isAttributeSelector())
- return true;
- }
- }
- return false;
-}
-
static inline bool isCommonAttributeSelectorAttribute(const QualifiedName& attribute)
{
// These are explicitly tested for equality in canShareStyleWithElement.
return attribute == typeAttr || attribute == readonlyAttr;
}
-static inline bool containsUncommonAttributeSelector(const CSSSelector* selector)
+static bool containsUncommonAttributeSelector(const CSSSelector& rootSelector, bool matchesRightmostElement)
{
- for (; selector; selector = selector->tagHistory()) {
- // Allow certain common attributes (used in the default style) in the selectors that match the current element.
- if (selector->isAttributeSelector() && !isCommonAttributeSelectorAttribute(selector->attribute()))
- return true;
- if (selectorListContainsAttributeSelector(selector))
- return true;
- if (selector->relation() != CSSSelector::SubSelector) {
- selector = selector->tagHistory();
- break;
+ const CSSSelector* selector = &rootSelector;
+ do {
+ if (selector->isAttributeSelector()) {
+ // FIXME: considering non-rightmost simple selectors is necessary because of the style sharing of cousins.
+ // It is a primitive solution which disable a lot of style sharing on pages that rely on attributes for styling.
+ // We should investigate better ways of doing this.
+ if (!isCommonAttributeSelectorAttribute(selector->attribute()) || !matchesRightmostElement)
+ return true;
}
- }
- for (; selector; selector = selector->tagHistory()) {
- if (selector->isAttributeSelector())
- return true;
- if (selectorListContainsAttributeSelector(selector))
- return true;
- }
+ if (const CSSSelectorList* selectorList = selector->selectorList()) {
+ for (const CSSSelector* subSelector = selectorList->first(); subSelector; subSelector = CSSSelectorList::next(subSelector)) {
+ if (containsUncommonAttributeSelector(*subSelector, matchesRightmostElement))
+ return true;
+ }
+ }
+
+ if (selector->relation() != CSSSelector::SubSelector)
+ matchesRightmostElement = false;
+
+ selector = selector->tagHistory();
+ } while (selector);
return false;
}
+static inline bool containsUncommonAttributeSelector(const CSSSelector& rootSelector)
+{
+ return containsUncommonAttributeSelector(rootSelector, true);
+}
+
static inline PropertyWhitelistType determinePropertyWhitelistType(const AddRuleFlags addRuleFlags, const CSSSelector* selector)
{
if (addRuleFlags & RuleIsInRegionRule)
@@ -161,7 +156,7 @@
, m_position(position)
, m_matchBasedOnRuleHash(static_cast<unsigned>(computeMatchBasedOnRuleHash(*selector())))
, m_canMatchPseudoElement(selectorCanMatchPseudoElement(*selector()))
- , m_containsUncommonAttributeSelector(WebCore::containsUncommonAttributeSelector(selector()))
+ , m_containsUncommonAttributeSelector(WebCore::containsUncommonAttributeSelector(*selector()))
, m_linkMatchType(SelectorChecker::determineLinkMatchType(selector()))
, m_propertyWhitelistType(determinePropertyWhitelistType(addRuleFlags, selector()))
#if ENABLE(CSS_SELECTOR_JIT) && CSS_SELECTOR_JIT_PROFILING
