From ab4778b28d971a674619704de7bb597ccd8c660a Mon Sep 17 00:00:00 2001 From: Jonas Sicking Date: Thu, 30 Jul 2009 02:27:24 -0700 Subject: [PATCH] Bug 507023: When inserting fragments into the DOM, use ContentAppended notification when possible. r/sr=jst --- content/base/src/nsGenericElement.cpp | 106 +++++++++++++++++--------- 1 file changed, 70 insertions(+), 36 deletions(-) diff --git a/content/base/src/nsGenericElement.cpp b/content/base/src/nsGenericElement.cpp index 148e049a66b..11ad999a9b2 100644 --- a/content/base/src/nsGenericElement.cpp +++ b/content/base/src/nsGenericElement.cpp @@ -3209,15 +3209,10 @@ nsGenericElement::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex, return rv; } - // The kid may have removed its parent from the document, so recheck that - // that's still in the document before proceeding. Also, the kid may have - // just removed itself, in which case we don't really want to fire - // ContentAppended or a mutation event. - // XXXbz What if the kid just moved us in the document? Scripts suck. We - // really need to stop running them while we're in the middle of modifying - // the DOM.... + NS_ASSERTION(aKid->GetNodeParent() == container, + "Did we run script inappropriately?"); - if (aNotify && aKid->GetNodeParent() == container) { + if (aNotify) { // Note that we always want to call ContentInserted when things are added // as kids to documents if (aParent && isAppend) { @@ -3730,7 +3725,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. - mozAutoDocConditionalContentUpdateBatch(aDocument, + mozAutoDocConditionalContentUpdateBatch batch(aDocument, aReplace || nodeType == nsIDOMNode::DOCUMENT_FRAGMENT_NODE); // If we're replacing @@ -3768,6 +3763,12 @@ nsGenericElement::doReplaceOrInsertBefore(PRBool aReplace, if (nodeType == nsIDOMNode::DOCUMENT_FRAGMENT_NODE) { PRUint32 count = newContent->GetChildCount(); + if (!count) { + returnVal.swap(*aReturn); + + return NS_OK; + } + // Copy the children into a separate array to avoid having to deal with // mutations to the fragment while we're inserting. nsCOMArray fragChildren; @@ -3793,25 +3794,12 @@ nsGenericElement::doReplaceOrInsertBefore(PRBool aReplace, mutated = mutated || guard.Mutated(1); } - // Iterate through the fragment's children, and insert them in the new - // parent - for (i = 0; i < count; ++i) { - // Get the n:th child from the array. - nsIContent* childContent = fragChildren[i]; - - // If we've had any unexpeted mutations so far we need to recheck that - // the child can still be inserted. - if (mutated) { - // We really only need to update insPos if we *just* got an unexpected - // mutation as opposed to 3 insertions ago. But this is an edgecase so - // no need to over optimize. - insPos = refContent ? container->IndexOf(refContent) : - container->GetChildCount(); - if (insPos < 0) { - // Someone seriously messed up the childlist. We have no idea - // where to insert the remaining children, so just bail. - return NS_ERROR_DOM_NOT_FOUND_ERR; - } + // If we've had any unexpeted mutations so far we need to recheck that + // the child can still be inserted. + if (mutated) { + for (i = 0; i < count; ++i) { + // Get the n:th child from the array. + nsIContent* childContent = fragChildren[i]; nsCOMPtr tmpNode = do_QueryInterface(childContent); PRUint16 tmpType = 0; @@ -3824,18 +3812,64 @@ nsGenericElement::doReplaceOrInsertBefore(PRBool aReplace, } } - nsMutationGuard guard; + insPos = refContent ? container->IndexOf(refContent) : + container->GetChildCount(); + if (insPos < 0) { + // Someone seriously messed up the childlist. We have no idea + // where to insert the remaining children, so just bail. + return NS_ERROR_DOM_NOT_FOUND_ERR; + } + } + + PRBool appending = aParent && (insPos == container->GetChildCount()); + PRBool firstInsPos = insPos; + + // Iterate through the fragment's children, and insert them in the new + // parent + for (i = 0; i < count; ++i, ++insPos) { + nsIContent* childContent = fragChildren[i]; // XXXbz how come no reparenting here? That seems odd... // Insert the child. - res = container->InsertChildAt(childContent, insPos, PR_TRUE); - NS_ENSURE_SUCCESS(res, res); + res = container->InsertChildAt(childContent, insPos, PR_FALSE); + if (NS_FAILED(res)) { + // Make sure to notify on any children that we did succeed to insert + if (appending && i != 0) { + nsNodeUtils::ContentAppended(aParent, firstInsPos); + } + return res; + } - // Check to see if any evil mutation events mucked around with the - // child list. - mutated = mutated || guard.Mutated(1); - - ++insPos; + if (!appending) { + nsNodeUtils::ContentInserted(container, childContent, insPos); + } + } + + // Notify + if (appending) { + nsNodeUtils::ContentAppended(aParent, firstInsPos); + } + + // Fire mutation events. Optimize for the case when there are no listeners + nsIDocument* doc = container->GetOwnerDoc(); + nsPIDOMWindow* window = nsnull; + if (doc && (window = doc->GetInnerWindow()) && + window->HasMutationListeners(NS_EVENT_BITS_MUTATION_NODEINSERTED)) { + + for (i = 0; i < count; ++i, ++insPos) { + nsIContent* childContent = fragChildren[i]; + + if (nsContentUtils::HasMutationListeners(childContent, + NS_EVENT_BITS_MUTATION_NODEINSERTED, container)) { + mozAutoRemovableBlockerRemover blockerRemover; + + nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEINSERTED); + mutation.mRelatedNode = do_QueryInterface(container); + + mozAutoSubtreeModified subtree(container->GetOwnerDoc(), container); + nsEventDispatcher::Dispatch(childContent, nsnull, &mutation); + } + } } } else {