Bug 1502875 - Remove AddElementToDocumentPre and RemoveSubtreeFromDocument. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 29 Oct 2018 15:01:51 +0100
changeset 499851 4fb88c6c2223597c9acf26f6e2e07b9fa1d83d3d
parent 499850 ed31fa097a650aa8e8f119584b516af3b2c388ac
child 499852 7a38544a9172918f63632958574c23e1329f030f
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1502875
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/xul/XULDocument.cpp
dom/xul/XULDocument.h
--- a/dom/xbl/nsXBLBinding.cpp
+++ b/dom/xbl/nsXBLBinding.cpp
@@ -200,46 +200,40 @@ nsXBLBinding::BindAnonymousContent(nsICo
     if (NS_FAILED(rv)) {
       // Oh, well... Just give up.
       // XXXbz This really shouldn't be a void method!
       child->UnbindFromTree();
       return;
     }
 
 #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);
     }
 #endif
   }
 }
 
 void
 nsXBLBinding::UnbindAnonymousContent(nsIDocument* aDocument,
                                      nsIContent* aAnonParent,
                                      bool aNullParent)
 {
   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
   }
 }
 
 void
 nsXBLBinding::SetBoundElement(Element* aElement)
 {
   mBoundElement = aElement;
   if (mNextBinding)
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -479,20 +479,19 @@ void
 XULDocument::ContentAppended(nsIContent* aFirstNewContent)
 {
     NS_ASSERTION(aFirstNewContent->OwnerDoc() == this, "unexpected doc");
 
     // Might not need this, but be safe for now.
     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);
     }
 }
 
 void
 XULDocument::ContentInserted(nsIContent* aChild)
 {
     NS_ASSERTION(aChild->OwnerDoc() == this, "unexpected doc");
 
@@ -500,22 +499,18 @@ XULDocument::ContentInserted(nsIContent*
     nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
 
     AddSubtreeToDocument(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?
 }
 
 //----------------------------------------------------------------------
 //
 // nsIDocument interface
 //
 
 
@@ -565,122 +560,60 @@ XULDocument::Persist(Element* aElement, 
         if (nsCOMPtr<nsIXULWindow> win = GetXULWindowIfToplevelChrome()) {
            return;
         }
     }
 
     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()) {
         ResetDocumentDirection();
     }
 
     if (aElement->IsXULElement(nsGkAtoms::link)) {
         LocalizationLinkAdded(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!");
 
     // If the content is not in the document, it must be in a shadow tree.
     //
     // The shadow root itself takes care of maintaining the ID tables and such,
     // and there's no use case for localization links in shadow trees, or at
     // 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);
 }
 
 //----------------------------------------------------------------------
 //
 // nsINode interface
 //
 
 nsresult
@@ -1246,19 +1179,16 @@ XULDocument::ResumeWalk()
                                                 getter_AddRefs(child),
                                                 false);
                 if (NS_FAILED(rv)) return rv;
 
                 // ...and append it to the content model.
                 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) {
                     rv = mContextStack.Push(protoele, child);
                     if (NS_FAILED(rv)) return rv;
                 }
                 else {
                     // If there are no children, do post-order document hookup
--- a/dom/xul/XULDocument.h
+++ b/dom/xul/XULDocument.h
@@ -83,21 +83,17 @@ public:
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
     NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
 
     /**
      * 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
      * disabled, whether the protototype was loaded, whether the
      * prototype was loaded from the cache or created by parsing the
      * actual XUL source, etc.
      *
      * @param aResumeWalk whether this should also call ResumeWalk().
@@ -150,21 +146,17 @@ protected:
                            nsIPrincipal* aDocumentPrincipal,
                            nsIParser** aResult);
 
     nsresult ApplyPersistentAttributes();
     nsresult ApplyPersistentAttributesInternal();
     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);
 
     // pseudo constants
     static int32_t gRefCnt;
 
     static LazyLogModule gXULLog;