Bug 423269: Don't add scriptblockers around StartUpdate/EndUpdate batches as script is likely to execute at the last EndUpdate. Also for now, restore full functionality to scripts running while there are scriptblockers. r/sr=bz a=beltzner

content/base/src/mozAutoDocUpdate.h

@@ -84,3 +84,35 @@ private:
 #define MOZ_AUTO_DOC_UPDATE(doc,type,notify) \
   mozAutoDocUpdate MOZ_AUTO_DOC_UPDATE_PASTE(_autoDocUpdater_, __LINE__) \
   (doc,type,notify)
+
+
+/**
+ * Creates an update batch only under certain conditions.
+ * Use this rather than mozAutoDocUpdate when you expect inner updates
+ * to notify but you don't always want to spec cycles creating a batch.
+ * This is needed to avoid having this batch always create a blocker,
+ * but then have inner mozAutoDocUpdate call the last EndUpdate before.
+ * we remove that blocker. See bug 423269.
+ */
+class mozAutoDocConditionalContentUpdateBatch
+{
+public:
+  mozAutoDocConditionalContentUpdateBatch(nsIDocument* aDocument,
+                                          PRBool aNotify) :
+    mDocument(aNotify ? aDocument : nsnull)
+  {
+    if (mDocument) {
+      mDocument->BeginUpdate(UPDATE_CONTENT_MODEL);
+    }
+  }
+
+  ~mozAutoDocConditionalContentUpdateBatch()
+  {
+    if (mDocument) {
+      mDocument->EndUpdate(UPDATE_CONTENT_MODEL);
+    }
+  }
+
+private:
+  nsCOMPtr<nsIDocument> mDocument;
+};

content/base/src/nsDocument.cpp

@@ -5888,10 +5888,8 @@ nsDocument::MutationEventDispatched(nsINode* aTarget)
     for (PRInt32 k = 0; k < realTargetCount; ++k) {
       mozAutoRemovableBlockerRemover blockerRemover;
 
-      if (nsContentUtils::IsSafeToRunScript()) {
-        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_SUBTREEMODIFIED);
-        nsEventDispatcher::Dispatch(realTargets[k], nsnull, &mutation);
-      }
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_SUBTREEMODIFIED);
+      nsEventDispatcher::Dispatch(realTargets[k], nsnull, &mutation);
     }
   }
 }

content/base/src/nsGenericDOMDataNode.cpp

@@ -500,19 +500,17 @@ nsGenericDOMDataNode::SetTextInternal(PRUint32 aOffset, PRUint32 aCount,
     if (haveMutationListeners) {
       mozAutoRemovableBlockerRemover blockerRemover;
 
-      if (nsContentUtils::IsSafeToRunScript()) {
-        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_CHARACTERDATAMODIFIED);
+      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);
+      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);
     }
   }
 

content/base/src/nsGenericElement.cpp

@@ -2754,13 +2754,11 @@ nsGenericElement::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex,
           NS_EVENT_BITS_MUTATION_NODEINSERTED, container)) {
       mozAutoRemovableBlockerRemover blockerRemover;
       
-      if (nsContentUtils::IsSafeToRunScript()) {
-        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEINSERTED);
-        mutation.mRelatedNode = do_QueryInterface(container);
+      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEINSERTED);
+      mutation.mRelatedNode = do_QueryInterface(container);
 
-        mozAutoSubtreeModified subtree(container->GetOwnerDoc(), container);
-        nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
-      }
+      mozAutoSubtreeModified subtree(container->GetOwnerDoc(), container);
+      nsEventDispatcher::Dispatch(aKid, nsnull, &mutation);
     }
   }
 
@@ -2826,13 +2824,11 @@ nsGenericElement::doRemoveChildAt(PRUint32 aIndex, PRBool aNotify,
         NS_EVENT_BITS_MUTATION_NODEREMOVED, container)) {
     mozAutoRemovableBlockerRemover blockerRemover;
 
-    if (nsContentUtils::IsSafeToRunScript()) {
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEREMOVED);
-      mutation.mRelatedNode = do_QueryInterface(container);
+    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
@@ -3249,7 +3245,7 @@ nsGenericElement::doReplaceOrInsertBefore(PRBool aReplace,
 
   // We want an update batch when we expect several mutations to be performed,
   // which is when we're replacing a node, or when we're inserting a fragment.
-  mozAutoDocUpdate updateBatch(aDocument, UPDATE_CONTENT_MODEL,
+  mozAutoDocConditionalContentUpdateBatch(aDocument,
     aReplace || nodeType == nsIDOMNode::DOCUMENT_FRAGMENT_NODE);
 
   // If we're replacing
@@ -3804,31 +3800,29 @@ nsGenericElement::SetAttrAndNotify(PRInt32 aNamespaceID,
   if (aFireMutation) {
     mozAutoRemovableBlockerRemover blockerRemover;
     
-    if (nsContentUtils::IsSafeToRunScript()) {
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+    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);
-      }
-      if (!aOldValue.IsEmpty()) {
-        mutation.mPrevAttrValue = do_GetAtom(aOldValue);
-      }
-      mutation.mAttrChange = modType;
-
-      mozAutoSubtreeModified subtree(GetOwnerDoc(), this);
-      nsEventDispatcher::Dispatch(this, nsnull, &mutation);
+    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 (aNamespaceID == kNameSpaceID_XMLEvents && 
@@ -4063,23 +4057,21 @@ nsGenericElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
   if (hasMutationListeners) {
     mozAutoRemovableBlockerRemover blockerRemover;
 
-    if (nsContentUtils::IsSafeToRunScript()) {
-      nsCOMPtr<nsIDOMEventTarget> node =
-        do_QueryInterface(static_cast<nsIContent *>(this));
-      nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+    nsCOMPtr<nsIDOMEventTarget> node =
+      do_QueryInterface(static_cast<nsIContent *>(this));
+    nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-      mutation.mRelatedNode = attrNode;
-      mutation.mAttrName = aName;
+    mutation.mRelatedNode = attrNode;
+    mutation.mAttrName = aName;
 
-      nsAutoString value;
-      oldValue.ToString(value);
-      if (!value.IsEmpty())
-        mutation.mPrevAttrValue = do_GetAtom(value);
-      mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
+    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);

content/xul/content/src/nsXULElement.cpp

@@ -1438,20 +1438,18 @@ nsXULElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, PRBool aNotify)
     if (hasMutationListeners) {
         mozAutoRemovableBlockerRemover blockerRemover;
 
-        if (nsContentUtils::IsSafeToRunScript()) {
-            nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
+        nsMutationEvent mutation(PR_TRUE, NS_MUTATION_ATTRMODIFIED);
 
-            mutation.mRelatedNode = attrNode;
-            mutation.mAttrName = aName;
+        mutation.mRelatedNode = attrNode;
+        mutation.mAttrName = aName;
 
-            if (!oldValue.IsEmpty())
-              mutation.mPrevAttrValue = do_GetAtom(oldValue);
-            mutation.mAttrChange = nsIDOMMutationEvent::REMOVAL;
+        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;

layout/base/nsPresShell.cpp

@@ -4513,8 +4513,7 @@ PresShell::IsSafeToFlush(PRBool& aIsSafeToFlush)
   // if any of the other flags are set.
   
   // Not safe if we are reflowing or in the middle of frame construction
-  aIsSafeToFlush = nsContentUtils::IsSafeToRunScript() &&
-                   !mIsReflowing &&
+  aIsSafeToFlush = !mIsReflowing &&
                    !mChangeNestCount;
 
   if (aIsSafeToFlush) {
@@ -5579,10 +5578,9 @@ PresShell::HandleEvent(nsIView         *aView,
     return HandleEventInternal(aEvent, aView, aEventStatus);
   }
 #endif
-  if (!nsContentUtils::IsSafeToRunScript()) {
-    NS_ERROR("How did we get here if it's not safe to run scripts?");
-    return NS_OK;
-  }
+
+  NS_ASSERTION(nsContentUtils::IsSafeToRunScript(),
+               "How did we get here if it's not safe to run scripts?");
 
   // Check for a theme change up front, since the frame type is irrelevant
   if (aEvent->message == NS_THEMECHANGED && mPresContext) {