Bug 1555944 - Make nsIContent::GetBindingParent return an element. r=bzbarsky

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

dom/base/BindContext.h

@@ -33,15 +33,11 @@ struct MOZ_STACK_CLASS BindContext final {
   Element* GetBindingParent() const { return mBindingParent; }
 
   // This constructor should be used for regular appends to content.
-  //
-  // FIXME(emilio, bug 1555944): nsIContent::GetBindingParent() should return an
-  // Element*.
   explicit BindContext(nsINode& aParentNode)
       : mDoc(*aParentNode.OwnerDoc()),
         mSubtreeRootChanges(true),
         mBindingParent(aParentNode.IsContent()
-                           ? static_cast<Element*>(
+                           ? aParentNode.AsContent()->GetBindingParent()
-                                 aParentNode.AsContent()->GetBindingParent())
                            : nullptr) {}
 
   // When re-binding a shadow host into a tree, we re-bind all the shadow tree

dom/base/FragmentOrElement.cpp

@@ -1123,6 +1123,28 @@ void nsIContent::SetXBLInsertionPoint(nsIContent* aContent) {
   }
 }
 
+#ifdef DEBUG
+void nsIContent::AssertAnonymousSubtreeRelatedInvariants() const {
+  NS_ASSERTION(!IsRootOfNativeAnonymousSubtree() ||
+                   (GetParent() && GetBindingParent() == GetParent()),
+               "root of native anonymous subtree must have parent equal "
+               "to binding parent");
+  NS_ASSERTION(!GetParent() ||
+                   ((GetBindingParent() == GetParent()) ==
+                    HasFlag(NODE_IS_ANONYMOUS_ROOT)) ||
+                   // Unfortunately default content for XBL insertion points
+                   // is anonymous content that is bound with the parent of
+                   // the insertion point as the parent but the bound element
+                   // for the binding as the binding parent.  So we have to
+                   // complicate the assert a bit here.
+                   (GetBindingParent() &&
+                    (GetBindingParent() == GetParent()->GetBindingParent()) ==
+                        HasFlag(NODE_IS_ANONYMOUS_ROOT)),
+               "For nodes with parent, flag and GetBindingParent() check "
+               "should match");
+}
+#endif
+
 void FragmentOrElement::GetTextContentInternal(nsAString& aTextContent,
                                                OOMReporter& aError) {
   if (!nsContentUtils::GetNodeTextContent(this, true, aTextContent, fallible)) {

dom/base/nsIContent.h

@@ -186,28 +186,18 @@ class nsIContent : public nsINode {
    */
   nsIContent* FindFirstNonChromeOnlyAccessContent() const;
 
+#ifdef DEBUG
+  void AssertAnonymousSubtreeRelatedInvariants() const;
+#endif
+
   /**
    * Returns true if and only if this node has a parent, but is not in
    * its parent's child list.
    */
   bool IsRootOfAnonymousSubtree() const {
-    NS_ASSERTION(!IsRootOfNativeAnonymousSubtree() ||
+#ifdef DEBUG
-                     (GetParent() && GetBindingParent() == GetParent()),
+    AssertAnonymousSubtreeRelatedInvariants();
-                 "root of native anonymous subtree must have parent equal "
+#endif
-                 "to binding parent");
-    NS_ASSERTION(!GetParent() ||
-                     ((GetBindingParent() == GetParent()) ==
-                      HasFlag(NODE_IS_ANONYMOUS_ROOT)) ||
-                     // Unfortunately default content for XBL insertion points
-                     // is anonymous content that is bound with the parent of
-                     // the insertion point as the parent but the bound element
-                     // for the binding as the binding parent.  So we have to
-                     // complicate the assert a bit here.
-                     (GetBindingParent() &&
-                      (GetBindingParent() == GetParent()->GetBindingParent()) ==
-                          HasFlag(NODE_IS_ANONYMOUS_ROOT)),
-                 "For nodes with parent, flag and GetBindingParent() check "
-                 "should match");
     return HasFlag(NODE_IS_ANONYMOUS_ROOT);
   }
 
@@ -401,7 +391,7 @@ class nsIContent : public nsINode {
    *
    * @return the binding parent
    */
-  virtual nsIContent* GetBindingParent() const {
+  virtual mozilla::dom::Element* GetBindingParent() const {
     const nsExtendedContentSlots* slots = GetExistingExtendedContentSlots();
     return slots ? slots->mBindingParent.get() : nullptr;
   }
@@ -735,11 +725,10 @@ class nsIContent : public nsINode {
 
     /**
      * The nearest enclosing content node with a binding that created us.
-     * TODO(emilio): This should be an Element*.
      *
      * @see nsIContent::GetBindingParent
      */
-    nsCOMPtr<nsIContent> mBindingParent;
+    RefPtr<mozilla::dom::Element> mBindingParent;
 
     /**
      * @see nsIContent::GetXBLInsertionPoint

dom/xul/nsXULElement.h

@@ -349,7 +349,7 @@ class nsXULElement : public nsStyledElement {
                                 bool aIsTrustedEvent) override;
   void ClickWithInputSource(uint16_t aInputSource, bool aIsTrustedEvent);
 
-  nsIContent* GetBindingParent() const final { return mBindingParent; }
+  Element* GetBindingParent() const final { return mBindingParent; }
 
   virtual bool IsNodeOfType(uint32_t aFlags) const override;
   virtual bool IsFocusableInternal(int32_t* aTabIndex,
@@ -368,7 +368,7 @@ class nsXULElement : public nsStyledElement {
   // This function should ONLY be used by BindToTree implementations.
   // The function exists solely because XUL elements store the binding
   // parent as a member instead of in the slots, as Element does.
-  void SetXULBindingParent(nsIContent* aBindingParent) {
+  void SetXULBindingParent(Element* aBindingParent) {
     mBindingParent = aBindingParent;
   }
 
@@ -541,7 +541,7 @@ class nsXULElement : public nsStyledElement {
    * The nearest enclosing content node with a binding
    * that created us.
    */
-  nsCOMPtr<nsIContent> mBindingParent;
+  RefPtr<Element> mBindingParent;
 
   /**
    * Abandon our prototype linkage, and copy all attributes locally

layout/style/GeckoBindings.cpp

@@ -1722,8 +1722,7 @@ FontSizePrefs Gecko_GetBaseSize(nsAtom* aLanguage) {
 }
 
 const Element* Gecko_GetBindingParent(const Element* aElement) {
-  nsIContent* parent = aElement->GetBindingParent();
+  return aElement->GetBindingParent();
-  return parent ? parent->AsElement() : nullptr;
 }
 
 static StaticRefPtr<UACacheReporter> gUACacheReporter;

servo/components/style/gecko/wrapper.rs

@@ -704,9 +704,8 @@ impl<'le> GeckoElement<'le> {
                     .map(GeckoElement)
             }
         } else {
-            let binding_parent = unsafe { self.non_xul_xbl_binding_parent_raw_content().as_ref() }
+            let binding_parent = unsafe { self.non_xul_xbl_binding_parent().as_ref() }
-                .map(GeckoNode::from_content)
+                .map(GeckoElement);
-                .and_then(|n| n.as_element());
 
             debug_assert!(
                 binding_parent ==
@@ -721,10 +720,10 @@ impl<'le> GeckoElement<'le> {
     }
 
     #[inline]
-    fn non_xul_xbl_binding_parent_raw_content(&self) -> *mut nsIContent {
+    fn non_xul_xbl_binding_parent(&self) -> *mut RawGeckoElement {
         debug_assert!(!self.is_xul_element());
         self.extended_slots().map_or(ptr::null_mut(), |slots| {
-            slots._base.mBindingParent.raw::<nsIContent>()
+            slots._base.mBindingParent.mRawPtr
         })
     }