From 1fa5bedc44c4458d04272b13a1848c84c0af3c37 Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Sun, 23 Feb 2025 07:54:05 +0000 Subject: [PATCH] Bug 1941002 - Make XPath tree walker hold a strong reference. r=smaug a=pascalc Differential Revision: https://phabricator.services.mozilla.com/D237259 --- dom/xslt/xml/txXMLParser.cpp | 25 +++---- dom/xslt/xml/txXMLParser.h | 11 +-- dom/xslt/xpath/XPathExpression.cpp | 6 +- dom/xslt/xpath/XPathResult.cpp | 4 +- dom/xslt/xpath/txForwardContext.h | 2 +- dom/xslt/xpath/txMozillaXPathTreeWalker.cpp | 58 +++------------- dom/xslt/xpath/txNodeSet.cpp | 12 ++-- dom/xslt/xpath/txNodeSet.h | 8 ++- dom/xslt/xpath/txSingleNodeContext.h | 3 +- dom/xslt/xpath/txXPathNode.h | 77 +++++++++++++++------ dom/xslt/xpath/txXPathTreeWalker.h | 33 +-------- dom/xslt/xslt/txEXSLTFunctions.cpp | 17 ++--- dom/xslt/xslt/txExecutionState.cpp | 61 ++++++---------- dom/xslt/xslt/txExecutionState.h | 35 ++-------- dom/xslt/xslt/txMozillaXSLTProcessor.cpp | 16 ++--- dom/xslt/xslt/txXSLTPatterns.cpp | 5 +- 16 files changed, 149 insertions(+), 224 deletions(-) diff --git a/dom/xslt/xml/txXMLParser.cpp b/dom/xslt/xml/txXMLParser.cpp index 779e198fb1da..1d576cdbacfc 100644 --- a/dom/xslt/xml/txXMLParser.cpp +++ b/dom/xslt/xml/txXMLParser.cpp @@ -12,16 +12,15 @@ #include "nsNetUtil.h" #include "nsIURI.h" +using namespace mozilla; using namespace mozilla::dom; -nsresult txParseDocumentFromURI(const nsAString& aHref, - const txXPathNode& aLoader, nsAString& aErrMsg, - txXPathNode** aResult) { - NS_ENSURE_ARG_POINTER(aResult); - *aResult = nullptr; +Result txParseDocumentFromURI(const nsAString& aHref, + const txXPathNode& aLoader, + nsAString& aErrMsg) { nsCOMPtr documentURI; nsresult rv = NS_NewURI(getter_AddRefs(documentURI), aHref); - NS_ENSURE_SUCCESS(rv, rv); + NS_ENSURE_SUCCESS(rv, Err(rv)); Document* loaderDocument = txXPathNativeNode::getDocument(aLoader); @@ -32,7 +31,7 @@ nsresult txParseDocumentFromURI(const nsAString& aHref, // Raw pointer, we want the resulting txXPathNode to hold a reference to // the document. - Document* theDocument = nullptr; + nsCOMPtr theDocument; nsAutoSyncOperation sync(loaderDocument, SyncOperationBehavior::eSuspendInput); rv = nsSyncLoadService::LoadDocument( @@ -40,20 +39,14 @@ nsresult txParseDocumentFromURI(const nsAString& aHref, loaderDocument->NodePrincipal(), nsILoadInfo::SEC_REQUIRE_CORS_INHERITS_SEC_CONTEXT, loadGroup, loaderDocument->CookieJarSettings(), true, - loaderDocument->GetReferrerPolicy(), &theDocument); + loaderDocument->GetReferrerPolicy(), getter_AddRefs(theDocument)); if (NS_FAILED(rv)) { aErrMsg.AppendLiteral("Document load of "); aErrMsg.Append(aHref); aErrMsg.AppendLiteral(" failed."); - return NS_FAILED(rv) ? rv : NS_ERROR_FAILURE; + return Err(rv); } - *aResult = txXPathNativeNode::createXPathNode(theDocument); - if (!*aResult) { - NS_RELEASE(theDocument); - return NS_ERROR_FAILURE; - } - - return NS_OK; + return txXPathNode(theDocument); } diff --git a/dom/xslt/xml/txXMLParser.h b/dom/xslt/xml/txXMLParser.h index 83f22215062b..ba800a7b2b39 100644 --- a/dom/xslt/xml/txXMLParser.h +++ b/dom/xslt/xml/txXMLParser.h @@ -10,6 +10,11 @@ class txXPathNode; +namespace mozilla { +template +class Result; +} // namespace mozilla + /** * API to load XML files into DOM datastructures. * Parsing is either done by expat, or by expat via the syncloaderservice @@ -19,9 +24,7 @@ class txXPathNode; * Parse a document from the aHref location, with referrer URI on behalf * of the document aLoader. */ -extern "C" nsresult txParseDocumentFromURI(const nsAString& aHref, - const txXPathNode& aLoader, - nsAString& aErrMsg, - txXPathNode** aResult); +mozilla::Result txParseDocumentFromURI( + const nsAString& aHref, const txXPathNode& aLoader, nsAString& aErrMsg); #endif diff --git a/dom/xslt/xpath/XPathExpression.cpp b/dom/xslt/xpath/XPathExpression.cpp index 0706763d79f2..bb290e77730f 100644 --- a/dom/xslt/xpath/XPathExpression.cpp +++ b/dom/xslt/xpath/XPathExpression.cpp @@ -21,7 +21,7 @@ namespace mozilla::dom { -class EvalContextImpl : public txIEvalContext { +class MOZ_STACK_CLASS EvalContextImpl : public txIEvalContext { public: EvalContextImpl(const txXPathNode& aContextNode, uint32_t aContextPosition, uint32_t aContextSize, txResultRecycler* aRecycler) @@ -121,8 +121,8 @@ already_AddRefed XPathExpression::EvaluateWithContext( return nullptr; } - UniquePtr contextNode( - txXPathNativeNode::createXPathNode(&aContextNode)); + Maybe contextNode = + txXPathNativeNode::createXPathNode(&aContextNode); if (!contextNode) { aRv.Throw(NS_ERROR_FAILURE); return nullptr; diff --git a/dom/xslt/xpath/XPathResult.cpp b/dom/xslt/xpath/XPathResult.cpp index 091a83528ff7..1fa82db486e8 100644 --- a/dom/xslt/xpath/XPathResult.cpp +++ b/dom/xslt/xpath/XPathResult.cpp @@ -238,13 +238,13 @@ nsresult XPathResult::GetExprResult(txAExprResult** aExprResult) { RefPtr nodeSet = new txNodeSet(nullptr); uint32_t i, count = mResultNodes.Length(); for (i = 0; i < count; ++i) { - UniquePtr node( + Maybe node( txXPathNativeNode::createXPathNode(mResultNodes[i])); if (!node) { return NS_ERROR_OUT_OF_MEMORY; } - nodeSet->append(*node); + nodeSet->append(node.extract()); } NS_ADDREF(*aExprResult = nodeSet); diff --git a/dom/xslt/xpath/txForwardContext.h b/dom/xslt/xpath/txForwardContext.h index 447f2fbfc133..5acca4d35c85 100644 --- a/dom/xslt/xpath/txForwardContext.h +++ b/dom/xslt/xpath/txForwardContext.h @@ -9,7 +9,7 @@ #include "txIXPathContext.h" #include "txNodeSet.h" -class txForwardContext : public txIEvalContext { +class MOZ_STACK_CLASS txForwardContext : public txIEvalContext { public: txForwardContext(txIMatchContext* aContext, const txXPathNode& aContextNode, txNodeSet* aContextNodeSet) diff --git a/dom/xslt/xpath/txMozillaXPathTreeWalker.cpp b/dom/xslt/xpath/txMozillaXPathTreeWalker.cpp index 8150d7307e5a..6fdd33f05531 100644 --- a/dom/xslt/xpath/txMozillaXPathTreeWalker.cpp +++ b/dom/xslt/xpath/txMozillaXPathTreeWalker.cpp @@ -23,8 +23,8 @@ #include #include +using namespace mozilla; using namespace mozilla::dom; -using mozilla::Maybe; txXPathTreeWalker::txXPathTreeWalker(const txXPathTreeWalker& aOther) = default; @@ -228,24 +228,6 @@ bool txXPathTreeWalker::moveToParent() { return true; } -txXPathNode::txXPathNode(const txXPathNode& aNode) - : mNode(aNode.mNode), - mRefCountRoot(aNode.mRefCountRoot), - mIndex(aNode.mIndex) { - MOZ_COUNT_CTOR(txXPathNode); - if (mRefCountRoot) { - NS_ADDREF(Root()); - } -} - -txXPathNode::~txXPathNode() { - MOZ_COUNT_DTOR(txXPathNode); - if (mRefCountRoot) { - nsINode* root = Root(); - NS_RELEASE(root); - } -} - /* static */ bool txXPathNodeUtils::getAttr(const txXPathNode& aNode, nsAtom* aLocalName, int32_t aNSID, nsAString& aValue) { @@ -443,8 +425,8 @@ bool txXPathNodeUtils::isWhitespace(const txXPathNode& aNode) { } /* static */ -txXPathNode* txXPathNodeUtils::getOwnerDocument(const txXPathNode& aNode) { - return new txXPathNode(aNode.mNode->OwnerDoc()); +txXPathNode txXPathNodeUtils::getOwnerDocument(const txXPathNode& aNode) { + return txXPathNode(aNode.mNode->OwnerDoc()); } const char gPrintfFmt[] = "id0x%" PRIxPTR; @@ -454,7 +436,8 @@ const char gPrintfFmtAttr[] = "id0x%" PRIxPTR "-%010i"; nsresult txXPathNodeUtils::getXSLTId(const txXPathNode& aNode, const txXPathNode& aBase, nsAString& aResult) { - uintptr_t nodeid = ((uintptr_t)aNode.mNode) - ((uintptr_t)aBase.mNode); + uintptr_t nodeid = + ((uintptr_t)aNode.mNode.get()) - ((uintptr_t)aBase.mNode.get()); if (!aNode.isAttribute()) { CopyASCIItoUTF16(nsPrintfCString(gPrintfFmt, nodeid), aResult); } else { @@ -585,16 +568,7 @@ int txXPathNodeUtils::comparePosition(const txXPathNode& aNode, } /* static */ -txXPathNode* txXPathNativeNode::createXPathNode(nsIContent* aContent, - bool aKeepRootAlive) { - nsINode* root = aKeepRootAlive ? txXPathNode::RootOf(aContent) : nullptr; - - return new txXPathNode(aContent, txXPathNode::eContent, root); -} - -/* static */ -txXPathNode* txXPathNativeNode::createXPathNode(nsINode* aNode, - bool aKeepRootAlive) { +Maybe txXPathNativeNode::createXPathNode(nsINode* aNode) { uint16_t nodeType = aNode->NodeType(); if (nodeType == nsINode::ATTRIBUTE_NODE) { auto* attr = static_cast(aNode); @@ -602,42 +576,30 @@ txXPathNode* txXPathNativeNode::createXPathNode(nsINode* aNode, NodeInfo* nodeInfo = attr->NodeInfo(); Element* parent = attr->GetElement(); if (!parent) { - return nullptr; + return Nothing(); } - nsINode* root = aKeepRootAlive ? txXPathNode::RootOf(parent) : nullptr; - uint32_t i, total = parent->GetAttrCount(); for (i = 0; i < total; ++i) { const nsAttrName* name = parent->GetAttrNameAt(i); if (nodeInfo->Equals(name->LocalName(), name->NamespaceID())) { - return new txXPathNode(parent, i, root); + return Some(txXPathNode(parent, i)); } } NS_ERROR("Couldn't find the attribute in its parent!"); - return nullptr; + return Nothing(); } uint32_t index; - nsINode* root = aKeepRootAlive ? aNode : nullptr; - if (nodeType == nsINode::DOCUMENT_NODE) { index = txXPathNode::eDocument; } else { index = txXPathNode::eContent; - if (root) { - root = txXPathNode::RootOf(root); - } } - return new txXPathNode(aNode, index, root); -} - -/* static */ -txXPathNode* txXPathNativeNode::createXPathNode(Document* aDocument) { - return new txXPathNode(aDocument); + return Some(txXPathNode(aNode, index)); } /* static */ diff --git a/dom/xslt/xpath/txNodeSet.cpp b/dom/xslt/xpath/txNodeSet.cpp index 60eed8952d3d..6b88299facfe 100644 --- a/dom/xslt/xpath/txNodeSet.cpp +++ b/dom/xslt/xpath/txNodeSet.cpp @@ -82,12 +82,12 @@ txNodeSet::~txNodeSet() { } } -nsresult txNodeSet::add(const txXPathNode& aNode) { +nsresult txNodeSet::add(txXPathNode&& aNode) { NS_ASSERTION(mDirection == kForward, "only append(aNode) is supported on reversed nodesets"); if (isEmpty()) { - return append(aNode); + return append(std::move(aNode)); } bool dupe; @@ -111,7 +111,7 @@ nsresult txNodeSet::add(const txXPathNode& aNode) { memmove(pos + 1, pos, moveSize * sizeof(txXPathNode)); } - new (pos) txXPathNode(aNode); + new (pos) txXPathNode(std::move(aNode)); ++mEnd; return NS_OK; @@ -293,19 +293,19 @@ nsresult txNodeSet::add(const txNodeSet& aNodes, transferOp aTransfer, * order info operations will be performed. */ -nsresult txNodeSet::append(const txXPathNode& aNode) { +nsresult txNodeSet::append(txXPathNode&& aNode) { if (!ensureGrowSize(1)) { return NS_ERROR_OUT_OF_MEMORY; } if (mDirection == kForward) { - new (mEnd) txXPathNode(aNode); + new (mEnd) txXPathNode(std::move(aNode)); ++mEnd; return NS_OK; } - new (--mStart) txXPathNode(aNode); + new (--mStart) txXPathNode(std::move(aNode)); return NS_OK; } diff --git a/dom/xslt/xpath/txNodeSet.h b/dom/xslt/xpath/txNodeSet.h index 8c5e805d6bcf..5885799923a1 100644 --- a/dom/xslt/xpath/txNodeSet.h +++ b/dom/xslt/xpath/txNodeSet.h @@ -44,7 +44,8 @@ class txNodeSet : public txAExprResult { * @param aNode the txXPathNode to add to the NodeSet * @return errorcode. */ - nsresult add(const txXPathNode& aNode); + nsresult add(const txXPathNode& aNode) { return add(txXPathNode(aNode)); } + nsresult add(txXPathNode&& aNode); /** * Adds the nodes in specified NodeSet to this NodeSet. The resulting @@ -72,7 +73,10 @@ class txNodeSet : public txAExprResult { * @param aNode the Node to append to the NodeSet * @return errorcode. */ - nsresult append(const txXPathNode& aNode); + nsresult append(const txXPathNode& aNode) { + return append(txXPathNode(aNode)); + } + nsresult append(txXPathNode&& aNode); /** * Appends the nodes in the specified NodeSet to the end of this NodeSet diff --git a/dom/xslt/xpath/txSingleNodeContext.h b/dom/xslt/xpath/txSingleNodeContext.h index 80b0680a2dd8..e6c1c5ee5596 100644 --- a/dom/xslt/xpath/txSingleNodeContext.h +++ b/dom/xslt/xpath/txSingleNodeContext.h @@ -8,6 +8,7 @@ #include "mozilla/Attributes.h" #include "txIXPathContext.h" +#include "txXPathNode.h" class txSingleNodeContext : public txIEvalContext { public: @@ -57,7 +58,7 @@ class txSingleNodeContext : public txIEvalContext { uint32_t position() override { return 1; } private: - const txXPathNode& mNode; + txXPathNode mNode; txIMatchContext* mInner; }; diff --git a/dom/xslt/xpath/txXPathNode.h b/dom/xslt/xpath/txXPathNode.h index 8c5d6f530838..ea7d1f6220c9 100644 --- a/dom/xslt/xpath/txXPathNode.h +++ b/dom/xslt/xpath/txXPathNode.h @@ -13,35 +13,70 @@ using txXPathNodeType = nsINode; +/** + * txXPathNode represents a node in XPath's data model (which is a bit different + * from the DOM's). While XPath 1.0 has 7 node types, we essentially deal with 3 + * kinds: a document node, any other node type that is backed by an + * implementation of nsIContent in Gecko, and attribute nodes. Because we try to + * avoid creating actual node objects for attribute nodes in Gecko's DOM, we + * store attribute nodes as a pointer to their owner element and an index into + * that element's attribute node list. So to represent the 3 kinds of node we + * need to store: + * + * - a pointer to a document (a nsIDocument as a nsINode pointer) + * - a pointer to a nsIContent object (as a nsINode pointer) + * - a pointer to a nsIContent object (a nsIContent as a nsINode pointer) and + * an index + * + * So we make txXPathNode store a |nsCOMPtr| and a uint32_t for the + * index. To be able to distinguish between the attribute nodes and other nodes + * we store a flag value for the other nodes in the index. To be able to quickly + * cast from the nsINode pointer to a nsIDocument or nsIContent pointer we + * actually use 2 different flag values (see txXPathNode::PositionType). + * + * We provide 2 fast constructors for txXPathNode, one that takes a nsIContent + * and one that takes a nsIDocument, since for those kinds of nodes we can + * simply store the pointer and the relevant flag value. For attribute nodes, or + * if you have just a nsINode pointer, then there is a helper function + * (txXPathNativeNode::createXPathNode) that either picks the right flag value + * or the correct index (for attribute nodes) when creating the txXPathNode. + */ class txXPathNode { public: + explicit txXPathNode(const txXPathNode& aNode) + : mNode(aNode.mNode), mIndex(aNode.mIndex) { + MOZ_COUNT_CTOR(txXPathNode); + } + txXPathNode(txXPathNode&& aNode) + : mNode(std::move(aNode.mNode)), mIndex(aNode.mIndex) { + MOZ_COUNT_CTOR(txXPathNode); + } + + explicit txXPathNode(mozilla::dom::Document* aDocument) + : mNode(aDocument), mIndex(eDocument) { + MOZ_COUNT_CTOR(txXPathNode); + } + explicit txXPathNode(nsIContent* aContent) + : mNode(aContent), mIndex(eContent) { + MOZ_COUNT_CTOR(txXPathNode); + } + + txXPathNode& operator=(txXPathNode&& aOther) = default; bool operator==(const txXPathNode& aNode) const; bool operator!=(const txXPathNode& aNode) const { return !(*this == aNode); } - ~txXPathNode(); + ~txXPathNode() { MOZ_COUNT_DTOR(txXPathNode); } private: - friend class txNodeSet; friend class txXPathNativeNode; friend class txXPathNodeUtils; friend class txXPathTreeWalker; - txXPathNode(const txXPathNode& aNode); - - explicit txXPathNode(mozilla::dom::Document* aDocument) - : mNode(aDocument), mRefCountRoot(0), mIndex(eDocument) { + txXPathNode(nsINode* aNode, uint32_t aIndex) : mNode(aNode), mIndex(aIndex) { MOZ_COUNT_CTOR(txXPathNode); } - txXPathNode(nsINode* aNode, uint32_t aIndex, nsINode* aRoot) - : mNode(aNode), mRefCountRoot(aRoot ? 1 : 0), mIndex(aIndex) { - MOZ_COUNT_CTOR(txXPathNode); - if (aRoot) { - NS_ADDREF(aRoot); - } - } static nsINode* RootOf(nsINode* aNode) { return aNode->SubtreeRoot(); } nsINode* Root() const { return RootOf(mNode); } - nsINode* GetRootToAddRef() const { return mRefCountRoot ? Root() : nullptr; } bool isDocument() const { return mIndex == eDocument; } bool isContent() const { return mIndex == eContent; } @@ -49,18 +84,20 @@ class txXPathNode { nsIContent* Content() const { NS_ASSERTION(isContent() || isAttribute(), "wrong type"); - return static_cast(mNode); + return static_cast(mNode.get()); } mozilla::dom::Document* Document() const { NS_ASSERTION(isDocument(), "wrong type"); - return static_cast(mNode); + return static_cast(mNode.get()); } - enum PositionType { eDocument = (1 << 30), eContent = eDocument - 1 }; + enum PositionType : uint32_t { + eDocument = UINT32_MAX, + eContent = eDocument - 1 + }; - nsINode* mNode; - uint32_t mRefCountRoot : 1; - uint32_t mIndex : 31; + nsCOMPtr mNode; + uint32_t mIndex; }; class txNamespaceManager { diff --git a/dom/xslt/xpath/txXPathTreeWalker.h b/dom/xslt/xpath/txXPathTreeWalker.h index 6b74b1e18d8f..e6bc14ae9984 100644 --- a/dom/xslt/xpath/txXPathTreeWalker.h +++ b/dom/xslt/xpath/txXPathTreeWalker.h @@ -60,7 +60,7 @@ class txXPathNodeUtils { static uint16_t getNodeType(const txXPathNode& aNode); static void appendNodeValue(const txXPathNode& aNode, nsAString& aResult); static bool isWhitespace(const txXPathNode& aNode); - static txXPathNode* getOwnerDocument(const txXPathNode& aNode); + static txXPathNode getOwnerDocument(const txXPathNode& aNode); static int32_t getUniqueIdentifier(const txXPathNode& aNode); static nsresult getXSLTId(const txXPathNode& aNode, const txXPathNode& aBase, nsAString& aResult); @@ -86,19 +86,10 @@ class txXPathNodeUtils { class txXPathNativeNode { public: - static txXPathNode* createXPathNode(nsINode* aNode, - bool aKeepRootAlive = false); - static txXPathNode* createXPathNode(nsIContent* aContent, - bool aKeepRootAlive = false); - static txXPathNode* createXPathNode(mozilla::dom::Document* aDocument); + static mozilla::Maybe createXPathNode(nsINode* aNode); static nsINode* getNode(const txXPathNode& aNode); static nsIContent* getContent(const txXPathNode& aNode); static mozilla::dom::Document* getDocument(const txXPathNode& aNode); - static void addRef(const txXPathNode& aNode) { NS_ADDREF(aNode.mNode); } - static void release(const txXPathNode& aNode) { - nsINode* node = aNode.mNode; - NS_RELEASE(node); - } }; inline const txXPathNode& txXPathTreeWalker::getCurrentPosition() const { @@ -123,21 +114,8 @@ inline void txXPathTreeWalker::getNodeName(nsAString& aName) const { } inline void txXPathTreeWalker::moveTo(const txXPathTreeWalker& aWalker) { - nsINode* root = nullptr; - if (mPosition.mRefCountRoot) { - root = mPosition.Root(); - } mPosition.mIndex = aWalker.mPosition.mIndex; - mPosition.mRefCountRoot = aWalker.mPosition.mRefCountRoot; mPosition.mNode = aWalker.mPosition.mNode; - nsINode* newRoot = nullptr; - if (mPosition.mRefCountRoot) { - newRoot = mPosition.Root(); - } - if (root != newRoot) { - NS_IF_ADDREF(newRoot); - NS_IF_RELEASE(root); - } } inline bool txXPathTreeWalker::isOnNode(const txXPathNode& aNode) const { @@ -147,12 +125,7 @@ inline bool txXPathTreeWalker::isOnNode(const txXPathNode& aNode) const { /* static */ inline int32_t txXPathNodeUtils::getUniqueIdentifier(const txXPathNode& aNode) { MOZ_ASSERT(!aNode.isAttribute(), "Not implemented for attributes."); - return NS_PTR_TO_INT32(aNode.mNode); -} - -/* static */ -inline void txXPathNodeUtils::release(txXPathNode* aNode) { - NS_RELEASE(aNode->mNode); + return NS_PTR_TO_INT32(aNode.mNode.get()); } /* static */ diff --git a/dom/xslt/xslt/txEXSLTFunctions.cpp b/dom/xslt/xslt/txEXSLTFunctions.cpp index d58ca33b5374..e61b47dcad03 100644 --- a/dom/xslt/xslt/txEXSLTFunctions.cpp +++ b/dom/xslt/xslt/txEXSLTFunctions.cpp @@ -73,11 +73,7 @@ static nsresult convertRtfToNode(txIEvalContext* aContext, NS_ENSURE_SUCCESS(rv, rv); // The txResultTreeFragment will own this. - const txXPathNode* node = - txXPathNativeNode::createXPathNode(domFragment, true); - NS_ENSURE_TRUE(node, NS_ERROR_OUT_OF_MEMORY); - - aRtf->setNode(node); + aRtf->setNode(new txXPathNode(domFragment)); return NS_OK; } @@ -95,8 +91,7 @@ static nsresult createTextNode(txIEvalContext* aContext, nsString& aValue, nsresult rv = text->SetText(aValue, false); NS_ENSURE_SUCCESS(rv, rv); - *aResult = txXPathNativeNode::createXPathNode(text, true); - NS_ENSURE_TRUE(*aResult, NS_ERROR_OUT_OF_MEMORY); + *aResult = new txXPathNode(text); return NS_OK; } @@ -126,11 +121,10 @@ static nsresult createAndAddToResult(nsAtom* aName, const nsAString& aValue, return error.StealNSResult(); } - UniquePtr xpathNode( - txXPathNativeNode::createXPathNode(elem, true)); + Maybe xpathNode(txXPathNativeNode::createXPathNode(elem)); NS_ENSURE_TRUE(xpathNode, NS_ERROR_OUT_OF_MEMORY); - aResultSet->append(*xpathNode); + aResultSet->append(xpathNode.extract()); return NS_OK; } @@ -705,8 +699,7 @@ nsresult txEXSLTRegExFunctionCall::evaluate(txIEvalContext* aContext, UniquePtr node; for (nsIContent* result = docFrag->GetFirstChild(); result; result = result->GetNextSibling()) { - node = WrapUnique(txXPathNativeNode::createXPathNode(result, true)); - rv = resultSet->add(*node); + rv = resultSet->add(txXPathNode(result)); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/dom/xslt/xslt/txExecutionState.cpp b/dom/xslt/xslt/txExecutionState.cpp index a3cfddac1c62..d05c3a70b79c 100644 --- a/dom/xslt/xslt/txExecutionState.cpp +++ b/dom/xslt/xslt/txExecutionState.cpp @@ -14,14 +14,12 @@ #include "txURIUtils.h" #include "txXMLParser.h" -using mozilla::UniquePtr; -using mozilla::Unused; -using mozilla::WrapUnique; +using namespace mozilla; const int32_t txExecutionState::kMaxRecursionDepth = 20000; nsresult txLoadedDocumentsHash::init(const txXPathNode& aSource) { - mSourceDocument = WrapUnique(txXPathNodeUtils::getOwnerDocument(aSource)); + mSourceDocument = Some(txXPathNodeUtils::getOwnerDocument(aSource)); nsAutoString baseURI; nsresult rv = txXPathNodeUtils::getBaseURI(*mSourceDocument, baseURI); @@ -39,24 +37,12 @@ nsresult txLoadedDocumentsHash::init(const txXPathNode& aSource) { // txMozillaXSLTProcessor::TransformToFragment) it makes more sense to return // the real root of the source tree, which is the node where the transform // started. - PutEntry(baseURI)->mDocument = WrapUnique( - txXPathNativeNode::createXPathNode(txXPathNativeNode::getNode(aSource))); + WithEntryHandle(baseURI, [&aSource](auto&& entry) { + entry.Insert(txLoadedDocumentEntry{txXPathNode(aSource)}); + }); return NS_OK; } -txLoadedDocumentsHash::~txLoadedDocumentsHash() { - if (mSourceDocument) { - nsAutoString baseURI; - nsresult rv = txXPathNodeUtils::getBaseURI(*mSourceDocument, baseURI); - if (NS_SUCCEEDED(rv)) { - txLoadedDocumentEntry* entry = GetEntry(baseURI); - if (entry) { - delete entry->mDocument.release(); - } - } - } -} - txExecutionState::txExecutionState(txStylesheet* aStylesheet, bool aDisableLoads) : mOutputHandler(nullptr), @@ -358,27 +344,24 @@ const txXPathNode* txExecutionState::retrieveDocument(const nsAString& aUri) { ("Retrieve Document %s", NS_LossyConvertUTF16toASCII(aUri).get())); // try to get already loaded document - txLoadedDocumentEntry* entry = mLoadedDocuments.PutEntry(aUri); - if (!entry) { - return nullptr; - } + const Variant& result = + mLoadedDocuments.LookupOrInsertWith(aUri, [&] { + // open URI + nsAutoString errMsg; + // XXX we should get the loader from the actual node + // triggering the load, but this will do for the time being + Result loadResult = txParseDocumentFromURI( + aUri, *mLoadedDocuments.mSourceDocument, errMsg); - if (!entry->mDocument && !entry->LoadingFailed()) { - // open URI - nsAutoString errMsg; - // XXX we should get the loader from the actual node - // triggering the load, but this will do for the time being - entry->mLoadResult = - txParseDocumentFromURI(aUri, *mLoadedDocuments.mSourceDocument, errMsg, - getter_Transfers(entry->mDocument)); - - if (entry->LoadingFailed()) { - receiveError(u"Couldn't load document '"_ns + aUri + u"': "_ns + errMsg, - entry->mLoadResult); - } - } - - return entry->mDocument.get(); + if (loadResult.isErr()) { + nsresult rv = loadResult.unwrapErr(); + receiveError( + u"Couldn't load document '"_ns + aUri + u"': "_ns + errMsg, rv); + return txLoadedDocumentEntry(rv); + } + return txLoadedDocumentEntry(loadResult.unwrap()); + }); + return result.is() ? &result.as() : nullptr; } nsresult txExecutionState::getKeyNodes(const txExpandedName& aKeyName, diff --git a/dom/xslt/xslt/txExecutionState.h b/dom/xslt/xslt/txExecutionState.h index 7a314cf8a4b8..89f5a1e19fca 100644 --- a/dom/xslt/xslt/txExecutionState.h +++ b/dom/xslt/xslt/txExecutionState.h @@ -23,41 +23,18 @@ class txAOutputHandlerFactory; class txAXMLEventHandler; class txInstruction; -class txLoadedDocumentEntry : public nsStringHashKey { +using txLoadedDocumentEntry = mozilla::Variant; + +class txLoadedDocumentsHash + : public nsTHashMap { public: - explicit txLoadedDocumentEntry(KeyTypePointer aStr) - : nsStringHashKey(aStr), mLoadResult(NS_OK) {} - txLoadedDocumentEntry(txLoadedDocumentEntry&& aOther) - : nsStringHashKey(std::move(aOther)), - mDocument(std::move(aOther.mDocument)), - mLoadResult(std::move(aOther.mLoadResult)) { - NS_ERROR("We're horked."); - } - ~txLoadedDocumentEntry() { - if (mDocument) { - txXPathNodeUtils::release(mDocument.get()); - } - } - bool LoadingFailed() { - NS_ASSERTION(NS_SUCCEEDED(mLoadResult) || !mDocument, - "Load failed but we still got a document?"); + txLoadedDocumentsHash() : nsTHashMap(4) {} - return NS_FAILED(mLoadResult); - } - - mozilla::UniquePtr mDocument; - nsresult mLoadResult; -}; - -class txLoadedDocumentsHash : public nsTHashtable { - public: - txLoadedDocumentsHash() : nsTHashtable(4) {} - ~txLoadedDocumentsHash(); [[nodiscard]] nsresult init(const txXPathNode& aSource); private: friend class txExecutionState; - mozilla::UniquePtr mSourceDocument; + mozilla::Maybe mSourceDocument; }; class txExecutionState : public txIMatchContext { diff --git a/dom/xslt/xslt/txMozillaXSLTProcessor.cpp b/dom/xslt/xslt/txMozillaXSLTProcessor.cpp index 91f60982fe28..144014f44299 100644 --- a/dom/xslt/xslt/txMozillaXSLTProcessor.cpp +++ b/dom/xslt/xslt/txMozillaXSLTProcessor.cpp @@ -326,7 +326,8 @@ txMozillaXSLTProcessor::AddXSLTParamNamespace(const nsString& aPrefix, return mParamNamespaceMap.mapNamespace(pre, aNamespace); } -class txXSLTParamContext : public txIParseContext, public txIEvalContext { +class MOZ_STACK_CLASS txXSLTParamContext : public txIParseContext, + public txIEvalContext { public: txXSLTParamContext(txNamespaceMap* aResolver, const txXPathNode& aContext, txResultRecycler* aRecycler) @@ -385,7 +386,7 @@ txMozillaXSLTProcessor::AddXSLTParam(const nsString& aName, uint16_t resultType; if (!aSelect.IsVoid()) { // Set up context - UniquePtr contextNode( + Maybe contextNode( txXPathNativeNode::createXPathNode(aContext)); NS_ENSURE_TRUE(contextNode, NS_ERROR_OUT_OF_MEMORY); @@ -655,8 +656,7 @@ XSLTProcessRequest::SetTRRMode(nsIRequest::TRRMode aTRRMode) { nsresult txMozillaXSLTProcessor::TransformToDoc(Document** aResult, bool aCreateDataDocument) { - UniquePtr sourceNode( - txXPathNativeNode::createXPathNode(mSource)); + Maybe sourceNode(txXPathNativeNode::createXPathNode(mSource)); if (!sourceNode) { return NS_ERROR_OUT_OF_MEMORY; } @@ -754,7 +754,7 @@ already_AddRefed txMozillaXSLTProcessor::TransformToFragment( source = &aSource; } - UniquePtr sourceNode(txXPathNativeNode::createXPathNode(source)); + Maybe sourceNode(txXPathNativeNode::createXPathNode(source)); if (!sourceNode) { aRv.Throw(NS_ERROR_OUT_OF_MEMORY); return nullptr; @@ -1185,7 +1185,7 @@ nsresult txVariable::convert(const OwningXSLTParameterValue& aUnionValue, if (aUnionValue.IsNode()) { nsINode& node = aUnionValue.GetAsNode(); - UniquePtr xpathNode(txXPathNativeNode::createXPathNode(&node)); + Maybe xpathNode(txXPathNativeNode::createXPathNode(&node)); if (!xpathNode) { return NS_ERROR_FAILURE; } @@ -1199,13 +1199,13 @@ nsresult txVariable::convert(const OwningXSLTParameterValue& aUnionValue, const Sequence>& values = aUnionValue.GetAsNodeSequence(); for (const auto& node : values) { - UniquePtr xpathNode( + Maybe xpathNode( txXPathNativeNode::createXPathNode(node.get())); if (!xpathNode) { return NS_ERROR_FAILURE; } - nodeSet->append(*xpathNode); + nodeSet->append(xpathNode.extract()); } nodeSet.forget(aValue); return NS_OK; diff --git a/dom/xslt/xslt/txXSLTPatterns.cpp b/dom/xslt/xslt/txXSLTPatterns.cpp index dc1b81e210e1..2c78dc45026b 100644 --- a/dom/xslt/xslt/txXSLTPatterns.cpp +++ b/dom/xslt/xslt/txXSLTPatterns.cpp @@ -329,12 +329,11 @@ void txIdPattern::toString(nsAString& aDest) { nsresult txKeyPattern::matches(const txXPathNode& aNode, txIMatchContext* aContext, bool& aMatched) { txExecutionState* es = (txExecutionState*)aContext->getPrivateContext(); - UniquePtr contextDoc(txXPathNodeUtils::getOwnerDocument(aNode)); - NS_ENSURE_TRUE(contextDoc, NS_ERROR_FAILURE); + txXPathNode contextDoc(txXPathNodeUtils::getOwnerDocument(aNode)); RefPtr nodes; nsresult rv = - es->getKeyNodes(mName, *contextDoc, mValue, true, getter_AddRefs(nodes)); + es->getKeyNodes(mName, contextDoc, mValue, true, getter_AddRefs(nodes)); NS_ENSURE_SUCCESS(rv, rv); aMatched = nodes->contains(aNode);