Bug 1375189 - Don't use aNodesWithProperties in nsNodeUtils::CloneAndAdopt if it is not needed r=bz
authorKirk Steuber <ksteuber@mozilla.com>
Wed, 21 Jun 2017 11:55:04 -0700
changeset 365665 98537bce9286be0d6d764ca7a201ed843ce3c236
parent 365664 a1f253f8a9f93cb804f55acdeb14855e12fe2c2b
child 365666 de9bb450199d86dc0b6610a8a9cf96f52ed3d841
push id91809
push usercbook@mozilla.com
push dateFri, 23 Jun 2017 09:44:41 +0000
treeherdermozilla-inbound@ab1d1b0135fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1375189
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1375189 - Don't use aNodesWithProperties in nsNodeUtils::CloneAndAdopt if it is not needed r=bz 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
@@ -6530,19 +6530,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->IsAnyOfSVGElements(nsGkAtoms::svg, 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);