Bug 1375189 - Don't use aNodesWithProperties in nsNodeUtils::CloneAndAdopt if it is not needed draft
authorKirk Steuber <ksteuber@mozilla.com>
Wed, 21 Jun 2017 11:55:04 -0700
changeset 598506 bebfa00123b1a8e15a370ea8ebc981e4c58346ed
parent 597455 464b2a3c25aa1065760d9ecbb0870bca4a66c62e
child 634488 7123340fac574e25b41afb2ebe10cb6916c5cf62
push id65216
push userksteuber@mozilla.com
push dateWed, 21 Jun 2017 18:55:44 +0000
bugs1375189
milestone56.0a1
Bug 1375189 - Don't use aNodesWithProperties in nsNodeUtils::CloneAndAdopt if it is not needed MozReview-Commit-ID: 9vu3HCQkDKf
dom/base/nsDocument.cpp
dom/base/nsNodeUtils.cpp
dom/base/nsNodeUtils.h
dom/svg/SVGUseElement.cpp
dom/xbl/nsXBLBinding.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -6526,19 +6526,18 @@ nsIDocument::ImportNode(nsINode& aNode, 
     case nsIDOMNode::ELEMENT_NODE:
     case nsIDOMNode::PROCESSING_INSTRUCTION_NODE:
     case nsIDOMNode::TEXT_NODE:
     case nsIDOMNode::CDATA_SECTION_NODE:
     case nsIDOMNode::COMMENT_NODE:
     case nsIDOMNode::DOCUMENT_TYPE_NODE:
     {
       nsCOMPtr<nsINode> newNode;
-      nsCOMArray<nsINode> nodesWithProperties;
-      rv = nsNodeUtils::Clone(imported, aDeep, mNodeInfoManager,
-                              nodesWithProperties, getter_AddRefs(newNode));
+      rv = nsNodeUtils::Clone(imported, aDeep, mNodeInfoManager, nullptr,
+                              getter_AddRefs(newNode));
       if (rv.Failed()) {
         return nullptr;
       }
       return newNode.forget();
     }
     default:
     {
       NS_WARNING("Don't know how to clone this nodetype for importNode.");
--- a/dom/base/nsNodeUtils.cpp
+++ b/dom/base/nsNodeUtils.cpp
@@ -403,31 +403,29 @@ nsNodeUtils::TraverseUserData(nsINode* a
 
 /* static */
 nsresult
 nsNodeUtils::CloneNodeImpl(nsINode *aNode, bool aDeep, nsINode **aResult)
 {
   *aResult = nullptr;
 
   nsCOMPtr<nsINode> newNode;
-  nsCOMArray<nsINode> nodesWithProperties;
-  nsresult rv = Clone(aNode, aDeep, nullptr, nodesWithProperties,
-                      getter_AddRefs(newNode));
+  nsresult rv = Clone(aNode, aDeep, nullptr, nullptr, getter_AddRefs(newNode));
   NS_ENSURE_SUCCESS(rv, rv);
 
   newNode.forget(aResult);
   return NS_OK;
 }
 
 /* static */
 nsresult
 nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep,
                            nsNodeInfoManager *aNewNodeInfoManager,
                            JS::Handle<JSObject*> aReparentScope,
-                           nsCOMArray<nsINode> &aNodesWithProperties,
+                           nsCOMArray<nsINode> *aNodesWithProperties,
                            nsINode *aParent, nsINode **aResult)
 {
   NS_PRECONDITION((!aClone && aNewNodeInfoManager) || !aReparentScope,
                   "If cloning or not getting a new nodeinfo we shouldn't "
                   "rewrap");
   NS_PRECONDITION(!aParent || aNode->IsNodeOfType(nsINode::eCONTENT),
                   "Can't insert document or attribute nodes into a parent");
 
@@ -645,20 +643,20 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNod
 #ifdef MOZ_XUL
   if (aClone && !aParent && aNode->IsXULElement()) {
     if (!aNode->OwnerDoc()->IsLoadedAsInteractiveData()) {
       clone->SetFlags(NODE_FORCE_XBL_BINDINGS);
     }
   }
 #endif
 
-  if (aNode->HasProperties()) {
-    bool ok = aNodesWithProperties.AppendObject(aNode);
+  if (aNodesWithProperties && aNode->HasProperties()) {
+    bool ok = aNodesWithProperties->AppendObject(aNode);
     if (aClone) {
-      ok = ok && aNodesWithProperties.AppendObject(clone);
+      ok = ok && aNodesWithProperties->AppendObject(clone);
     }
 
     NS_ENSURE_TRUE(ok, NS_ERROR_OUT_OF_MEMORY);
   }
 
   clone.forget(aResult);
 
   return NS_OK;
--- a/dom/base/nsNodeUtils.h
+++ b/dom/base/nsNodeUtils.h
@@ -176,36 +176,36 @@ public:
    * @param aDeep If true the function will be called recursively on
    *              descendants of the node
    * @param aNewNodeInfoManager The nodeinfo manager to use to create new
    *                            nodeinfos for aNode and its attributes and
    *                            descendants. May be null if the nodeinfos
    *                            shouldn't be changed.
    * @param aNodesWithProperties All nodes (from amongst aNode and its
    *                             descendants) with properties. Every node will
-   *                             be followed by its clone.
+   *                             be followed by its clone. Null can be passed to
+   *                             prevent this from being used.
    * @param aResult *aResult will contain the cloned node.
    */
   static nsresult Clone(nsINode *aNode, bool aDeep,
                         nsNodeInfoManager *aNewNodeInfoManager,
-                        nsCOMArray<nsINode> &aNodesWithProperties,
+                        nsCOMArray<nsINode> *aNodesWithProperties,
                         nsINode **aResult)
   {
     return CloneAndAdopt(aNode, true, aDeep, aNewNodeInfoManager,
                          nullptr, aNodesWithProperties, nullptr, aResult);
   }
 
   /**
    * Clones aNode, its attributes and, if aDeep is true, its descendant nodes
    */
   static nsresult Clone(nsINode *aNode, bool aDeep, nsINode **aResult)
   {
-    nsCOMArray<nsINode> dummyNodeWithProperties;
-    return CloneAndAdopt(aNode, true, aDeep, nullptr, nullptr,
-                         dummyNodeWithProperties, aNode->GetParent(), aResult);
+    return CloneAndAdopt(aNode, true, aDeep, nullptr, nullptr, nullptr,
+                         aNode->GetParent(), aResult);
   }
 
   /**
    * Walks aNode, its attributes and descendant nodes. If aNewNodeInfoManager is
    * not null, it is used to create new nodeinfos for the nodes. Also reparents
    * the XPConnect wrappers for the nodes into aReparentScope if non-null.
    * aNodesWithProperties will be filled with all the nodes that have
    * properties.
@@ -221,17 +221,17 @@ public:
    *                             descendants) with properties.
    */
   static nsresult Adopt(nsINode *aNode, nsNodeInfoManager *aNewNodeInfoManager,
                         JS::Handle<JSObject*> aReparentScope,
                         nsCOMArray<nsINode> &aNodesWithProperties)
   {
     nsCOMPtr<nsINode> node;
     nsresult rv = CloneAndAdopt(aNode, false, true, aNewNodeInfoManager,
-                                aReparentScope, aNodesWithProperties,
+                                aReparentScope, &aNodesWithProperties,
                                 nullptr, getter_AddRefs(node));
 
     nsMutationGuard::DidMutate();
 
     return rv;
   }
 
   /**
@@ -294,27 +294,28 @@ private:
    *                            nodeinfos for aNode and its attributes and
    *                            descendants. May be null if the nodeinfos
    *                            shouldn't be changed.
    * @param aReparentScope Scope into which wrappers should be reparented, or
    *                             null if no reparenting should be done.
    * @param aNodesWithProperties All nodes (from amongst aNode and its
    *                             descendants) with properties. If aClone is
    *                             true every node will be followed by its
-   *                             clone.
+   *                             clone. Null can be passed to prevent this from
+   *                             being populated.
    * @param aParent If aClone is true the cloned node will be appended to
    *                aParent's children. May be null. If not null then aNode
    *                must be an nsIContent.
    * @param aResult If aClone is true then *aResult will contain the cloned
    *                node.
    */
   static nsresult CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep,
                                 nsNodeInfoManager *aNewNodeInfoManager,
                                 JS::Handle<JSObject*> aReparentScope,
-                                nsCOMArray<nsINode> &aNodesWithProperties,
+                                nsCOMArray<nsINode> *aNodesWithProperties,
                                 nsINode *aParent, nsINode **aResult);
 
   enum class AnimationMutationType
   {
     Added,
     Changed,
     Removed
   };
--- a/dom/svg/SVGUseElement.cpp
+++ b/dom/svg/SVGUseElement.cpp
@@ -258,21 +258,20 @@ SVGUseElement::CreateAnonymousContent()
       if (content->IsSVGElement(nsGkAtoms::use) &&
           static_cast<SVGUseElement*>(content.get())->mOriginal == mOriginal) {
         return nullptr;
       }
     }
   }
 
   nsCOMPtr<nsINode> newnode;
-  nsCOMArray<nsINode> unused;
   nsNodeInfoManager* nodeInfoManager =
     targetContent->OwnerDoc() == OwnerDoc() ?
       nullptr : OwnerDoc()->NodeInfoManager();
-  nsNodeUtils::Clone(targetContent, true, nodeInfoManager, unused,
+  nsNodeUtils::Clone(targetContent, true, nodeInfoManager, nullptr,
                      getter_AddRefs(newnode));
 
   nsCOMPtr<nsIContent> newcontent = do_QueryInterface(newnode);
 
   if (!newcontent)
     return nullptr;
 
   if (newcontent->IsSVGElement(nsGkAtoms::symbol)) {
--- a/dom/xbl/nsXBLBinding.cpp
+++ b/dom/xbl/nsXBLBinding.cpp
@@ -316,19 +316,18 @@ nsXBLBinding::GenerateAnonymousContent()
   uint32_t contentCount = content->GetChildCount();
 
   // Plan to build the content by default.
   bool hasContent = (contentCount > 0);
   if (hasContent) {
     nsIDocument* doc = mBoundElement->OwnerDoc();
 
     nsCOMPtr<nsINode> clonedNode;
-    nsCOMArray<nsINode> nodesWithProperties;
-    nsNodeUtils::Clone(content, true, doc->NodeInfoManager(),
-                       nodesWithProperties, getter_AddRefs(clonedNode));
+    nsNodeUtils::Clone(content, true, doc->NodeInfoManager(), nullptr,
+                       getter_AddRefs(clonedNode));
     mContent = clonedNode->AsElement();
 
     // Search for <xbl:children> elements in the XBL content. In the presence
     // of multiple default insertion points, we use the last one in document
     // order.
     for (nsIContent* child = mContent; child; child = child->GetNextNode(mContent)) {
       if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
         XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);