Bug 1309720 - Ensure expected DOM tree operations when calling insertBefore. r=ehsan, a=abillings
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Fri, 14 Oct 2016 15:33:42 +0300
changeset 348640 af995f34ee2e5342db8f33ae3d2b9f164ad6273e
parent 348639 6d65311378a748b84f98ede85d2c541addd8f266
child 348641 752c1320cca5a9757d0d159a3c8d303160aae0b0
push id6506
push userryanvm@gmail.com
push dateFri, 14 Oct 2016 19:02:44 +0000
treeherdermozilla-beta@752c1320cca5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan, abillings
bugs1309720
milestone50.0
Bug 1309720 - Ensure expected DOM tree operations when calling insertBefore. r=ehsan, a=abillings
dom/base/Element.cpp
dom/base/nsTreeSanitizer.cpp
dom/html/HTMLSelectElement.cpp
dom/html/HTMLTableElement.cpp
dom/html/HTMLTableElement.h
dom/html/HTMLTableSectionElement.cpp
dom/html/UndoManager.cpp
editor/libeditor/CreateElementTransaction.cpp
editor/libeditor/DeleteNodeTransaction.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/JoinNodeTransaction.cpp
editor/libeditor/SplitNodeTransaction.cpp
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3670,25 +3670,27 @@ Element::InsertAdjacent(const nsAString&
 {
   if (aWhere.LowerCaseEqualsLiteral("beforebegin")) {
     nsCOMPtr<nsINode> parent = GetParentNode();
     if (!parent) {
       return nullptr;
     }
     parent->InsertBefore(*aNode, this, aError);
   } else if (aWhere.LowerCaseEqualsLiteral("afterbegin")) {
-    static_cast<nsINode*>(this)->InsertBefore(*aNode, GetFirstChild(), aError);
+    nsCOMPtr<nsINode> refNode = GetFirstChild();
+    static_cast<nsINode*>(this)->InsertBefore(*aNode, refNode, aError);
   } else if (aWhere.LowerCaseEqualsLiteral("beforeend")) {
     static_cast<nsINode*>(this)->AppendChild(*aNode, aError);
   } else if (aWhere.LowerCaseEqualsLiteral("afterend")) {
     nsCOMPtr<nsINode> parent = GetParentNode();
     if (!parent) {
       return nullptr;
     }
-    parent->InsertBefore(*aNode, GetNextSibling(), aError);
+    nsCOMPtr<nsINode> refNode = GetNextSibling();
+    parent->InsertBefore(*aNode, refNode, aError);
   } else {
     aError.Throw(NS_ERROR_DOM_SYNTAX_ERR);
     return nullptr;
   }
 
   return aError.Failed() ? nullptr : aNode;
 }
 
--- a/dom/base/nsTreeSanitizer.cpp
+++ b/dom/base/nsTreeSanitizer.cpp
@@ -1411,17 +1411,18 @@ nsTreeSanitizer::SanitizeChildren(nsINod
       }
       if (MustFlatten(ns, localName)) {
         RemoveAllAttributes(node);
         nsCOMPtr<nsIContent> next = node->GetNextNode(aRoot);
         nsCOMPtr<nsIContent> parent = node->GetParent();
         nsCOMPtr<nsIContent> child; // Must keep the child alive during move
         ErrorResult rv;
         while ((child = node->GetFirstChild())) {
-          parent->InsertBefore(*child, node, rv);
+          nsCOMPtr<nsINode> refNode = node;
+          parent->InsertBefore(*child, refNode, rv);
           if (rv.Failed()) {
             break;
           }
         }
         node->RemoveFromParent();
         node = next;
         continue;
       }
--- a/dom/html/HTMLSelectElement.cpp
+++ b/dom/html/HTMLSelectElement.cpp
@@ -582,17 +582,18 @@ HTMLSelectElement::Add(nsGenericHTMLElem
     // NOT_FOUND_ERR: Raised if before is not a descendant of the SELECT
     // element.
     aError.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
     return;
   }
 
   // If the before parameter is not null, we are equivalent to the
   // insertBefore method on the parent of before.
-  parent->InsertBefore(aElement, aBefore, aError);
+  nsCOMPtr<nsINode> refNode = aBefore;
+  parent->InsertBefore(aElement, refNode, aError);
 }
 
 NS_IMETHODIMP
 HTMLSelectElement::Add(nsIDOMHTMLElement* aElement,
                        nsIVariant* aBefore)
 {
   uint16_t dataType;
   nsresult rv = aBefore->GetDataType(&dataType);
--- a/dom/html/HTMLTableElement.cpp
+++ b/dom/html/HTMLTableElement.cpp
@@ -407,17 +407,18 @@ HTMLTableElement::CreateTHead()
                                 getter_AddRefs(nodeInfo));
 
     head = NS_NewHTMLTableSectionElement(nodeInfo.forget());
     if (!head) {
       return nullptr;
     }
 
     ErrorResult rv;
-    nsINode::InsertBefore(*head, nsINode::GetFirstChild(), rv);
+    nsCOMPtr<nsINode> refNode = nsINode::GetFirstChild();
+    nsINode::InsertBefore(*head, refNode, rv);
   }
   return head.forget();
 }
 
 void
 HTMLTableElement::DeleteTHead()
 {
   HTMLTableSectionElement* tHead = GetTHead();
@@ -498,17 +499,17 @@ HTMLTableElement::CreateTBody()
                                                kNameSpaceID_XHTML,
                                                nsIDOMNode::ELEMENT_NODE);
   MOZ_ASSERT(nodeInfo);
 
   RefPtr<nsGenericHTMLElement> newBody =
     NS_NewHTMLTableSectionElement(nodeInfo.forget());
   MOZ_ASSERT(newBody);
 
-  nsIContent* referenceNode = nullptr;
+  nsCOMPtr<nsIContent> referenceNode = nullptr;
   for (nsIContent* child = nsINode::GetLastChild();
        child;
        child = child->GetPreviousSibling()) {
     if (child->IsHTMLElement(nsGkAtoms::tbody)) {
       referenceNode = child->GetNextSibling();
       break;
     }
   }
@@ -610,17 +611,18 @@ HTMLTableElement::InsertRow(int32_t aInd
       nsContentUtils::NameChanged(mNodeInfo, nsGkAtoms::tr,
                                   getter_AddRefs(nodeInfo));
 
       newRow = NS_NewHTMLTableRowElement(nodeInfo.forget());
       if (newRow) {
         HTMLTableSectionElement* section =
           static_cast<HTMLTableSectionElement*>(rowGroup.get());
         nsIHTMLCollection* rows = section->Rows();
-        rowGroup->InsertBefore(*newRow, rows->Item(0), aError);
+        nsCOMPtr<nsINode> refNode = rows->Item(0);
+        rowGroup->InsertBefore(*newRow, refNode, aError);
       }
     }
   }
 
   return newRow.forget();
 }
 
 void
--- a/dom/html/HTMLTableElement.h
+++ b/dom/html/HTMLTableElement.h
@@ -54,17 +54,18 @@ public:
   {
     if (aTHead && !aTHead->IsHTMLElement(nsGkAtoms::thead)) {
       aError.Throw(NS_ERROR_DOM_HIERARCHY_REQUEST_ERR);
       return;
     }
 
     DeleteTHead();
     if (aTHead) {
-      nsINode::InsertBefore(*aTHead, nsINode::GetFirstChild(), aError);
+      nsCOMPtr<nsINode> refNode = nsINode::GetFirstChild();
+      nsINode::InsertBefore(*aTHead, refNode, aError);
     }
   }
   already_AddRefed<nsGenericHTMLElement> CreateTHead();
 
   void DeleteTHead();
 
   HTMLTableSectionElement* GetTFoot() const
   {
--- a/dom/html/HTMLTableSectionElement.cpp
+++ b/dom/html/HTMLTableSectionElement.cpp
@@ -86,17 +86,18 @@ HTMLTableSectionElement::InsertRow(int32
   RefPtr<nsGenericHTMLElement> rowContent =
     NS_NewHTMLTableRowElement(nodeInfo.forget());
   if (!rowContent) {
     aError.Throw(NS_ERROR_OUT_OF_MEMORY);
     return nullptr;
   }
 
   if (doInsert) {
-    nsINode::InsertBefore(*rowContent, rows->Item(aIndex), aError);
+    nsCOMPtr<nsINode> refNode = rows->Item(aIndex);
+    nsINode::InsertBefore(*rowContent, refNode, aError);
   } else {
     nsINode::AppendChild(*rowContent, aError);
   }
   return rowContent.forget();
 }
 
 void
 HTMLTableSectionElement::DeleteRow(int32_t aValue, ErrorResult& aError)
--- a/dom/html/UndoManager.cpp
+++ b/dom/html/UndoManager.cpp
@@ -438,17 +438,18 @@ UndoContentInsert::RedoTransaction()
   }
 
   // Check to see if next sibling has same parent.
   if (mNextNode && mNextNode->GetParentNode() != mContent) {
     return NS_OK;
   }
 
   IgnoredErrorResult error;
-  mContent->InsertBefore(*mChild, mNextNode, error);
+  nsCOMPtr<nsIContent> refNode = mNextNode;
+  mContent->InsertBefore(*mChild, refNode, error);
   return NS_OK;
 }
 
 nsresult
 UndoContentInsert::UndoTransaction()
 {
   if (!mChild) {
     return NS_ERROR_UNEXPECTED;
@@ -532,17 +533,18 @@ UndoContentRemove::UndoTransaction()
   }
 
   // Make sure next sibling is still under same parent.
   if (mNextNode && mNextNode->GetParentNode() != mContent) {
     return NS_OK;
   }
 
   IgnoredErrorResult error;
-  mContent->InsertBefore(*mChild, mNextNode, error);
+  nsCOMPtr<nsIContent> refNode = mNextNode;
+  mContent->InsertBefore(*mChild, refNode, error);
   return NS_OK;
 }
 
 nsresult
 UndoContentRemove::RedoTransaction()
 {
   if (!mChild) {
     return NS_ERROR_UNEXPECTED;
--- a/editor/libeditor/CreateElementTransaction.cpp
+++ b/editor/libeditor/CreateElementTransaction.cpp
@@ -79,17 +79,18 @@ CreateElementTransaction::DoTransaction(
   }
 
   mOffsetInParent = std::min(mOffsetInParent,
                              static_cast<int32_t>(mParent->GetChildCount()));
 
   // Note, it's ok for mRefNode to be null. That means append
   mRefNode = mParent->GetChildAt(mOffsetInParent);
 
-  mParent->InsertBefore(*mNewNode, mRefNode, rv);
+  nsCOMPtr<nsIContent> refNode = mRefNode;
+  mParent->InsertBefore(*mNewNode, refNode, rv);
   NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
 
   // Only set selection to insertion point if editor gives permission
   if (!mEditorBase->GetShouldTxnSetSelection()) {
     // Do nothing - DOM range gravity will adjust selection
     return NS_OK;
   }
 
@@ -119,17 +120,18 @@ CreateElementTransaction::RedoTransactio
   MOZ_ASSERT(mEditorBase && mParent);
 
   // First, reset mNewNode so it has no attributes or content
   // XXX We never actually did this, we only cleared mNewNode's contents if it
   // was a CharacterData node (which it's not, it's an Element)
 
   // Now, reinsert mNewNode
   ErrorResult rv;
-  mParent->InsertBefore(*mNewNode, mRefNode, rv);
+  nsCOMPtr<nsIContent> refNode = mRefNode;
+  mParent->InsertBefore(*mNewNode, refNode, rv);
   return rv.StealNSResult();
 }
 
 NS_IMETHODIMP
 CreateElementTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("CreateElementTransaction: ");
   aString += nsDependentAtomString(mTag);
--- a/editor/libeditor/DeleteNodeTransaction.cpp
+++ b/editor/libeditor/DeleteNodeTransaction.cpp
@@ -83,17 +83,18 @@ DeleteNodeTransaction::UndoTransaction()
     // this is a legal state, the txn is a no-op
     return NS_OK;
   }
   if (!mNode) {
     return NS_ERROR_NULL_POINTER;
   }
 
   ErrorResult error;
-  mParent->InsertBefore(*mNode, mRefNode, error);
+  nsCOMPtr<nsIContent> refNode = mRefNode;
+  mParent->InsertBefore(*mNode, refNode, error);
   return error.StealNSResult();
 }
 
 NS_IMETHODIMP
 DeleteNodeTransaction::RedoTransaction()
 {
   if (!mParent) {
     // this is a legal state, the txn is a no-op
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2699,17 +2699,18 @@ EditorBase::SplitNodeImpl(nsIContent& aE
       savedRanges.AppendElement(range);
     }
   }
 
   nsCOMPtr<nsINode> parent = aExistingRightNode.GetParentNode();
   NS_ENSURE_TRUE(parent, NS_ERROR_NULL_POINTER);
 
   ErrorResult rv;
-  parent->InsertBefore(aNewLeftNode, &aExistingRightNode, rv);
+  nsCOMPtr<nsINode> refNode = &aExistingRightNode;
+  parent->InsertBefore(aNewLeftNode, refNode, rv);
   NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
 
   // Split the children between the two nodes.  At this point,
   // aExistingRightNode has all the children.  Move all the children whose
   // index is < aOffset to aNewLeftNode.
   if (aOffset < 0) {
     // This means move no children
     return NS_OK;
--- a/editor/libeditor/JoinNodeTransaction.cpp
+++ b/editor/libeditor/JoinNodeTransaction.cpp
@@ -89,17 +89,18 @@ JoinNodeTransaction::UndoTransaction()
         return NS_ERROR_NULL_POINTER;
       }
       nsCOMPtr<nsIContent> nextSibling = child->GetNextSibling();
       mLeftNode->AppendChild(*child, rv);
       child = nextSibling;
     }
   }
   // Second, re-insert the left node into the tree
-  mParent->InsertBefore(*mLeftNode, mRightNode, rv);
+  nsCOMPtr<nsINode> refNode = mRightNode;
+  mParent->InsertBefore(*mLeftNode, refNode, rv);
   return rv.StealNSResult();
 }
 
 NS_IMETHODIMP
 JoinNodeTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("JoinNodeTransaction");
   return NS_OK;
--- a/editor/libeditor/SplitNodeTransaction.cpp
+++ b/editor/libeditor/SplitNodeTransaction.cpp
@@ -101,17 +101,18 @@ SplitNodeTransaction::RedoTransaction()
       mExistingRightNode->RemoveChild(*child, rv);
       if (!rv.Failed()) {
         mNewLeftNode->AppendChild(*child, rv);
       }
       child = nextSibling;
     }
   }
   // Second, re-insert the left node into the tree
-  mParent->InsertBefore(*mNewLeftNode, mExistingRightNode, rv);
+  nsCOMPtr<nsIContent> refNode = mExistingRightNode;
+  mParent->InsertBefore(*mNewLeftNode, refNode, rv);
   return rv.StealNSResult();
 }
 
 
 NS_IMETHODIMP
 SplitNodeTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("SplitNodeTransaction");