Modified: trunk/Source/WebCore/ChangeLog (128328 => 128329)
--- trunk/Source/WebCore/ChangeLog 2012-09-12 16:43:17 UTC (rev 128328)
+++ trunk/Source/WebCore/ChangeLog 2012-09-12 16:50:33 UTC (rev 128329)
@@ -1,3 +1,29 @@
+2012-09-12 Dimitri Glazkov <dglaz...@chromium.org>
+
+ Remove transient state regarding uknown pseudoelements from SelectorChecker.
+ https://bugs.webkit.org/show_bug.cgi?id=96425
+
+ Reviewed by Eric Seidel.
+
+ The fact that an unknown pseudoelement was found when checking selector was stored as extra state on SelectorChecker. That's bad,
+ because this state is at odds with the lifecycle of the instance and had to be explicitly reset prior to each checking. To address
+ this, I made checkSelector report the value as its result (by-ref parameter, actually).
+
+ No new tests, refactoring only. Covered by existing tests.
+
+ * css/SelectorChecker.cpp:
+ (WebCore::SelectorChecker::SelectorChecker): Removed the now-unneded state initialization.
+ (WebCore::SelectorChecker::checkSelector): Changed to take extra parameter.
+ (WebCore):
+ (WebCore::SelectorChecker::checkOneSelector): Ditto.
+ * css/SelectorChecker.h:
+ (SelectorChecker): Changed decls accordingly.
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::StyleResolver): Added state to StyleResolver.
+ (WebCore::StyleResolver::collectMatchingRulesForList): Changed to use own state, rather than StyleChecker's state.
+ * css/StyleResolver.h:
+ (StyleResolver): Moved state here from StyleChecker.
+
2012-09-12 Andrei Onea <o...@adobe.com>
[CSSRegions]Use RefPtr's instead of weak references on DOMNamedFlowCollection
Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (128328 => 128329)
--- trunk/Source/WebCore/css/SelectorChecker.cpp 2012-09-12 16:43:17 UTC (rev 128328)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp 2012-09-12 16:50:33 UTC (rev 128329)
@@ -71,7 +71,6 @@
, m_documentIsHTML(document->isHTMLDocument())
, m_mode(ResolvingStyle)
, m_pseudoStyle(NOPSEUDO)
- , m_hasUnknownPseudoElements(false)
{
}
@@ -268,7 +267,8 @@
}
PseudoId dynamicPseudo = NOPSEUDO;
- return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), dynamicPseudo) == SelectorMatches;
+ bool hasUnknownPseudoElements = false;
+ return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), dynamicPseudo, hasUnknownPseudoElements) == SelectorMatches;
}
namespace {
@@ -439,10 +439,10 @@
// * 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) const
+SelectorChecker::SelectorMatch SelectorChecker::checkSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo, bool& hasUnknownPseudoElements) const
{
// first selector has to match
- if (!checkOneSelector(context, dynamicPseudo))
+ if (!checkOneSelector(context, dynamicPseudo, hasUnknownPseudoElements))
return SelectorFailsLocally;
// The rest of the selectors has to match
@@ -477,7 +477,7 @@
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
for (; nextContext.element; nextContext.element = nextContext.element->parentElement()) {
- SelectorMatch match = checkSelector(nextContext, dynamicPseudo);
+ SelectorMatch match = checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
if (match == SelectorMatches || match == SelectorFailsCompletely)
return match;
if (nextContext.element == nextContext.scope)
@@ -492,7 +492,7 @@
nextContext.isSubSelector = false;
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
- return checkSelector(nextContext, dynamicPseudo);
+ return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
case CSSSelector::DirectAdjacent:
if (m_mode == ResolvingStyle && context.element->parentElement()) {
@@ -506,7 +506,7 @@
nextContext.isSubSelector = false;
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
- return checkSelector(nextContext, dynamicPseudo);
+ return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
case CSSSelector::IndirectAdjacent:
if (m_mode == ResolvingStyle && context.element->parentElement()) {
@@ -519,7 +519,7 @@
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
for (; nextContext.element; nextContext.element = nextContext.element->previousElementSibling()) {
- SelectorMatch match = checkSelector(nextContext, dynamicPseudo);
+ SelectorMatch match = checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
if (match == SelectorMatches || match == SelectorFailsAllSiblings || match == SelectorFailsCompletely)
return match;
};
@@ -533,7 +533,7 @@
&& !((RenderScrollbar::scrollbarForStyleResolve() || dynamicPseudo == SCROLLBAR_CORNER || dynamicPseudo == RESIZER) && nextContext.selector->m_match == CSSSelector::PseudoClass))
return SelectorFailsCompletely;
nextContext.isSubSelector = true;
- return checkSelector(nextContext, dynamicPseudo);
+ return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
case CSSSelector::ShadowDescendant:
{
@@ -547,7 +547,7 @@
nextContext.isSubSelector = false;
nextContext.elementStyle = 0;
nextContext.elementParentStyle = 0;
- return checkSelector(nextContext, dynamicPseudo);
+ return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
}
}
@@ -701,7 +701,7 @@
return false;
}
-bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo) const
+bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo, bool& hasUnknownPseudoElements) const
{
Element* const & element = context.element;
CSSSelector* const & selector = context.selector;
@@ -748,7 +748,7 @@
// We select between :visited and :link when applying. We don't know which one applied (or not) yet.
if (subContext.selector->pseudoType() == CSSSelector::PseudoVisited || (subContext.selector->pseudoType() == CSSSelector::PseudoLink && subContext.visitedMatchType == VisitedMatchEnabled))
return true;
- if (!checkOneSelector(subContext, dynamicPseudo))
+ if (!checkOneSelector(subContext, dynamicPseudo, hasUnknownPseudoElements))
return true;
}
} else if (dynamicPseudo != NOPSEUDO && (RenderScrollbar::scrollbarForStyleResolve() || dynamicPseudo == SCROLLBAR_CORNER || dynamicPseudo == RESIZER)) {
@@ -998,7 +998,7 @@
SelectorCheckingContext subContext(context);
subContext.isSubSelector = true;
for (subContext.selector = selector->selectorList()->first(); subContext.selector; subContext.selector = CSSSelectorList::next(subContext.selector)) {
- if (checkSelector(subContext, dynamicPseudo) == SelectorMatches)
+ if (checkSelector(subContext, dynamicPseudo, hasUnknownPseudoElements) == SelectorMatches)
return true;
}
}
@@ -1178,7 +1178,7 @@
}
if (selector->m_match == CSSSelector::PseudoElement) {
if (selector->isUnknownPseudoElement()) {
- m_hasUnknownPseudoElements = true;
+ hasUnknownPseudoElements = true;
return element->shadowPseudoId() == selector->value();
}
Modified: trunk/Source/WebCore/css/SelectorChecker.h (128328 => 128329)
--- trunk/Source/WebCore/css/SelectorChecker.h 2012-09-12 16:43:17 UTC (rev 128328)
+++ trunk/Source/WebCore/css/SelectorChecker.h 2012-09-12 16:50:33 UTC (rev 128329)
@@ -76,7 +76,7 @@
};
bool checkSelector(CSSSelector*, Element*, bool isFastCheckableSelector = false) const;
- SelectorMatch checkSelector(const SelectorCheckingContext&, PseudoId&) const;
+ SelectorMatch checkSelector(const SelectorCheckingContext&, PseudoId&, bool& hasUnknownPseudoElements) const;
static bool isFastCheckableSelector(const CSSSelector*);
bool fastCheckSelector(const CSSSelector*, const Element*) const;
@@ -103,9 +103,6 @@
PseudoId pseudoStyle() const { return m_pseudoStyle; }
void setPseudoStyle(PseudoId pseudoId) { m_pseudoStyle = pseudoId; }
- bool hasUnknownPseudoElements() const { return m_hasUnknownPseudoElements; }
- void clearHasUnknownPseudoElements() { m_hasUnknownPseudoElements = false; }
-
static bool tagMatches(const Element*, const CSSSelector*);
static bool attributeNameMatches(const Attribute*, const QualifiedName&);
static bool isCommonPseudoClassSelector(const CSSSelector*);
@@ -121,7 +118,7 @@
static bool elementMatchesSelectorScopes(const StyledElement*, const HashSet<AtomicStringImpl*>& idScopes, const HashSet<AtomicStringImpl*>& classScopes);
private:
- bool checkOneSelector(const SelectorCheckingContext&, PseudoId&) const;
+ bool checkOneSelector(const SelectorCheckingContext&, PseudoId&, bool& hasUnknownPseudoElements) const;
bool checkScrollbarPseudoClass(CSSSelector*, PseudoId& dynamicPseudo) const;
static bool isFrameFocused(const Element*);
@@ -138,7 +135,6 @@
bool m_documentIsHTML;
Mode m_mode;
PseudoId m_pseudoStyle;
- mutable bool m_hasUnknownPseudoElements;
mutable HashSet<LinkHash, LinkHashHash> m_linksCheckedForVisitedState;
struct ParentStackFrame {
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (128328 => 128329)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2012-09-12 16:43:17 UTC (rev 128328)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2012-09-12 16:50:33 UTC (rev 128329)
@@ -380,6 +380,7 @@
, m_matchAuthorAndUserStyles(matchAuthorAndUserStyles)
, m_sameOriginOnly(false)
, m_distributedToInsertionPoint(false)
+ , m_hasUnknownPseudoElements(false)
, m_fontSelector(CSSFontSelector::create(document))
, m_applyPropertyToRegularStyle(true)
, m_applyPropertyToVisitedLinkStyle(false)
@@ -1102,7 +1103,7 @@
#if ENABLE(STYLE_SCOPED)
&& (!options.scope || options.scope->treeScope() != treeScope)
#endif
- && !m_checker.hasUnknownPseudoElements()) {
+ && !m_hasUnknownPseudoElements) {
InspectorInstrumentation::didMatchRule(cookie, false);
continue;
@@ -2400,7 +2401,7 @@
inline bool StyleResolver::checkSelector(const RuleData& ruleData, const ContainerNode* scope)
{
m_dynamicPseudo = NOPSEUDO;
- m_checker.clearHasUnknownPseudoElements();
+ m_hasUnknownPseudoElements = false;
if (ruleData.hasFastCheckableSelector()) {
// We know this selector does not include any pseudo elements.
@@ -2423,7 +2424,7 @@
context.elementStyle = style();
context.elementParentStyle = m_parentNode ? m_parentNode->renderStyle() : 0;
context.scope = scope;
- SelectorChecker::SelectorMatch match = m_checker.checkSelector(context, m_dynamicPseudo);
+ SelectorChecker::SelectorMatch match = m_checker.checkSelector(context, m_dynamicPseudo, m_hasUnknownPseudoElements);
if (match != SelectorChecker::SelectorMatches)
return false;
if (m_checker.pseudoStyle() != NOPSEUDO && m_checker.pseudoStyle() != m_dynamicPseudo)
@@ -2436,7 +2437,7 @@
if (!regionSelector || !regionElement)
return false;
- m_checker.clearHasUnknownPseudoElements();
+ m_hasUnknownPseudoElements = false;
m_checker.setPseudoStyle(NOPSEUDO);
for (CSSSelector* s = regionSelector; s; s = CSSSelectorList::next(s))
