Changeset 130089 in webkit

Timestamp:

Oct 1, 2012 3:54:29 PM (12 years ago)

Author:

Dimitri Glazkov

Message:

Kill transitive effects of SelectorChecker::checkOneSelector.
​https://bugs.webkit.org/show_bug.cgi?id=97953

Reviewed by Eric Seidel.

The dynamicPseudo/hasUnknownPseudoelements by-ref parameters that are passed into checkOneSelector make the logic harder to understand and aren't needed. Refactor the code to rid of them, replacing them instead with two flags in SelectorCheckingContext.

No change in behavior, covered by existing tests.

• css/SelectorChecker.cpp:

(WebCore::SelectorChecker::checkSelector): Rolled pseudo-element-checking code out of checkOneSelector into here, since that is where t
(WebCore::SelectorChecker::checkOneSelector): Changed to use SelectorCheckingContext rather than transitive params.

• css/SelectorChecker.h:

(WebCore::SelectorChecker::SelectorCheckingContext::SelectorCheckingContext): Added two new flags.
(SelectorCheckingContext): Ditto.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/SelectorChecker.cpp (modified) (7 diffs)
• css/SelectorChecker.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-01  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Kill transitive effects of SelectorChecker::checkOneSelector.
+        https://bugs.webkit.org/show_bug.cgi?id=97953
+
+        Reviewed by Eric Seidel.
+
+        The dynamicPseudo/hasUnknownPseudoelements by-ref parameters that are passed into checkOneSelector make the logic harder to understand and aren't needed. Refactor the code to rid of them, replacing them instead with two flags in SelectorCheckingContext.
+
+        No change in behavior, covered by existing tests.
+
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::checkSelector): Rolled pseudo-element-checking code out of checkOneSelector into here, since that is where t
+        (WebCore::SelectorChecker::checkOneSelector): Changed to use SelectorCheckingContext rather than transitive params.
+        * css/SelectorChecker.h:
+        (WebCore::SelectorChecker::SelectorCheckingContext::SelectorCheckingContext): Added two new flags.
+        (SelectorCheckingContext): Ditto.
+
+
 2012-10-01  Andreas Kling  <kling@webkit.org>
 

trunk/Source/WebCore/css/SelectorChecker.cpp

@@ -267 +267 @@
     }
 
-    PseudoId dynamicPseudo = NOPSEUDO;
+    PseudoId ignoreDynamicPseudo = NOPSEUDO;
     bool hasUnknownPseudoElements = false;
-    return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), dynamicPseudo, hasUnknownPseudoElements) == SelectorMatches;
+    return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), ignoreDynamicPseudo, hasUnknownPseudoElements) == SelectorMatches;
 }
 
@@ -443 +443 @@
 {
     // first selector has to match
-    if (!checkOneSelector(context, dynamicPseudo, hasUnknownPseudoElements))
+    if (!checkOneSelector(context))
         return SelectorFailsLocally;
+
+    if (context.selector->m_match == CSSSelector::PseudoElement) {
+        if (context.selector->isUnknownPseudoElement()) {
+            hasUnknownPseudoElements = true;
+            if (context.element->shadowPseudoId() != context.selector->value())
+                return SelectorFailsLocally;
+        } else {
+            if ((!context.elementStyle && m_mode == ResolvingStyle) || m_mode == QueryingRules)
+                return SelectorFailsLocally;
+
+            PseudoId pseudoId = CSSSelector::pseudoId(context.selector->pseudoType());
+            if (pseudoId == FIRST_LETTER) {
+                if (Document* document = context.element->document())
+                    document->styleSheetCollection()->setUsesFirstLetterRules(true);
+            }
+            if (pseudoId != NOPSEUDO)
+                dynamicPseudo = pseudoId;
+        }
+    }
 
     // The rest of the selectors has to match
@@ -533 +552 @@
         // We make an exception for scrollbar pseudo elements and allow a set of pseudo classes (but nothing else)
         // to follow the pseudo elements.
-        if ((context.elementStyle || m_mode == CollectingRules || m_mode == QueryingRules) && dynamicPseudo != NOPSEUDO && dynamicPseudo != SELECTION
-             && !((RenderScrollbar::scrollbarForStyleResolve() || dynamicPseudo == SCROLLBAR_CORNER || dynamicPseudo == RESIZER) && nextContext.selector->m_match == CSSSelector::PseudoClass))
+        nextContext.hasScrollbarPseudo = RenderScrollbar::scrollbarForStyleResolve() || dynamicPseudo == SCROLLBAR_CORNER || dynamicPseudo == RESIZER;
+        nextContext.hasSelectionPseudo = dynamicPseudo == SELECTION;
+        if ((context.elementStyle || m_mode == CollectingRules || m_mode == QueryingRules) && dynamicPseudo != NOPSEUDO
+            && !nextContext.hasSelectionPseudo
+            && !(nextContext.hasScrollbarPseudo && nextContext.selector->m_match == CSSSelector::PseudoClass))
             return SelectorFailsCompletely;
         nextContext.isSubSelector = true;
@@ -705 +727 @@
 }
 
-bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo, bool& hasUnknownPseudoElements) const
+bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context) const
 {
     Element* const & element = context.element;
@@ -752 +774 @@
                 if (subContext.selector->pseudoType() == CSSSelector::PseudoVisited || (subContext.selector->pseudoType() == CSSSelector::PseudoLink && subContext.visitedMatchType == VisitedMatchEnabled))
                     return true;
-                if (!checkOneSelector(subContext, dynamicPseudo, hasUnknownPseudoElements))
+                if (!checkOneSelector(subContext))
                     return true;
             }
-        } else if (dynamicPseudo != NOPSEUDO && (RenderScrollbar::scrollbarForStyleResolve() || dynamicPseudo == SCROLLBAR_CORNER || dynamicPseudo == RESIZER)) {
+        } else if (context.hasScrollbarPseudo) {
             // CSS scrollbars match a specific subset of pseudo classes, and they have specialized rules for each
             // (since there are no elements involved).
             return checkScrollbarPseudoClass(selector);
-        } else if (dynamicPseudo == SELECTION) {
+        } else if (context.hasSelectionPseudo) {
             if (selector->pseudoType() == CSSSelector::PseudoWindowInactive)
                 return !m_document->page()->focusController()->isActive();
@@ -1001 +1023 @@
                 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, dynamicPseudo, hasUnknownPseudoElements) == SelectorMatches)
+                    if (checkSelector(subContext, ignoreDynamicPseudo, hasUnknownPseudoElements) == SelectorMatches)
                         return true;
                 }
@@ -1175 +1199 @@
         }
         return false;
-    }
-    if (selector->m_match == CSSSelector::PseudoElement) {
-        if (selector->isUnknownPseudoElement()) {
-            hasUnknownPseudoElements = true;
-            return element->shadowPseudoId() == selector->value();
-        }
-
-        if ((!context.elementStyle && m_mode == ResolvingStyle) || m_mode == QueryingRules)
-            return false;
-
-        PseudoId pseudoId = CSSSelector::pseudoId(selector->pseudoType());
-        if (pseudoId == FIRST_LETTER) {
-            if (Document* document = element->document())
-                document->styleSheetCollection()->setUsesFirstLetterRules(true);
-        }
-        if (pseudoId != NOPSEUDO)
-            dynamicPseudo = pseudoId;
     }
     // ### add the rest of the checks...

trunk/Source/WebCore/css/SelectorChecker.h

@@ -66 +66 @@
             , isSubSelector(false)
             , pseudoStyle(NOPSEUDO)
+            , hasScrollbarPseudo(false)
+            , hasSelectionPseudo(false)
         { }
 
@@ -76 +78 @@
         bool isSubSelector;
         PseudoId pseudoStyle;
+        bool hasScrollbarPseudo;
+        bool hasSelectionPseudo;
     };
 
@@ -118 +122 @@
 
 private:
-    bool checkOneSelector(const SelectorCheckingContext&, PseudoId&, bool& hasUnknownPseudoElements) const;
+    bool checkOneSelector(const SelectorCheckingContext&) const;
     bool checkScrollbarPseudoClass(CSSSelector*) const;
     static bool isFrameFocused(const Element*);