Backing out half of previous changes, since they're not necessary to fix the crash and I think they cause problems.

content/base/src/nsDocument.cpp

@@ -530,20 +530,21 @@ nsDocument::~nsDocument()
 {
   delete mXPathDocument;
 
-  mInDestructor = PR_TRUE;
-
   // XXX Inform any remaining observers that we are going away.
   // Note that this currently contradicts the rule that all
   // observers must hold on to live references to the document.
   // This notification will occur only after the reference has
   // been dropped.
-
+  mInDestructor = PR_TRUE;
-  // if an observer removes itself, we're ok (not if it removes others though)
   PRInt32 indx;
-  for (indx = mObservers.Count() - 1; indx >= 0; --indx) {
+  for (indx = 0; indx < mObservers.Count(); ++indx) {
     // XXX Should this be a kungfudeathgrip?!!!!
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers.ElementAt(indx);
     observer->DocumentWillBeDestroyed(this);
+    // Test to see if the observer was removed
+    if (observer != (nsIDocumentObserver*)mObservers.ElementAt(indx)) {
+      indx--;
+    }
   }
 
   mLoadFlags = nsIRequest::LOAD_NORMAL; // XXX maybe not required
@@ -1792,9 +1793,17 @@ NS_IMETHODIMP
 nsDocument::BeginUpdate()
 {
   PRInt32 i;
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver* observer = (nsIDocumentObserver*) mObservers[i];
     observer->BeginUpdate(this);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -1803,9 +1812,17 @@ NS_IMETHODIMP
 nsDocument::EndUpdate()
 {
   PRInt32 i;
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver* observer = (nsIDocumentObserver*) mObservers[i];
     observer->EndUpdate(this);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -1814,9 +1831,17 @@ NS_IMETHODIMP
 nsDocument::BeginLoad()
 {
   PRInt32 i;
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver* observer = (nsIDocumentObserver*) mObservers[i];
     observer->BeginLoad(this);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -1843,9 +1868,18 @@ NS_IMETHODIMP
 nsDocument::EndLoad()
 {
   PRInt32 i;
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver* observer = (nsIDocumentObserver*)mObservers[i];
     observer->EndLoad(this);
+
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
 
   // Fire a DOM event notifying listeners that this document has been
@@ -1976,9 +2010,17 @@ nsDocument::ContentChanged(nsIContent* aContent,
   NS_ABORT_IF_FALSE(aContent, "Null content!");
 
   PRInt32 i;
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     observer->ContentChanged(this, aContent, aSubContent);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -1989,9 +2031,17 @@ nsDocument::ContentStatesChanged(nsIContent* aContent1,
                                  PRInt32 aStateMask)
 {
   PRInt32 i;
-  for (i = mObservers.Count(); i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     observer->ContentStatesChanged(this, aContent1, aContent2, aStateMask);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -2007,14 +2057,13 @@ nsDocument::ContentAppended(nsIContent* aContainer,
   volatile
 #endif
   PRInt32 i;
-  // XXXdwh There is a hacky ordering dependency between the binding manager 
+  // Get new value of count for every iteration in case
-  // and the frame constructor that forces us to walk the observer list 
+  // observers remove themselves during the loop.
-  // in a forward order
   for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     observer->ContentAppended(this, aContainer, aNewIndexInContainer);
     // Make sure that the observer didn't remove itself during the
-    // notification. If it did, update our index
+    // notification. If it did, update our index and count.
     if (i < mObservers.Count() &&
         observer != (nsIDocumentObserver*)mObservers[i]) {
       i--;
@@ -2031,14 +2080,13 @@ nsDocument::ContentInserted(nsIContent* aContainer,
   NS_ABORT_IF_FALSE(aChild, "Null child!");
 
   PRInt32 i;
-  // XXXdwh There is a hacky ordering dependency between the binding manager 
+  // Get new value of count for every iteration in case
-  // and the frame constructor that forces us to walk the observer list 
+  // observers remove themselves during the loop.
-  // in a forward order
   for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     observer->ContentInserted(this, aContainer, aChild, aIndexInContainer);
     // Make sure that the observer didn't remove itself during the
-    // notification. If it did, update our index.
+    // notification. If it did, update our index and count.
     if (i < mObservers.Count() &&
         observer != (nsIDocumentObserver*)mObservers[i]) {
       i--;
@@ -2056,13 +2104,18 @@ nsDocument::ContentReplaced(nsIContent* aContainer,
   NS_ABORT_IF_FALSE(aOldChild && aNewChild, "Null old or new child child!");
 
   PRInt32 i;
-  // XXXdwh There is a hacky ordering dependency between the binding manager 
+  // Get new value of count for every iteration in case
-  // and the frame constructor that forces us to walk the observer list 
+  // observers remove themselves during the loop.
-  // in a reverse order
+  for (i = 0; i < mObservers.Count(); i++) {
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     observer->ContentReplaced(this, aContainer, aOldChild, aNewChild,
                               aIndexInContainer);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -2075,13 +2128,18 @@ nsDocument::ContentRemoved(nsIContent* aContainer,
   NS_ABORT_IF_FALSE(aChild, "Null child!");
 
   PRInt32 i;
-  // XXXdwh There is a hacky ordering dependency between the binding manager 
+  // Get new value of count for every iteration in case
-  // and the frame constructor that forces us to walk the observer list 
+  // observers remove themselves during the loop.
-  // in a reverse order
+  for (i = 0; i < mObservers.Count(); i++) {
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     observer->ContentRemoved(this, aContainer, 
                              aChild, aIndexInContainer);
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return NS_OK;
 }
@@ -2107,11 +2165,19 @@ nsDocument::AttributeChanged(nsIContent* aChild,
 
   PRInt32 i;
   nsresult result = NS_OK;
-  for (i = mObservers.Count() - 1; i >= 0; --i) {
+  // Get new value of count for every iteration in case
+  // observers remove themselves during the loop.
+  for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
     nsresult rv = observer->AttributeChanged(this, aChild, aNameSpaceID, aAttribute, aModType, aHint);
     if (NS_FAILED(rv) && NS_SUCCEEDED(result))
       result = rv;
+    // Make sure that the observer didn't remove itself during the
+    // notification. If it did, update our index and count.
+    if (i < mObservers.Count() &&
+        observer != (nsIDocumentObserver*)mObservers[i]) {
+      i--;
+    }
   }
   return result;
 }
@@ -2126,8 +2192,6 @@ nsDocument::StyleRuleChanged(nsIStyleSheet* aStyleSheet, nsIStyleRule* aStyleRul
   // observers remove themselves during the loop.
   for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
-    // XXXbz We should _not_ be calling BeginUpdate from in here! The
-    // caller of StyleRuleChanged should do that!
     observer->BeginUpdate(this);
     observer->StyleRuleChanged(this, aStyleSheet, aStyleRule, aHint);
     // Make sure that the observer didn't remove itself during the
@@ -2151,8 +2215,6 @@ nsDocument::StyleRuleAdded(nsIStyleSheet* aStyleSheet, nsIStyleRule* aStyleRule)
   // observers remove themselves during the loop.
   for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
-    // XXXbz We should _not_ be calling BeginUpdate from in here! The
-    // caller of StyleRuleChanged should do that!
     observer->BeginUpdate(this);
     observer->StyleRuleAdded(this, aStyleSheet, aStyleRule);
     // Make sure that the observer didn't remove itself during the
@@ -2176,8 +2238,6 @@ nsDocument::StyleRuleRemoved(nsIStyleSheet* aStyleSheet, nsIStyleRule* aStyleRul
   // observers remove themselves during the loop.
   for (i = 0; i < mObservers.Count(); i++) {
     nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
-    // XXXbz We should _not_ be calling BeginUpdate from in here! The
-    // caller of StyleRuleChanged should do that!
     observer->BeginUpdate(this);
     observer->StyleRuleRemoved(this, aStyleSheet, aStyleRule);
     // Make sure that the observer didn't remove itself during the