diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 97059586c8..7d7d72910a 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -5823,14 +5823,9 @@ nsIDocument::ImportNode(nsINode& aNode, bool aDeep, ErrorResult& rv) const case nsIDOMNode::COMMENT_NODE: case nsIDOMNode::DOCUMENT_TYPE_NODE: { - nsCOMPtr newNode; nsCOMArray nodesWithProperties; - rv = nsNodeUtils::Clone(imported, aDeep, mNodeInfoManager, - nodesWithProperties, getter_AddRefs(newNode)); - if (rv.Failed()) { - return nullptr; - } - return newNode.forget(); + return nsNodeUtils::Clone(imported, aDeep, mNodeInfoManager, + &nodesWithProperties, rv); } default: { @@ -6996,8 +6991,8 @@ nsIDocument::AdoptNode(nsINode& aAdoptedNode, ErrorResult& rv) } nsCOMArray nodesWithProperties; - rv = nsNodeUtils::Adopt(adoptedNode, sameDocument ? nullptr : mNodeInfoManager, - newScope, nodesWithProperties); + nsNodeUtils::Adopt(adoptedNode, sameDocument ? nullptr : mNodeInfoManager, + newScope, nodesWithProperties, rv); if (rv.Failed()) { // Disconnect all nodes from their parents, since some have the old document // as their ownerDocument and some have this as their ownerDocument. diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp index 37980db614..46b019b943 100644 --- a/dom/base/nsINode.cpp +++ b/dom/base/nsINode.cpp @@ -3084,9 +3084,7 @@ nsINode::WrapObject(JSContext *aCx, JS::Handle aGivenProto) already_AddRefed nsINode::CloneNode(bool aDeep, ErrorResult& aError) { - nsCOMPtr result; - aError = nsNodeUtils::CloneNodeImpl(this, aDeep, getter_AddRefs(result)); - return result.forget(); + return nsNodeUtils::CloneNodeImpl(this, aDeep, aError); } nsDOMAttributeMap* diff --git a/dom/base/nsNodeUtils.cpp b/dom/base/nsNodeUtils.cpp index 63da8b04df..3eefc544f8 100644 --- a/dom/base/nsNodeUtils.cpp +++ b/dom/base/nsNodeUtils.cpp @@ -404,28 +404,20 @@ nsNodeUtils::TraverseUserData(nsINode* aNode, } /* static */ -nsresult -nsNodeUtils::CloneNodeImpl(nsINode *aNode, bool aDeep, nsINode **aResult) +already_AddRefed +nsNodeUtils::CloneNodeImpl(nsINode *aNode, bool aDeep, ErrorResult& aError) { - *aResult = nullptr; - - nsCOMPtr newNode; nsCOMArray nodesWithProperties; - nsresult rv = Clone(aNode, aDeep, nullptr, nodesWithProperties, - getter_AddRefs(newNode)); - NS_ENSURE_SUCCESS(rv, rv); - - newNode.forget(aResult); - return NS_OK; + return Clone(aNode, aDeep, nullptr, &nodesWithProperties, aError); } /* static */ -nsresult +already_AddRefed nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, nsNodeInfoManager *aNewNodeInfoManager, JS::Handle aReparentScope, - nsCOMArray &aNodesWithProperties, - nsINode *aParent, nsINode **aResult) + nsCOMArray *aNodesWithProperties, + nsINode *aParent, ErrorResult& aError) { NS_PRECONDITION((!aClone && aNewNodeInfoManager) || !aReparentScope, "If cloning or not getting a new nodeinfo we shouldn't " @@ -433,14 +425,11 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, NS_PRECONDITION(!aParent || aNode->IsNodeOfType(nsINode::eCONTENT), "Can't insert document or attribute nodes into a parent"); - *aResult = nullptr; - // First deal with aNode and walk its attributes (and their children). Then, // if aDeep is true, deal with aNode's children (and recurse into their // attributes and children). nsAutoScriptBlocker scriptBlocker; - nsresult rv; nsNodeInfoManager *nodeInfoManager = aNewNodeInfoManager; @@ -452,14 +441,20 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, // Don't allow importing/adopting nodes from non-privileged "scriptable" // documents to "non-scriptable" documents. nsIDocument* newDoc = nodeInfoManager->GetDocument(); - NS_ENSURE_STATE(newDoc); + if (NS_WARN_IF(!newDoc)) { + aError.Throw(NS_ERROR_UNEXPECTED); + return nullptr; + } bool hasHadScriptHandlingObject = false; if (!newDoc->GetScriptHandlingObject(hasHadScriptHandlingObject) && !hasHadScriptHandlingObject) { nsIDocument* currentDoc = aNode->OwnerDoc(); - NS_ENSURE_STATE((nsContentUtils::IsChromeDoc(currentDoc) || - (!currentDoc->GetScriptHandlingObject(hasHadScriptHandlingObject) && - !hasHadScriptHandlingObject))); + if (NS_WARN_IF(!nsContentUtils::IsChromeDoc(currentDoc) && + (currentDoc->GetScriptHandlingObject(hasHadScriptHandlingObject) || + hasHadScriptHandlingObject))) { + aError.Throw(NS_ERROR_UNEXPECTED); + return nullptr; + } } newNodeInfo = nodeInfoManager->GetNodeInfo(nodeInfo->NameAtom(), @@ -475,8 +470,11 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, nsCOMPtr clone; if (aClone) { - rv = aNode->Clone(nodeInfo, getter_AddRefs(clone)); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = aNode->Clone(nodeInfo, getter_AddRefs(clone)); + if (NS_WARN_IF(NS_FAILED(rv))) { + aError.Throw(rv); + return nullptr; + } if (CustomElementRegistry::IsCustomElementEnabled() && clone->IsHTMLElement()) { @@ -514,7 +512,10 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, // parent. rv = aParent->AppendChildTo(static_cast(clone.get()), false); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + aError.Throw(rv); + return nullptr; + } } else if (aDeep && clone->IsNodeOfType(nsINode::eDOCUMENT)) { // After cloning the document itself, we want to clone the children into @@ -613,7 +614,7 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, if ((wrapper = aNode->GetWrapper())) { MOZ_ASSERT(IsDOMObject(wrapper)); JSAutoCompartment ac(cx, wrapper); - rv = ReparentWrapper(cx, wrapper); + nsresult rv = ReparentWrapper(cx, wrapper); if (NS_FAILED(rv)) { if (wasRegistered) { aNode->OwnerDoc()->UnregisterActivityObserver(aNode->AsElement()); @@ -622,17 +623,18 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, if (wasRegistered) { aNode->OwnerDoc()->RegisterActivityObserver(aNode->AsElement()); } - return rv; + aError.Throw(rv); + return nullptr; } } } } if (aNode->HasProperties()) { - bool ok = aNodesWithProperties.AppendObject(aNode); + bool ok = aNodesWithProperties->AppendObject(aNode); MOZ_RELEASE_ASSERT(ok, "Out of memory"); if (aClone) { - ok = aNodesWithProperties.AppendObject(clone); + ok = aNodesWithProperties->AppendObject(clone); MOZ_RELEASE_ASSERT(ok, "Out of memory"); } } @@ -642,22 +644,24 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, for (nsIContent* cloneChild = aNode->GetFirstChild(); cloneChild; cloneChild = cloneChild->GetNextSibling()) { - nsCOMPtr child; - rv = CloneAndAdopt(cloneChild, aClone, true, nodeInfoManager, - aReparentScope, aNodesWithProperties, clone, - getter_AddRefs(child)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr child = + CloneAndAdopt(cloneChild, aClone, true, nodeInfoManager, + aReparentScope, aNodesWithProperties, clone, + aError); + if (NS_WARN_IF(aError.Failed())) { + return nullptr; + } } } - // TODO: update this if we choose to land bug 1393806. if (aDeep && !aClone && aNode->IsElement()) { if (ShadowRoot* shadowRoot = aNode->AsElement()->GetShadowRoot()) { - nsCOMPtr child; - rv = CloneAndAdopt(shadowRoot, aClone, aDeep, nodeInfoManager, - aReparentScope, aNodesWithProperties, clone, - getter_AddRefs(child)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr child = CloneAndAdopt(shadowRoot, aClone, aDeep, nodeInfoManager, + aReparentScope, aNodesWithProperties, clone, + aError); + if (NS_WARN_IF(aError.Failed())) { + return nullptr; + } } } @@ -676,11 +680,13 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, for (nsIContent* cloneChild = origContent->GetFirstChild(); cloneChild; cloneChild = cloneChild->GetNextSibling()) { - nsCOMPtr child; - rv = CloneAndAdopt(cloneChild, aClone, aDeep, ownerNodeInfoManager, - aReparentScope, aNodesWithProperties, cloneContent, - getter_AddRefs(child)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr child = + CloneAndAdopt(cloneChild, aClone, aDeep, ownerNodeInfoManager, + aReparentScope, aNodesWithProperties, cloneContent, + aError); + if (NS_WARN_IF(aError.Failed())) { + return nullptr; + } } } @@ -703,9 +709,7 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, } #endif - clone.forget(aResult); - - return NS_OK; + return clone.forget(); } diff --git a/dom/base/nsNodeUtils.h b/dom/base/nsNodeUtils.h index 971f8d17c7..6f6f293d0f 100644 --- a/dom/base/nsNodeUtils.h +++ b/dom/base/nsNodeUtils.h @@ -17,6 +17,7 @@ template class nsCOMArray; class nsCycleCollectionTraversalCallback; namespace mozilla { struct NonOwningAnimationTarget; +class ErrorResult; namespace dom { class Animation; } // namespace dom @@ -181,25 +182,17 @@ public: * @param aNodesWithProperties All nodes (from amongst aNode and its * descendants) with properties. Every node will * be followed by its clone. - * @param aResult *aResult will contain the cloned node. + * @param aError The error, if any. + * + * @return The newly created node. Null in error conditions. */ - static nsresult Clone(nsINode *aNode, bool aDeep, - nsNodeInfoManager *aNewNodeInfoManager, - nsCOMArray &aNodesWithProperties, - nsINode **aResult) + static already_AddRefed Clone(nsINode *aNode, bool aDeep, + nsNodeInfoManager *aNewNodeInfoManager, + nsCOMArray *aNodesWithProperties, + mozilla::ErrorResult& aError) { 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 dummyNodeWithProperties; - return CloneAndAdopt(aNode, true, aDeep, nullptr, nullptr, - dummyNodeWithProperties, aNode->GetParent(), aResult); + nullptr, aNodesWithProperties, nullptr, aError); } /** @@ -218,19 +211,20 @@ public: * should be done. * @param aNodesWithProperties All nodes (from amongst aNode and its * descendants) with properties. + * @param aError The error, if any. */ - static nsresult Adopt(nsINode *aNode, nsNodeInfoManager *aNewNodeInfoManager, - JS::Handle aReparentScope, - nsCOMArray &aNodesWithProperties) + static void Adopt(nsINode *aNode, nsNodeInfoManager *aNewNodeInfoManager, + JS::Handle aReparentScope, + nsCOMArray& aNodesWithProperties, + mozilla::ErrorResult& aError) { - nsCOMPtr node; - nsresult rv = CloneAndAdopt(aNode, false, true, aNewNodeInfoManager, - aReparentScope, aNodesWithProperties, - nullptr, getter_AddRefs(node)); + // Just need to store the return value of CloneAndAdopt in a + // temporary nsCOMPtr to make sure we release it. + nsCOMPtr node = CloneAndAdopt(aNode, false, true, aNewNodeInfoManager, + aReparentScope, &aNodesWithProperties, + nullptr, aError); nsMutationGuard::DidMutate(); - - return rv; } /** @@ -248,9 +242,12 @@ public: * * @param aNode the node to clone * @param aDeep if true all descendants will be cloned too - * @param aResult the clone + * @param aError the error, if any. + * + * @return the clone, or null if an error occurs. */ - static nsresult CloneNodeImpl(nsINode *aNode, bool aDeep, nsINode **aResult); + static already_AddRefed CloneNodeImpl(nsINode *aNode, bool aDeep, + mozilla::ErrorResult& aError); /** * Release the UserData for aNode. @@ -286,7 +283,7 @@ private: * * @param aNode Node to adopt/clone. * @param aClone If true the node will be cloned and the cloned node will - * be in aResult. + * be returned. * @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 @@ -302,14 +299,18 @@ private: * @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. + * @param aError The error, if any. + * + * @return If aClone is true then the cloned node will be returned, + * unless an error occurred. In error conditions, null + * will be returned. */ - static nsresult CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, - nsNodeInfoManager *aNewNodeInfoManager, - JS::Handle aReparentScope, - nsCOMArray &aNodesWithProperties, - nsINode *aParent, nsINode **aResult); + static already_AddRefed + CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, + nsNodeInfoManager* aNewNodeInfoManager, + JS::Handle aReparentScope, + nsCOMArray *aNodesWithProperties, + nsINode *aParent, mozilla::ErrorResult& aError); enum class AnimationMutationType { diff --git a/dom/svg/SVGUseElement.cpp b/dom/svg/SVGUseElement.cpp index aceac1bb8a..31470ede23 100644 --- a/dom/svg/SVGUseElement.cpp +++ b/dom/svg/SVGUseElement.cpp @@ -259,14 +259,12 @@ SVGUseElement::CreateAnonymousContent() } } - nsCOMPtr newnode; - nsCOMArray unused; nsNodeInfoManager* nodeInfoManager = targetContent->OwnerDoc() == OwnerDoc() ? nullptr : OwnerDoc()->NodeInfoManager(); - nsNodeUtils::Clone(targetContent, true, nodeInfoManager, unused, - getter_AddRefs(newnode)); - + IgnoredErrorResult rv; + nsCOMPtr newnode = + nsNodeUtils::Clone(targetContent, true, nodeInfoManager, nullptr, rv); nsCOMPtr newcontent = do_QueryInterface(newnode); if (!newcontent) diff --git a/dom/xbl/nsXBLBinding.cpp b/dom/xbl/nsXBLBinding.cpp index b7e0d9094c..49e23965d2 100644 --- a/dom/xbl/nsXBLBinding.cpp +++ b/dom/xbl/nsXBLBinding.cpp @@ -321,10 +321,11 @@ nsXBLBinding::GenerateAnonymousContent() if (hasContent) { nsIDocument* doc = mBoundElement->OwnerDoc(); - nsCOMPtr clonedNode; - nsCOMArray nodesWithProperties; - nsNodeUtils::Clone(content, true, doc->NodeInfoManager(), - nodesWithProperties, getter_AddRefs(clonedNode)); + IgnoredErrorResult rv; + nsCOMPtr clonedNode = + nsNodeUtils::Clone(content, true, doc->NodeInfoManager(), nullptr, rv); + // FIXME: Bug 1399558, Why is this code OK assuming that nsNodeUtils::Clone + // never fails? mContent = clonedNode->AsElement(); // Search for elements in the XBL content. In the presence