Use full white/black listing for invalidation selectors.

Source/core/css/RuleFeature.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 23 matching lines @@
 #include "core/css/CSSSelectorList.h"
 #include "core/css/RuleSet.h"
 #include "core/css/StyleRule.h"
 #include "core/css/invalidation/DescendantInvalidationSet.h"
 #include "core/dom/Element.h"
 #include "core/dom/Node.h"
 #include "wtf/BitVector.h"
 
 namespace blink {
 
-static bool isSkippableComponentForInvalidation(const CSSSelector& selector)
+static void assertSupportedMatch(CSSSelector::Match match)
 {
-    if (selector.match() == CSSSelector::Tag) {
-        ASSERT(selector.tagQName().localName() == starAtom);
-        return true;
+    switch (match) {
+    case CSSSelector::Tag:
+    case CSSSelector::Id:
+    case CSSSelector::Class:
+    case CSSSelector::AttributeExact:
+    case CSSSelector::AttributeSet:
+    case CSSSelector::AttributeHyphen:
+    case CSSSelector::AttributeList:
+    case CSSSelector::AttributeContain:
+    case CSSSelector::AttributeBegin:
+    case CSSSelector::AttributeEnd:
+        break;
+    case CSSSelector::Unknown:
+    case CSSSelector::PagePseudoClass:
+        // These should not appear in StyleRule selectors.
+        ASSERT_NOT_REACHED();
+        break;
+    default:
+        // New match type added. Figure out if it needs a subtree invalidation or not.
+        ASSERT_NOT_REACHED();
+        break;
     }
-    if (selector.match() == CSSSelector::PseudoElement) {
-        switch (selector.pseudoType()) {
-        case CSSSelector::PseudoBefore:
-        case CSSSelector::PseudoAfter:
-        case CSSSelector::PseudoBackdrop:
-        case CSSSelector::PseudoShadow:
-            return true;
-        default:
-            ASSERT(!selector.isCustomPseudoElement());
-            return false;
-        }
-    }
-    if (selector.match() != CSSSelector::PseudoClass)
-        return false;
-    switch (selector.pseudoType()) {
+}
+
+static void assertSupportedPseudo(CSSSelector::PseudoType type)

[esprehn, 2014/10/15 17:55:23]

supports what? This name is too generic.

[rune, 2014/10/15 20:43:31]

I used the name you proposed in the ASSERT comment.

+{
+    switch (type) {
     case CSSSelector::PseudoEmpty:
     case CSSSelector::PseudoFirstChild:
     case CSSSelector::PseudoFirstOfType:
     case CSSSelector::PseudoLastChild:
     case CSSSelector::PseudoLastOfType:
     case CSSSelector::PseudoOnlyChild:
     case CSSSelector::PseudoOnlyOfType:
     case CSSSelector::PseudoNthChild:
     case CSSSelector::PseudoNthOfType:
     case CSSSelector::PseudoNthLastChild:
     case CSSSelector::PseudoNthLastOfType:
     case CSSSelector::PseudoLink:
     case CSSSelector::PseudoVisited:
+    case CSSSelector::PseudoAny:
     case CSSSelector::PseudoAnyLink:
     case CSSSelector::PseudoHover:
     case CSSSelector::PseudoDrag:
     case CSSSelector::PseudoFocus:
     case CSSSelector::PseudoActive:
     case CSSSelector::PseudoChecked:
     case CSSSelector::PseudoEnabled:
     case CSSSelector::PseudoDefault:
     case CSSSelector::PseudoDisabled:
     case CSSSelector::PseudoOptional:
     case CSSSelector::PseudoRequired:
     case CSSSelector::PseudoReadOnly:
     case CSSSelector::PseudoReadWrite:
     case CSSSelector::PseudoValid:
     case CSSSelector::PseudoInvalid:
     case CSSSelector::PseudoIndeterminate:
     case CSSSelector::PseudoTarget:
+    case CSSSelector::PseudoBefore:
+    case CSSSelector::PseudoAfter:
+    case CSSSelector::PseudoBackdrop:
     case CSSSelector::PseudoLang:
+    case CSSSelector::PseudoNot:
     case CSSSelector::PseudoRoot:
     case CSSSelector::PseudoScope:
     case CSSSelector::PseudoInRange:
     case CSSSelector::PseudoOutOfRange:
     case CSSSelector::PseudoUnresolved:
+    case CSSSelector::PseudoHost:
+    case CSSSelector::PseudoShadow:
     case CSSSelector::PseudoListBox:
+        break;
+    case CSSSelector::PseudoNotParsed:
+    case CSSSelector::PseudoUnknown:
+    case CSSSelector::PseudoLeftPage:
+    case CSSSelector::PseudoRightPage:
+    case CSSSelector::PseudoFirstPage:
+        // These should not appear in StyleRule selectors.
+        ASSERT_NOT_REACHED();
+        break;
+    default:
+        // New pseudo type added. Figure out if it needs a subtree invalidation or not.
+        ASSERT_NOT_REACHED();
+        break;
+    }
+}
+
+static bool requiresSubtreeInvalidation(const CSSSelector& selector)
+{
+    if (!selector.matchesPseudoElement() && selector.match() != CSSSelector::PseudoClass) {
+        assertSupportedMatch(selector.match());
+        return false;
+    }
+
+    switch (selector.pseudoType()) {
+    case CSSSelector::PseudoAutofill:
+    case CSSSelector::PseudoFullPageMedia:
+    case CSSSelector::PseudoResizer:
+    case CSSSelector::PseudoScrollbar:
+    case CSSSelector::PseudoScrollbarBack:
+    case CSSSelector::PseudoScrollbarButton:
+    case CSSSelector::PseudoScrollbarCorner:
+    case CSSSelector::PseudoScrollbarForward:
+    case CSSSelector::PseudoScrollbarThumb:
+    case CSSSelector::PseudoScrollbarTrack:
+    case CSSSelector::PseudoScrollbarTrackPiece:
+    case CSSSelector::PseudoWindowInactive:
+    case CSSSelector::PseudoCornerPresent:
+    case CSSSelector::PseudoDecrement:
+    case CSSSelector::PseudoIncrement:
+    case CSSSelector::PseudoHorizontal:
+    case CSSSelector::PseudoVertical:
+    case CSSSelector::PseudoStart:
+    case CSSSelector::PseudoEnd:
+    case CSSSelector::PseudoDoubleButton:
+    case CSSSelector::PseudoSingleButton:
+    case CSSSelector::PseudoNoButton:
+    case CSSSelector::PseudoSelection:
+    case CSSSelector::PseudoFullScreen:
+    case CSSSelector::PseudoFullScreenDocument:
+    case CSSSelector::PseudoFullScreenAncestor:
+    case CSSSelector::PseudoUserAgentCustomElement:
+    case CSSSelector::PseudoWebKitCustomElement:
+    case CSSSelector::PseudoCue:
+    case CSSSelector::PseudoFutureCue:
+    case CSSSelector::PseudoPastCue:
+    case CSSSelector::PseudoContent:
+    case CSSSelector::PseudoSpatialNavigationFocus:
+        // FIXME: Most pseudo classes/elements above can be supported and moved
+        // to assertSupportedPseudo(). Move on a case-by-case basis. If they
+        // require subtree invalidation, document why.
+    case CSSSelector::PseudoFirstLine:
+        // :first-line pseudo can apply to elements arbitrarily deep down in the
+        // DOM from its container given an arbitrary number of block descendants
+        // with no inline flow content in between.
+    case CSSSelector::PseudoFirstLetter:

[esprehn, 2014/10/15 17:55:23]

I don't think this is true (or for first-line). The way they work is that we
mark the element that matches the :first-letter for a recalc and it traverses
down the tree. You don't need to do this subtree recalc.

[rune, 2014/10/15 20:43:31]

It currently breaks fast/dynamic/first-letter-display-change.html. I have a hope
that https://codereview.chromium.org/571603003/ will make it not require a
subtree invalidation.

I have removed the comments and will revisit.

+        // :first-letter pseudo elements can be arbitrarily deep down in the
+        // DOM from its container given an arbitrary number of block descendants
+        // with no text content in between.
+    case CSSSelector::PseudoHostContext:
+        // :host-context matches a shadow host, yet the simple selectors inside
+        // :host-context matches an ancestor of the shadow host.
         return true;
     default:
+        assertSupportedPseudo(selector.pseudoType());

[esprehn, 2014/10/15 17:55:23]

I'd rather you made these methods return booleans and added an
ASSERT(supportsInvalidation(...)) here.

[rune, 2014/10/15 20:43:30]

Done.

I've kept the ASSERT_NOT_REACHED(), though.

         return false;
     }
 }
 
 RuleFeature::RuleFeature(StyleRule* rule, unsigned selectorIndex, bool hasDocumentSecurityOrigin)
     : rule(rule)
     , selectorIndex(selectorIndex)
     , hasDocumentSecurityOrigin(hasDocumentSecurityOrigin)
 {
 }
@@ skipping 49 matching lines @@
                     // invalidation set is concerned.
                     if (subSelectorMode == UseLocalStyleChange)
                         foundUniversal = true;
                 }
                 if (!foundUniversal && !foundCombinator) {
                     // All sub-selectors contained invalidation set features and
                     // we are in the rightmost compound selector.
                     foundIdent = true;
                 }
             }
-        } else if (!isSkippableComponentForInvalidation(*component)) {
+        } else if (requiresSubtreeInvalidation(*component)) {
             return foundCombinator ? UseLocalStyleChange : UseSubtreeStyleChange;
         }
         if (component->relation() != CSSSelector::SubSelector)
             foundCombinator = true;
     }
     return foundIdent ? AddFeatures : UseLocalStyleChange;
 }
 
 void RuleFeatureSet::extractInvalidationSetFeature(const CSSSelector& selector, InvalidationSetFeatures& features)
 {
@@ skipping 367 matching lines @@
     visitor->trace(uncommonAttributeRules);
     visitor->trace(m_classInvalidationSets);
     visitor->trace(m_attributeInvalidationSets);
     visitor->trace(m_idInvalidationSets);
     visitor->trace(m_pseudoInvalidationSets);
     visitor->trace(m_styleInvalidator);
 #endif
 }
 
 } // namespace blink