Bug 1502875 - Remove AddElementToDocumentPre and RemoveSubtreeFromDocument. r=smaug

The ID table is managed in BindToTree / UnbindFromTree.

I guess this was more important when XUL templates were a thing.

Differential Revision: https://phabricator.services.mozilla.com/D10056

dom/xbl/nsXBLBinding.cpp

@@ -205,9 +205,11 @@ nsXBLBinding::BindAnonymousContent(nsIContent* aAnonParent,
     }
 
 #ifdef MOZ_XUL
-    // To make XUL templates work (and other goodies that happen when
-    // an element is added to a XUL document), we need to notify the
-    // XUL document using its special API.
+    // To make other goodies that happen when an element is added to a XUL
+    // document work, we need to notify the XUL document using its special API.
+    //
+    // FIXME(emilio): Is this needed anymore? Do we really use <linkset> or
+    // <link> from XBL stuff?
     if (doc && doc->IsXULDocument()) {
       doc->AsXULDocument()->AddSubtreeToDocument(child);
     }
@@ -223,18 +225,10 @@ nsXBLBinding::UnbindAnonymousContent(nsIDocument* aDocument,
   nsAutoScriptBlocker scriptBlocker;
   // Hold a strong ref while doing this, just in case.
   nsCOMPtr<nsIContent> anonParent = aAnonParent;
-#ifdef MOZ_XUL
-  const bool isXULDocument = aDocument && aDocument->IsXULDocument();
-#endif
   for (nsIContent* child = aAnonParent->GetFirstChild();
        child;
        child = child->GetNextSibling()) {
     child->UnbindFromTree(true, aNullParent);
-#ifdef MOZ_XUL
-    if (isXULDocument) {
-      aDocument->AsXULDocument()->RemoveSubtreeFromDocument(child);
-    }
-#endif
   }
 }
 

dom/xul/XULDocument.cpp

@@ -484,10 +484,9 @@ XULDocument::ContentAppended(nsIContent* aFirstNewContent)
     nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
 
     // Update our element map
-    nsresult rv = NS_OK;
-    for (nsIContent* cur = aFirstNewContent; cur && NS_SUCCEEDED(rv);
+    for (nsIContent* cur = aFirstNewContent; cur;
          cur = cur->GetNextSibling()) {
-        rv = AddSubtreeToDocument(cur);
+        AddSubtreeToDocument(cur);
     }
 }
 
@@ -505,12 +504,8 @@ XULDocument::ContentInserted(nsIContent* aChild)
 void
 XULDocument::ContentRemoved(nsIContent* aChild, nsIContent* aPreviousSibling)
 {
-    NS_ASSERTION(aChild->OwnerDoc() == this, "unexpected doc");
-
-    // Might not need this, but be safe for now.
-    nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
-
-    RemoveSubtreeFromDocument(aChild);
+    // FIXME(emilio): Doesn't this need to remove the l10n links if they go
+    // away, or something?
 }
 
 //----------------------------------------------------------------------
@@ -570,25 +565,7 @@ XULDocument::Persist(Element* aElement, int32_t aNameSpaceID,
     mLocalStore->SetValue(uri, id, attrstr, valuestr);
 }
 
-nsresult
-XULDocument::AddElementToDocumentPre(Element* aElement)
-{
-    // Do a bunch of work that's necessary when an element gets added
-    // to the XUL Document.
-
-    // 1. Add the element to the id map, since it seems this can be
-    // called when creating elements from prototypes.
-    nsAtom* id = aElement->GetID();
-    if (id) {
-        // FIXME: Shouldn't BindToTree take care of this?
-        nsAutoScriptBlocker scriptBlocker;
-        AddToIdTable(aElement, id);
-    }
-
-    return NS_OK;
-}
-
-nsresult
+void
 XULDocument::AddElementToDocumentPost(Element* aElement)
 {
     if (aElement == GetRootElement()) {
@@ -600,11 +577,9 @@ XULDocument::AddElementToDocumentPost(Element* aElement)
     } else if (aElement->IsXULElement(nsGkAtoms::linkset)) {
         OnL10nResourceContainerParsed();
     }
-
-    return NS_OK;
 }
 
-nsresult
+void
 XULDocument::AddSubtreeToDocument(nsIContent* aContent)
 {
     MOZ_ASSERT(aContent->GetComposedDoc() == this, "Element not in doc!");
@@ -616,66 +591,24 @@ XULDocument::AddSubtreeToDocument(nsIContent* aContent)
     // least they don't work in regular HTML documents either as of today so...
     if (MOZ_UNLIKELY(!aContent->IsInUncomposedDoc())) {
         MOZ_ASSERT(aContent->IsInShadowTree());
-        return NS_OK;
+        return;
     }
 
     // From here on we only care about elements.
     Element* aElement = Element::FromNode(aContent);
     if (!aElement) {
-        return NS_OK;
+        return;
     }
 
-    // Do pre-order addition magic
-    nsresult rv = AddElementToDocumentPre(aElement);
-    if (NS_FAILED(rv)) return rv;
-
     // Recurse to children
     for (nsIContent* child = aElement->GetLastChild();
          child;
          child = child->GetPreviousSibling()) {
-
-        rv = AddSubtreeToDocument(child);
-        if (NS_FAILED(rv))
-            return rv;
+        AddSubtreeToDocument(child);
     }
 
     // Do post-order addition magic
-    return AddElementToDocumentPost(aElement);
-}
-
-nsresult
-XULDocument::RemoveSubtreeFromDocument(nsIContent* aContent)
-{
-    // From here on we only care about elements.
-    Element* aElement = Element::FromNode(aContent);
-    if (!aElement) {
-        return NS_OK;
-    }
-
-    // Do a bunch of cleanup to remove an element from the XUL
-    // document.
-    nsresult rv;
-
-    // Remove any children from the document.
-    for (nsIContent* child = aElement->GetLastChild();
-         child;
-         child = child->GetPreviousSibling()) {
-
-        rv = RemoveSubtreeFromDocument(child);
-        if (NS_FAILED(rv))
-            return rv;
-    }
-
-    // Remove the element from the id map, since we added it in
-    // AddElementToDocumentPre().
-    nsAtom* id = aElement->GetID();
-    if (id) {
-        // FIXME: Shouldn't UnbindFromTree take care of this?
-        nsAutoScriptBlocker scriptBlocker;
-        RemoveFromIdTable(aElement, id);
-    }
-
-    return NS_OK;
+    AddElementToDocumentPost(aElement);
 }
 
 //----------------------------------------------------------------------
@@ -1251,9 +1184,6 @@ XULDocument::ResumeWalk()
                 rv = element->AppendChildTo(child, false);
                 if (NS_FAILED(rv)) return rv;
 
-                // do pre-order document-level hookup.
-                AddElementToDocumentPre(child);
-
                 // If it has children, push the element onto the context
                 // stack and begin to process them.
                 if (protoele->mChildren.Length() > 0) {

dom/xul/XULDocument.h

@@ -88,11 +88,7 @@ public:
     /**
      * Notify the XUL document that a subtree has been added
      */
-    nsresult AddSubtreeToDocument(nsIContent* aContent);
-    /**
-     * Notify the XUL document that a subtree has been removed
-     */
-    nsresult RemoveSubtreeFromDocument(nsIContent* aContent);
+    void AddSubtreeToDocument(nsIContent* aContent);
     /**
      * This is invoked whenever the prototype for this document is loaded
      * and should be walked, regardless of whether the XUL cache is
@@ -155,11 +151,7 @@ protected:
     nsresult ApplyPersistentAttributesToElements(const nsAString &aID,
                                                  nsCOMArray<Element>& aElements);
 
-    nsresult
-    AddElementToDocumentPre(Element* aElement);
-
-    nsresult
-    AddElementToDocumentPost(Element* aElement);
+    void AddElementToDocumentPost(Element* aElement);
 
     static void DirectionChanged(const char* aPrefName, XULDocument* aData);