Add a first pass of a class descendant invalidator, and a containing RuleSetAnalyzer

Source/core/css/analyzer/DescendantInvalidationSet.cpp

Index: Source/core/css/analyzer/DescendantInvalidationSet.cpp
diff --git a/Source/core/css/analyzer/DescendantInvalidationSet.cpp b/Source/core/css/analyzer/DescendantInvalidationSet.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..e84ff5ddbfa92be7b619695987fffa0e555ea13f
--- /dev/null
+++ b/Source/core/css/analyzer/DescendantInvalidationSet.cpp
@@ -0,0 +1,120 @@
+/*
+ * Copyright (C) 2014 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "core/css/analyzer/DescendantInvalidationSet.h"
+
+#include "core/css/resolver/StyleResolver.h"
+#include "core/dom/Element.h"
+
+namespace WebCore {
+
+DescendantInvalidationSet::DescendantInvalidationSet()
+    : allDescendantsMightBeInvalid(false)
+{
+}
+
+void DescendantInvalidationSet::combine(const DescendantInvalidationSet& other)
+{
+    HashSet<AtomicString>::const_iterator end = other.m_classes.end();
+    for (HashSet<AtomicString>::const_iterator it = other.m_classes.begin(); it != end; ++it)
+        addClass(*it);
+
+    end = other.m_ids.end();
+    for (HashSet<AtomicString>::const_iterator it = other.m_ids.begin(); it != end; ++it)
+        addId(*it);
+
+    end = other.m_tagNames.end();
+    for (HashSet<AtomicString>::const_iterator it = other.m_tagNames.begin(); it != end; ++it)
+        addTagName(*it);

[esprehn, 2014/01/14 21:32:56]

If allDescendantsMightBeInvalid is true we should probably just abort all the
add calls and early return.

m_allDescendantsMightBeInvalid = m_allDescendantsMightBeInvalid ||
other.m_allDescendantsMightBeInvalid;
if (m...)
  return;

at the top of this function.

[chrishtr, 2014/01/14 23:33:00]

Done.

+    allDescendantsMightBeInvalid = allDescendantsMightBeInvalid || other.allDescendantsMightBeInvalid;
+}
+
+void DescendantInvalidationSet::invalidateElementSubtree(Element* element)
+{
+    if (allDescendantsMightBeInvalid) {

[esprehn, 2014/01/14 21:32:56]

Braces are not needed.

[chrishtr, 2014/01/14 23:33:00]

Done.

+        element->setNeedsStyleRecalc(SubtreeStyleChange);
+    } else {
+        invalidateElementSubtreeInternal(element);
+    }
+}
+
+bool DescendantInvalidationSet::invalidateElementSubtreeInternal(Element* element)
+{
+    bool shouldRecalcThisElement = false;
+
+    if (element->hasClass()) {
+        for (HashSet<AtomicString>::const_iterator it = m_classes.begin(); it != m_classes.end(); ++it) {
+            if (element->classNames().contains(*it)) {

[esprehn, 2014/01/14 21:32:56]

Braces are not needed and this should be a return otherwise you look at all the
classes even once one of them matches,

if (element->classNames().contains(*it)) 
  return true;

[chrishtr, 2014/01/14 23:33:00]

Done.

+                shouldRecalcThisElement = true;
+            }
+        }
+    }
+
+    if (m_tagNames.contains(element->tagQName().localName()))
+        shouldRecalcThisElement = true;

[esprehn, 2014/01/14 21:32:56]

This function needs to be split and have an early return, not set some variable
like this.

if (m_tagNames.contains(element->tagQName().localName()))
  return true;

Right now you do lots of needless hash lookups even though you already know
something matches.

[chrishtr, 2014/01/14 23:33:00]

Done.

+
+    if (element->hasID() && m_ids.contains(element->idForStyleResolution()))
+        shouldRecalcThisElement = true;
+
+    for (Node* child = element->firstChild(); child; child = child->nextSibling()) {

[esprehn, 2014/01/14 21:32:56]

This doesn't handle ShadowRoots, but I think we always skip rules that could
apply to ShadowRoots (ex. host or ^^ or ^) so they should never end up down this
path.

It might be worth adding a comment that system doesn't handle tree boundary
crossing rules on purpose.

[chrishtr, 2014/01/14 23:33:00]

Done.

+        if (child->isElementNode()) {
+            Element* childElement = toElement(child);
+            bool elementDidRecalc = invalidateElementSubtreeInternal(childElement);
+            shouldRecalcThisElement = shouldRecalcThisElement || elementDidRecalc;
+        }
+    }

[esprehn, 2014/01/14 21:32:56]

This function needs to be split, you want

bool invalidatesElement(Element&) that just has lots of returns in it, and
another one that walks the tree and calls it.

[chrishtr, 2014/01/14 23:33:00]

Done.

+    if (shouldRecalcThisElement) {
+        element->setNeedsStyleRecalc(LocalStyleChange);
+    } else if (element->supportsStyleSharing()) {
+        // FIXME: this doesn't really belong here. Style sharing needs to be cleanly separated
+        // somehow from style recalc invalidation.
+        element->document().ensureStyleResolver().addToStyleSharingList(*element);

[esprehn, 2014/01/14 21:32:56]

Remove this, you shouldn't be messing with the style sharing list from inside
here. There's no reason for that (and it does nothing right now since you're not
inside recalcStyle when this code runs).

[chrishtr, 2014/01/14 23:33:00]

Done.

+    }
+
+    return shouldRecalcThisElement;
+}
+
+void DescendantInvalidationSet::addClass(const AtomicString& className)
+{
+    m_classes.add(className);
+}
+
+void DescendantInvalidationSet::addId(const AtomicString& id)
+{
+    m_ids.add(id);
+}
+
+void DescendantInvalidationSet::addTagName(const AtomicString& tagName)
+{
+    m_tagNames.add(tagName);
+}
+
+} // namespace WebCore