Fix issue in which removing a class does not always invalidate style

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 362 matching lines @@
             Element* childElement = toElement(child);
             bool childRecalced = invalidateStyleForClassChange(childElement, invalidationClasses, foundInvalidationSet);
             someChildrenNeedStyleRecalc = someChildrenNeedStyleRecalc || childRecalced;
         }
     }
     return someChildrenNeedStyleRecalc;
 }
 
 bool RuleFeatureSet::invalidateStyleForClassChange(Element* element, Vector<AtomicString>& invalidationClasses, bool foundInvalidationSet)
 {
+    bool thisElementNeedsStyleRecalc = false;
     int oldSize = invalidationClasses.size();
+
     if (element->needsStyleInvalidation()) {
         if (InvalidationList* invalidationList = m_pendingInvalidationMap.get(element)) {
+            thisElementNeedsStyleRecalc = true;
             foundInvalidationSet = true;

[esprehn, 2014/02/18 22:52:33]

Isn't this bool identical to foundInvalidationSet then?

[chrishtr, 2014/02/18 23:14:31]

No, because not all elements in the subtree of an element with a pending
invalidation list needs style recalc. The code recalculates style if one of
these is true:

1. The element has a pending invalidation list
2. The element has a class matching one in invalidationClasses
3. One or more of the element's children needed style recalc.

This patch addresses #1, but should not affect any other cases. In particular 

             for (InvalidationList::const_iterator it = invalidationList->begin(); it != invalidationList->end(); ++it) {
                 if ((*it)->wholeSubtreeInvalid()) {
                     element->setNeedsStyleRecalc(SubtreeStyleChange);
                     invalidationClasses.remove(oldSize, invalidationClasses.size() - oldSize);
                     element->clearChildNeedsStyleInvalidation();
                     return true;
                 }
                 (*it)->getClasses(invalidationClasses);
             }
         }
     }
 
-    bool thisElementNeedsStyleRecalc = false;
-
     if (element->hasClass()) {
         const SpaceSplitString& classNames = element->classNames();
         for (Vector<AtomicString>::const_iterator it = invalidationClasses.begin(); it != invalidationClasses.end(); ++it) {
             if (classNames.contains(*it)) {
                 thisElementNeedsStyleRecalc = true;
                 break;
             }
         }
     }
 
     // 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 (foundInvalidationSet || element->childNeedsStyleInvalidation()) {
         bool someChildrenNeedStyleRecalc = invalidateStyleForClassChangeOnChildren(element, invalidationClasses, foundInvalidationSet);
         // We only need to possibly recalc style if this node is in the subtree of a node with a DescendantInvalidationSet on it.
         if (foundInvalidationSet)
             thisElementNeedsStyleRecalc = thisElementNeedsStyleRecalc || someChildrenNeedStyleRecalc;

[esprehn, 2014/02/18 22:52:33]

This doesn't make sense now, thisElementNeedsStyleRecalc is always true if
foundInvalidationSet is true.

[chrishtr, 2014/02/18 23:14:31]

It's the other way around - foundInvalidationSet is always true if
thisElementNeedsStyleRecalc is true.

This code is still needed for case #3 above. In that case
thisElementNeedsStyleRecalc would be false before line 418, since the element
has neither a pending invalidation list nor a matching class. However, we know
we're a descendant of an invalidation list element (via foundInvalidationSet),
so we recalc style if any child did.

     }
 
     if (thisElementNeedsStyleRecalc)
         element->setNeedsStyleRecalc(LocalStyleChange);
 
     invalidationClasses.remove(oldSize, invalidationClasses.size() - oldSize);
     element->clearChildNeedsStyleInvalidation();
     return thisElementNeedsStyleRecalc;
 }
 
 } // namespace WebCore