Bug 373911, r=bz, sr=dbaron, a=1.9+
authorOlli.Pettay@helsinki.fi
Tue, 21 Aug 2007 14:45:00 -0700
changeset 4876 939a94729316426c57de344a0840eab3a05f69ca
parent 4875 d6abc6197346e44c4294ac83008a96e307a64dea
child 4877 e6c36c34c67a42ba12a49727b56853dd97f1a1e9
push idunknown
push userunknown
push dateunknown
reviewersbz, dbaron, 1.9
bugs373911
milestone1.9a8pre
Bug 373911, r=bz, sr=dbaron, a=1.9+
content/xbl/src/nsBindingManager.cpp
layout/base/nsPresShell.cpp
--- a/content/xbl/src/nsBindingManager.cpp
+++ b/content/xbl/src/nsBindingManager.cpp
@@ -838,46 +838,48 @@ nsBindingManager::ProcessAttachedQueue()
 
   mProcessingAttachedStack = PR_FALSE;
 
   NS_ASSERTION(mAttachedStack.Length() == 0, "How did we get here?");
   
   mAttachedStack.Compact();
 }
 
+// Keep bindings and bound elements alive while executing detached handlers.
+struct BindingTableReadClosure
+{
+  nsCOMArray<nsIContent> mBoundElements;
+  nsBindingList          mBindings;
+};
+
 PR_STATIC_CALLBACK(PLDHashOperator)
 AccumulateBindingsToDetach(nsISupports *aKey, nsXBLBinding *aBinding,
-                           void* aVoidArray)
-{
-  nsVoidArray* arr = static_cast<nsVoidArray*>(aVoidArray);
-  // Hold an owning reference to this binding, just in case
-  if (arr->AppendElement(aBinding))
-    NS_ADDREF(aBinding);
+                           void* aClosure)
+ {
+  BindingTableReadClosure* closure =
+    static_cast<BindingTableReadClosure*>(aClosure);
+  if (aBinding && closure->mBindings.AppendElement(aBinding)) {
+    if (!closure->mBoundElements.AppendObject(aBinding->GetBoundElement())) {
+      closure->mBindings.RemoveElementAt(closure->mBindings.Length() - 1);
+    }
+  }
   return PL_DHASH_NEXT;
 }
 
-PR_STATIC_CALLBACK(PRBool)
-ExecuteDetachedHandler(void* aBinding, void* aClosure)
-{
-  NS_PRECONDITION(aBinding, "Null binding in list?");
-  nsXBLBinding* binding = static_cast<nsXBLBinding*>(aBinding);
-  binding->ExecuteDetachedHandler();
-  // Drop our ref to the binding now
-  NS_RELEASE(binding);
-  return PR_TRUE;
-}
-
 void
 nsBindingManager::ExecuteDetachedHandlers()
 {
   // Walk our hashtable of bindings.
   if (mBindingTable.IsInitialized()) {
-    nsVoidArray bindingsToDetach;
-    mBindingTable.EnumerateRead(AccumulateBindingsToDetach, &bindingsToDetach);
-    bindingsToDetach.EnumerateForwards(ExecuteDetachedHandler, nsnull);
+    BindingTableReadClosure closure;
+    mBindingTable.EnumerateRead(AccumulateBindingsToDetach, &closure);
+    PRUint32 i, count = closure.mBindings.Length();
+    for (i = 0; i < count; ++i) {
+      closure.mBindings[i]->ExecuteDetachedHandler();
+    }
   }
 }
 
 nsresult
 nsBindingManager::PutXBLDocumentInfo(nsIXBLDocumentInfo* aDocumentInfo)
 {
   NS_PRECONDITION(aDocumentInfo, "Must have a non-null documentinfo!");
   
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -4372,60 +4372,52 @@ PresShell::FlushPendingNotifications(moz
   NS_ASSERTION(aType & (Flush_StyleReresolves | Flush_OnlyReflow |
                         Flush_OnlyPaint),
                "Why did we get called?");
   
   PRBool isSafeToFlush;
   IsSafeToFlush(isSafeToFlush);
 
   NS_ASSERTION(!isSafeToFlush || mViewManager, "Must have view manager");
-  if (isSafeToFlush && mViewManager) {
+  // Make sure the view manager stays alive while batching view updates.
+  // XXX FIXME: If viewmanager hierarchy is modified while we're in update
+  //            batch... We need to address that somehow.  See bug 369165.
+  nsCOMPtr<nsIViewManager> viewManager = mViewManager;
+  if (isSafeToFlush && viewManager) {
     // Processing pending notifications can kill us, and some callers only
     // hold weak refs when calling FlushPendingNotifications().  :(
     nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
 
     // Style reresolves not in conjunction with reflows can't cause
     // painting or geometry changes, so don't bother with view update
     // batching if we only have style reresolve
-    mViewManager->BeginUpdateViewBatch();
+    viewManager->BeginUpdateViewBatch();
 
     if (aType & Flush_StyleReresolves) {
       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) {
+    if (aType & Flush_OnlyReflow && !mIsDestroying) {
       mFrameConstructor->RecalcQuotesAndCounters();
       ProcessReflowCommands(PR_FALSE);
-      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;
-      }
     }
 
     PRUint32 updateFlags = NS_VMREFRESH_NO_SYNC;
     if (aType & Flush_OnlyPaint) {
       // Flushing paints, so perform the invalidates and drawing
       // immediately
       updateFlags = NS_VMREFRESH_IMMEDIATE;
     }
     else if (!(aType & Flush_OnlyReflow)) {
       // Not flushing reflows, so do deferred invalidates.  This will keep us
       // from possibly flushing out reflows due to invalidates being processed
       // at the end of this view batch.
       updateFlags = NS_VMREFRESH_DEFERRED;
     }
-    mViewManager->EndUpdateViewBatch(updateFlags);
+    viewManager->EndUpdateViewBatch(updateFlags);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 PresShell::IsReflowLocked(PRBool* aIsReflowLocked) 
 {