Bug 423355: Make sure that we never remove removable blockers more than once. Also make sure to never fire mutation events when it's not safe to run script. r/sr=bz/roc a=beltzner

git-svn-id: svn://10.0.0.236/trunk@250233 18797224-902f-48f8-a5cc-f745e15eee43

mozilla/content/base/public/nsContentUtils.h

@@ -1223,6 +1223,29 @@ public:
     return sScriptBlockerCount == 0;
   }
 
+  /**
+   * 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.
+   */
+  static void AddRemovableScriptBlocker()
+  {
+    AddScriptBlocker();
+    ++sRemovableScriptBlockerCount;
+  }
+  static void RemoveRemovableScriptBlocker()
+  {
+    NS_ASSERTION(sRemovableScriptBlockerCount != 0,
+                "Number of removable blockers should never go below zero");
+    --sRemovableScriptBlockerCount;
+    RemoveScriptBlocker();
+  }
+  static PRUint32 GetRemovableScriptBlockerLevel()
+  {
+    return sRemovableScriptBlockerCount;
+  }
+
 private:
 
   static PRBool InitializeEventTable();
@@ -1296,6 +1319,7 @@ private:
 
   static PRBool sInitialized;
   static PRUint32 sScriptBlockerCount;
+  static PRUint32 sRemovableScriptBlockerCount;
   static nsCOMArray<nsIRunnable>* sBlockedScriptRunners;
   static PRUint32 sRunnersCountAtFirstBlocker;
 };
@@ -1377,6 +1401,32 @@ public:
   }
 };
 
+class mozAutoRemovableBlockerRemover
+{
+public:
+  mozAutoRemovableBlockerRemover()
+  {
+    mNestingLevel = nsContentUtils::GetRemovableScriptBlockerLevel();
+    for (PRUint32 i = 0; i < mNestingLevel; ++i) {
+      nsContentUtils::RemoveRemovableScriptBlocker();
+    }
+
+    NS_ASSERTION(nsContentUtils::IsSafeToRunScript(), "killing mutation events");
+  }
+
+  ~mozAutoRemovableBlockerRemover()
+  {
+    NS_ASSERTION(nsContentUtils::GetRemovableScriptBlockerLevel() == 0,
+                 "Should have had none");
+    for (PRUint32 i = 0; i < mNestingLevel; ++i) {
+      nsContentUtils::AddRemovableScriptBlocker();
+    }
+  }
+
+private:
+  PRUint32 mNestingLevel;
+};
+
 #define NS_AUTO_GCROOT_PASTE2(tok,line) tok##line
 #define NS_AUTO_GCROOT_PASTE(tok,line) \
   NS_AUTO_GCROOT_PASTE2(tok,line)

mozilla/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

mozilla/content/base/src/mozAutoDocUpdate.h

@@ -52,6 +52,12 @@ public:
     if (mDocument) {
       mDocument->BeginUpdate(mUpdateType);
     }
+    else if (aUpdateType == UPDATE_CONTENT_MODEL) {
+      nsContentUtils::AddRemovableScriptBlocker();
+    }
+    else {
+      nsContentUtils::AddScriptBlocker();
+    }
   }
 
   ~mozAutoDocUpdate()
@@ -59,6 +65,12 @@ public:
     if (mDocument) {
       mDocument->EndUpdate(mUpdateType);
     }
+    else if (mUpdateType == UPDATE_CONTENT_MODEL) {
+      nsContentUtils::RemoveRemovableScriptBlocker();
+    }
+    else {
+      nsContentUtils::RemoveScriptBlocker();
+    }
   }
 
 private:

mozilla/content/base/src/nsContentUtils.cpp

@@ -200,6 +200,7 @@ PRUint32 nsContentUtils::sJSGCThingRootCount;
 nsIBidiKeyboard *nsContentUtils::sBidiKeyboard = nsnull;
 #endif
 PRUint32 nsContentUtils::sScriptBlockerCount = 0;
+PRUint32 nsContentUtils::sRemovableScriptBlockerCount = 0;
 nsCOMArray<nsIRunnable>* nsContentUtils::sBlockedScriptRunners = nsnull;
 PRUint32 nsContentUtils::sRunnersCountAtFirstBlocker = 0;
 
@@ -4014,6 +4015,7 @@ nsContentUtils::AddScriptBlocker()
 void
 nsContentUtils::RemoveScriptBlocker()
 {
+  NS_ASSERTION(sScriptBlockerCount != 0, "Negative script blockers");
   --sScriptBlockerCount;
   if (sScriptBlockerCount) {
     return;

mozilla/content/base/src/nsDocument.cpp

@@ -2711,23 +2711,27 @@ nsDocument::BeginUpdate(nsUpdateType aUpdateType)
   }
   
   ++mUpdateNestLevel;
-  if (aUpdateType == UPDATE_CONTENT_MODEL) {
-    ++mContentUpdateNestLevel;
-  }
   NS_DOCUMENT_NOTIFY_OBSERVERS(BeginUpdate, (this, aUpdateType));
 
-  nsContentUtils::AddScriptBlocker();
+  if (aUpdateType == UPDATE_CONTENT_MODEL) {
+    nsContentUtils::AddRemovableScriptBlocker();
+  }
+  else {
+    nsContentUtils::AddScriptBlocker();
+  }
 }
 
 void
 nsDocument::EndUpdate(nsUpdateType aUpdateType)
 {
-  nsContentUtils::RemoveScriptBlocker();
+  if (aUpdateType == UPDATE_CONTENT_MODEL) {
+    nsContentUtils::RemoveRemovableScriptBlocker();
+  }
+  else {
+    nsContentUtils::RemoveScriptBlocker();
+  }
   NS_DOCUMENT_NOTIFY_OBSERVERS(EndUpdate, (this, aUpdateType));
 
-  if (aUpdateType == UPDATE_CONTENT_MODEL) {
-    --mContentUpdateNestLevel;
-  }
   --mUpdateNestLevel;
   if (mUpdateNestLevel == 0) {
     // This set of updates may have created XBL bindings.  Let the
@@ -2756,18 +2760,6 @@ nsDocument::EndUpdate(nsUpdateType aUpdateType)
   }
 }
 
-PRUint32
-nsDocument::GetUpdateNestingLevel()
-{
-  return mUpdateNestLevel;
-}
-
-PRBool
-nsDocument::AllUpdatesAreContent()
-{
-  return mContentUpdateNestLevel == mUpdateNestLevel;
-}
-
 void
 nsDocument::BeginLoad()
 {
@@ -5882,10 +5874,12 @@ nsDocument::MutationEventDispatched(nsINode* aTarget)
 
     PRInt32 realTargetCount = realTargets.Count();
     for (PRInt32 k = 0; k < realTargetCount; ++k) {
-      mozAutoDocUpdateContentUnnest updateUnnest(this);
+      mozAutoRemovableBlockerRemover blockerRemover;
 
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_SUBTREEMODIFIED);
-      nsEventDispatcher::Dispatch(realTargets[k], nsnull, &mutation);
+      if (nsContentUtils::IsSafeToRunScript()) {
+        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_SUBTREEMODIFIED);
+        nsEventDispatcher::Dispatch(realTargets[k], nsnull, &mutation);
+      }
     }
   }
 }

mozilla/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,
@@ -805,8 +803,6 @@ protected:
 
   // Our update nesting level
   PRUint32 mUpdateNestLevel;
-  // Our UPDATE_CONTENT_MODEL update nesting level
-  PRUint32 mContentUpdateNestLevel;
 
 private:
   friend class nsUnblockOnloadEvent;

mozilla/content/base/src/nsGenericDOMDataNode.cpp

@@ -498,19 +498,21 @@ nsGenericDOMDataNode::SetTextInternal(PRUint32 aOffset, PRUint32 aCount,
     nsNodeUtils::CharacterDataChanged(this, &info);
 
     if (haveMutationListeners) {
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_CHARACTERDATAMODIFIED);
+      mozAutoRemovableBlockerRemover blockerRemover;
 
-      mutation.mPrevAttrValue = oldValue;
-      if (aLength > 0) {
-        nsAutoString val;
-        mText.AppendTo(val);
-        mutation.mNewAttrValue = do_GetAtom(val);
+      if (nsContentUtils::IsSafeToRunScript()) {
+        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_CHARACTERDATAMODIFIED);
+
+        mutation.mPrevAttrValue = oldValue;
+        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);
     }
   }
 

mozilla/content/base/src/nsGenericElement.cpp

@@ -2752,13 +2752,15 @@ nsGenericElement::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex,
 
     if (nsContentUtils::HasMutationListeners(aKid,
           NS_EVENT_BITS_MUTATION_NODEINSERTED, container)) {
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEINSERTED);
-      mutation.mRelatedNode = do_QueryInterface(container);
+      mozAutoRemovableBlockerRemover blockerRemover;
+      
+      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);
+      }
     }
   }
 
@@ -2822,13 +2824,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);
-    mutation.mRelatedNode = do_QueryInterface(container);
+    mozAutoRemovableBlockerRemover blockerRemover;
 
-    mozAutoDocUpdateContentUnnest updateUnnest(aDocument);
+    if (nsContentUtils::IsSafeToRunScript()) {
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEREMOVED);
+      mutation.mRelatedNode = do_QueryInterface(container);
 
-    subtree.UpdateTarget(container->GetOwnerDoc(), container);
-    nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
+      subtree.UpdateTarget(container->GetOwnerDoc(), container);
+      nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
+    }
   }
 
   // Someone may have removed the kid or any of its siblings while that event
@@ -3798,31 +3802,33 @@ nsGenericElement::SetAttrAndNotify(PRInt32 aNamespaceID,
   }
   
   if (aFireMutation) {
-    nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+    mozAutoRemovableBlockerRemover blockerRemover;
+    
+    if (nsContentUtils::IsSafeToRunScript()) {
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-    nsAutoString attrName;
-    aName->ToString(attrName);
-    nsCOMPtr<nsIDOMAttr> attrNode;
-    nsAutoString ns;
-    nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
-    GetAttributeNodeNS(ns, attrName, getter_AddRefs(attrNode));
-    mutation.mRelatedNode = attrNode;
+      nsAutoString attrName;
+      aName->ToString(attrName);
+      nsCOMPtr<nsIDOMAttr> attrNode;
+      nsAutoString ns;
+      nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
+      GetAttributeNodeNS(ns, attrName, getter_AddRefs(attrNode));
+      mutation.mRelatedNode = attrNode;
 
-    mutation.mAttrName = aName;
-    nsAutoString newValue;
-    GetAttr(aNamespaceID, aName, newValue);
-    if (!newValue.IsEmpty()) {
-      mutation.mNewAttrValue = do_GetAtom(newValue);
+      mutation.mAttrName = aName;
+      nsAutoString newValue;
+      GetAttr(aNamespaceID, aName, newValue);
+      if (!newValue.IsEmpty()) {
+        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 && 
@@ -4055,23 +4061,25 @@ nsGenericElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
   }
 
   if (hasMutationListeners) {
-    nsCOMPtr<nsIDOMEventTarget> node =
-      do_QueryInterface(static_cast<nsIContent *>(this));
-    nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+    mozAutoRemovableBlockerRemover blockerRemover;
 
-    mutation.mRelatedNode = attrNode;
-    mutation.mAttrName = aName;
+    if (nsContentUtils::IsSafeToRunScript()) {
+      nsCOMPtr<nsIDOMEventTarget> node =
+        do_QueryInterface(static_cast<nsIContent *>(this));
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-    nsAutoString value;
-    oldValue.ToString(value);
-    if (!value.IsEmpty())
-      mutation.mPrevAttrValue = do_GetAtom(value);
-    mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
+      mutation.mRelatedNode = attrNode;
+      mutation.mAttrName = aName;
 
-    mozAutoDocUpdateContentUnnest updateUnnest(document);
+      nsAutoString value;
+      oldValue.ToString(value);
+      if (!value.IsEmpty())
+        mutation.mPrevAttrValue = do_GetAtom(value);
+      mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
 
-    mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
-    nsEventDispatcher::Dispatch(this, nsnull, &mutation);
+      mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
+      nsEventDispatcher::Dispatch(this, nsnull, &mutation);
+    }
   }
 
   return AfterSetAttr(aNameSpaceID, aName, nsnull, aNotify);

mozilla/content/base/src/nsGenericElement.h

@@ -1069,33 +1069,4 @@ private:
   nsRefPtr<nsGenericElement> mContent;
 };
 
-class mozAutoDocUpdateContentUnnest
-{
-public:
-  mozAutoDocUpdateContentUnnest(nsIDocument* aDocument)
-  {
-    if (aDocument) {
-      NS_ASSERTION(aDocument->AllUpdatesAreContent(),
-                   "There are non-content updates in progress");
-      mNestingLevel = aDocument->GetUpdateNestingLevel();
-      for (PRUint32 i = 0; i < mNestingLevel; ++i) {
-        nsContentUtils::RemoveScriptBlocker();
-      }
-    }
-    else {
-      mNestingLevel = 0;
-    }
-  }
-
-  ~mozAutoDocUpdateContentUnnest()
-  {
-    for (PRUint32 i = 0; i < mNestingLevel; ++i) {
-      nsContentUtils::AddScriptBlocker();
-    }
-  }
-
-private:
-  PRUint32 mNestingLevel;
-};
-
 #endif /* nsGenericElement_h___ */

mozilla/content/xul/content/src/nsXULElement.cpp

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

mozilla/layout/base/nsLayoutUtils.cpp

@@ -2662,3 +2662,31 @@ nsLayoutUtils::GetRectDifferenceStrips(const nsRect& aR1, const nsRect& aR2,
   aHStrip->height -= HStripStart;
 }
 
+nsSetAttrRunnable::nsSetAttrRunnable(nsIContent* aContent, nsIAtom* aAttrName,
+                                     const nsAString& aValue)
+  : mContent(aContent),
+    mAttrName(aAttrName),
+    mValue(aValue)
+{
+  NS_ASSERTION(aContent && aAttrName, "Missing stuff, prepare to crash");
+}
+
+NS_IMETHODIMP
+nsSetAttrRunnable::Run()
+{
+  return mContent->SetAttr(kNameSpaceID_None, mAttrName, mValue, PR_TRUE);
+}
+
+nsUnsetAttrRunnable::nsUnsetAttrRunnable(nsIContent* aContent,
+                                         nsIAtom* aAttrName)
+  : mContent(aContent),
+    mAttrName(aAttrName)
+{
+  NS_ASSERTION(aContent && aAttrName, "Missing stuff, prepare to crash");
+}
+
+NS_IMETHODIMP
+nsUnsetAttrRunnable::Run()
+{
+  return mContent->UnsetAttr(kNameSpaceID_None, mAttrName, PR_TRUE);
+}

mozilla/layout/base/nsLayoutUtils.h

@@ -58,6 +58,7 @@ class nsIFontMetrics;
 #include "nsStyleSet.h"
 #include "nsIView.h"
 #include "nsIFrame.h"
+#include "nsThreadUtils.h"
 
 class nsBlockFrame;
 
@@ -815,4 +816,28 @@ private:
 #endif  
 };
 
+class nsSetAttrRunnable : public nsRunnable
+{
+public:
+  nsSetAttrRunnable(nsIContent* aContent, nsIAtom* aAttrName,
+                    const nsAString& aValue);
+
+  NS_DECL_NSIRUNNABLE
+
+  nsCOMPtr<nsIContent> mContent;
+  nsCOMPtr<nsIAtom> mAttrName;
+  nsAutoString mValue;
+};
+
+class nsUnsetAttrRunnable : public nsRunnable
+{
+public:
+  nsUnsetAttrRunnable(nsIContent* aContent, nsIAtom* aAttrName);
+
+  NS_DECL_NSIRUNNABLE
+
+  nsCOMPtr<nsIContent> mContent;
+  nsCOMPtr<nsIAtom> mAttrName;
+};
+
 #endif // nsLayoutUtils_h__

mozilla/layout/xul/base/src/nsSliderFrame.cpp

@@ -228,7 +228,6 @@ nsSliderFrame::GetIntegerAttribute(nsIContent* content, nsIAtom* atom, PRInt32 d
     return defaultValue;
 }
 
-
 NS_IMETHODIMP
 nsSliderFrame::AttributeChanged(PRInt32 aNameSpaceID,
                                 nsIAtom* aAttribute,
@@ -271,7 +270,8 @@ nsSliderFrame::AttributeChanged(PRInt32 aNameSpaceID,
 
         nsAutoString currentStr;
         currentStr.AppendInt(current);
-        scrollbar->SetAttr(kNameSpaceID_None, nsGkAtoms::curpos, currentStr, PR_TRUE);
+        nsContentUtils::AddScriptRunner(
+          new nsSetAttrRunnable(scrollbar, nsGkAtoms::curpos, currentStr));
       }
   }
 

mozilla/layout/xul/base/src/nsSplitterFrame.cpp

@@ -989,15 +989,16 @@ nsSplitterFrameInner::UpdateState()
           // CollapsedBefore -> Dragging
           // CollapsedAfter -> Open
           // CollapsedAfter -> Dragging
-          sibling->UnsetAttr(kNameSpaceID_None, nsGkAtoms::collapsed,
-                             PR_TRUE);
+          nsContentUtils::AddScriptRunner(
+            new nsUnsetAttrRunnable(sibling, nsGkAtoms::collapsed));
         } else if ((mState == Open || mState == Dragging)
                    && (newState == CollapsedBefore ||
                        newState == CollapsedAfter)) {
           // Open -> CollapsedBefore / CollapsedAfter
           // Dragging -> CollapsedBefore / CollapsedAfter
-          sibling->SetAttr(kNameSpaceID_None, nsGkAtoms::collapsed,
-                           NS_LITERAL_STRING("true"), PR_TRUE);
+          nsContentUtils::AddScriptRunner(
+            new nsSetAttrRunnable(sibling, nsGkAtoms::collapsed,
+                                  NS_LITERAL_STRING("true")));
         }
       }
     }