Bug 100952, round two. This time, maintain a stack of active resources that we're currently building content for. r=tingley@sundell.net, sr=brendan

content/xul/templates/src/nsXULContentBuilder.cpp

@@ -1203,6 +1203,12 @@ nsXULContentBuilder::CreateContainerContents(nsIContent* aElement,
                                              nsIContent** aContainer,
                                              PRInt32* aNewIndexInContainer)
 {
+    // Avoid re-entrant builds for the same resource.
+    if (IsActivated(aResource))
+        return NS_OK;
+
+    ActivationEntry entry(aResource, &mTop);
+
     // Create the contents of a container by iterating over all of the
     // "containment" arcs out of the element's resource.
     nsresult rv;
@@ -1663,12 +1669,6 @@ nsXULContentBuilder::Rebuild(nsIContent* aElement)
     if (! IsElementInWidget(aElement))
         return NS_OK;
 
-    // Forbid re-entrant rebuilds
-    if (mIsBuilding)
-        return NS_OK;
-
-    AutoLatch latch(&mIsBuilding);
-
     nsresult rv;
 
     // Next, see if it's a XUL element whose contents have never even

content/xul/templates/src/nsXULTemplateBuilder.cpp

@@ -160,8 +160,8 @@ nsXULTemplateBuilder::nsXULTemplateBuilder(void)
       mTimer(nsnull),
       mUpdateBatchNest(0),
       mRulesCompiled(PR_FALSE),
-      mIsBuilding(PR_FALSE),
-      mFlags(0)
+      mFlags(0),
+      mTop(nsnull)
 {
     NS_INIT_REFCNT();
 }
@@ -565,11 +565,14 @@ nsXULTemplateBuilder::OnAssert(nsIRDFDataSource* aDataSource,
                                nsIRDFResource* aProperty,
                                nsIRDFNode* aTarget)
 {
-    // Ignore re-entrant updates while we're building content.
-    if (mIsBuilding || mUpdateBatchNest != 0)
+    // Ignore updates if we're batching
+    if (mUpdateBatchNest)
         return(NS_OK);
 
-    AutoLatch latch(&mIsBuilding);
+    // Ignore re-entrant builds for content that is currently in our
+    // activation stack.
+    if (IsActivated(aSource))
+        return NS_OK;
 
     nsresult rv;
 
@@ -635,11 +638,14 @@ nsXULTemplateBuilder::OnUnassert(nsIRDFDataSource* aDataSource,
                                  nsIRDFResource* aProperty,
                                  nsIRDFNode* aTarget)
 {
-    // Ignore re-entrant updates while we're building content.
-    if (mIsBuilding || mUpdateBatchNest != 0)
+    // Ignore updates if we're batching
+    if (mUpdateBatchNest)
         return NS_OK;
 
-    AutoLatch latch(&mIsBuilding);
+    // Ignore re-entrant builds for content that is currently in our
+    // activation stack.
+    if (IsActivated(aSource))
+        return NS_OK;
 
     nsresult rv;
 
@@ -665,11 +671,14 @@ nsXULTemplateBuilder::OnChange(nsIRDFDataSource* aDataSource,
                                nsIRDFNode* aOldTarget,
                                nsIRDFNode* aNewTarget)
 {
-    // Ignore re-entrant updates while we're building content.
-    if (mIsBuilding || mUpdateBatchNest != 0)
+    // Ignore updates if we're batching
+    if (mUpdateBatchNest)
         return NS_OK;
 
-    AutoLatch latch(&mIsBuilding);
+    // Ignore re-entrant builds for content that is currently in our
+    // activation stack.
+    if (IsActivated(aSource))
+        return NS_OK;
 
 	if (mCache) {
 		if (aOldTarget)
@@ -715,12 +724,10 @@ nsXULTemplateBuilder::OnMove(nsIRDFDataSource* aDataSource,
                              nsIRDFResource* aProperty,
                              nsIRDFNode* aTarget)
 {
-    // Ignore re-entrant updates while we're building content.
-    if (mIsBuilding || mUpdateBatchNest != 0)
+    // Ignore updates if we're batching
+    if (mUpdateBatchNest)
         return NS_OK;
 
-    AutoLatch latch(&mIsBuilding);
-
 	if (mCache)
 		mCache->Move(aOldSource, aNewSource, aProperty, aTarget);
 
@@ -2439,3 +2446,14 @@ nsXULTemplateBuilder::IsSystemPrincipal(nsIPrincipal *principal, PRBool *result)
   return NS_OK;
 }
 
+PRBool
+nsXULTemplateBuilder::IsActivated(nsIRDFResource *aResource)
+{
+    for (ActivationEntry *entry = mTop;
+         entry != nsnull;
+         entry = entry->mPrevious) {
+        if (entry->mResource == aResource)
+            return PR_TRUE;
+    }
+    return PR_FALSE;
+}

content/xul/templates/src/nsXULTemplateBuilder.h

@@ -60,6 +60,7 @@
 #include "nsFixedSizeAllocator.h"
 #include "nsResourceSet.h"
 #include "nsRuleNetwork.h"
+#include "nsVoidArray.h"
 
 #include "prlog.h"
 #ifdef PR_LOGGING
@@ -403,8 +404,6 @@ protected:
     static PRInt32  kNameSpaceID_RDF;
     static PRInt32  kNameSpaceID_XUL;
 
-    PRBool mIsBuilding;
-
     enum {
         eDontTestEmpty = (1 << 0)
     };
@@ -412,21 +411,36 @@ protected:
     PRInt32 mFlags;
 
     /**
-     * Stack-based helper class to maintain a latch variable without
-     * worrying about control flow headaches.
+     * Stack-based helper class to maintain a list of ``activated''
+     * resources; i.e., resources for which we are currently building
+     * content.
      */
-    class AutoLatch {
-    protected:
-        PRBool* mVariable;
-
+    class ActivationEntry {
     public:
-        AutoLatch(PRBool* aVariable) : mVariable(aVariable) {
-            NS_ASSERTION(! *mVariable, "latch already set");
-            *mVariable = PR_TRUE; }
+        nsIRDFResource   *mResource;
+        ActivationEntry  *mPrevious;
+        ActivationEntry **mLink;
 
-        ~AutoLatch() { *mVariable = PR_FALSE; }
+        ActivationEntry(nsIRDFResource *aResource, ActivationEntry **aLink)
+            : mResource(aResource),
+              mPrevious(*aLink),
+              mLink(aLink) { *mLink = this; }
+
+        ~ActivationEntry() { *mLink = mPrevious; }
     };
 
+    /**
+     * The top of the stack of resources that we're currently building
+     * content for.
+     */
+    ActivationEntry *mTop;
+
+    /**
+     * Determine if a resource is currently on the activation stack.
+     */
+    PRBool
+    IsActivated(nsIRDFResource *aResource);
+
     /**
      * Must be implemented by subclasses. Handle replacing aOldMatch
      * with aNewMatch. Either aOldMatch or aNewMatch may be null.