- Revision
- 132754
- Author
- [email protected]
- Date
- 2012-10-28 15:35:33 -0700 (Sun, 28 Oct 2012)
Log Message
Get rid of StyleResolver state related to unknown pseudo-elements.
https://bugs.webkit.org/show_bug.cgi?id=100582
Reviewed by Eric Seidel.
All of the state, related to unknown pseudo-elements is already understood at the time of collecting rules.
We can just get rid of most of this code in StyleResolver.
At the time of matching rules, we know for certain that only rules that contain unknown pseudo-elements,
or are UA rules, or are explicitly invited by a TreeScope will match. So we can just return early in many cases.
No change in behavior, covered by existing tests.
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkSelector): Removed now-unnecessary param.
(WebCore::SelectorChecker::checkOneSelector): Ditto.
* css/SelectorChecker.h:
(SelectorChecker): Ditto.
* css/StyleResolver.cpp:
(WebCore::StyleResolver::StyleResolver): Ditto.
(MatchingUARulesScope): Moved class definition here, since we now use it in a different place.
(WebCore::StyleResolver::collectMatchingRules): Changed the logic to stop matching rules that definitely won't match in a different scope.
(WebCore::StyleResolver::collectMatchingRulesForList): Removed code that's now unnecesssary.
(WebCore::StyleResolver::checkSelector): Removed now-unnecessary param.
(WebCore::StyleResolver::checkRegionSelector): Removed weird dead code.
* css/StyleResolver.h:
(StyleResolver): Removed now-unnecessary member.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (132753 => 132754)
--- trunk/Source/WebCore/ChangeLog 2012-10-28 21:42:58 UTC (rev 132753)
+++ trunk/Source/WebCore/ChangeLog 2012-10-28 22:35:33 UTC (rev 132754)
@@ -1,3 +1,33 @@
+2012-10-28 Dimitri Glazkov <[email protected]>
+
+ Get rid of StyleResolver state related to unknown pseudo-elements.
+ https://bugs.webkit.org/show_bug.cgi?id=100582
+
+ Reviewed by Eric Seidel.
+
+ All of the state, related to unknown pseudo-elements is already understood at the time of collecting rules.
+ We can just get rid of most of this code in StyleResolver.
+
+ At the time of matching rules, we know for certain that only rules that contain unknown pseudo-elements,
+ or are UA rules, or are explicitly invited by a TreeScope will match. So we can just return early in many cases.
+
+ No change in behavior, covered by existing tests.
+
+ * css/SelectorChecker.cpp:
+ (WebCore::SelectorChecker::checkSelector): Removed now-unnecessary param.
+ (WebCore::SelectorChecker::checkOneSelector): Ditto.
+ * css/SelectorChecker.h:
+ (SelectorChecker): Ditto.
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::StyleResolver): Ditto.
+ (MatchingUARulesScope): Moved class definition here, since we now use it in a different place.
+ (WebCore::StyleResolver::collectMatchingRules): Changed the logic to stop matching rules that definitely won't match in a different scope.
+ (WebCore::StyleResolver::collectMatchingRulesForList): Removed code that's now unnecesssary.
+ (WebCore::StyleResolver::checkSelector): Removed now-unnecessary param.
+ (WebCore::StyleResolver::checkRegionSelector): Removed weird dead code.
+ * css/StyleResolver.h:
+ (StyleResolver): Removed now-unnecessary member.
+
2012-10-28 Sheriff Bot <[email protected]>
Unreviewed, rolling out r132696.
Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (132753 => 132754)
--- trunk/Source/WebCore/css/SelectorChecker.cpp 2012-10-28 21:42:58 UTC (rev 132753)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp 2012-10-28 22:35:33 UTC (rev 132754)
@@ -268,8 +268,7 @@
}
PseudoId ignoreDynamicPseudo = NOPSEUDO;
- bool hasUnknownPseudoElements = false;
- return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), ignoreDynamicPseudo, hasUnknownPseudoElements) == SelectorMatches;
+ return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), ignoreDynamicPseudo) == SelectorMatches;
}
namespace {
@@ -440,7 +439,7 @@
// * SelectorFailsLocally - the selector fails for the element e
// * SelectorFailsAllSiblings - the selector fails for e and any sibling of e
// * SelectorFailsCompletely - the selector fails for e and any sibling or ancestor of e
-SelectorChecker::SelectorMatch SelectorChecker::checkSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo, bool& hasUnknownPseudoElements) const
+SelectorChecker::SelectorMatch SelectorChecker::checkSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo) const
{
// first selector has to match
if (!checkOneSelector(context, DOMSiblingTraversalStrategy()))
@@ -448,7 +447,6 @@
if (context.selector->m_match == CSSSelector::PseudoElement) {
if (context.selector->isUnknownPseudoElement()) {
- hasUnknownPseudoElements = true;
if (context.element->shadowPseudoId() != context.selector->value())
return SelectorFailsLocally;
} else {
@@ -500,7 +498,7 @@
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
for (; nextContext.element; nextContext.element = nextContext.element->parentElement()) {
- SelectorMatch match = checkSelector(nextContext, ignoreDynamicPseudo, hasUnknownPseudoElements);
+ SelectorMatch match = checkSelector(nextContext, ignoreDynamicPseudo);
if (match == SelectorMatches || match == SelectorFailsCompletely)
return match;
if (nextContext.element == nextContext.scope)
@@ -515,7 +513,7 @@
nextContext.isSubSelector = false;
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
- return checkSelector(nextContext, ignoreDynamicPseudo, hasUnknownPseudoElements);
+ return checkSelector(nextContext, ignoreDynamicPseudo);
case CSSSelector::DirectAdjacent:
if (m_mode == ResolvingStyle && context.element->parentElement()) {
@@ -529,7 +527,7 @@
nextContext.isSubSelector = false;
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
- return checkSelector(nextContext, ignoreDynamicPseudo, hasUnknownPseudoElements);
+ return checkSelector(nextContext, ignoreDynamicPseudo);
case CSSSelector::IndirectAdjacent:
if (m_mode == ResolvingStyle && context.element->parentElement()) {
@@ -542,7 +540,7 @@
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
for (; nextContext.element; nextContext.element = nextContext.element->previousElementSibling()) {
- SelectorMatch match = checkSelector(nextContext, ignoreDynamicPseudo, hasUnknownPseudoElements);
+ SelectorMatch match = checkSelector(nextContext, ignoreDynamicPseudo);
if (match == SelectorMatches || match == SelectorFailsAllSiblings || match == SelectorFailsCompletely)
return match;
};
@@ -559,7 +557,7 @@
&& !(nextContext.hasScrollbarPseudo && nextContext.selector->m_match == CSSSelector::PseudoClass))
return SelectorFailsCompletely;
nextContext.isSubSelector = true;
- return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+ return checkSelector(nextContext, dynamicPseudo);
case CSSSelector::ShadowDescendant:
{
@@ -573,7 +571,7 @@
nextContext.isSubSelector = false;
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
- return checkSelector(nextContext, ignoreDynamicPseudo, hasUnknownPseudoElements);
+ return checkSelector(nextContext, ignoreDynamicPseudo);
}
}
@@ -979,10 +977,9 @@
{
SelectorCheckingContext subContext(context);
subContext.isSubSelector = true;
- bool hasUnknownPseudoElements = false;
PseudoId ignoreDynamicPseudo = NOPSEUDO;
for (subContext.selector = selector->selectorList()->first(); subContext.selector; subContext.selector = CSSSelectorList::next(subContext.selector)) {
- if (checkSelector(subContext, ignoreDynamicPseudo, hasUnknownPseudoElements) == SelectorMatches)
+ if (checkSelector(subContext, ignoreDynamicPseudo) == SelectorMatches)
return true;
}
}
Modified: trunk/Source/WebCore/css/SelectorChecker.h (132753 => 132754)
--- trunk/Source/WebCore/css/SelectorChecker.h 2012-10-28 21:42:58 UTC (rev 132753)
+++ trunk/Source/WebCore/css/SelectorChecker.h 2012-10-28 22:35:33 UTC (rev 132754)
@@ -82,7 +82,7 @@
};
bool checkSelector(CSSSelector*, Element*, bool isFastCheckableSelector = false) const;
- SelectorMatch checkSelector(const SelectorCheckingContext&, PseudoId&, bool& hasUnknownPseudoElements) const;
+ SelectorMatch checkSelector(const SelectorCheckingContext&, PseudoId&) const;
template<typename SiblingTraversalStrategy>
bool checkOneSelector(const SelectorCheckingContext&, const SiblingTraversalStrategy&) const;
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (132753 => 132754)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2012-10-28 21:42:58 UTC (rev 132753)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2012-10-28 22:35:33 UTC (rev 132754)
@@ -273,7 +273,6 @@
, m_matchAuthorAndUserStyles(matchAuthorAndUserStyles)
, m_sameOriginOnly(false)
, m_distributedToInsertionPoint(false)
- , m_hasUnknownPseudoElements(false)
, m_fontSelector(CSSFontSelector::create(document))
, m_applyPropertyToRegularStyle(true)
, m_applyPropertyToVisitedLinkStyle(false)
@@ -637,11 +636,56 @@
result.isCacheable = false;
}
+class MatchingUARulesScope {
+public:
+ MatchingUARulesScope();
+ ~MatchingUARulesScope();
+
+ static bool isMatchingUARules();
+
+private:
+ static bool m_matchingUARules;
+};
+
+MatchingUARulesScope::MatchingUARulesScope()
+{
+ ASSERT(!m_matchingUARules);
+ m_matchingUARules = true;
+}
+
+MatchingUARulesScope::~MatchingUARulesScope()
+{
+ m_matchingUARules = false;
+}
+
+inline bool MatchingUARulesScope::isMatchingUARules()
+{
+ return m_matchingUARules;
+}
+
+bool MatchingUARulesScope::m_matchingUARules = false;
+
void StyleResolver::collectMatchingRules(RuleSet* rules, int& firstRuleIndex, int& lastRuleIndex, const MatchOptions& options)
{
ASSERT(rules);
ASSERT(m_element);
+ const AtomicString& pseudoId = m_element->shadowPseudoId();
+ if (!pseudoId.isEmpty()) {
+ ASSERT(m_styledElement);
+ collectMatchingRulesForList(rules->shadowPseudoElementRules(pseudoId.impl()), firstRuleIndex, lastRuleIndex, options);
+ }
+
+ // Check whether other types of rules are applicable in the current tree scope. Criteria for this:
+ // a) it's a UA rule
+ // b) the tree scope allows author rules
+ // c) the rules comes from a scoped style sheet within the same tree scope
+ TreeScope* treeScope = m_element->treeScope();
+ if (!MatchingUARulesScope::isMatchingUARules()
+ && !treeScope->applyAuthorStyles()
+ && (!options.scope || options.scope->treeScope() != treeScope))
+ return;
+
// We need to collect the rules for id, class, tag, and everything else into a buffer and
// then sort the buffer.
if (m_element->hasID())
@@ -650,11 +694,7 @@
for (size_t i = 0; i < m_styledElement->classNames().size(); ++i)
collectMatchingRulesForList(rules->classRules(m_styledElement->classNames()[i].impl()), firstRuleIndex, lastRuleIndex, options);
}
- const AtomicString& pseudoId = m_element->shadowPseudoId();
- if (!pseudoId.isEmpty()) {
- ASSERT(m_styledElement);
- collectMatchingRulesForList(rules->shadowPseudoElementRules(pseudoId.impl()), firstRuleIndex, lastRuleIndex, options);
- }
+
if (m_element->isLink())
collectMatchingRulesForList(rules->linkPseudoClassRules(), firstRuleIndex, lastRuleIndex, options);
if (m_checker.matchesFocusPseudoClass(m_element))
@@ -818,42 +858,11 @@
sortAndTransferMatchedRules(result);
}
-class MatchingUARulesScope {
-public:
- MatchingUARulesScope();
- ~MatchingUARulesScope();
-
- static bool isMatchingUARules();
-
-private:
- static bool m_matchingUARules;
-};
-
-MatchingUARulesScope::MatchingUARulesScope()
-{
- ASSERT(!m_matchingUARules);
- m_matchingUARules = true;
-}
-
-MatchingUARulesScope::~MatchingUARulesScope()
-{
- m_matchingUARules = false;
-}
-
-inline bool MatchingUARulesScope::isMatchingUARules()
-{
- return m_matchingUARules;
-}
-
-bool MatchingUARulesScope::m_matchingUARules = false;
-
void StyleResolver::collectMatchingRulesForList(const Vector<RuleData>* rules, int& firstRuleIndex, int& lastRuleIndex, const MatchOptions& options)
{
if (!rules)
return;
- TreeScope* treeScope = m_element->treeScope();
-
// In some cases we may end up looking up style for random elements in the middle of a recursive tree resolve.
// Ancestor identifier filter won't be up-to-date in that case and we can't use the fast path.
bool canUseFastReject = m_checker.parentStackIsConsistent(m_parentNode);
@@ -867,19 +876,6 @@
StyleRule* rule = ruleData.rule();
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willMatchRule(document(), rule);
if (checkSelector(ruleData, options.scope)) {
- // Check whether the rule is applicable in the current tree scope. Criteria for this:
- // a) it's a UA rule
- // b) the tree scope allows author rules
- // c) the rules comes from a scoped style sheet within the same tree scope
- // d) the rule contains shadow-ID pseudo elements
- if (!MatchingUARulesScope::isMatchingUARules()
- && !treeScope->applyAuthorStyles()
- && (!options.scope || options.scope->treeScope() != treeScope)
- && !m_hasUnknownPseudoElements) {
-
- InspectorInstrumentation::didMatchRule(cookie, false);
- continue;
- }
// If the rule has no properties to apply, then ignore it in the non-debug mode.
const StylePropertySet* properties = rule->properties();
if (!properties || (properties->isEmpty() && !options.includeEmptyRules)) {
@@ -2196,7 +2192,6 @@
inline bool StyleResolver::checkSelector(const RuleData& ruleData, const ContainerNode* scope)
{
m_dynamicPseudo = NOPSEUDO;
- m_hasUnknownPseudoElements = false;
if (ruleData.hasFastCheckableSelector()) {
// We know this selector does not include any pseudo elements.
@@ -2220,7 +2215,7 @@
context.elementParentStyle = m_parentNode ? m_parentNode->renderStyle() : 0;
context.scope = scope;
context.pseudoStyle = m_pseudoStyle;
- SelectorChecker::SelectorMatch match = m_checker.checkSelector(context, m_dynamicPseudo, m_hasUnknownPseudoElements);
+ SelectorChecker::SelectorMatch match = m_checker.checkSelector(context, m_dynamicPseudo);
if (match != SelectorChecker::SelectorMatches)
return false;
if (m_pseudoStyle != NOPSEUDO && m_pseudoStyle != m_dynamicPseudo)
@@ -2233,7 +2228,6 @@
if (!regionSelector || !regionElement)
return false;
- m_hasUnknownPseudoElements = false;
m_pseudoStyle = NOPSEUDO;
for (CSSSelector* s = regionSelector; s; s = CSSSelectorList::next(s))
Modified: trunk/Source/WebCore/css/StyleResolver.h (132753 => 132754)
--- trunk/Source/WebCore/css/StyleResolver.h 2012-10-28 21:42:58 UTC (rev 132753)
+++ trunk/Source/WebCore/css/StyleResolver.h 2012-10-28 22:35:33 UTC (rev 132754)
@@ -357,7 +357,7 @@
void sortMatchedRules();
void sortAndTransferMatchedRules(MatchResult&);
- bool checkSelector(const RuleData&, const ContainerNode* scope = 0);
+ bool checkSelector(const RuleData&, const ContainerNode* scope);
bool checkRegionSelector(CSSSelector* regionSelector, Element* regionElement);
void applyMatchedProperties(const MatchResult&, const Element*);
enum StyleApplicationPass {
@@ -492,7 +492,6 @@
bool m_matchAuthorAndUserStyles;
bool m_sameOriginOnly;
bool m_distributedToInsertionPoint;
- bool m_hasUnknownPseudoElements;
RefPtr<CSSFontSelector> m_fontSelector;
Vector<OwnPtr<MediaQueryResult> > m_viewportDependentMediaQueryResults;
