Changeset 128329 in webkit


Ignore:
Timestamp:
Sep 12, 2012 9:50:33 AM (12 years ago)
Author:
Dimitri Glazkov
Message:

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.

Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r128325 r128329  
     12012-09-12  Dimitri Glazkov  <dglazkov@chromium.org>
     2
     3        Remove transient state regarding uknown pseudoelements from SelectorChecker.
     4        https://bugs.webkit.org/show_bug.cgi?id=96425
     5
     6        Reviewed by Eric Seidel.
     7
     8        The fact that an unknown pseudoelement was found when checking selector was stored as extra state on SelectorChecker. That's bad,
     9        because this state is at odds with the lifecycle of the instance and had to be explicitly reset prior to each checking. To address
     10        this, I made checkSelector report the value as its result (by-ref parameter, actually).
     11
     12        No new tests, refactoring only. Covered by existing tests.
     13
     14        * css/SelectorChecker.cpp:
     15        (WebCore::SelectorChecker::SelectorChecker): Removed the now-unneded state initialization.
     16        (WebCore::SelectorChecker::checkSelector): Changed to take extra parameter.
     17        (WebCore):
     18        (WebCore::SelectorChecker::checkOneSelector): Ditto.
     19        * css/SelectorChecker.h:
     20        (SelectorChecker): Changed decls accordingly.
     21        * css/StyleResolver.cpp:
     22        (WebCore::StyleResolver::StyleResolver): Added state to StyleResolver.
     23        (WebCore::StyleResolver::collectMatchingRulesForList): Changed to use own state, rather than StyleChecker's state.
     24        * css/StyleResolver.h:
     25        (StyleResolver): Moved state here from StyleChecker.
     26
    1272012-09-12  Andrei Onea  <onea@adobe.com>
    228

  • trunk/Source/WebCore/css/SelectorChecker.cpp

    r124180 r128329  
    7272    , m_mode(ResolvingStyle)
    7373    , m_pseudoStyle(NOPSEUDO)
    74     , m_hasUnknownPseudoElements(false)
    7574{
    7675}
    … …  
    269268
    270269    PseudoId dynamicPseudo = NOPSEUDO;
    271     return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), dynamicPseudo) == SelectorMatches;
     270    bool hasUnknownPseudoElements = false;
     271    return checkSelector(SelectorCheckingContext(sel, element, SelectorChecker::VisitedMatchDisabled), dynamicPseudo, hasUnknownPseudoElements) == SelectorMatches;
    272272}
    273273
    … …  
    440440// * SelectorFailsAllSiblings - the selector fails for e and any sibling of e
    441441// * SelectorFailsCompletely  - the selector fails for e and any sibling or ancestor of e
    442 SelectorChecker::SelectorMatch SelectorChecker::checkSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo) const
     442SelectorChecker::SelectorMatch SelectorChecker::checkSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo, bool& hasUnknownPseudoElements) const
    443443{
    444444    // first selector has to match
    445     if (!checkOneSelector(context, dynamicPseudo))
     445    if (!checkOneSelector(context, dynamicPseudo, hasUnknownPseudoElements))
    446446        return SelectorFailsLocally;
    447447
    … …  
    478478        nextContext.elementParentStyle = 0;
    479479        for (; nextContext.element; nextContext.element = nextContext.element->parentElement()) {
    480             SelectorMatch match = checkSelector(nextContext, dynamicPseudo);
     480            SelectorMatch match = checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
    481481            if (match == SelectorMatches || match == SelectorFailsCompletely)
    482482                return match;
    … …  
    493493        nextContext.elementStyle = 0;
    494494        nextContext.elementParentStyle = 0;
    495         return checkSelector(nextContext, dynamicPseudo);
     495        return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
    496496
    497497    case CSSSelector::DirectAdjacent:
    … …  
    507507        nextContext.elementStyle = 0;
    508508        nextContext.elementParentStyle = 0;
    509         return checkSelector(nextContext, dynamicPseudo);
     509        return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
    510510
    511511    case CSSSelector::IndirectAdjacent:
    … …  
    520520        nextContext.elementParentStyle = 0;
    521521        for (; nextContext.element; nextContext.element = nextContext.element->previousElementSibling()) {
    522             SelectorMatch match = checkSelector(nextContext, dynamicPseudo);
     522            SelectorMatch match = checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
    523523            if (match == SelectorMatches || match == SelectorFailsAllSiblings || match == SelectorFailsCompletely)
    524524                return match;
    … …  
    534534            return SelectorFailsCompletely;
    535535        nextContext.isSubSelector = true;
    536         return checkSelector(nextContext, dynamicPseudo);
     536        return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
    537537
    538538    case CSSSelector::ShadowDescendant:
    … …  
    548548            nextContext.elementStyle = 0;
    549549            nextContext.elementParentStyle = 0;
    550             return checkSelector(nextContext, dynamicPseudo);
     550            return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
    551551        }
    552552    }
    … …  
    702702}
    703703
    704 bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo) const
     704bool SelectorChecker::checkOneSelector(const SelectorCheckingContext& context, PseudoId& dynamicPseudo, bool& hasUnknownPseudoElements) const
    705705{
    706706    Element* const & element = context.element;
    … …  
    749749                if (subContext.selector->pseudoType() == CSSSelector::PseudoVisited || (subContext.selector->pseudoType() == CSSSelector::PseudoLink && subContext.visitedMatchType == VisitedMatchEnabled))
    750750                    return true;
    751                 if (!checkOneSelector(subContext, dynamicPseudo))
     751                if (!checkOneSelector(subContext, dynamicPseudo, hasUnknownPseudoElements))
    752752                    return true;
    753753            }
    … …  
    999999                subContext.isSubSelector = true;
    10001000                for (subContext.selector = selector->selectorList()->first(); subContext.selector; subContext.selector = CSSSelectorList::next(subContext.selector)) {
    1001                     if (checkSelector(subContext, dynamicPseudo) == SelectorMatches)
     1001                    if (checkSelector(subContext, dynamicPseudo, hasUnknownPseudoElements) == SelectorMatches)
    10021002                        return true;
    10031003                }
    … …  
    11791179    if (selector->m_match == CSSSelector::PseudoElement) {
    11801180        if (selector->isUnknownPseudoElement()) {
    1181             m_hasUnknownPseudoElements = true;
     1181            hasUnknownPseudoElements = true;
    11821182            return element->shadowPseudoId() == selector->value();
    11831183        }

  • trunk/Source/WebCore/css/SelectorChecker.h

    r123636 r128329  
    7777
    7878    bool checkSelector(CSSSelector*, Element*, bool isFastCheckableSelector = false) const;
    79     SelectorMatch checkSelector(const SelectorCheckingContext&, PseudoId&) const;
     79    SelectorMatch checkSelector(const SelectorCheckingContext&, PseudoId&, bool& hasUnknownPseudoElements) const;
    8080    static bool isFastCheckableSelector(const CSSSelector*);
    8181    bool fastCheckSelector(const CSSSelector*, const Element*) const;
    … …  
    103103    PseudoId pseudoStyle() const { return m_pseudoStyle; }
    104104    void setPseudoStyle(PseudoId pseudoId) { m_pseudoStyle = pseudoId; }
    105 
    106     bool hasUnknownPseudoElements() const { return m_hasUnknownPseudoElements; }
    107     void clearHasUnknownPseudoElements() { m_hasUnknownPseudoElements = false; }
    108105
    109106    static bool tagMatches(const Element*, const CSSSelector*);
    … …  
    122119
    123120private:
    124     bool checkOneSelector(const SelectorCheckingContext&, PseudoId&) const;
     121    bool checkOneSelector(const SelectorCheckingContext&, PseudoId&, bool& hasUnknownPseudoElements) const;
    125122    bool checkScrollbarPseudoClass(CSSSelector*, PseudoId& dynamicPseudo) const;
    126123    static bool isFrameFocused(const Element*);
    … …  
    139136    Mode m_mode;
    140137    PseudoId m_pseudoStyle;
    141     mutable bool m_hasUnknownPseudoElements;
    142138    mutable HashSet<LinkHash, LinkHashHash> m_linksCheckedForVisitedState;
    143139

  • trunk/Source/WebCore/css/StyleResolver.cpp

    r128301 r128329  
    381381    , m_sameOriginOnly(false)
    382382    , m_distributedToInsertionPoint(false)
     383    , m_hasUnknownPseudoElements(false)
    383384    , m_fontSelector(CSSFontSelector::create(document))
    384385    , m_applyPropertyToRegularStyle(true)
    … …  
    11031104                && (!options.scope || options.scope->treeScope() != treeScope)
    11041105#endif
    1105                 && !m_checker.hasUnknownPseudoElements()) {
     1106                && !m_hasUnknownPseudoElements) {
    11061107
    11071108                InspectorInstrumentation::didMatchRule(cookie, false);
    … …  
    24012402{
    24022403    m_dynamicPseudo = NOPSEUDO;
    2403     m_checker.clearHasUnknownPseudoElements();
     2404    m_hasUnknownPseudoElements = false;
    24042405
    24052406    if (ruleData.hasFastCheckableSelector()) {
    … …  
    24242425    context.elementParentStyle = m_parentNode ? m_parentNode->renderStyle() : 0;
    24252426    context.scope = scope;
    2426     SelectorChecker::SelectorMatch match = m_checker.checkSelector(context, m_dynamicPseudo);
     2427    SelectorChecker::SelectorMatch match = m_checker.checkSelector(context, m_dynamicPseudo, m_hasUnknownPseudoElements);
    24272428    if (match != SelectorChecker::SelectorMatches)
    24282429        return false;
    … …  
    24372438        return false;
    24382439
    2439     m_checker.clearHasUnknownPseudoElements();
     2440    m_hasUnknownPseudoElements = false;
    24402441    m_checker.setPseudoStyle(NOPSEUDO);
    24412442

  • trunk/Source/WebCore/css/StyleResolver.h

    r128228 r128329  
    498498    bool m_sameOriginOnly;
    499499    bool m_distributedToInsertionPoint;
     500    bool m_hasUnknownPseudoElements;
    500501
    501502    RefPtr<CSSFontSelector> m_fontSelector;
Note: See TracChangeset for help on using the changeset viewer.