diff --git a/dom/xslt/xslt/txNodeSorter.cpp b/dom/xslt/xslt/txNodeSorter.cpp index cc7ac3e35404..2da7871cc042 100644 --- a/dom/xslt/xslt/txNodeSorter.cpp +++ b/dom/xslt/xslt/txNodeSorter.cpp @@ -92,8 +92,12 @@ nsresult txNodeSorter::addSortElement(Expr* aSelectExpr, Expr* aLangExpr, } } - key->mComparator = - new txResultStringComparator(ascending, upperFirst, lang); + UniquePtr comparator = + MakeUnique(ascending, upperFirst); + rv = comparator->init(lang); + NS_ENSURE_SUCCESS(rv, rv); + + key->mComparator = comparator.release(); } else if (TX_StringEqualsAtom(dataType, nsGkAtoms::number)) { // Number comparator key->mComparator = new txResultNumberComparator(ascending); @@ -134,7 +138,7 @@ nsresult txNodeSorter::sortNodeSet(txNodeSet* aNodes, txExecutionState* aEs, nsTArray indexes(len.value()); indexes.SetLengthAndRetainStorage(len.value()); - nsTArray sortValues(numSortValues.value()); + nsTArray> sortValues(numSortValues.value()); sortValues.SetLengthAndRetainStorage(numSortValues.value()); // txObject* has no null initializing constructor, so we init manually. memset(sortValues.Elements(), 0, sortValuesSize.value()); @@ -147,11 +151,7 @@ nsresult txNodeSorter::sortNodeSet(txNodeSet* aNodes, txExecutionState* aEs, auto nodeSetContext = MakeUnique(aNodes, aEs); // Sort the indexarray - SortData sortData{}; - sortData.mNodeSorter = this; - sortData.mContext = nodeSetContext.get(); - sortData.mSortValues = sortValues.Elements(); - sortData.mRv = NS_OK; + SortData sortData{this, nodeSetContext.get(), sortValues.Elements(), NS_OK}; aEs->pushEvalContext(nodeSetContext.release()); @@ -159,12 +159,6 @@ nsresult txNodeSorter::sortNodeSet(txNodeSet* aNodes, txExecutionState* aEs, return compareNodes(left, right, sortData); }); - // Delete these here so we don't have to deal with them at every possible - // failurepoint - for (i = 0; i < numSortValues.value(); ++i) { - delete sortValues[i]; - } - if (NS_FAILED(sortData.mRv)) { // The txExecutionState owns the evalcontext so no need to handle it return sortData.mRv; @@ -188,12 +182,10 @@ nsresult txNodeSorter::sortNodeSet(txNodeSet* aNodes, txExecutionState* aEs, int txNodeSorter::compareNodes(uint32_t aIndexA, uint32_t aIndexB, SortData& aSortData) { - NS_ENSURE_SUCCESS(aSortData.mRv, -1); - txListIterator iter(&aSortData.mNodeSorter->mSortKeys); - txObject** sortValuesA = + UniquePtr* sortValuesA = aSortData.mSortValues + aIndexA * aSortData.mNodeSorter->mNKeys; - txObject** sortValuesB = + UniquePtr* sortValuesB = aSortData.mSortValues + aIndexB * aSortData.mNodeSorter->mNKeys; unsigned int i; @@ -201,19 +193,19 @@ int txNodeSorter::compareNodes(uint32_t aIndexA, uint32_t aIndexB, for (i = 0; i < aSortData.mNodeSorter->mNKeys; ++i) { SortKey* key = (SortKey*)iter.next(); // Lazy create sort values - if (!sortValuesA[i] && - !calcSortValue(sortValuesA[i], key, &aSortData, aIndexA)) { - return -1; + if (!sortValuesA[i]) { + sortValuesA[i] = calcSortValue(key, &aSortData, aIndexA); } - if (!sortValuesB[i] && - !calcSortValue(sortValuesB[i], key, &aSortData, aIndexB)) { - return 1; + if (!sortValuesB[i]) { + sortValuesB[i] = calcSortValue(key, &aSortData, aIndexB); } // Compare node values - int compRes = - key->mComparator->compareValues(sortValuesA[i], sortValuesB[i]); - if (compRes != 0) return compRes; + int compRes = key->mComparator->compareValues(sortValuesA[i].get(), + sortValuesB[i].get()); + if (compRes != 0) { + return compRes; + } } // All keys have the same value for these nodes. @@ -221,16 +213,18 @@ int txNodeSorter::compareNodes(uint32_t aIndexA, uint32_t aIndexB, } // static -bool txNodeSorter::calcSortValue(txObject*& aSortValue, SortKey* aKey, - SortData* aSortData, uint32_t aNodeIndex) { +UniquePtr txNodeSorter::calcSortValue(SortKey* aKey, + SortData* aSortData, + uint32_t aNodeIndex) { aSortData->mContext->setPosition(aNodeIndex + 1); // position is 1-based - nsresult rv = aKey->mComparator->createSortableValue( - aKey->mExpr, aSortData->mContext, aSortValue); - if (NS_FAILED(rv)) { + UniquePtr sortValue; + nsresult rv; + std::tie(sortValue, rv) = + aKey->mComparator->createSortableValue(aKey->mExpr, aSortData->mContext); + if (NS_FAILED(rv) && NS_SUCCEEDED(aSortData->mRv)) { + // Record the first failure. aSortData->mRv = rv; - return false; } - - return true; + return sortValue; } diff --git a/dom/xslt/xslt/txNodeSorter.h b/dom/xslt/xslt/txNodeSorter.h index b6c883b6ea54..3ab74763815c 100644 --- a/dom/xslt/xslt/txNodeSorter.h +++ b/dom/xslt/xslt/txNodeSorter.h @@ -36,7 +36,7 @@ class txNodeSorter { struct SortData { txNodeSorter* mNodeSorter; txNodeSetContext* mContext; - txObject** mSortValues; + mozilla::UniquePtr* mSortValues; nsresult mRv; }; struct SortKey { @@ -46,8 +46,9 @@ class txNodeSorter { static int compareNodes(uint32_t aIndexA, uint32_t aIndexB, SortData& aSortData); - static bool calcSortValue(txObject*& aSortValue, SortKey* aKey, - SortData* aSortData, uint32_t aNodeIndex); + static mozilla::UniquePtr calcSortValue(SortKey* aKey, + SortData* aSortData, + uint32_t aNodeIndex); txList mSortKeys; unsigned int mNKeys; }; diff --git a/dom/xslt/xslt/txXPathResultComparator.cpp b/dom/xslt/xslt/txXPathResultComparator.cpp index 470a55dc5032..0331dac2913f 100644 --- a/dom/xslt/xslt/txXPathResultComparator.cpp +++ b/dom/xslt/xslt/txXPathResultComparator.cpp @@ -19,13 +19,10 @@ using Collator = mozilla::intl::Collator; #define kUpperFirst (1 << 1) txResultStringComparator::txResultStringComparator(bool aAscending, - bool aUpperFirst, - const nsString& aLanguage) { + bool aUpperFirst) { mSorting = 0; if (aAscending) mSorting |= kAscending; if (aUpperFirst) mSorting |= kUpperFirst; - nsresult rv = init(aLanguage); - if (NS_FAILED(rv)) NS_ERROR("Failed to initialize txResultStringComparator"); } nsresult txResultStringComparator::init(const nsString& aLanguage) { @@ -49,61 +46,35 @@ nsresult txResultStringComparator::init(const nsString& aLanguage) { return NS_OK; } -nsresult txResultStringComparator::createSortableValue(Expr* aExpr, - txIEvalContext* aContext, - txObject*& aResult) { - UniquePtr val(new StringValue); - - if (!mCollator) { - return NS_ERROR_FAILURE; - } - - val->mString = MakeUnique(); - nsString& string = *val->mString; - nsresult rv = aExpr->evaluateToString(aContext, string); - NS_ENSURE_SUCCESS(rv, rv); - - aResult = val.release(); - - return NS_OK; +std::pair, nsresult> +txResultStringComparator::createSortableValue(Expr* aExpr, + txIEvalContext* aContext) { + UniquePtr string = MakeUnique(); + nsresult rv = aExpr->evaluateToString(aContext, *string); + return std::make_pair(MakeUnique(std::move(string)), rv); } int txResultStringComparator::compareValues(txObject* aVal1, txObject* aVal2) { nsString& dval1 = *((StringValue*)aVal1)->mString; nsString& dval2 = *((StringValue*)aVal2)->mString; - if (!mCollator) { - MOZ_ASSERT_UNREACHABLE("No mCollator"); - return -1; - } - int32_t result = mCollator->CompareStrings(dval1, dval2); return (mSorting & kAscending) ? result : -result; } -txResultStringComparator::StringValue::StringValue() = default; - -txResultStringComparator::StringValue::~StringValue() = default; - txResultNumberComparator::txResultNumberComparator(bool aAscending) { mAscending = aAscending ? 1 : -1; } -nsresult txResultNumberComparator::createSortableValue(Expr* aExpr, - txIEvalContext* aContext, - txObject*& aResult) { - UniquePtr numval(new NumberValue); - +std::pair, nsresult> +txResultNumberComparator::createSortableValue(Expr* aExpr, + txIEvalContext* aContext) { RefPtr exprRes; nsresult rv = aExpr->evaluate(aContext, getter_AddRefs(exprRes)); - NS_ENSURE_SUCCESS(rv, rv); - - numval->mVal = exprRes->numberValue(); - - aResult = numval.release(); - - return NS_OK; + return std::make_pair( + MakeUnique(NS_SUCCEEDED(rv) ? exprRes->numberValue() : 0), + rv); } int txResultNumberComparator::compareValues(txObject* aVal1, txObject* aVal2) { diff --git a/dom/xslt/xslt/txXPathResultComparator.h b/dom/xslt/xslt/txXPathResultComparator.h index 165cc5f2edd8..3ca547cbb4d9 100644 --- a/dom/xslt/xslt/txXPathResultComparator.h +++ b/dom/xslt/xslt/txXPathResultComparator.h @@ -31,10 +31,11 @@ class txXPathResultComparator { virtual int compareValues(txObject* val1, txObject* val2) = 0; /* - * Create a sortable value. + * Create a sortable value. This always needs to return a value, but can + * indicate failure in the nsresult. */ - virtual nsresult createSortableValue(Expr* aExpr, txIEvalContext* aContext, - txObject*& aResult) = 0; + virtual std::pair, nsresult> createSortableValue( + Expr* aExpr, txIEvalContext* aContext) = 0; }; /* @@ -42,22 +43,21 @@ class txXPathResultComparator { */ class txResultStringComparator : public txXPathResultComparator { public: - txResultStringComparator(bool aAscending, bool aUpperFirst, - const nsString& aLanguage); + txResultStringComparator(bool aAscending, bool aUpperFirst); + nsresult init(const nsString& aLanguage); int compareValues(txObject* aVal1, txObject* aVal2) override; - nsresult createSortableValue(Expr* aExpr, txIEvalContext* aContext, - txObject*& aResult) override; + std::pair, nsresult> createSortableValue( + Expr* aExpr, txIEvalContext* aContext) override; private: mozilla::UniquePtr mCollator; - nsresult init(const nsString& aLanguage); int mSorting; class StringValue : public txObject { public: - StringValue(); - ~StringValue(); + explicit StringValue(mozilla::UniquePtr aString) + : mString(std::move(aString)) {} mozilla::UniquePtr mString; }; @@ -71,14 +71,16 @@ class txResultNumberComparator : public txXPathResultComparator { explicit txResultNumberComparator(bool aAscending); int compareValues(txObject* aVal1, txObject* aVal2) override; - nsresult createSortableValue(Expr* aExpr, txIEvalContext* aContext, - txObject*& aResult) override; + std::pair, nsresult> createSortableValue( + Expr* aExpr, txIEvalContext* aContext) override; private: int mAscending; class NumberValue : public txObject { public: + explicit NumberValue(double aVal) : mVal(aVal) {} + double mVal; }; };