Bug 650493 Part 2: Simplify userdata by firing adoption handlers off of scriptrunners. r=smaug/peterv

content/base/src/nsDocument.cpp

@@ -5984,6 +5984,30 @@ BlastSubtreeToPieces(nsINode *aNode)
   }
 }
 
+
+class nsUserDataCaller : public nsRunnable
+{
+public:
+  nsUserDataCaller(nsCOMArray<nsINode>& aNodesWithProperties,
+                   nsIDocument* aOwnerDoc)
+    : mNodesWithProperties(aNodesWithProperties),
+      mOwnerDoc(aOwnerDoc)
+  {
+  }
+
+  NS_IMETHOD Run()
+  {
+    nsNodeUtils::CallUserDataHandlers(mNodesWithProperties, mOwnerDoc,
+                                      nsIDOMUserDataHandler::NODE_ADOPTED,
+                                      PR_FALSE);
+    return NS_OK;
+  }
+
+private:
+  nsCOMArray<nsINode> mNodesWithProperties;
+  nsCOMPtr<nsIDocument> mOwnerDoc;
+};
+
 NS_IMETHODIMP
 nsDocument::AdoptNode(nsIDOMNode *aAdoptedNode, nsIDOMNode **aResult)
 {
@@ -5994,15 +6018,28 @@ nsDocument::AdoptNode(nsIDOMNode *aAdoptedNode, nsIDOMNode **aResult)
   nsresult rv = nsContentUtils::CheckSameOrigin(this, aAdoptedNode);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsCOMPtr<nsINode> adoptedNode;
+  nsCOMPtr<nsINode> adoptedNode = do_QueryInterface(aAdoptedNode);
+  
+  // Scope firing mutation events so that we don't carry any state that
+  // might be stale
+  {
+    nsINode* parent = adoptedNode->GetNodeParent();
+    if (parent) {
+      nsContentUtils::MaybeFireNodeRemoved(adoptedNode, parent,
+                                           adoptedNode->GetOwnerDoc());
+    }
+  }
+
+  nsAutoScriptBlocker scriptBlocker;
+
   PRUint16 nodeType;
   aAdoptedNode->GetNodeType(&nodeType);
   switch (nodeType) {
     case nsIDOMNode::ATTRIBUTE_NODE:
     {
       // Remove from ownerElement.
-      nsCOMPtr<nsIDOMAttr> adoptedAttr = do_QueryInterface(aAdoptedNode, &rv);
-      NS_ENSURE_SUCCESS(rv, rv);
+      nsCOMPtr<nsIDOMAttr> adoptedAttr = do_QueryInterface(aAdoptedNode);
+      NS_ASSERTION(adoptedAttr, "Attribute not implementing nsIDOMAttr");
 
       nsCOMPtr<nsIDOMElement> ownerElement;
       rv = adoptedAttr->GetOwnerElement(getter_AddRefs(ownerElement));
@@ -6017,8 +6054,6 @@ nsDocument::AdoptNode(nsIDOMNode *aAdoptedNode, nsIDOMNode **aResult)
         newAttr.swap(adoptedAttr);
       }
 
-      adoptedNode = do_QueryInterface(adoptedAttr, &rv);
-      NS_ENSURE_SUCCESS(rv, rv);
       break;
     }
     case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
@@ -6028,9 +6063,6 @@ nsDocument::AdoptNode(nsIDOMNode *aAdoptedNode, nsIDOMNode **aResult)
     case nsIDOMNode::CDATA_SECTION_NODE:
     case nsIDOMNode::COMMENT_NODE:
     {
-      adoptedNode = do_QueryInterface(aAdoptedNode, &rv);
-      NS_ENSURE_SUCCESS(rv, rv);
-
       // We don't want to adopt an element into its own contentDocument or into
       // a descendant contentDocument, so we check if the frameElement of this
       // document or any of its parents is the adopted node or one of its
@@ -6049,13 +6081,9 @@ nsDocument::AdoptNode(nsIDOMNode *aAdoptedNode, nsIDOMNode **aResult)
       } while ((doc = doc->GetParentDocument()));
 
       // Remove from parent.
-      nsCOMPtr<nsIDOMNode> parent;
-      aAdoptedNode->GetParentNode(getter_AddRefs(parent));
-      NS_ENSURE_SUCCESS(rv, rv);
-
+      nsINode* parent = adoptedNode->GetNodeParent();
       if (parent) {
-        nsCOMPtr<nsIDOMNode> newChild;
-        rv = parent->RemoveChild(aAdoptedNode, getter_AddRefs(newChild));
+        rv = parent->RemoveChildAt(parent->IndexOf(adoptedNode), PR_TRUE);
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
@@ -6135,15 +6163,14 @@ nsDocument::AdoptNode(nsIDOMNode *aAdoptedNode, nsIDOMNode **aResult)
     }
   }
 
-  rv = nsNodeUtils::CallUserDataHandlers(nodesWithProperties, this,
-                                         nsIDOMUserDataHandler::NODE_ADOPTED,
-                                         PR_FALSE);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (adoptedNode->GetOwnerDoc() != this) {
-    return NS_ERROR_DOM_WRONG_DOCUMENT_ERR;
+  if (nodesWithProperties.Count()) {
+    nsContentUtils::AddScriptRunner(new nsUserDataCaller(nodesWithProperties,
+                                                         this));
   }
 
+  NS_ASSERTION(adoptedNode->GetOwnerDoc() == this,
+               "Should still be in the document we just got adopted into");
+
   return CallQueryInterface(adoptedNode, aResult);
 }
 

content/base/src/nsGenericElement.cpp

@@ -3570,10 +3570,8 @@ AdoptNodeIntoOwnerDoc(nsINode *aParent, nsINode *aNode)
   rv = domDoc->AdoptNode(node, getter_AddRefs(adoptedNode));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (doc != aParent->GetOwnerDoc()) {
-    return NS_ERROR_DOM_WRONG_DOCUMENT_ERR;
-  }
-
+  NS_ASSERTION(aParent->GetOwnerDoc() == doc,
+               "ownerDoc chainged while adopting");
   NS_ASSERTION(adoptedNode == node, "Uh, adopt node changed nodes?");
   NS_ASSERTION(aParent->HasSameOwnerDoc(aNode),
                "ownerDocument changed again after adopting!");
@@ -3589,10 +3587,18 @@ nsINode::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex,
                   "Inserting node that already has parent");
   nsresult rv;
 
+  // The id-handling code, and in the future possibly other code, need to
+  // react to unexpected attribute changes.
+  nsMutationGuard::DidMutate();
+
+  // Do this before checking the child-count since this could cause mutations
+  nsIDocument* doc = GetCurrentDoc();
+  mozAutoDocUpdate updateBatch(doc, UPDATE_CONTENT_MODEL, aNotify);
+
   if (!HasSameOwnerDoc(aKid)) {
     nsCOMPtr<nsIDOMNode> kid = do_QueryInterface(aKid, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
- 
+
     PRUint16 nodeType = 0;
     rv = kid->GetNodeType(&nodeType);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -3607,14 +3613,6 @@ nsINode::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex,
     }
   }
 
-  // The id-handling code, and in the future possibly other code, need to
-  // react to unexpected attribute changes.
-  nsMutationGuard::DidMutate();
-
-  // Do this before checking the child-count since this could cause mutations
-  nsIDocument* doc = GetCurrentDoc();
-  mozAutoDocUpdate updateBatch(doc, UPDATE_CONTENT_MODEL, aNotify);
-
   PRUint32 childCount = aChildArray.ChildCount();
   NS_ENSURE_TRUE(aIndex <= childCount, NS_ERROR_ILLEGAL_VALUE);
   PRBool isAppend = (aIndex == childCount);

content/base/src/nsNodeUtils.cpp

@@ -364,6 +364,16 @@ nsNodeUtils::CallUserDataHandlers(nsCOMArray<nsINode> &aNodesWithProperties,
                   "Expected aNodesWithProperties to contain original and "
                   "cloned nodes.");
 
+  if (!nsContentUtils::IsSafeToRunScript()) {
+    if (nsContentUtils::IsChromeDoc(aOwnerDocument)) {
+      NS_WARNING("Fix the caller! Userdata callback disabled.");
+    } else {
+      NS_ERROR("This is unsafe! Fix the caller! Userdata callback disabled.");
+    }
+
+    return NS_OK;
+  }
+
   nsPropertyTable *table = aOwnerDocument->PropertyTable(DOM_USER_DATA_HANDLER);
 
   // Keep the document alive, just in case one of the handlers causes it to go

content/base/test/test_bug548463.html

@@ -76,7 +76,7 @@ catch (e) {
   thrown = true;
 }
 
-ok(thrown, "adoptNode while adopting should throw");
+ok(!thrown, "adoptNode while adopting should not throw");
 
 </script>
 </pre>

content/base/test/test_bug626262.html

@@ -44,7 +44,7 @@ addLoadEvent(function() {
     thrown = true;
   }
 
-  ok(thrown, "changing ownerDocument during adoptNode should throw");
+  ok(!thrown, "changing ownerDocument during adoptNode should not throw");
 
   SimpleTest.finish();
 });