Avoid looking at invalidation sets in a SubtreeStyleChange subtree

We were matching all the rulesets in a subtree even after finding a ruleset
that would invalidate the whole subtree which is wasted work. We were also
storing all the invalidation sets for an element even after one of them
would have invalidated the entire subtree.

This patch makes us only ever store one invalidation set for an element once
one of them would invalidate the whole subtree. It also removes the
foundInvalidationSet() bit since that's identical to just checking if the
Vector of invalidation sets is non-empty.

I also removed an ASSERT(renderer->style()), all renderers have a style, so
there's no reason to check this.

Review URL: https://codereview.chromium.org/233243003

git-svn-id: svn://svn.chromium.org/blink/trunk@171417 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp

@@ -28,7 +28,14 @@ void StyleInvalidator::invalidate(Document& document)
 
 void StyleInvalidator::scheduleInvalidation(PassRefPtr<DescendantInvalidationSet> invalidationSet, Element& element)
 {
-    ensurePendingInvalidationList(element).append(invalidationSet);
+    InvalidationList& list = ensurePendingInvalidationList(element);
+    // If we're already going to invalidate the whole subtree we don't need to store any new sets.
+    if (!list.isEmpty() && list.last()->wholeSubtreeInvalid())
+        return;
+    // If this set would invalidate the whole subtree we can discard all existing sets.
+    if (invalidationSet->wholeSubtreeInvalid())
+        list.clear();
+    list.append(invalidationSet);
     element.setNeedsStyleInvalidation();
 }
 
@@ -63,13 +70,19 @@ StyleInvalidator::~StyleInvalidator()
 
 void StyleInvalidator::RecursionData::pushInvalidationSet(const DescendantInvalidationSet& invalidationSet)
 {
+    ASSERT(!m_wholeSubtreeInvalid);
+    if (invalidationSet.wholeSubtreeInvalid()) {
+        m_wholeSubtreeInvalid = true;
+        return;
+    }
     m_invalidationSets.append(&invalidationSet);
     m_invalidateCustomPseudo = invalidationSet.customPseudoInvalid();
-    m_foundInvalidationSet = true;
 }
 
 bool StyleInvalidator::RecursionData::matchesCurrentInvalidationSets(Element& element)
 {
+    ASSERT(!m_wholeSubtreeInvalid);
+
     if (m_invalidateCustomPseudo && element.shadowPseudoId() != nullAtom)
         return true;
 
@@ -83,27 +96,17 @@ bool StyleInvalidator::RecursionData::matchesCurrentInvalidationSets(Element& el
 
 bool StyleInvalidator::checkInvalidationSetsAgainstElement(Element& element)
 {
-    bool thisElementNeedsStyleRecalc = false;
+    if (m_recursionData.wholeSubtreeInvalid())
+        return true;
     if (element.needsStyleInvalidation()) {
         if (InvalidationList* invalidationList = m_pendingInvalidationMap.get(&element)) {
-            // FIXME: it's really only necessary to clone the render style for this element, not full style recalc.
-            thisElementNeedsStyleRecalc = true;
-            for (InvalidationList::const_iterator it = invalidationList->begin(); it != invalidationList->end(); ++it) {
+            for (InvalidationList::const_iterator it = invalidationList->begin(); it != invalidationList->end(); ++it)
                 m_recursionData.pushInvalidationSet(**it);
-                if ((*it)->wholeSubtreeInvalid()) {
-                    element.setNeedsStyleRecalc(SubtreeStyleChange);
-                    // Even though we have set needsStyleRecalc on the whole subtree, we need to keep walking over the subtree
-                    // in order to clear the invalidation dirty bits on all elements.
-                    // FIXME: we can optimize this by having a dedicated function that just traverses the tree and removes the dirty bits,
-                    // without checking classes etc.
-                    break;
-                }
-            }
+            // FIXME: It's really only necessary to clone the render style for this element, not full style recalc.
+            return true;
         }
     }
-    if (!thisElementNeedsStyleRecalc)
-        thisElementNeedsStyleRecalc = m_recursionData.matchesCurrentInvalidationSets(element);
-    return thisElementNeedsStyleRecalc;
+    return m_recursionData.matchesCurrentInvalidationSets(element);
 }
 
 bool StyleInvalidator::invalidateChildren(Element& element)
@@ -131,21 +134,17 @@ bool StyleInvalidator::invalidate(Element& element)
     bool thisElementNeedsStyleRecalc = checkInvalidationSetsAgainstElement(element);
 
     bool someChildrenNeedStyleRecalc = false;
-    // foundInvalidationSet() will be true if we are in a subtree of a node with a DescendantInvalidationSet on it.
-    // We need to check all nodes in the subtree of such a node.
-    if (m_recursionData.foundInvalidationSet() || element.childNeedsStyleInvalidation())
+    if (m_recursionData.hasInvalidationSets() || element.childNeedsStyleInvalidation())
         someChildrenNeedStyleRecalc = invalidateChildren(element);
 
     if (thisElementNeedsStyleRecalc) {
-        element.setNeedsStyleRecalc(LocalStyleChange);
-    } else if (m_recursionData.foundInvalidationSet() && someChildrenNeedStyleRecalc) {
+        element.setNeedsStyleRecalc(m_recursionData.wholeSubtreeInvalid() ? SubtreeStyleChange : LocalStyleChange);
+    } else if (m_recursionData.hasInvalidationSets() && someChildrenNeedStyleRecalc) {
         // Clone the RenderStyle in order to preserve correct style sharing, if possible. Otherwise recalc style.
-        if (RenderObject* renderer = element.renderer()) {
-            ASSERT(renderer->style());
+        if (RenderObject* renderer = element.renderer())
             renderer->setStyleInternal(RenderStyle::clone(renderer->style()));
-        } else {
+        else
             element.setNeedsStyleRecalc(LocalStyleChange);
-        }
     }
 
     element.clearChildNeedsStyleInvalidation();

third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.h

@@ -33,38 +33,39 @@ private:
 
     struct RecursionData {
         RecursionData()
-            : m_foundInvalidationSet(false)
-            , m_invalidateCustomPseudo(false)
+            : m_invalidateCustomPseudo(false)
+            , m_wholeSubtreeInvalid(false)
         { }
         void pushInvalidationSet(const DescendantInvalidationSet&);
         bool matchesCurrentInvalidationSets(Element&);
-        bool foundInvalidationSet() { return m_foundInvalidationSet; }
+        bool hasInvalidationSets() const { return m_invalidationSets.size(); }
+        bool wholeSubtreeInvalid() const { return m_wholeSubtreeInvalid; }
 
         typedef Vector<const DescendantInvalidationSet*, 16> InvalidationSets;
         InvalidationSets m_invalidationSets;
-        bool m_foundInvalidationSet;
         bool m_invalidateCustomPseudo;
+        bool m_wholeSubtreeInvalid;
     };
 
     class RecursionCheckpoint {
     public:
         RecursionCheckpoint(RecursionData* data)
             : m_prevInvalidationSetsSize(data->m_invalidationSets.size())
-            , m_prevFoundInvalidationSet(data->m_foundInvalidationSet)
             , m_prevInvalidateCustomPseudo(data->m_invalidateCustomPseudo)
+            , m_prevWholeSubtreeInvalid(data->m_wholeSubtreeInvalid)
             , m_data(data)
         { }
         ~RecursionCheckpoint()
         {
             m_data->m_invalidationSets.remove(m_prevInvalidationSetsSize, m_data->m_invalidationSets.size() - m_prevInvalidationSetsSize);
-            m_data->m_foundInvalidationSet = m_prevFoundInvalidationSet;
             m_data->m_invalidateCustomPseudo = m_prevInvalidateCustomPseudo;
+            m_data->m_wholeSubtreeInvalid = m_prevWholeSubtreeInvalid;
         }
 
     private:
         int m_prevInvalidationSetsSize;
-        bool m_prevFoundInvalidationSet;
         bool m_prevInvalidateCustomPseudo;
+        bool m_prevWholeSubtreeInvalid;
         RecursionData* m_data;
     };