Bug 1026008 Incorrect XBL behaviour with multiple insertion points r=mrbkap

content/base/src/ChildIterator.cpp

@@ -25,19 +25,19 @@ public:
   uint32_t Length() const
   {
     return mIsContentElement ? mContentElement->MatchedNodes().Length()
-                             : mChildrenElement->mInsertedChildren.Length();
+                             : mChildrenElement->InsertedChildrenLength();
   }
 
   nsIContent* operator[](int32_t aIndex) const
   {
     return mIsContentElement ? mContentElement->MatchedNodes()[aIndex]
-                             : mChildrenElement->mInsertedChildren[aIndex];
+                             : mChildrenElement->InsertedChild(aIndex);
   }
 
   bool IsEmpty() const
   {
     return mIsContentElement ? mContentElement->MatchedNodes().IsEmpty()
-                             : mChildrenElement->mInsertedChildren.IsEmpty();
+                             : !mChildrenElement->HasInsertedChildren();
   }
 protected:
   bool mIsContentElement;

dom/xbl/XBLChildrenElement.cpp

@@ -103,8 +103,8 @@ nsAnonymousContentList::GetLength(uint32_t* aLength)
        child = child->GetNextSibling()) {
     if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
       XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);
-      if (!point->mInsertedChildren.IsEmpty()) {
-        count += point->mInsertedChildren.Length();
+      if (point->HasInsertedChildren()) {
+        count += point->InsertedChildrenLength();
       }
       else {
         count += point->GetChildCount();
@@ -144,11 +144,11 @@ nsAnonymousContentList::Item(uint32_t aIndex)
        child = child->GetNextSibling()) {
     if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
       XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);
-      if (!point->mInsertedChildren.IsEmpty()) {
-        if (remIndex < point->mInsertedChildren.Length()) {
-          return point->mInsertedChildren[remIndex];
+      if (point->HasInsertedChildren()) {
+        if (remIndex < point->InsertedChildrenLength()) {
+          return point->InsertedChild(remIndex);
         }
-        remIndex -= point->mInsertedChildren.Length();
+        remIndex -= point->InsertedChildrenLength();
       }
       else {
         if (remIndex < point->GetChildCount()) {
@@ -179,23 +179,23 @@ nsAnonymousContentList::IndexOf(nsIContent* aContent)
     return -1;
   }
 
-  size_t index = 0;
+  int32_t index = 0;
   for (nsIContent* child = mParent->GetFirstChild();
        child;
        child = child->GetNextSibling()) {
     if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
       XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);
-      if (!point->mInsertedChildren.IsEmpty()) {
-        size_t insIndex = point->mInsertedChildren.IndexOf(aContent);
-        if (insIndex != point->mInsertedChildren.NoIndex) {
+      if (point->HasInsertedChildren()) {
+        int32_t insIndex = point->IndexOfInsertedChild(aContent);
+        if (insIndex != -1) {
           return index + insIndex;
         }
-        index += point->mInsertedChildren.Length();
+        index += point->InsertedChildrenLength();
       }
       else {
         int32_t insIndex = point->IndexOf(aContent);
         if (insIndex != -1) {
-          return index + (size_t)insIndex;
+          return index + insIndex;
         }
         index += point->GetChildCount();
       }

dom/xbl/XBLChildrenElement.h

@@ -22,8 +22,6 @@ class ExplicitChildIterator;
 class XBLChildrenElement : public nsXMLElement
 {
 public:
-  friend class mozilla::dom::ExplicitChildIterator;
-  friend class nsAnonymousContentList;
   XBLChildrenElement(already_AddRefed<nsINodeInfo>& aNodeInfo)
     : nsXMLElement(aNodeInfo)
   {
@@ -115,10 +113,7 @@ public:
     return !mInsertedChildren.IsEmpty();
   }
 
-  enum {
-    NoIndex = uint32_t(-1)
-  };
-  uint32_t IndexOfInsertedChild(nsIContent* aChild)
+  int32_t IndexOfInsertedChild(nsIContent* aChild)
   {
     return mInsertedChildren.IndexOf(aChild);
   }
@@ -135,9 +130,13 @@ public:
     return mIncludes.IsEmpty();
   }
 
-  nsTArray<nsIContent*> mInsertedChildren;
+  nsIContent* InsertedChild(uint32_t aIndex)
+  {
+    return mInsertedChildren[aIndex];
+  }
 
 private:
+  nsCOMArray<nsIContent> mInsertedChildren;
   nsTArray<nsCOMPtr<nsIAtom> > mIncludes;
 };
 

dom/xbl/nsBindingManager.cpp

@@ -834,12 +834,12 @@ static void
 InsertAppendedContent(XBLChildrenElement* aPoint,
                       nsIContent* aFirstNewContent)
 {
-  size_t insertionIndex;
+  int32_t insertionIndex;
   if (nsIContent* prevSibling = aFirstNewContent->GetPreviousSibling()) {
     // If we have a previous sibling, then it must already be in aPoint. Find
     // it and insert after it.
     insertionIndex = aPoint->IndexOfInsertedChild(prevSibling);
-    MOZ_ASSERT(insertionIndex != aPoint->NoIndex);
+    MOZ_ASSERT(insertionIndex != -1);
 
     // Our insertion index is one after our previous sibling's index.
     ++insertionIndex;
@@ -847,7 +847,7 @@ InsertAppendedContent(XBLChildrenElement* aPoint,
     // Otherwise, we append.
     // TODO This is wrong for nested insertion points. In that case, we need to
     // keep track of the right index to insert into.
-    insertionIndex = aPoint->mInsertedChildren.Length();
+    insertionIndex = aPoint->InsertedChildrenLength();
   }
 
   // Do the inserting.
@@ -1077,15 +1077,15 @@ nsBindingManager::HandleChildInsertion(nsIContent* aContainer,
     // TODO If there were multiple insertion points, this approximation can be
     // wrong. We need to re-run the distribution algorithm. In the meantime,
     // this should work well enough.
-    size_t index = aAppend ? point->mInsertedChildren.Length() : 0;
+    uint32_t index = aAppend ? point->InsertedChildrenLength() : 0;
     for (nsIContent* currentSibling = aChild->GetPreviousSibling();
          currentSibling;
          currentSibling = currentSibling->GetPreviousSibling()) {
       // If we find one of our previous siblings in the insertion point, the
       // index following it is the correct insertion point. Otherwise, we guess
       // based on whether we're appending or inserting.
-      size_t pointIndex = point->IndexOfInsertedChild(currentSibling);
-      if (pointIndex != point->NoIndex) {
+      int32_t pointIndex = point->IndexOfInsertedChild(currentSibling);
+      if (pointIndex != -1) {
         index = pointIndex + 1;
         break;
       }

dom/xbl/nsXBLBinding.cpp

@@ -702,7 +702,7 @@ UpdateInsertionParent(XBLChildrenElement* aPoint,
   }
 
   for (size_t i = 0; i < aPoint->InsertedChildrenLength(); ++i) {
-    nsIContent* child = aPoint->mInsertedChildren[i];
+    nsIContent* child = aPoint->InsertedChild(i);
 
     MOZ_ASSERT(child->GetParentNode());