Bug 423355: Fix bug in blocker unnesting code. Also make sure to never fire mutation events when it's not safe to run script, even if the event isn't catchable by content. r/sr=bz

content/base/public/nsIDocument.h

@@ -595,8 +595,6 @@ public:
   // To make this easy and painless, use the mozAutoDocUpdate helper class.
   virtual void BeginUpdate(nsUpdateType aUpdateType) = 0;
   virtual void EndUpdate(nsUpdateType aUpdateType) = 0;
-  virtual PRUint32 GetUpdateNestingLevel() = 0;
-  virtual PRBool AllUpdatesAreContent() = 0;
   virtual void BeginLoad() = 0;
   virtual void EndLoad() = 0;
   // notify that one or two content nodes changed state
@@ -977,6 +975,22 @@ protected:
   friend class mozAutoSubtreeModified;
   friend class nsPresShellIterator;
 
+  /**
+   * Get/Set the current number of removable updates. Currently only
+   * UPDATE_CONTENT_MODEL updates are removable, and only when firing mutation
+   * events. These functions should only be called by mozAutoDocUpdateRemover.
+   * The count is also adjusted by the normal calls to BeginUpdate/EndUpdate.
+   */
+  PRUint32 GetRemovableUpdateLevel()
+  {
+    return mRemovableUpdateLevel;
+  }
+  void SetRemovableUpdateLevel(PRUint32 aLevel)
+  {
+    mRemovableUpdateLevel = aLevel;
+  }
+  friend class mozAutoDocUpdateRemover;
+
   nsString mDocumentTitle;
   nsCOMPtr<nsIURI> mDocumentURI;
   nsCOMPtr<nsIURI> mDocumentBaseURI;
@@ -1047,6 +1061,9 @@ protected:
   // won't be collected
   PRUint32 mMarkedCCGeneration;
 
+  // Current number of removable updates.
+  PRUint32 mRemovableUpdateLevel;
+
   nsTObserverArray<nsIPresShell*> mPresShells;
 
   nsCOMArray<nsINode> mSubtreeModifiedTargets;

content/base/src/nsContentUtils.cpp

@@ -4005,6 +4005,7 @@ nsContentUtils::AddScriptBlocker()
 void
 nsContentUtils::RemoveScriptBlocker()
 {
+  NS_ASSERTION(sScriptBlockerCount != 0, "Negative script blockers");
   --sScriptBlockerCount;
   if (sScriptBlockerCount) {
     return;

content/base/src/nsDocument.cpp

@@ -2706,7 +2706,7 @@ nsDocument::BeginUpdate(nsUpdateType aUpdateType)
   
   ++mUpdateNestLevel;
   if (aUpdateType == UPDATE_CONTENT_MODEL) {
-    ++mContentUpdateNestLevel;
+    ++mRemovableUpdateLevel;
   }
   NS_DOCUMENT_NOTIFY_OBSERVERS(BeginUpdate, (this, aUpdateType));
 
@@ -2720,7 +2720,8 @@ nsDocument::EndUpdate(nsUpdateType aUpdateType)
   NS_DOCUMENT_NOTIFY_OBSERVERS(EndUpdate, (this, aUpdateType));
 
   if (aUpdateType == UPDATE_CONTENT_MODEL) {
-    --mContentUpdateNestLevel;
+    NS_ASSERTION(mRemovableUpdateLevel != 0, "level going below 0");
+    --mRemovableUpdateLevel;
   }
   --mUpdateNestLevel;
   if (mUpdateNestLevel == 0) {
@@ -2750,18 +2751,6 @@ nsDocument::EndUpdate(nsUpdateType aUpdateType)
   }
 }
 
-PRUint32
-nsDocument::GetUpdateNestingLevel()
-{
-  return mUpdateNestLevel;
-}
-
-PRBool
-nsDocument::AllUpdatesAreContent()
-{
-  return mContentUpdateNestLevel == mUpdateNestLevel;
-}
-
 void
 nsDocument::BeginLoad()
 {
@@ -5863,10 +5852,12 @@ nsDocument::MutationEventDispatched(nsINode* aTarget)
 
     PRInt32 realTargetCount = realTargets.Count();
     for (PRInt32 k = 0; k < realTargetCount; ++k) {
-      mozAutoDocUpdateContentUnnest updateUnnest(this);
+      mozAutoDocUpdateRemover updateRemover(this);
 
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_SUBTREEMODIFIED);
+      if (nsContentUtils::IsSafeToRunScript()) {
-      nsEventDispatcher::Dispatch(realTargets[k], nsnull, &mutation);
+        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_SUBTREEMODIFIED);
+        nsEventDispatcher::Dispatch(realTargets[k], nsnull, &mutation);
+      }
     }
   }
 }

content/base/src/nsDocument.h

@@ -474,8 +474,6 @@ public:
   // observers.
   virtual void BeginUpdate(nsUpdateType aUpdateType);
   virtual void EndUpdate(nsUpdateType aUpdateType);
-  virtual PRUint32 GetUpdateNestingLevel();
-  virtual PRBool AllUpdatesAreContent();
   virtual void BeginLoad();
   virtual void EndLoad();
   virtual void ContentStatesChanged(nsIContent* aContent1,
@@ -802,8 +800,6 @@ protected:
 
   // Our update nesting level
   PRUint32 mUpdateNestLevel;
-  // Our UPDATE_CONTENT_MODEL update nesting level
-  PRUint32 mContentUpdateNestLevel;
 
 private:
   friend class nsUnblockOnloadEvent;

content/base/src/nsGenericDOMDataNode.cpp

@@ -497,19 +497,21 @@ nsGenericDOMDataNode::SetTextInternal(PRUint32 aOffset, PRUint32 aCount,
     nsNodeUtils::CharacterDataChanged(this, &info);
 
     if (haveMutationListeners) {
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_CHARACTERDATAMODIFIED);
+      mozAutoDocUpdateRemover updateRemover(document);
 
-      mutation.mPrevAttrValue = oldValue;
+      if (nsContentUtils::IsSafeToRunScript()) {
-      if (aLength > 0) {
+        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_CHARACTERDATAMODIFIED);
-        nsAutoString val;
+
-        mText.AppendTo(val);
+        mutation.mPrevAttrValue = oldValue;
-        mutation.mNewAttrValue = do_GetAtom(val);
+        if (aLength > 0) {
+          nsAutoString val;
+          mText.AppendTo(val);
+          mutation.mNewAttrValue = do_GetAtom(val);
+        }
+
+        mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
+        nsEventDispatcher::Dispatch(this, nsnull, &mutation);
       }
-
-      mozAutoDocUpdateContentUnnest updateUnnest(document);
-
-      mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
-      nsEventDispatcher::Dispatch(this, nsnull, &mutation);
     }
   }
 

content/base/src/nsGenericElement.cpp

@@ -2756,13 +2756,15 @@ nsGenericElement::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex,
 
     if (nsContentUtils::HasMutationListeners(aKid,
           NS_EVENT_BITS_MUTATION_NODEINSERTED, container)) {
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEINSERTED);
+      mozAutoDocUpdateRemover updateRemover(aDocument);
-      mutation.mRelatedNode = do_QueryInterface(container);
+      
+      if (nsContentUtils::IsSafeToRunScript()) {
+        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEINSERTED);
+        mutation.mRelatedNode = do_QueryInterface(container);
 
-      mozAutoDocUpdateContentUnnest updateUnnest(aDocument);
+        mozAutoSubtreeModified subtree(container->GetOwnerDoc(), container);
-
+        nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
-      mozAutoSubtreeModified subtree(container->GetOwnerDoc(), container);
+      }
-      nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
     }
   }
 
@@ -2826,13 +2828,15 @@ nsGenericElement::doRemoveChildAt(PRUint32 aIndex, PRBool aNotify,
   if (aNotify &&
       nsContentUtils::HasMutationListeners(aKid,
         NS_EVENT_BITS_MUTATION_NODEREMOVED, container)) {
-    nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEREMOVED);
+    mozAutoDocUpdateRemover updateRemover(aDocument);
-    mutation.mRelatedNode = do_QueryInterface(container);
 
-    mozAutoDocUpdateContentUnnest updateUnnest(aDocument);
+    if (nsContentUtils::IsSafeToRunScript()) {
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEREMOVED);
+      mutation.mRelatedNode = do_QueryInterface(container);
 
-    subtree.UpdateTarget(container->GetOwnerDoc(), container);
+      subtree.UpdateTarget(container->GetOwnerDoc(), container);
-    nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
+      nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
+    }
   }
 
   // Someone may have removed the kid or any of its siblings while that event
@@ -3789,31 +3793,33 @@ nsGenericElement::SetAttrAndNotify(PRInt32 aNamespaceID,
   }
   
   if (aFireMutation) {
-    nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+    mozAutoDocUpdateRemover updateRemover(document);
+    
+    if (nsContentUtils::IsSafeToRunScript()) {
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-    nsAutoString attrName;
+      nsAutoString attrName;
-    aName->ToString(attrName);
+      aName->ToString(attrName);
-    nsCOMPtr<nsIDOMAttr> attrNode;
+      nsCOMPtr<nsIDOMAttr> attrNode;
-    nsAutoString ns;
+      nsAutoString ns;
-    nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
+      nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
-    GetAttributeNodeNS(ns, attrName, getter_AddRefs(attrNode));
+      GetAttributeNodeNS(ns, attrName, getter_AddRefs(attrNode));
-    mutation.mRelatedNode = attrNode;
+      mutation.mRelatedNode = attrNode;
 
-    mutation.mAttrName = aName;
+      mutation.mAttrName = aName;
-    nsAutoString newValue;
+      nsAutoString newValue;
-    GetAttr(aNamespaceID, aName, newValue);
+      GetAttr(aNamespaceID, aName, newValue);
-    if (!newValue.IsEmpty()) {
+      if (!newValue.IsEmpty()) {
-      mutation.mNewAttrValue = do_GetAtom(newValue);
+        mutation.mNewAttrValue = do_GetAtom(newValue);
+      }
+      if (!aOldValue.IsEmpty()) {
+        mutation.mPrevAttrValue = do_GetAtom(aOldValue);
+      }
+      mutation.mAttrChange = modType;
+
+      mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
+      nsEventDispatcher::Dispatch(this, nsnull, &mutation);
     }
-    if (!aOldValue.IsEmpty()) {
-      mutation.mPrevAttrValue = do_GetAtom(aOldValue);
-    }
-    mutation.mAttrChange = modType;
-
-    mozAutoDocUpdateContentUnnest updateUnnest(document);
-
-    mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
-    nsEventDispatcher::Dispatch(this, nsnull, &mutation);
   }
 
   if (aNamespaceID == kNameSpaceID_XMLEvents && 
@@ -4046,23 +4052,25 @@ nsGenericElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
   }
 
   if (hasMutationListeners) {
-    nsCOMPtr<nsIDOMEventTarget> node =
+    mozAutoDocUpdateRemover updateRemover(document);
-      do_QueryInterface(static_cast<nsIContent *>(this));
-    nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-    mutation.mRelatedNode = attrNode;
+    if (nsContentUtils::IsSafeToRunScript()) {
-    mutation.mAttrName = aName;
+      nsCOMPtr<nsIDOMEventTarget> node =
+        do_QueryInterface(static_cast<nsIContent *>(this));
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-    nsAutoString value;
+      mutation.mRelatedNode = attrNode;
-    oldValue.ToString(value);
+      mutation.mAttrName = aName;
-    if (!value.IsEmpty())
-      mutation.mPrevAttrValue = do_GetAtom(value);
-    mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
 
-    mozAutoDocUpdateContentUnnest updateUnnest(document);
+      nsAutoString value;
+      oldValue.ToString(value);
+      if (!value.IsEmpty())
+        mutation.mPrevAttrValue = do_GetAtom(value);
+      mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
 
-    mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
+      mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
-    nsEventDispatcher::Dispatch(this, nsnull, &mutation);
+      nsEventDispatcher::Dispatch(this, nsnull, &mutation);
+    }
   }
 
   return AfterSetAttr(aNameSpaceID, aName, nsnull, aNotify);

content/base/src/nsGenericElement.h

@@ -1067,15 +1067,15 @@ private:
   nsRefPtr<nsGenericElement> mContent;
 };
 
-class mozAutoDocUpdateContentUnnest
+class mozAutoDocUpdateRemover
 {
 public:
-  mozAutoDocUpdateContentUnnest(nsIDocument* aDocument)
+  mozAutoDocUpdateRemover(nsIDocument* aDocument)
+    : mDocument(aDocument)
   {
     if (aDocument) {
-      NS_ASSERTION(aDocument->AllUpdatesAreContent(),
+      mNestingLevel = aDocument->GetRemovableUpdateLevel();
-                   "There are non-content updates in progress");
+      aDocument->SetRemovableUpdateLevel(0);
-      mNestingLevel = aDocument->GetUpdateNestingLevel();
       for (PRUint32 i = 0; i < mNestingLevel; ++i) {
         nsContentUtils::RemoveScriptBlocker();
       }
@@ -1085,15 +1085,21 @@ public:
     }
   }
 
-  ~mozAutoDocUpdateContentUnnest()
+  ~mozAutoDocUpdateRemover()
   {
-    for (PRUint32 i = 0; i < mNestingLevel; ++i) {
+    NS_ASSERTION(mNestingLevel == 0 || mDocument,
-      nsContentUtils::AddScriptBlocker();
+                 "Count should be zero if there's no document");
+    if (mDocument) {
+      for (PRUint32 i = 0; i < mNestingLevel; ++i) {
+        nsContentUtils::AddScriptBlocker();
+      }
+      mDocument->SetRemovableUpdateLevel(mNestingLevel);
     }
   }
 
 private:
   PRUint32 mNestingLevel;
+  nsCOMPtr<nsIDocument> mDocument;
 };
 
 #endif /* nsGenericElement_h___ */

content/xul/content/src/nsXULElement.cpp

@@ -1435,20 +1435,22 @@ nsXULElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, PRBool aNotify)
     }
 
     if (hasMutationListeners) {
-        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+        mozAutoDocUpdateRemover updateRemover(doc);
 
-        mutation.mRelatedNode = attrNode;
+        if (nsContentUtils::IsSafeToRunScript()) {
-        mutation.mAttrName = aName;
+            nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-        if (!oldValue.IsEmpty())
+            mutation.mRelatedNode = attrNode;
-          mutation.mPrevAttrValue = do_GetAtom(oldValue);
+            mutation.mAttrName = aName;
-        mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
 
-        mozAutoDocUpdateContentUnnest updateUnnest(doc);
+            if (!oldValue.IsEmpty())
+              mutation.mPrevAttrValue = do_GetAtom(oldValue);
+            mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
 
-        mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
+            mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
-        nsEventDispatcher::Dispatch(static_cast<nsIContent*>(this),
+            nsEventDispatcher::Dispatch(static_cast<nsIContent*>(this),
-                                    nsnull, &mutation);
+                                        nsnull, &mutation);
+        }
     }
 
     return NS_OK;