Bug 564591: Speed up BindToTree/UnbindFromTree by only doing XBL related work when needed. r=smaug

content/base/src/nsGenericDOMDataNode.cpp

@@ -565,7 +565,7 @@ nsGenericDOMDataNode::UnbindFromTree(PRBool aDeep, PRBool aNullParent)
     // Notify XBL- & nsIAnonymousContentCreator-generated
     // anonymous content that the document is changing.
     // This is needed to update the insertion point.
-    document->BindingManager()->ChangeDocumentFor(this, document, nsnull);
+    document->BindingManager()->RemovedFromDocument(this, document);
   }
 
   mParentPtrBits = aNullParent ? 0 : mParentPtrBits & ~PARENT_BIT_INDOCUMENT;

content/base/src/nsGenericElement.cpp

@@ -2969,7 +2969,7 @@ nsGenericElement::UnbindFromTree(PRBool aDeep, PRBool aNullParent)
   if (document) {
     // Notify XBL- & nsIAnonymousContentCreator-generated
     // anonymous content that the document is changing.
-    document->BindingManager()->ChangeDocumentFor(this, document, nsnull);
+    document->BindingManager()->RemovedFromDocument(this, document);
 
     document->ClearBoxObjectFor(this);
   }
@@ -3766,7 +3766,7 @@ nsGenericElement::DestroyContent()
 {
   nsIDocument *document = GetOwnerDoc();
   if (document) {
-    document->BindingManager()->ChangeDocumentFor(this, document, nsnull);
+    document->BindingManager()->RemovedFromDocument(this, document);
     document->ClearBoxObjectFor(this);
   }
 
@@ -4327,9 +4327,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGenericElement)
 
   {
     nsIDocument *doc;
-    if (!tmp->GetNodeParent() && (doc = tmp->GetOwnerDoc()) &&
+    if (!tmp->GetNodeParent() && (doc = tmp->GetOwnerDoc())) {
-        tmp->HasFlag(NODE_MAY_BE_IN_BINDING_MNGR)) {
+      doc->BindingManager()->RemovedFromDocument(tmp, doc);
-      doc->BindingManager()->ChangeDocumentFor(tmp, doc, nsnull);
     }
   }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END

content/base/src/nsNodeUtils.cpp

@@ -284,7 +284,7 @@ nsNodeUtils::LastRelease(nsINode* aNode)
     // attached
     if (aNode->HasFlag(NODE_FORCE_XBL_BINDINGS) &&
         ownerDoc && ownerDoc->BindingManager()) {
-      ownerDoc->BindingManager()->ChangeDocumentFor(elem, ownerDoc, nsnull);
+      ownerDoc->BindingManager()->RemovedFromDocument(elem, ownerDoc);
     }
   }
 

content/xbl/src/nsBindingManager.cpp

@@ -621,17 +621,11 @@ nsBindingManager::SetWrappedJS(nsIContent* aContent, nsIXPConnectWrappedJS* aWra
   return SetOrRemoveObject(mWrapperTable, aContent, aWrappedJS);
 }
 
-nsresult
+void
-nsBindingManager::ChangeDocumentFor(nsIContent* aContent, nsIDocument* aOldDocument,
+nsBindingManager::RemovedFromDocumentInternal(nsIContent* aContent,
-                                    nsIDocument* aNewDocument)
+                                              nsIDocument* aOldDocument)
 {
-  // XXXbz this code is pretty broken, since moving from one document
-  // to another always passes through a null document!
   NS_PRECONDITION(aOldDocument != nsnull, "no old document");
-  NS_PRECONDITION(!aNewDocument,
-                  "Changing to a non-null new document not supported yet");
-  if (! aOldDocument)
-    return NS_ERROR_NULL_POINTER;
 
   // Hold a ref to the binding so it won't die when we remove it from our
   // table.
@@ -650,18 +644,14 @@ nsBindingManager::ChangeDocumentFor(nsIContent* aContent, nsIDocument* aOldDocum
   }
 
   if (binding) {
-    binding->ChangeDocument(aOldDocument, aNewDocument);
+    binding->ChangeDocument(aOldDocument, nsnull);
     SetBinding(aContent, nsnull);
-    if (aNewDocument)
-      aNewDocument->BindingManager()->SetBinding(aContent, binding);
   }
 
   // Clear out insertion parents and content lists.
   SetInsertionParent(aContent, nsnull);
   SetContentListFor(aContent, nsnull);
   SetAnonymousNodesFor(aContent, nsnull);
-
-  return NS_OK;
 }
 
 nsIAtom*

content/xbl/src/nsBindingManager.h

@@ -84,21 +84,22 @@ public:
 
   /**
    * Notify the binding manager that an element
-   * has been moved from one document to another,
+   * has been removed from its document,
    * so that it can update any bindings or
    * nsIAnonymousContentCreator-created anonymous
    * content that may depend on the document.
    * @param aContent the element that's being moved
    * @param aOldDocument the old document in which the
-   *   content resided. May be null if the the content
+   *   content resided.
-   *   was not in any document.
-   * @param aNewDocument the document in which the
-   *   content will reside. May be null if the content
-   *   will not reside in any document, or if the
-   *   content is being destroyed.
    */
-  nsresult ChangeDocumentFor(nsIContent* aContent, nsIDocument* aOldDocument,
+  void RemovedFromDocument(nsIContent* aContent, nsIDocument* aOldDocument)
-                             nsIDocument* aNewDocument);
+  {
+    if (aContent->HasFlag(NODE_MAY_BE_IN_BINDING_MNGR)) {
+      RemovedFromDocumentInternal(aContent, aOldDocument);
+    }
+  }
+  void RemovedFromDocumentInternal(nsIContent* aContent,
+                                   nsIDocument* aOldDocument);
 
   nsIAtom* ResolveTag(nsIContent* aContent, PRInt32* aNameSpaceID);