Bug 1427001: Move SetXBLBinding and SetShadowRoot to Element. r?smaug draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 23 Dec 2017 09:52:05 +0100
changeset 714780 ca2cfa730f8ead1c0462ec9d6f8ea0aee7ad6383
parent 714779 cb842b53f36e8fcb7b66b4d941ba16b2bd1f9f0f
child 744679 430f6a881a31c8c8219d7f7691be670fe37c8d4e
push id94027
push userbmo:emilio@crisal.io
push dateFri, 29 Dec 2017 16:26:14 +0000
reviewerssmaug
bugs1427001
milestone59.0a1
Bug 1427001: Move SetXBLBinding and SetShadowRoot to Element. r?smaug MozReview-Commit-ID: 6FL1HR2Isa
dom/base/Element.cpp
dom/base/Element.h
dom/base/FragmentOrElement.cpp
dom/base/FragmentOrElement.h
dom/base/nsDocument.cpp
dom/base/nsGenericDOMDataNode.cpp
dom/base/nsGenericDOMDataNode.h
dom/base/nsIContent.h
dom/xbl/nsBindingManager.cpp
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -359,16 +359,64 @@ Element::SetTabIndex(int32_t aTabIndex, 
 {
   nsAutoString value;
   value.AppendInt(aTabIndex);
 
   SetAttr(nsGkAtoms::tabindex, value, aError);
 }
 
 void
+Element::SetXBLBinding(nsXBLBinding* aBinding,
+                       nsBindingManager* aOldBindingManager)
+{
+  nsBindingManager* bindingManager;
+  if (aOldBindingManager) {
+    MOZ_ASSERT(!aBinding, "aOldBindingManager should only be provided "
+                          "when removing a binding.");
+    bindingManager = aOldBindingManager;
+  } else {
+    bindingManager = OwnerDoc()->BindingManager();
+  }
+
+  // After this point, aBinding will be the most-derived binding for aContent.
+  // If we already have a binding for aContent, make sure to
+  // remove it from the attached stack.  Otherwise we might end up firing its
+  // constructor twice (if aBinding inherits from it) or firing its constructor
+  // after aContent has been deleted (if aBinding is null and the content node
+  // dies before we process mAttachedStack).
+  RefPtr<nsXBLBinding> oldBinding = GetXBLBinding();
+  if (oldBinding) {
+    bindingManager->RemoveFromAttachedQueue(oldBinding);
+  }
+
+  if (aBinding) {
+    SetFlags(NODE_MAY_BE_IN_BINDING_MNGR);
+    nsExtendedDOMSlots* slots = ExtendedDOMSlots();
+    slots->mXBLBinding = aBinding;
+    bindingManager->AddBoundContent(this);
+  } else {
+    nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
+    if (slots) {
+      slots->mXBLBinding = nullptr;
+    }
+    bindingManager->RemoveBoundContent(this);
+    if (oldBinding) {
+      oldBinding->SetBoundElement(nullptr);
+    }
+  }
+}
+
+void
+Element::SetShadowRoot(ShadowRoot* aShadowRoot)
+{
+  nsExtendedDOMSlots* slots = ExtendedDOMSlots();
+  slots->mShadowRoot = aShadowRoot;
+}
+
+void
 Element::Blur(mozilla::ErrorResult& aError)
 {
   if (!ShouldBlur(this)) {
     return;
   }
 
   nsIDocument* doc = GetComposedDoc();
   if (!doc) {
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -262,16 +262,40 @@ public:
   int32_t TabIndex();
 
   /**
    * Set tabIndex value to this element.
    */
   void SetTabIndex(int32_t aTabIndex, mozilla::ErrorResult& aError);
 
   /**
+   * Sets or unsets an XBL binding for this element. Setting a
+   * binding on an element that already has a binding will remove the
+   * old binding.
+   *
+   * @param aBinding The binding to bind to this content. If nullptr is
+   *        provided as the argument, then existing binding will be
+   *        removed.
+   *
+   * @param aOldBindingManager The old binding manager that contains
+   *                           this content if this content was adopted
+   *                           to another document.
+   */
+  void SetXBLBinding(nsXBLBinding* aBinding,
+                     nsBindingManager* aOldBindingManager = nullptr);
+
+  /**
+   * Sets the ShadowRoot binding for this element. The contents of the
+   * binding is rendered in place of this node's children.
+   *
+   * @param aShadowRoot The ShadowRoot to be bound to this element.
+   */
+  void SetShadowRoot(ShadowRoot* aShadowRoot);
+
+  /**
    * Make focus on this element.
    */
   virtual void Focus(mozilla::ErrorResult& aError);
 
   /**
    * Show blur and clear focus.
    */
   virtual void Blur(mozilla::ErrorResult& aError);
--- a/dom/base/FragmentOrElement.cpp
+++ b/dom/base/FragmentOrElement.cpp
@@ -1187,64 +1187,16 @@ nsXBLBinding*
 FragmentOrElement::DoGetXBLBinding() const
 {
   MOZ_ASSERT(HasFlag(NODE_MAY_BE_IN_BINDING_MNGR));
   const nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
   return slots ? slots->mXBLBinding.get() : nullptr;
 }
 
 void
-FragmentOrElement::SetXBLBinding(nsXBLBinding* aBinding,
-                                 nsBindingManager* aOldBindingManager)
-{
-  nsBindingManager* bindingManager;
-  if (aOldBindingManager) {
-    MOZ_ASSERT(!aBinding, "aOldBindingManager should only be provided "
-                          "when removing a binding.");
-    bindingManager = aOldBindingManager;
-  } else {
-    bindingManager = OwnerDoc()->BindingManager();
-  }
-
-  // After this point, aBinding will be the most-derived binding for aContent.
-  // If we already have a binding for aContent, make sure to
-  // remove it from the attached stack.  Otherwise we might end up firing its
-  // constructor twice (if aBinding inherits from it) or firing its constructor
-  // after aContent has been deleted (if aBinding is null and the content node
-  // dies before we process mAttachedStack).
-  RefPtr<nsXBLBinding> oldBinding = GetXBLBinding();
-  if (oldBinding) {
-    bindingManager->RemoveFromAttachedQueue(oldBinding);
-  }
-
-  if (aBinding) {
-    SetFlags(NODE_MAY_BE_IN_BINDING_MNGR);
-    nsExtendedDOMSlots* slots = ExtendedDOMSlots();
-    slots->mXBLBinding = aBinding;
-    bindingManager->AddBoundContent(this);
-  } else {
-    nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
-    if (slots) {
-      slots->mXBLBinding = nullptr;
-    }
-    bindingManager->RemoveBoundContent(this);
-    if (oldBinding) {
-      oldBinding->SetBoundElement(nullptr);
-    }
-  }
-}
-
-void
-FragmentOrElement::SetShadowRoot(ShadowRoot* aShadowRoot)
-{
-  nsExtendedDOMSlots* slots = ExtendedDOMSlots();
-  slots->mShadowRoot = aShadowRoot;
-}
-
-void
 nsIContent::SetAssignedSlot(HTMLSlotElement* aSlot)
 {
   ExtendedContentSlots()->mAssignedSlot = aSlot;
 }
 
 void
 nsIContent::SetXBLInsertionPoint(nsIContent* aContent)
 {
--- a/dom/base/FragmentOrElement.h
+++ b/dom/base/FragmentOrElement.h
@@ -144,19 +144,16 @@ public:
                               bool aNotify) override;
   virtual bool TextIsOnlyWhitespace() override;
   virtual bool ThreadSafeTextIsOnlyWhitespace() const override;
   virtual bool HasTextForTranslation() override;
   virtual void AppendTextTo(nsAString& aResult) override;
   MOZ_MUST_USE
   virtual bool AppendTextTo(nsAString& aResult, const mozilla::fallible_t&) override;
   virtual nsXBLBinding* DoGetXBLBinding() const override;
-  virtual void SetXBLBinding(nsXBLBinding* aBinding,
-                             nsBindingManager* aOldBindingManager = nullptr) override;
-  virtual void SetShadowRoot(ShadowRoot* aBinding) override;
   virtual bool IsLink(nsIURI** aURI) const override;
 
   virtual void DestroyContent() override;
   virtual void SaveSubtreeState() override;
 
   nsIHTMLCollection* Children();
   uint32_t ChildElementCount()
   {
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -7753,17 +7753,19 @@ nsIDocument::AdoptNode(nsINode& aAdopted
         parent->RemoveChildAt(idx, true);
       } else {
         MOZ_ASSERT(!adoptedNode->IsInUncomposedDoc());
 
         // If we're adopting a node that's not in a document, it might still
         // have a binding applied. Remove the binding from the element now
         // that it's getting adopted into a new document.
         // TODO Fully tear down the binding.
-        adoptedNode->AsContent()->SetXBLBinding(nullptr);
+        if (adoptedNode->IsElement()) {
+          adoptedNode->AsElement()->SetXBLBinding(nullptr);
+        }
       }
 
       break;
     }
     case nsIDOMNode::DOCUMENT_NODE:
     {
       rv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
       return nullptr;
--- a/dom/base/nsGenericDOMDataNode.cpp
+++ b/dom/base/nsGenericDOMDataNode.cpp
@@ -664,27 +664,16 @@ nsGenericDOMDataNode::RemoveChildAt(uint
 }
 
 nsXBLBinding *
 nsGenericDOMDataNode::DoGetXBLBinding() const
 {
   return nullptr;
 }
 
-void
-nsGenericDOMDataNode::SetXBLBinding(nsXBLBinding* aBinding,
-                                    nsBindingManager* aOldBindingManager)
-{
-}
-
-void
-nsGenericDOMDataNode::SetShadowRoot(ShadowRoot* aShadowRoot)
-{
-}
-
 bool
 nsGenericDOMDataNode::IsNodeOfType(uint32_t aFlags) const
 {
   return !(aFlags & ~eDATA_NODE);
 }
 
 void
 nsGenericDOMDataNode::SaveSubtreeState()
--- a/dom/base/nsGenericDOMDataNode.h
+++ b/dom/base/nsGenericDOMDataNode.h
@@ -154,19 +154,16 @@ public:
   virtual void SaveSubtreeState() override;
 
 #ifdef DEBUG
   virtual void List(FILE* out, int32_t aIndent) const override;
   virtual void DumpContent(FILE* out, int32_t aIndent, bool aDumpAll) const override;
 #endif
 
   virtual nsXBLBinding* DoGetXBLBinding() const override;
-  virtual void SetXBLBinding(nsXBLBinding* aBinding,
-                             nsBindingManager* aOldBindingManager = nullptr) override;
-  virtual void SetShadowRoot(mozilla::dom::ShadowRoot* aShadowRoot) override;
   virtual bool IsNodeOfType(uint32_t aFlags) const override;
   virtual bool IsLink(nsIURI** aURI) const override;
 
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const override
   {
     nsCOMPtr<nsINode> result = CloneDataNode(aNodeInfo, true);
     result.forget(aResult);
--- a/dom/base/nsIContent.h
+++ b/dom/base/nsIContent.h
@@ -525,42 +525,16 @@ public:
     }
 
     return DoGetXBLBinding();
   }
 
   virtual nsXBLBinding* DoGetXBLBinding() const = 0;
 
   /**
-   * Sets or unsets an XBL binding for this element. Setting a
-   * binding on an element that already has a binding will remove the
-   * old binding.
-   *
-   * @param aBinding The binding to bind to this content. If nullptr is
-   *        provided as the argument, then existing binding will be
-   *        removed.
-   *
-   * @param aOldBindingManager The old binding manager that contains
-   *                           this content if this content was adopted
-   *                           to another document.
-   */
-  virtual void SetXBLBinding(nsXBLBinding* aBinding,
-                             nsBindingManager* aOldBindingManager = nullptr) = 0;
-
-  /**
-   * Sets the ShadowRoot binding for this element. The contents of the
-   * binding is rendered in place of this node's children.
-   *
-   * @param aShadowRoot The ShadowRoot to be bound to this element.
-   *
-   * FIXME(emilio): No reason this lives in nsIContent, should move to Element.
-   */
-  virtual void SetShadowRoot(mozilla::dom::ShadowRoot* aShadowRoot) = 0;
-
-  /**
    * Gets the ShadowRoot binding for this element.
    *
    * @return The ShadowRoot currently bound to this element.
    */
   inline mozilla::dom::ShadowRoot *GetShadowRoot() const;
 
   /**
    * Gets the root of the node tree for this content if it is in a shadow tree.
--- a/dom/xbl/nsBindingManager.cpp
+++ b/dom/xbl/nsBindingManager.cpp
@@ -211,17 +211,17 @@ nsBindingManager::RemovedFromDocumentInt
     // BindingDetached (which may invoke a XBL destructor) and
     // ChangeDocument, but we still want to clear out the binding
     // and insertion parent that may hold references.
     if (!mDestroyed && aDestructorHandling == eRunDtor) {
       binding->PrototypeBinding()->BindingDetached(binding->GetBoundElement());
       binding->ChangeDocument(aOldDocument, nullptr);
     }
 
-    aContent->SetXBLBinding(nullptr, this);
+    aContent->AsElement()->SetXBLBinding(nullptr, this);
   }
 
   // Clear out insertion point and content lists.
   aContent->SetXBLInsertionPoint(nullptr);
 }
 
 nsAtom*
 nsBindingManager::ResolveTag(nsIContent* aContent, int32_t* aNameSpaceID)