Bug 661746 - Part 1: Delay setting slow selector flags during selector matching to avoid mutating Elements. r=bzbarsky
authorDavid Zbarsky <dzbarsky@gmail.com>
Mon, 14 Nov 2011 16:29:56 +1300
changeset 80217 03970d1fc9191ff84c8d1ec538b56ba4ecda16b6
parent 80216 61094ae6da186b46ed268d93be2e87f35c0f12b4
child 80218 1cb3705557d9d5cae86418bc2bb3402be7d5a62b
push id323
push userrcampbell@mozilla.com
push dateTue, 15 Nov 2011 21:58:36 +0000
treeherderfx-team@3ea216303184 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs661746
milestone11.0a1
Bug 661746 - Part 1: Delay setting slow selector flags during selector matching to avoid mutating Elements. r=bzbarsky
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsRuleProcessorData.h
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1446,17 +1446,17 @@ edgeChildMatches(Element* aElement, Tree
                  bool checkFirst, bool checkLast)
 {
   nsIContent *parent = aElement->GetParent();
   if (!parent) {
     return false;
   }
 
   if (aTreeMatchContext.mForStyling)
-    parent->SetFlags(NODE_HAS_EDGE_CHILD_SELECTOR);
+    aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_EDGE_CHILD_SELECTOR);
 
   return (!checkFirst ||
           aTreeMatchContext.mNthIndexCache.
             GetNthIndex(aElement, false, false, true) == 1) &&
          (!checkLast ||
           aTreeMatchContext.mNthIndexCache.
             GetNthIndex(aElement, false, true, true) == 1);
 }
@@ -1469,19 +1469,19 @@ nthChildGenericMatches(Element* aElement
 {
   nsIContent *parent = aElement->GetParent();
   if (!parent) {
     return false;
   }
 
   if (aTreeMatchContext.mForStyling) {
     if (isFromEnd)
-      parent->SetFlags(NODE_HAS_SLOW_SELECTOR);
+      aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_SLOW_SELECTOR);
     else
-      parent->SetFlags(NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS);
+      aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS);
   }
 
   const PRInt32 index = aTreeMatchContext.mNthIndexCache.
     GetNthIndex(aElement, isOfType, isFromEnd, false);
   if (index <= 0) {
     // Node is anonymous content (not really a child of its parent).
     return false;
   }
@@ -1507,19 +1507,19 @@ edgeOfTypeMatches(Element* aElement, Tre
 {
   nsIContent *parent = aElement->GetParent();
   if (!parent) {
     return false;
   }
 
   if (aTreeMatchContext.mForStyling) {
     if (checkLast)
-      parent->SetFlags(NODE_HAS_SLOW_SELECTOR);
+      aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_SLOW_SELECTOR);
     else
-      parent->SetFlags(NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS);
+      aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS);
   }
 
   return (!checkFirst ||
           aTreeMatchContext.mNthIndexCache.
             GetNthIndex(aElement, true, false, true) == 1) &&
          (!checkLast ||
           aTreeMatchContext.mNthIndexCache.
             GetNthIndex(aElement, true, true, true) == 1);
@@ -1529,17 +1529,17 @@ static inline bool
 checkGenericEmptyMatches(Element* aElement,
                          TreeMatchContext& aTreeMatchContext,
                          bool isWhitespaceSignificant)
 {
   nsIContent *child = nsnull;
   PRInt32 index = -1;
 
   if (aTreeMatchContext.mForStyling)
-    aElement->SetFlags(NODE_HAS_EMPTY_SELECTOR);
+    aTreeMatchContext.RecordFlagsToAdd(aElement, NODE_HAS_EMPTY_SELECTOR);
 
   do {
     child = aElement->GetChildAt(++index);
     // stop at first non-comment (and non-whitespace for
     // :-moz-only-whitespace) node        
   } while (child && !IsSignificantChild(child, true, isWhitespaceSignificant));
   return (child == nsnull);
 }
@@ -1691,17 +1691,17 @@ static bool SelectorMatches(Element* aEl
           PRInt32 index = -1;
 
           if (aTreeMatchContext.mForStyling)
             // FIXME:  This isn't sufficient to handle:
             //   :-moz-empty-except-children-with-localname() + E
             //   :-moz-empty-except-children-with-localname() ~ E
             // because we don't know to restyle the grandparent of the
             // inserted/removed element (as in bug 534804 for :empty).
-            aElement->SetFlags(NODE_HAS_SLOW_SELECTOR);
+            aTreeMatchContext.RecordFlagsToAdd(aElement, NODE_HAS_SLOW_SELECTOR);
           do {
             child = aElement->GetChildAt(++index);
           } while (child &&
                    (!IsSignificantChild(child, true, false) ||
                     (child->GetNameSpaceID() == aElement->GetNameSpaceID() &&
                      child->Tag()->Equals(nsDependentString(pseudoClass->u.mString)))));
           if (child != nsnull) {
             return false;
@@ -1805,17 +1805,17 @@ static bool SelectorMatches(Element* aEl
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_firstNode:
         {
           nsIContent *firstNode = nsnull;
           nsIContent *parent = aElement->GetParent();
           if (parent) {
             if (aTreeMatchContext.mForStyling)
-              parent->SetFlags(NODE_HAS_EDGE_CHILD_SELECTOR);
+              aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_EDGE_CHILD_SELECTOR);
 
             PRInt32 index = -1;
             do {
               firstNode = parent->GetChildAt(++index);
               // stop at first non-comment and non-whitespace node
             } while (firstNode &&
                      !IsSignificantChild(firstNode, true, false));
           }
@@ -1832,17 +1832,17 @@ static bool SelectorMatches(Element* aEl
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_lastNode:
         {
           nsIContent *lastNode = nsnull;
           nsIContent *parent = aElement->GetParent();
           if (parent) {
             if (aTreeMatchContext.mForStyling)
-              parent->SetFlags(NODE_HAS_EDGE_CHILD_SELECTOR);
+              aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_EDGE_CHILD_SELECTOR);
             
             PRUint32 index = parent->GetChildCount();
             do {
               lastNode = parent->GetChildAt(--index);
               // stop at first non-comment and non-whitespace node
             } while (lastNode &&
                      !IsSignificantChild(lastNode, true, false));
           }
@@ -2167,17 +2167,17 @@ static bool SelectorMatchesTree(Element*
     Element* element = nsnull;
     if (PRUnichar('+') == selector->mOperator ||
         PRUnichar('~') == selector->mOperator) {
       // The relevant link must be an ancestor of the node being matched.
       aLookForRelevantLink = false;
       nsIContent* parent = prevElement->GetParent();
       if (parent) {
         if (aTreeMatchContext.mForStyling)
-          parent->SetFlags(NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS);
+          aTreeMatchContext.RecordFlagsToAdd(parent, NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS);
 
         PRInt32 index = parent->IndexOf(prevElement);
         while (0 <= --index) {
           nsIContent* content = parent->GetChildAt(index);
           if (content->IsElement()) {
             element = content->AsElement();
             break;
           }
--- a/layout/style/nsRuleProcessorData.h
+++ b/layout/style/nsRuleProcessorData.h
@@ -46,22 +46,33 @@
 
 #include "nsPresContext.h" // for nsCompatibility
 #include "nsString.h"
 #include "nsChangeHint.h"
 #include "nsIContent.h"
 #include "nsCSSPseudoElements.h"
 #include "nsRuleWalker.h"
 #include "nsNthIndexCache.h"
+#include "nsTHashtable.h"
+#include "nsHashKeys.h"
 
 class nsIStyleSheet;
 class nsIAtom;
 class nsICSSPseudoComparator;
 class nsAttrValue;
 
+struct NodeFlagEntry : public nsPtrHashKey<const nsINode>
+{
+  NodeFlagEntry(const nsINode* aNode) : nsPtrHashKey<const nsINode>(aNode), mFlags(0) {}
+
+  PRUint32 mFlags;
+};
+
+typedef nsTHashtable<NodeFlagEntry> NodeFlagsTable;
+
 /**
  * A |TreeMatchContext| has data about a matching operation.  The
  * data are not node-specific but are invariants of the DOM tree the
  * nodes being matched against are in.
  *
  * Most of the members are in parameters to selector matching.  The
  * one out parameter is mHaveRelevantLink.  Consumers that use a
  * TreeMatchContext for more than one matching operation and care
@@ -103,17 +114,30 @@ struct NS_STACK_CLASS TreeMatchContext {
   // to be rerun with eRelevantLinkVisited.  Otherwise, its behavior is
   // undefined (it might get set appropriately, or might not).
   bool mHaveRelevantLink;
 
   // How matching should be performed.  See the documentation for
   // nsRuleWalker::VisitedHandlingType.
   nsRuleWalker::VisitedHandlingType mVisitedHandling;
 
+  NodeFlagsTable mFlagsToAdd;
+
+  static PLDHashOperator
+  AddFlagsToNode(NodeFlagEntry* aEntry, void* aData) {
+    const_cast<nsINode*>(aEntry->GetKey())->SetFlags(aEntry->mFlags);
+    return PL_DHASH_NEXT; 
+  }
+
  public:
+  void RecordFlagsToAdd(const nsINode* aNode, PRUint32 aFlags) {
+    NodeFlagEntry* entry = mFlagsToAdd.PutEntry(aNode);
+    entry->mFlags |= aFlags;
+  }
+  
   // The document we're working with.
   nsIDocument* const mDocument;
 
   // Root of scoped stylesheet (set and unset by the supplier of the
   // scoped stylesheet).
   nsIContent* mScopedRoot;
 
   // Whether our document is HTML (as opposed to XML of some sort,
@@ -135,16 +159,21 @@ struct NS_STACK_CLASS TreeMatchContext {
     : mForStyling(aForStyling)
     , mHaveRelevantLink(false)
     , mVisitedHandling(aVisitedHandling)
     , mDocument(aDocument)
     , mScopedRoot(nsnull)
     , mIsHTMLDocument(aDocument->IsHTML())
     , mCompatMode(aDocument->GetCompatibilityMode())
   {
+    mFlagsToAdd.Init();
+  }
+
+  ~TreeMatchContext() {
+    mFlagsToAdd.EnumerateEntries(AddFlagsToNode, nsnull);
   }
 };
 
 // The implementation of the constructor and destructor are currently in
 // nsCSSRuleProcessor.cpp.
 
 struct NS_STACK_CLASS RuleProcessorData  {
   RuleProcessorData(nsPresContext* aPresContext,