Bug 1410895: Make XBL slots hold the insertion point, not the XBL parent. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 23 Oct 2017 15:52:08 +0200
changeset 686215 3f931e3d5e1443cfa843330481930f5ba8ce35a2
parent 686074 dfb54d604158f5605fb07f41751e36bfef641a2f
child 686216 b239b7576fe0b9d8e74e89b3cb0315575b8f3d8a
push id86120
push userbmo:emilio@crisal.io
push dateWed, 25 Oct 2017 15:24:20 +0000
reviewersbz
bugs1410895
milestone58.0a1
Bug 1410895: Make XBL slots hold the insertion point, not the XBL parent. r?bz This is pretty much a straight-forward change except for a single thing, the UpdateInsertionParent calls. However, I cannot make any sense of them. They go through the inserted children setting the insertion point, but then ClearInsertionPoints() is called. ClearInsertionPoints calls XBLChildrenElement::ClearInsertedChildren, which sets all the insertion points to null anyway. Thus, I've removed that function completely. MozReview-Commit-ID: 80daGQfLZrV
dom/base/FragmentOrElement.cpp
dom/base/FragmentOrElement.h
dom/base/nsGenericDOMDataNode.cpp
dom/base/nsGenericDOMDataNode.h
dom/base/nsIContent.h
dom/xbl/XBLChildrenElement.h
dom/xbl/nsBindingManager.cpp
dom/xbl/nsXBLBinding.cpp
--- a/dom/base/FragmentOrElement.cpp
+++ b/dom/base/FragmentOrElement.cpp
@@ -701,18 +701,18 @@ FragmentOrElement::nsDOMSlots::Traverse(
 
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mExtendedSlots->mAssignedSlot");
   cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIContent*, mExtendedSlots->mAssignedSlot.get()));
 
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mExtendedSlots->mXBLBinding");
   cb.NoteNativeChild(mExtendedSlots->mXBLBinding,
                      NS_CYCLE_COLLECTION_PARTICIPANT(nsXBLBinding));
 
-  NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mExtendedSlots->mXBLInsertionParent");
-  cb.NoteXPCOMChild(mExtendedSlots->mXBLInsertionParent.get());
+  NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mExtendedSlots->mXBLInsertionPoint");
+  cb.NoteXPCOMChild(mExtendedSlots->mXBLInsertionPoint.get());
 
   if (mExtendedSlots->mCustomElementData) {
     for (uint32_t i = 0;
          i < mExtendedSlots->mCustomElementData->mReactionQueue.Length(); i++) {
       if (mExtendedSlots->mCustomElementData->mReactionQueue[i]) {
         mExtendedSlots->mCustomElementData->mReactionQueue[i]->Traverse(cb);
       }
     }
@@ -746,17 +746,17 @@ FragmentOrElement::nsDOMSlots::Unlink()
 
   mExtendedSlots->mSMILOverrideStyle = nullptr;
   mExtendedSlots->mControllers = nullptr;
   mExtendedSlots->mLabelsList = nullptr;
   mExtendedSlots->mShadowRoot = nullptr;
   mExtendedSlots->mContainingShadow = nullptr;
   mExtendedSlots->mAssignedSlot = nullptr;
   MOZ_ASSERT(!(mExtendedSlots->mXBLBinding));
-  mExtendedSlots->mXBLInsertionParent = nullptr;
+  mExtendedSlots->mXBLInsertionPoint = nullptr;
   if (mExtendedSlots->mCustomElementData) {
     if (mExtendedSlots->mCustomElementData->mCustomElementDefinition) {
       mExtendedSlots->mCustomElementData->mCustomElementDefinition = nullptr;
     }
     mExtendedSlots->mCustomElementData = nullptr;
   }
   nsCOMPtr<nsIFrameLoader> frameLoader =
     do_QueryInterface(mExtendedSlots->mFrameLoaderOrOpener);
@@ -1169,22 +1169,22 @@ FragmentOrElement::SetXBLBinding(nsXBLBi
     bindingManager->RemoveBoundContent(this);
     if (oldBinding) {
       oldBinding->SetBoundElement(nullptr);
     }
   }
 }
 
 nsIContent*
-FragmentOrElement::GetXBLInsertionParent() const
+FragmentOrElement::GetXBLInsertionPoint() const
 {
   if (HasFlag(NODE_MAY_BE_IN_BINDING_MNGR)) {
     nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
     if (slots) {
-      return slots->mXBLInsertionParent;
+      return slots->mXBLInsertionPoint;
     }
   }
 
   return nullptr;
 }
 
 ShadowRoot*
 FragmentOrElement::GetContainingShadow() const
@@ -1230,34 +1230,34 @@ FragmentOrElement::GetAssignedSlot() con
 void
 FragmentOrElement::SetAssignedSlot(HTMLSlotElement* aSlot)
 {
   nsExtendedDOMSlots* slots = ExtendedDOMSlots();
   slots->mAssignedSlot = aSlot;
 }
 
 void
-FragmentOrElement::SetXBLInsertionParent(nsIContent* aContent)
+FragmentOrElement::SetXBLInsertionPoint(nsIContent* aContent)
 {
-  nsCOMPtr<nsIContent> oldInsertionParent = nullptr;
+  nsCOMPtr<nsIContent> oldInsertionPoint = nullptr;
   if (aContent) {
     nsExtendedDOMSlots* slots = ExtendedDOMSlots();
     SetFlags(NODE_MAY_BE_IN_BINDING_MNGR);
-    oldInsertionParent = slots->mXBLInsertionParent.forget();
-    slots->mXBLInsertionParent = aContent;
+    oldInsertionPoint = slots->mXBLInsertionPoint.forget();
+    slots->mXBLInsertionPoint = aContent;
   } else {
     if (nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots()) {
-      oldInsertionParent = slots->mXBLInsertionParent.forget();
-      slots->mXBLInsertionParent = nullptr;
+      oldInsertionPoint = slots->mXBLInsertionPoint.forget();
+      slots->mXBLInsertionPoint = nullptr;
     }
   }
 
   // We just changed the flattened tree, so any Servo style data is now invalid.
   // We rely on nsXBLService::LoadBindings to re-traverse the subtree afterwards.
-  if (oldInsertionParent != aContent &&
+  if (oldInsertionPoint != aContent &&
       IsStyledByServo() && IsElement() && AsElement()->HasServoData()) {
     ServoRestyleManager::ClearServoDataFromSubtree(AsElement());
   }
 }
 
 nsresult
 FragmentOrElement::InsertChildAt(nsIContent* aKid,
                                 uint32_t aIndex,
--- a/dom/base/FragmentOrElement.h
+++ b/dom/base/FragmentOrElement.h
@@ -152,18 +152,18 @@ public:
   virtual void SetXBLBinding(nsXBLBinding* aBinding,
                              nsBindingManager* aOldBindingManager = nullptr) override;
   virtual ShadowRoot *GetContainingShadow() const override;
   virtual nsTArray<nsIContent*> &DestInsertionPoints() override;
   virtual nsTArray<nsIContent*> *GetExistingDestInsertionPoints() const override;
   virtual void SetShadowRoot(ShadowRoot* aBinding) override;
   virtual mozilla::dom::HTMLSlotElement* GetAssignedSlot() const override;
   virtual void SetAssignedSlot(mozilla::dom::HTMLSlotElement* aSlot) override;
-  virtual nsIContent *GetXBLInsertionParent() const override;
-  virtual void SetXBLInsertionParent(nsIContent* aContent) override;
+  virtual nsIContent *GetXBLInsertionPoint() const override;
+  virtual void SetXBLInsertionPoint(nsIContent* aContent) override;
   virtual bool IsLink(nsIURI** aURI) const override;
 
   virtual void DestroyContent() override;
   virtual void SaveSubtreeState() override;
 
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker) override;
 
   nsIHTMLCollection* Children();
@@ -305,19 +305,19 @@ public:
     RefPtr<mozilla::dom::HTMLSlotElement> mAssignedSlot;
 
     /**
      * XBL binding installed on the element.
      */
     RefPtr<nsXBLBinding> mXBLBinding;
 
     /**
-     * XBL binding installed on the lement.
+     * XBL binding insertion point.
      */
-    nsCOMPtr<nsIContent> mXBLInsertionParent;
+    nsCOMPtr<nsIContent> mXBLInsertionPoint;
 
     /**
      * Web components custom element data.
      */
     RefPtr<CustomElementData> mCustomElementData;
 
     /**
      * For XUL to hold either frameloader or opener.
--- a/dom/base/nsGenericDOMDataNode.cpp
+++ b/dom/base/nsGenericDOMDataNode.cpp
@@ -762,39 +762,39 @@ nsGenericDOMDataNode::GetXBLBinding() co
 
 void
 nsGenericDOMDataNode::SetXBLBinding(nsXBLBinding* aBinding,
                                     nsBindingManager* aOldBindingManager)
 {
 }
 
 nsIContent *
-nsGenericDOMDataNode::GetXBLInsertionParent() const
+nsGenericDOMDataNode::GetXBLInsertionPoint() const
 {
   if (HasFlag(NODE_MAY_BE_IN_BINDING_MNGR)) {
     nsDataSlots *slots = GetExistingDataSlots();
     if (slots) {
-      return slots->mXBLInsertionParent;
+      return slots->mXBLInsertionPoint;
     }
   }
 
   return nullptr;
 }
 
 void
-nsGenericDOMDataNode::SetXBLInsertionParent(nsIContent* aContent)
+nsGenericDOMDataNode::SetXBLInsertionPoint(nsIContent* aContent)
 {
   if (aContent) {
     nsDataSlots *slots = DataSlots();
     SetFlags(NODE_MAY_BE_IN_BINDING_MNGR);
-    slots->mXBLInsertionParent = aContent;
+    slots->mXBLInsertionPoint = aContent;
   } else {
     nsDataSlots *slots = GetExistingDataSlots();
     if (slots) {
-      slots->mXBLInsertionParent = nullptr;
+      slots->mXBLInsertionPoint = nullptr;
     }
   }
 }
 
 bool
 nsGenericDOMDataNode::IsNodeOfType(uint32_t aFlags) const
 {
   return !(aFlags & ~(eCONTENT | eDATA_NODE));
@@ -834,30 +834,30 @@ nsGenericDOMDataNode::CreateSlots()
 nsGenericDOMDataNode::nsDataSlots::nsDataSlots()
   : nsINode::nsSlots(), mBindingParent(nullptr)
 {
 }
 
 void
 nsGenericDOMDataNode::nsDataSlots::Traverse(nsCycleCollectionTraversalCallback &cb)
 {
-  NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mXBLInsertionParent");
-  cb.NoteXPCOMChild(mXBLInsertionParent.get());
+  NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mXBLInsertionPoint");
+  cb.NoteXPCOMChild(mXBLInsertionPoint.get());
 
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mContainingShadow");
   cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIContent*, mContainingShadow));
 
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mAssignedSlot");
   cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIContent*, mAssignedSlot.get()));
 }
 
 void
 nsGenericDOMDataNode::nsDataSlots::Unlink()
 {
-  mXBLInsertionParent = nullptr;
+  mXBLInsertionPoint = nullptr;
   mContainingShadow = nullptr;
   mAssignedSlot = nullptr;
 }
 
 //----------------------------------------------------------------------
 
 // Implementation of the nsIDOMText interface
 
--- a/dom/base/nsGenericDOMDataNode.h
+++ b/dom/base/nsGenericDOMDataNode.h
@@ -173,18 +173,18 @@ public:
   virtual void SetXBLBinding(nsXBLBinding* aBinding,
                              nsBindingManager* aOldBindingManager = nullptr) override;
   virtual mozilla::dom::ShadowRoot *GetContainingShadow() const override;
   virtual nsTArray<nsIContent*> &DestInsertionPoints() override;
   virtual nsTArray<nsIContent*> *GetExistingDestInsertionPoints() const override;
   virtual void SetShadowRoot(mozilla::dom::ShadowRoot* aShadowRoot) override;
   virtual mozilla::dom::HTMLSlotElement* GetAssignedSlot() const override;
   virtual void SetAssignedSlot(mozilla::dom::HTMLSlotElement* aSlot) override;
-  virtual nsIContent *GetXBLInsertionParent() const override;
-  virtual void SetXBLInsertionParent(nsIContent* aContent) override;
+  virtual nsIContent *GetXBLInsertionPoint() const override;
+  virtual void SetXBLInsertionPoint(nsIContent* aContent) override;
   virtual bool IsNodeOfType(uint32_t aFlags) const override;
   virtual bool IsLink(nsIURI** aURI) const override;
 
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker) override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const;
   virtual nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
                                               int32_t aModType) const;
 
@@ -273,19 +273,19 @@ protected:
 
     /**
      * The nearest enclosing content node with a binding that created us.
      * @see nsIContent::GetBindingParent
      */
     nsIContent* mBindingParent;  // [Weak]
 
     /**
-     * @see nsIContent::GetXBLInsertionParent
+     * @see nsIContent::GetXBLInsertionPoint
      */
-    nsCOMPtr<nsIContent> mXBLInsertionParent;
+    nsCOMPtr<nsIContent> mXBLInsertionPoint;
 
     /**
      * @see nsIContent::GetContainingShadow
      */
     RefPtr<mozilla::dom::ShadowRoot> mContainingShadow;
 
     /**
      * @see nsIContent::GetDestInsertionPoints
--- a/dom/base/nsIContent.h
+++ b/dom/base/nsIContent.h
@@ -753,30 +753,36 @@ public:
 
   /**
    * Sets the assigned slot associated with this content.
    *
    * @param aSlot The assigned slot.
    */
   virtual void SetAssignedSlot(mozilla::dom::HTMLSlotElement* aSlot) = 0;
 
+  nsIContent* GetXBLInsertionParent() const
+  {
+    nsIContent* ip = GetXBLInsertionPoint();
+    return ip ? ip->GetParent() : nullptr;
+  }
+
   /**
    * Gets the insertion parent element of the XBL binding.
    * The insertion parent is our one true parent in the transformed DOM.
    *
    * @return the insertion parent element.
    */
-  virtual nsIContent *GetXBLInsertionParent() const = 0;
+  virtual nsIContent* GetXBLInsertionPoint() const = 0;
 
   /**
    * Sets the insertion parent element of the XBL binding.
    *
    * @param aContent The insertion parent element.
    */
-  virtual void SetXBLInsertionParent(nsIContent* aContent) = 0;
+  virtual void SetXBLInsertionPoint(nsIContent* aContent) = 0;
 
   /**
    * Same as GetFlattenedTreeParentNode, but returns null if the parent is
    * non-nsIContent.
    */
   inline nsIContent *GetFlattenedTreeParent() const;
 
   // Helper method, which we leave public so that it's accessible from nsINode.
--- a/dom/xbl/XBLChildrenElement.h
+++ b/dom/xbl/XBLChildrenElement.h
@@ -36,27 +36,27 @@ public:
   virtual nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult,
                          bool aPreallocateChildren) const override;
 
   virtual nsIDOMNode* AsDOMNode() override { return this; }
 
   void AppendInsertedChild(nsIContent* aChild)
   {
     mInsertedChildren.AppendElement(aChild);
-    aChild->SetXBLInsertionParent(GetParent());
+    aChild->SetXBLInsertionPoint(this);
 
     // Appending an inserted child causes the inserted
     // children to be projected instead of default content.
     MaybeRemoveDefaultContent();
   }
 
   void InsertInsertedChildAt(nsIContent* aChild, uint32_t aIndex)
   {
     mInsertedChildren.InsertElementAt(aIndex, aChild);
-    aChild->SetXBLInsertionParent(GetParent());
+    aChild->SetXBLInsertionPoint(this);
 
     // Inserting an inserted child causes the inserted
     // children to be projected instead of default content.
     MaybeRemoveDefaultContent();
   }
 
   void RemoveInsertedChild(nsIContent* aChild)
   {
@@ -68,44 +68,44 @@ public:
 
     // After removing the inserted child, default content
     // may be projected into this insertion point.
     MaybeSetupDefaultContent();
   }
 
   void ClearInsertedChildren()
   {
-    for (uint32_t c = 0; c < mInsertedChildren.Length(); ++c) {
-      mInsertedChildren[c]->SetXBLInsertionParent(nullptr);
+    for (auto* child : mInsertedChildren) {
+      child->SetXBLInsertionPoint(nullptr);
     }
     mInsertedChildren.Clear();
 
     // After clearing inserted children, default content
     // will be projected into this insertion point.
     MaybeSetupDefaultContent();
   }
 
   void MaybeSetupDefaultContent()
   {
     if (!HasInsertedChildren()) {
       for (nsIContent* child = static_cast<nsINode*>(this)->GetFirstChild();
            child;
            child = child->GetNextSibling()) {
-        child->SetXBLInsertionParent(GetParent());
+        child->SetXBLInsertionPoint(this);
       }
     }
   }
 
   void MaybeRemoveDefaultContent()
   {
     if (!HasInsertedChildren()) {
       for (nsIContent* child = static_cast<nsINode*>(this)->GetFirstChild();
            child;
            child = child->GetNextSibling()) {
-        child->SetXBLInsertionParent(nullptr);
+        child->SetXBLInsertionPoint(nullptr);
       }
     }
   }
 
   uint32_t InsertedChildrenLength()
   {
     return mInsertedChildren.Length();
   }
--- a/dom/xbl/nsBindingManager.cpp
+++ b/dom/xbl/nsBindingManager.cpp
@@ -214,18 +214,18 @@ nsBindingManager::RemovedFromDocumentInt
     if (!mDestroyed && aDestructorHandling == eRunDtor) {
       binding->PrototypeBinding()->BindingDetached(binding->GetBoundElement());
       binding->ChangeDocument(aOldDocument, nullptr);
     }
 
     aContent->SetXBLBinding(nullptr, this);
   }
 
-  // Clear out insertion parent and content lists.
-  aContent->SetXBLInsertionParent(nullptr);
+  // Clear out insertion point and content lists.
+  aContent->SetXBLInsertionPoint(nullptr);
 }
 
 nsAtom*
 nsBindingManager::ResolveTag(nsIContent* aContent, int32_t* aNameSpaceID)
 {
   nsXBLBinding *binding = aContent->GetXBLBinding();
 
   if (binding) {
@@ -910,17 +910,17 @@ nsBindingManager::ContentInserted(nsIDoc
 }
 
 void
 nsBindingManager::ContentRemoved(nsIDocument* aDocument,
                                  nsIContent* aContainer,
                                  nsIContent* aChild,
                                  nsIContent* aPreviousSibling)
 {
-  aChild->SetXBLInsertionParent(nullptr);
+  aChild->SetXBLInsertionPoint(nullptr);
 
   XBLChildrenElement* point = nullptr;
   nsIContent* parent = aContainer;
 
   // Handle appending of default content.
   if (parent && parent->IsActiveChildrenElement()) {
     XBLChildrenElement* childrenEl = static_cast<XBLChildrenElement*>(parent);
     if (childrenEl->HasInsertedChildren()) {
--- a/dom/xbl/nsXBLBinding.cpp
+++ b/dom/xbl/nsXBLBinding.cpp
@@ -462,18 +462,18 @@ nsXBLBinding::FindInsertionPointForInter
 
 void
 nsXBLBinding::ClearInsertionPoints()
 {
   if (mDefaultInsertionPoint) {
     mDefaultInsertionPoint->ClearInsertedChildren();
   }
 
-  for (uint32_t i = 0; i < mInsertionPoints.Length(); ++i) {
-    mInsertionPoints[i]->ClearInsertedChildren();
+  for (const auto& insertionPoint : mInsertionPoints) {
+    insertionPoint->ClearInsertedChildren();
   }
 }
 
 nsAnonymousContentList*
 nsXBLBinding::GetAnonymousNodeList()
 {
   if (!mContent) {
     return mNextBinding ? mNextBinding->GetAnonymousNodeList() : nullptr;
@@ -703,44 +703,16 @@ nsXBLBinding::UnhookEventHandlers()
         flags.mInSystemGroup = true;
       }
 
       manager->RemoveEventListenerByType(handler, type, flags);
     }
   }
 }
 
-static void
-UpdateInsertionParent(XBLChildrenElement* aPoint,
-                      nsIContent* aOldBoundElement)
-{
-  if (aPoint->IsDefaultInsertion()) {
-    return;
-  }
-
-  for (size_t i = 0; i < aPoint->InsertedChildrenLength(); ++i) {
-    nsIContent* child = aPoint->InsertedChild(i);
-
-    MOZ_ASSERT(child->GetParentNode());
-
-    // Here, we're iterating children that we inserted. There are two cases:
-    // either |child| is an explicit child of |aOldBoundElement| and is no
-    // longer inserted anywhere or it's a child of a <children> element
-    // parented to |aOldBoundElement|. In the former case, the child is no
-    // longer inserted anywhere, so we set its insertion parent to null. In the
-    // latter case, the child is now inserted into |aOldBoundElement| from some
-    // binding above us, so we set its insertion parent to aOldBoundElement.
-    if (child->GetParentNode() == aOldBoundElement) {
-      child->SetXBLInsertionParent(nullptr);
-    } else {
-      child->SetXBLInsertionParent(aOldBoundElement);
-    }
-  }
-}
-
 void
 nsXBLBinding::ChangeDocument(nsIDocument* aOldDocument, nsIDocument* aNewDocument)
 {
   if (aOldDocument == aNewDocument)
     return;
 
   // Now the binding dies.  Unhook our prototypes.
   if (mPrototypeBinding->HasImplementation()) {
@@ -824,29 +796,16 @@ nsXBLBinding::ChangeDocument(nsIDocument
     }
 
     // Update the anonymous content.
     // XXXbz why not only for style bindings?
     if (mContent && !mIsShadowRootBinding) {
       nsXBLBinding::UnbindAnonymousContent(aOldDocument, mContent);
     }
 
-    // Now that we've unbound our anonymous content from the tree and updated
-    // its binding parent, update the insertion parent for content inserted
-    // into our <children> elements.
-    if (mDefaultInsertionPoint) {
-      UpdateInsertionParent(mDefaultInsertionPoint, mBoundElement);
-    }
-
-    for (size_t i = 0; i < mInsertionPoints.Length(); ++i) {
-      UpdateInsertionParent(mInsertionPoints[i], mBoundElement);
-    }
-
-    // Now that our inserted children no longer think they're inserted
-    // anywhere, make sure our internal state reflects that as well.
     ClearInsertionPoints();
   }
 }
 
 bool
 nsXBLBinding::InheritsStyle() const
 {
   // XXX Will have to change if we ever allow multiple bindings to contribute anonymous content.