Bug 485808. Create an iterator class to do forward iteration over the result of GetChildArray(). r+sr=sicking

2009-04-09 21:36:41 -04:00 · 2009-04-09 21:36:41 -04:00 · 46a80c50b9
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@ -1589,78 +1589,6 @@ private:
  PRUint32 mNestingLevel;
 };

-/**
- * Class used to detect unexpected mutations. To use the class create an
- * nsMutationGuard on the stack before unexpected mutations could occur.
- * You can then at any time call Mutated to check if any unexpected mutations
- * have occured.
- *
- * When a guard is instantiated sMutationCount is set to 300. It is then
- * decremented by every mutation (capped at 0). This means that we can only
- * detect 300 mutations during the lifetime of a single guard, however that
- * should be more then we ever care about as we usually only care if more then
- * one mutation has occured.
- *
- * When the guard goes out of scope it will adjust sMutationCount so that over
- * the lifetime of the guard the guard itself has not affected sMutationCount,
- * while mutations that happened while the guard was alive still will. This
- * allows a guard to be instantiated even if there is another guard higher up
- * on the callstack watching for mutations.
- *
- * The only thing that has to be avoided is for an outer guard to be used
- * while an inner guard is alive. This can be avoided by only ever
- * instantiating a single guard per scope and only using the guard in the
- * current scope.
- */
-class nsMutationGuard {
-public:
-  nsMutationGuard()
-  {
-    mDelta = eMaxMutations - sMutationCount;
-    sMutationCount = eMaxMutations;
-  }
-  ~nsMutationGuard()
-  {
-    sMutationCount =
-      mDelta > sMutationCount ? 0 : sMutationCount - mDelta;
-  }
-
-  /**
-   * Returns true if any unexpected mutations have occured. You can pass in
-   * an 8-bit ignore count to ignore a number of expected mutations.
-   */
-  PRBool Mutated(PRUint8 aIgnoreCount)
-  {
-    return sMutationCount < static_cast<PRUint32>(eMaxMutations - aIgnoreCount);
-  }
-
-  // This function should be called whenever a mutation that we want to keep
-  // track of happen. For now this is only done when children are added or
-  // removed, but we might do it for attribute changes too in the future.
-  static void DidMutate()
-  {
-    if (sMutationCount) {
-      --sMutationCount;
-    }
-  }
-
-private:
-  // mDelta is the amount sMutationCount was adjusted when the guard was
-  // initialized. It is needed so that we can undo that adjustment once
-  // the guard dies.
-  PRUint32 mDelta;
-
-  // The value 300 is not important, as long as it is bigger then anything
-  // ever passed to Mutated().
-  enum { eMaxMutations = 300 };
-
-  
-  // sMutationCount is a global mutation counter which is decreased by one at
-  // every mutation. It is capped at 0 to avoid wrapping.
-  // Its value is always between 0 and 300, inclusive.
-  static PRUint32 sMutationCount;
-};
-
 #define NS_AUTO_GCROOT_PASTE2(tok,line) tok##line
 #define NS_AUTO_GCROOT_PASTE(tok,line) \
  NS_AUTO_GCROOT_PASTE2(tok,line)
--- a/content/base/public/nsINode.h
+++ b/content/base/public/nsINode.h
@ -165,6 +165,77 @@ inline nsINode* NODE_FROM(C& aContent, D& aDocument)
  return static_cast<nsINode*>(aDocument);
 }

+/**
+ * Class used to detect unexpected mutations. To use the class create an
+ * nsMutationGuard on the stack before unexpected mutations could occur.
+ * You can then at any time call Mutated to check if any unexpected mutations
+ * have occured.
+ *
+ * When a guard is instantiated sMutationCount is set to 300. It is then
+ * decremented by every mutation (capped at 0). This means that we can only
+ * detect 300 mutations during the lifetime of a single guard, however that
+ * should be more then we ever care about as we usually only care if more then
+ * one mutation has occured.
+ *
+ * When the guard goes out of scope it will adjust sMutationCount so that over
+ * the lifetime of the guard the guard itself has not affected sMutationCount,
+ * while mutations that happened while the guard was alive still will. This
+ * allows a guard to be instantiated even if there is another guard higher up
+ * on the callstack watching for mutations.
+ *
+ * The only thing that has to be avoided is for an outer guard to be used
+ * while an inner guard is alive. This can be avoided by only ever
+ * instantiating a single guard per scope and only using the guard in the
+ * current scope.
+ */
+class nsMutationGuard {
+public:
+  nsMutationGuard()
+  {
+    mDelta = eMaxMutations - sMutationCount;
+    sMutationCount = eMaxMutations;
+  }
+  ~nsMutationGuard()
+  {
+    sMutationCount =
+      mDelta > sMutationCount ? 0 : sMutationCount - mDelta;
+  }
+
+  /**
+   * Returns true if any unexpected mutations have occured. You can pass in
+   * an 8-bit ignore count to ignore a number of expected mutations.
+   */
+  PRBool Mutated(PRUint8 aIgnoreCount)
+  {
+    return sMutationCount < static_cast<PRUint32>(eMaxMutations - aIgnoreCount);
+  }
+
+  // This function should be called whenever a mutation that we want to keep
+  // track of happen. For now this is only done when children are added or
+  // removed, but we might do it for attribute changes too in the future.
+  static void DidMutate()
+  {
+    if (sMutationCount) {
+      --sMutationCount;
+    }
+  }
+
+private:
+  // mDelta is the amount sMutationCount was adjusted when the guard was
+  // initialized. It is needed so that we can undo that adjustment once
+  // the guard dies.
+  PRUint32 mDelta;
+
+  // The value 300 is not important, as long as it is bigger then anything
+  // ever passed to Mutated().
+  enum { eMaxMutations = 300 };
+
+  
+  // sMutationCount is a global mutation counter which is decreased by one at
+  // every mutation. It is capped at 0 to avoid wrapping.
+  // Its value is always between 0 and 300, inclusive.
+  static PRUint32 sMutationCount;
+};

 // IID for the nsINode interface
 #define NS_INODE_IID \
@ -739,6 +810,42 @@ public:
   */
  nsIDocument* GetOwnerDocument() const;

+  /**
+   * Iterator that can be used to easily iterate over the children.  This has
+   * the same restrictions on its use as GetChildArray does.
+   */
+  class ChildIterator {
+  public:
+    ChildIterator(const nsINode* aNode) { Init(aNode); }
+    ChildIterator(const nsINode* aNode, PRUint32 aOffset) {
+      Init(aNode);
+      Advance(aOffset);
+    }
+    ~ChildIterator() {
+      NS_ASSERTION(!mGuard.Mutated(0), "Unexpected mutations happened");
+    }
+
+    PRBool IsDone() const { return mCur == mEnd; }
+    operator nsIContent* const () { return *mCur; }
+    void Next() { NS_PRECONDITION(mCur != mEnd, "Check IsDone"); ++mCur; }
+    void Advance(PRUint32 aOffset) {
+      NS_ASSERTION(mCur + aOffset <= mEnd, "Unexpected offset");
+      mCur += aOffset;
+    }
+  private:
+    void Init(const nsINode* aNode) {
+      NS_PRECONDITION(aNode, "Must have node here!");
+      PRUint32 childCount;
+      mCur = aNode->GetChildArray(&childCount);
+      mEnd = mCur + childCount;
+    }
+#ifdef DEBUG
+    nsMutationGuard mGuard;
+#endif
+    nsIContent* const * mCur;
+    nsIContent* const * mEnd;
+  };
+
 protected:

  // Override this function to create a custom slots class.
--- a/content/base/src/nsContentList.cpp
+++ b/content/base/src/nsContentList.cpp
@ -623,8 +623,10 @@ nsContentList::ContentAppended(nsIDocument *aDocument, nsIContent* aContainer,
    if (!appendToList) {
      // The new stuff is somewhere in the middle of our list; check
      // whether we need to invalidate
-      for (i = aNewIndexInContainer; i <= count-1; ++i) {
-        if (MatchSelf(aContainer->GetChildAt(i))) {
+      for (nsINode::ChildIterator iter(aContainer, aNewIndexInContainer);
+           !iter.IsDone();
+           iter.Next()) {
+        if (MatchSelf(iter)) {
          // Uh-oh.  We're gonna have to add elements into the middle
          // of our list. That's not worth the effort.
          SetDirty();
@ -649,9 +651,11 @@ nsContentList::ContentAppended(nsIDocument *aDocument, nsIContent* aContainer,
     * We're up to date.  That means someone's actively using us; we
     * may as well grab this content....
     */
-    for (i = aNewIndexInContainer; i <= count-1; ++i) {
+    for (nsINode::ChildIterator iter(aContainer, aNewIndexInContainer);
+         !iter.IsDone();
+         iter.Next()) {
      PRUint32 limit = PRUint32(-1);
-      nsIContent* newContent = aContainer->GetChildAt(i);
+      nsIContent* newContent = iter;
      if (newContent->IsNodeOfType(nsINode::eELEMENT)) {
        PopulateWith(newContent, limit);
      }
@ -747,10 +751,8 @@ nsContentList::MatchSelf(nsIContent *aContent)
  if (!mDeep)
    return PR_FALSE;

-  PRUint32 i, count = aContent->GetChildCount();
-
-  for (i = 0; i < count; i++) {
-    if (MatchSelf(aContent->GetChildAt(i))) {
+  for (nsINode::ChildIterator iter(aContent); !iter.IsDone(); iter.Next()) {
+    if (MatchSelf(iter)) {
      return PR_TRUE;
    }
  }
@ -778,25 +780,15 @@ nsContentList::PopulateWith(nsIContent *aContent, PRUint32& aElementsToAppend)
  // Don't recurse down if we're not doing a deep match.
  if (!mDeep)
    return;
-  
-#ifdef DEBUG
-  nsMutationGuard debugMutationGuard;
-#endif  
-  PRUint32 count;
-  nsIContent* const* curChildPtr = aContent->GetChildArray(&count);
-  nsIContent* const* stop = curChildPtr + count;
-  for (; curChildPtr != stop; ++curChildPtr) {
-    nsIContent* curContent = *curChildPtr;
+
+  for (nsINode::ChildIterator iter(aContent); !iter.IsDone(); iter.Next()) {
+    nsIContent* curContent = iter;
    if (curContent->IsNodeOfType(nsINode::eELEMENT)) {
-      PopulateWith(*curChildPtr, aElementsToAppend);
+      PopulateWith(curContent, aElementsToAppend);
      if (aElementsToAppend == 0)
        break;
    }
  }
-#ifdef DEBUG
-  NS_ASSERTION(!debugMutationGuard.Mutated(0),
-               "Unexpected mutations happened.  Check your match function!");
-#endif  
 }

 void 
@ -820,17 +812,11 @@ nsContentList::PopulateWithStartingAfter(nsINode *aStartRoot,
      ++i;  // move to one past
    }

-#ifdef DEBUG
-    nsMutationGuard debugMutationGuard;
-#endif  
-    PRUint32 childCount;
-    nsIContent* const* curChildPtr = aStartRoot->GetChildArray(&childCount);
-    nsIContent* const* stop = curChildPtr + childCount;
-    // Now advance curChildPtr to the child we want to be starting with
-    NS_ASSERTION(i <= childCount, "Unexpected index");
-    curChildPtr += i;
-    for ( ; curChildPtr != stop; ++curChildPtr) {
-      nsIContent* content = *curChildPtr;
+    // Now start an iterator with the child we want to be starting with
+    for (nsINode::ChildIterator iter(aStartRoot, i);
+         !iter.IsDone();
+         iter.Next()) {
+      nsIContent* content = iter;
      if (content->IsNodeOfType(nsINode::eELEMENT)) {
        PopulateWith(content, aElementsToAppend);

@ -840,10 +826,6 @@ nsContentList::PopulateWithStartingAfter(nsINode *aStartRoot,
          break;
      }
    }
-#ifdef DEBUG
-    NS_ASSERTION(!debugMutationGuard.Mutated(0),
-                 "Unexpected mutations happened.  Check your match function!");
-#endif
  }

  if (aElementsToAppend == 0) {
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@ -220,8 +220,6 @@ nsIInterfaceRequestor* nsContentUtils::sSameOriginChecker = nsnull;
 nsIJSRuntimeService *nsAutoGCRoot::sJSRuntimeService;
 JSRuntime *nsAutoGCRoot::sJSScriptRuntime;

-PRUint32 nsMutationGuard::sMutationCount = 0;
-
 PRBool nsContentUtils::sInitialized = PR_FALSE;

 static PLDHashTable sEventListenerManagersHash;
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@ -2327,18 +2327,9 @@ nsDocument::UnregisterNamedItems(nsIContent *aContent)
  RemoveFromNameTable(aContent);
  RemoveFromIdTable(aContent);

-#ifdef DEBUG
-  nsMutationGuard debugMutationGuard;
-#endif
-
-  PRUint32 count;
-  nsIContent * const * kidSlot = aContent->GetChildArray(&count);
-  nsIContent * const * end = kidSlot + count;
-  for (; kidSlot != end; ++kidSlot) {
-    UnregisterNamedItems(*kidSlot);
+  for (nsINode::ChildIterator iter(aContent); !iter.IsDone(); iter.Next()) {
+    UnregisterNamedItems(iter);
  }
-
-  NS_ASSERTION(!debugMutationGuard.Mutated(0), "Unexpected mutations happened");
 }

 void
@ -2352,18 +2343,9 @@ nsDocument::RegisterNamedItems(nsIContent *aContent)
  UpdateNameTableEntry(aContent);
  UpdateIdTableEntry(aContent);

-#ifdef DEBUG
-  nsMutationGuard debugMutationGuard;
-#endif
-
-  PRUint32 count;
-  nsIContent * const * kidSlot = aContent->GetChildArray(&count);
-  nsIContent * const * end = kidSlot + count;
-  for (; kidSlot != end; ++kidSlot) {
-    RegisterNamedItems(*kidSlot);
+  for (nsINode::ChildIterator iter(aContent); !iter.IsDone(); iter.Next()) {
+    RegisterNamedItems(iter);
  }
-
-  NS_ASSERTION(!debugMutationGuard.Mutated(0), "Unexpected mutations happened");
 }

 void
@ -2373,18 +2355,11 @@ nsDocument::ContentAppended(nsIDocument* aDocument,
 {
  NS_ASSERTION(aDocument == this, "unexpected doc");

-#ifdef DEBUG
-  nsMutationGuard debugMutationGuard;
-#endif
-
-  PRUint32 count;
-  nsIContent * const * kidSlot = aContainer->GetChildArray(&count);
-  nsIContent * const * end = kidSlot + count;
-  for (kidSlot += aNewIndexInContainer; kidSlot != end; ++kidSlot) {
-    RegisterNamedItems(*kidSlot);
+  for (nsINode::ChildIterator iter(aContainer, aNewIndexInContainer);
+       !iter.IsDone();
+       iter.Next()) {
+    RegisterNamedItems(iter);
  }
-
-  NS_ASSERTION(!debugMutationGuard.Mutated(0), "Unexpected mutations happened");
 }

 void
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@ -173,6 +173,8 @@ NS_DEFINE_IID(kThisPtrOffsetsSID, NS_THISPTROFFSETS_SID);

 PRInt32 nsIContent::sTabFocusModel = eTabFocus_any;
 PRBool nsIContent::sTabFocusModelAppliesToXUL = PR_FALSE;
+PRUint32 nsMutationGuard::sMutationCount = 0;
+
 nsresult NS_NewContentIterator(nsIContentIterator** aInstancePtrResult);

 //----------------------------------------------------------------------
@ -5123,17 +5125,10 @@ TryMatchingElementsInSubtree(nsINode* aRoot,
  char databuf[2 * sizeof(RuleProcessorData)];
  RuleProcessorData* prevSibling = nsnull;
  RuleProcessorData* data = reinterpret_cast<RuleProcessorData*>(databuf);
-  PRUint32 count;
-  nsIContent * const * kidSlot = aRoot->GetChildArray(&count);
-  nsIContent * const * end = kidSlot + count;

-#ifdef DEBUG
-  nsMutationGuard debugMutationGuard;
-#endif
-  
  PRBool continueIteration = PR_TRUE;
-  for (; kidSlot != end; ++kidSlot) {
-    nsIContent* kid = *kidSlot;
+  for (nsINode::ChildIterator iter(aRoot); !iter.IsDone(); iter.Next()) {
+    nsIContent* kid = iter;
    if (!kid->IsNodeOfType(nsINode::eELEMENT)) {
      continue;
    }
@ -5193,10 +5188,6 @@ TryMatchingElementsInSubtree(nsINode* aRoot,
    prevSibling->~RuleProcessorData();
  }

-#ifdef DEBUG
-  NS_ASSERTION(!debugMutationGuard.Mutated(0), "Unexpected mutations happened");
-#endif
-
  return continueIteration;
 }