Fire XBL constructors asynchronously after binding attachment, unless we're in

the middle of an update.  In that case, fire them at the end of the update.
Bug 267833, r+sr=sicking, fingers-crossed=me

content/base/src/nsContentSink.cpp

@@ -968,8 +968,12 @@ nsContentSink::NotifyAppend(nsIContent* aContainer, PRUint32 aStartIndex)
   MOZ_TIMER_SAVE(mWatch)
   MOZ_TIMER_STOP(mWatch);
 
+  {
+    // Scope so we call EndUpdate before we decrease mInNotification
+    MOZ_AUTO_DOC_UPDATE(mDocument, UPDATE_CONTENT_MODEL, !mBeganUpdate);
     nsNodeUtils::ContentAppended(aContainer, aStartIndex);
     mLastNotificationTime = PR_Now();
+  }
 
   MOZ_TIMER_DEBUGLOG(("Restore: nsHTMLContentSink::NotifyAppend()\n"));
   MOZ_TIMER_RESTORE(mWatch);

content/base/src/nsContentSink.h

@@ -245,7 +245,9 @@ protected:
   // Do we notify based on time?
   PRPackedBool mNotifyOnTimer;
 
-  PRPackedBool mLayoutStarted;
+  // Have we already called BeginUpdate for this set of content changes?
+  PRUint8 mBeganUpdate : 1;
+  PRUint8 mLayoutStarted : 1;
   PRUint8 mScrolledToRefAlready : 1;
   PRUint8 mCanInterruptParser : 1;
   PRUint8 mDynamicLowerValue : 1;

content/base/src/nsDocument.cpp

@@ -2535,6 +2535,7 @@ nsDocument::RemoveObserver(nsIDocumentObserver* aObserver)
 void
 nsDocument::BeginUpdate(nsUpdateType aUpdateType)
 {
+  ++mUpdateNestLevel;
   if (mScriptLoader) {
     mScriptLoader->AddExecuteBlocker();
   }
@@ -2545,6 +2546,14 @@ void
 nsDocument::EndUpdate(nsUpdateType aUpdateType)
 {
   NS_DOCUMENT_NOTIFY_OBSERVERS(EndUpdate, (this, aUpdateType));
+
+  --mUpdateNestLevel;
+  if (mUpdateNestLevel == 0) {
+    // This set of updates may have created XBL bindings.  Run the
+    // constructors.
+    mBindingManager->ProcessAttachedQueue();
+  }
+
   if (mScriptLoader) {
     mScriptLoader->RemoveExecuteBlocker();
   }
@@ -4695,6 +4704,8 @@ nsDocument::FlushPendingNotifications(mozFlushType aType)
     }
   }
 
+  // Should we be flushing pending binding constructors in here?
+
   nsPIDOMWindow *window = GetWindow();
 
   if (aType == (aType & (Flush_Content | Flush_SinkNotifications)) ||

content/base/src/nsDocument.h

@@ -826,6 +826,9 @@ private:
 
   // Member to store out last-selected stylesheet set.
   nsString mLastStyleSheetSet;
+
+  // Our update nesting level
+  PRUint32 mUpdateNestLevel;
 };
 
 

content/html/content/src/nsGenericHTMLElement.cpp

@@ -508,27 +508,6 @@ nsGenericHTMLElement::GetStyle(nsIDOMCSSStyleDeclaration** aStyle)
   return NS_OK;
 }
 
-void
-nsGenericHTMLElement::RecreateFrames()
-{
-  nsIDocument* document = GetCurrentDoc();
-  
-  if (!document) {
-    return;
-  }
-
-  PRInt32 numShells = document->GetNumberOfShells();
-  for (PRInt32 i = 0; i < numShells; ++i) {
-    nsIPresShell *shell = document->GetShellAt(i);
-    if (shell) {
-      nsIFrame* frame = shell->GetPrimaryFrameFor(this);
-      if (frame) {
-        shell->RecreateFramesFor(this);
-      }
-    }
-  }
-}
-
 static PRBool
 IsBody(nsIContent *aContent)
 {

content/html/content/src/nsGenericHTMLElement.h

@@ -726,12 +726,6 @@ protected:
    */
   NS_HIDDEN_(nsresult) GetURIListAttr(nsIAtom* aAttr, nsAString& aResult);
 
-  /**
-   * Helper method to recreate all frames for this content, if there
-   * are any.
-   */
-  void RecreateFrames();
-
   /**
    * Locate an nsIEditor rooted at this content node, if there is one.
    */

content/html/document/src/nsHTMLContentSink.cpp

@@ -1345,9 +1345,12 @@ SinkContext::AddText(const nsAString& aText)
 nsresult
 SinkContext::FlushTags()
 {
+  PRBool oldBeganUpdate = mSink->mBeganUpdate;
+
   ++(mSink->mInNotification);
   mozAutoDocUpdate updateBatch(mSink->mDocument, UPDATE_CONTENT_MODEL,
                                PR_TRUE);
+  mSink->mBeganUpdate = PR_TRUE;
 
   // Don't release last text node in case we need to add to it again
   FlushText();
@@ -1400,6 +1403,8 @@ SinkContext::FlushTags()
   mNotifyLevel = mStackPos - 1;
   --(mSink->mInNotification);
 
+  mSink->mBeganUpdate = oldBeganUpdate;
+
   return NS_OK;
 }
 
@@ -3026,9 +3031,13 @@ HTMLContentSink::NotifyInsert(nsIContent* aContent,
   MOZ_TIMER_SAVE(mWatch)
   MOZ_TIMER_STOP(mWatch);
 
+  {
+    // Scope so we call EndUpdate before we decrease mInNotification
+    MOZ_AUTO_DOC_UPDATE(mDocument, UPDATE_CONTENT_MODEL, !mBeganUpdate);
     nsNodeUtils::ContentInserted(NODE_FROM(aContent, mDocument),
                                  aChildContent, aIndexInContainer);
     mLastNotificationTime = PR_Now();
+  }
 
   MOZ_TIMER_DEBUGLOG(("Restore: nsHTMLContentSink::NotifyInsert()\n"));
   MOZ_TIMER_RESTORE(mWatch);

content/xbl/src/nsBindingManager.cpp

@@ -81,6 +81,8 @@
 #include "nsIScriptContext.h"
 #include "nsBindingManager.h"
 
+#include "nsThreadUtils.h"
+
 // ==================================================================
 // = nsAnonymousContentList 
 // ==================================================================
@@ -361,6 +363,8 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsBindingManager)
       tmp->mDocumentTable.EnumerateRead(&DocumentInfoHashtableTraverser, &cb);
   if (tmp->mLoadingDocTable.IsInitialized())
       tmp->mLoadingDocTable.EnumerateRead(&LoadingDocHashtableTraverser, &cb);
+  // No need to traverse mProcessAttachedQueueEvent, since it'll just
+  // fire at some point.
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsBindingManager)
@@ -816,13 +820,33 @@ nsBindingManager::AddToAttachedQueue(nsXBLBinding* aBinding)
   if (!mAttachedStack.AppendElement(aBinding))
     return NS_ERROR_OUT_OF_MEMORY;
 
+  // If we're in the middle of processing our queue already, don't
+  // bother posting the event.
+  if (!mProcessingAttachedStack && !mProcessAttachedQueueEvent) {
+    mProcessAttachedQueueEvent =
+      new nsRunnableMethod<nsBindingManager>(
+        this, &nsBindingManager::DoProcessAttachedQueue);
+    NS_DispatchToCurrentThread(mProcessAttachedQueueEvent);
+  }
+
   return NS_OK;
 }
 
+void
+nsBindingManager::DoProcessAttachedQueue()
+{
+  ProcessAttachedQueue();
+
+  NS_ASSERTION(mAttachedStack.Length() == 0,
+               "Shouldn't have pending bindings!");
+  
+  mProcessAttachedQueueEvent = nsnull;
+}
+
 void
 nsBindingManager::ProcessAttachedQueue()
 {
-  if (mProcessingAttachedStack)
+  if (mProcessingAttachedStack || mAttachedStack.Length() == 0)
     return;
 
   mProcessingAttachedStack = PR_TRUE;
@@ -837,7 +861,10 @@ nsBindingManager::ProcessAttachedQueue()
   }
 
   mProcessingAttachedStack = PR_FALSE;
-  mAttachedStack.Clear();
+
+  NS_ASSERTION(mAttachedStack.Length() == 0, "How did we get here?");
+  
+  mAttachedStack.Compact();
 }
 
 PR_STATIC_CALLBACK(PLDHashOperator)
@@ -1352,5 +1379,8 @@ nsBindingManager::ContentRemoved(nsIDocument* aDocument,
 void
 nsBindingManager::NodeWillBeDestroyed(const nsINode *aNode)
 {
+  // Make sure to not run any more XBL constructors
+  mProcessingAttachedStack = PR_TRUE;
+  
   NS_BINDINGMANAGER_NOTIFY_OBSERVERS(NodeWillBeDestroyed, (aNode));
 }

content/xbl/src/nsBindingManager.h

@@ -61,6 +61,7 @@ class nsStyleSet;
 class nsXBLBinding;
 template<class E> class nsRefPtr;
 typedef nsTArray<nsRefPtr<nsXBLBinding> > nsBindingList;
+template<class T> class nsRunnableMethod;
 
 class nsBindingManager : public nsIMutationObserver
 {
@@ -218,6 +219,10 @@ protected:
   NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(mObservers, nsIMutationObserver, \
                                      func_, params_);
 
+  // Same as ProcessAttachedQueue, but also nulls out
+  // mProcessAttachedQueueEvent
+  void DoProcessAttachedQueue();
+
 // MEMBER VARIABLES
 protected: 
   // A mapping from nsIContent* to the nsXBLBinding* that is
@@ -271,6 +276,10 @@ protected:
   // A queue of binding attached event handlers that are awaiting execution.
   nsBindingList mAttachedStack;
   PRBool mProcessingAttachedStack;
+
+  // Our posted event to process the attached queue, if any
+  friend class nsRunnableMethod<nsBindingManager>;
+  nsCOMPtr<nsIRunnable> mProcessAttachedQueueEvent;
 };
 
 #endif

content/xbl/src/nsXBLResourceLoader.cpp

@@ -235,12 +235,6 @@ nsXBLResourceLoader::NotifyBoundElements()
         // Flush first to make sure we can get the frame for content
         doc->FlushPendingNotifications(Flush_Frames);
 
-        // Notify
-        nsIContent* parent = content->GetParent();
-        PRInt32 index = 0;
-        if (parent)
-          index = parent->IndexOf(content);
-        
         // If |content| is (in addition to having binding |mBinding|)
         // also a descendant of another element with binding |mBinding|,
         // then we might have just constructed it due to the
@@ -259,8 +253,7 @@ nsXBLResourceLoader::NotifyBoundElements()
               shell->FrameManager()->GetUndisplayedContent(content);
 
             if (!sc) {
-              nsCOMPtr<nsIDocumentObserver> obs(do_QueryInterface(shell));
-              obs->ContentInserted(doc, parent, content, index);
+              shell->RecreateFramesFor(content);
             }
           }
         }

content/xml/document/src/nsXMLContentSink.cpp

@@ -1537,11 +1537,14 @@ nsXMLContentSink::FlushPendingNotifications(mozFlushType aType)
 nsresult
 nsXMLContentSink::FlushTags()
 {
-  // Don't release last text node in case we need to add to it again
-  FlushText();
+  PRBool oldBeganUpdate = mBeganUpdate;
 
   ++mInNotification;
   mozAutoDocUpdate updateBatch(mDocument, UPDATE_CONTENT_MODEL, PR_TRUE);
+  mBeganUpdate = PR_TRUE;
+
+  // Don't release last text node in case we need to add to it again
+  FlushText();
 
   // Start from the base of the stack (growing downward) and do
   // a notification from the node that is closest to the root of
@@ -1568,6 +1571,8 @@ nsXMLContentSink::FlushTags()
 
   --mInNotification;
 
+  mBeganUpdate = oldBeganUpdate;
+  
   return NS_OK;
 }
 

content/xul/templates/src/nsXULContentBuilder.cpp

@@ -2004,6 +2004,8 @@ nsXULContentBuilder::OpenContainer(nsIContent* aElement)
     if (container && IsLazyWidgetItem(aElement)) {
         // The tree widget is special, and has to be spanked every
         // time we add content to a container.
+        MOZ_AUTO_DOC_UPDATE(container->GetCurrentDoc(), UPDATE_CONTENT_MODEL,
+                            PR_TRUE);
         nsNodeUtils::ContentAppended(container, newIndex);
     }
 
@@ -2070,6 +2072,8 @@ nsXULContentBuilder::RebuildAll()
     CreateTemplateAndContainerContents(mRoot, getter_AddRefs(container), &newIndex);
 
     if (container) {
+        MOZ_AUTO_DOC_UPDATE(container->GetCurrentDoc(), UPDATE_CONTENT_MODEL,
+                            PR_TRUE);
         nsNodeUtils::ContentAppended(container, newIndex);
     }
 

layout/base/nsCSSFrameConstructor.cpp

@@ -8514,11 +8514,6 @@ nsCSSFrameConstructor::ContentAppended(nsIContent*     aContainer,
     }
   }
 
-  // We built some new frames.  Initialize any newly-constructed bindings.
-  LAYOUT_PHASE_TEMP_EXIT();
-  mDocument->BindingManager()->ProcessAttachedQueue();
-  LAYOUT_PHASE_TEMP_REENTER();
-
   // process the current pseudo frame state
   if (!state.mPseudoFrames.IsEmpty()) {
     ProcessPseudoFrames(state, frameItems);
@@ -8851,10 +8846,6 @@ nsCSSFrameConstructor::ContentInserted(nsIContent*            aContainer,
 #endif
     }
 
-    LAYOUT_PHASE_TEMP_EXIT();
-    mDocument->BindingManager()->ProcessAttachedQueue();
-    LAYOUT_PHASE_TEMP_REENTER();
-
     // otherwise this is not a child of the root element, and we
     // won't let it have a frame.
     return NS_OK;
@@ -9080,12 +9071,6 @@ nsCSSFrameConstructor::ContentInserted(nsIContent*            aContainer,
     }
   }
 
-  // Now that we've created frames, run the attach queue.
-  //XXXwaterson should we do this after we've processed pseudos, too?
-  LAYOUT_PHASE_TEMP_EXIT();
-  mDocument->BindingManager()->ProcessAttachedQueue();
-  LAYOUT_PHASE_TEMP_REENTER();
-
   // process the current pseudo frame state
   if (!state.mPseudoFrames.IsEmpty())
     ProcessPseudoFrames(state, frameItems);
@@ -12232,8 +12217,6 @@ nsCSSFrameConstructor::CreateListBoxContent(nsPresContext* aPresContext,
     *aNewFrame = newFrame;
 
     if (NS_SUCCEEDED(rv) && (nsnull != newFrame)) {
-      mDocument->BindingManager()->ProcessAttachedQueue();
-
       // Notify the parent frame
       if (aIsAppend)
         rv = ((nsListBoxBodyFrame*)aParentFrame)->ListBoxAppendFrames(newFrame);
@@ -12938,6 +12921,11 @@ nsCSSFrameConstructor::ProcessPendingRestyles()
 
   delete [] restylesToProcess;
 
+  // Run the XBL binding constructors for any new frames we've constructed.
+  // Note that the restyle event is holding a strong ref to us, so we're ok
+  // here.
+  mDocument->BindingManager()->ProcessAttachedQueue();
+
   EndUpdate();
 
 #ifdef DEBUG

layout/base/nsCSSFrameConstructor.h

@@ -138,16 +138,27 @@ public:
 
   // Note: It's the caller's responsibility to make sure to wrap a
   // ProcessRestyledFrames call in a view update batch.
+  // This function does not call ProcessAttachedQueue() on the binding manager.
+  // If the caller wants that to happen synchronously, it needs to handle that
+  // itself.
   nsresult ProcessRestyledFrames(nsStyleChangeList& aRestyleArray);
 
+private:
   // Note: It's the caller's responsibility to make sure to wrap a
   // ProcessOneRestyle call in a view update batch.
+  // This function does not call ProcessAttachedQueue() on the binding manager.
+  // If the caller wants that to happen synchronously, it needs to handle that
+  // itself.
   void ProcessOneRestyle(nsIContent* aContent, nsReStyleHint aRestyleHint,
                          nsChangeHint aChangeHint);
 
+public:
   // Note: It's the caller's responsibility to make sure to wrap a
   // ProcessPendingRestyles call in a view update batch.
+  // ProcessPendingRestyles will handle calling ProcessAttachedQueue() on the
+  // binding manager.
   void ProcessPendingRestyles();
+
   void PostRestyleEvent(nsIContent* aContent, nsReStyleHint aRestyleHint,
                         nsChangeHint aMinChangeHint);
 

layout/base/nsPresShell.cpp

@@ -2561,6 +2561,9 @@ PresShell::InitialReflow(nscoord aWidth, nscoord aHeight)
     }
   }
 
+  // Run the XBL binding constructors for any new frames we've constructed
+  mDocument->BindingManager()->ProcessAttachedQueue();
+
   return NS_OK; //XXX this needs to be real. MMP
 }
 
@@ -4512,7 +4515,16 @@ PresShell::FlushPendingNotifications(mozFlushType aType)
     mViewManager->BeginUpdateViewBatch();
 
     if (aType & Flush_StyleReresolves) {
+      // Processing pending restyles can kill us, and some callers only
+      // hold weak refs when calling FlushPendingNotifications().  :(
+      nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
       mFrameConstructor->ProcessPendingRestyles();
+      if (mIsDestroying) {
+        // We no longer have a view manager and all that.
+        // XXX FIXME: Except we're in the middle of a view update batch...  We
+        // need to address that somehow.  See bug 369165.
+        return NS_OK;
+      }
     }
 
     if (aType & Flush_OnlyReflow) {