From 6b75d0f84f42da15b43a4426bc654e20056fbc26 Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Thu, 2 Dec 2010 11:12:27 -0500 Subject: [PATCH] Fix for bug 603844 (Leak txUnknownHandler+ with transformToDocument(textnode)). r=sicking a=b:final --- content/xslt/crashtests/603844.html | 32 +++++ content/xslt/crashtests/crashtests.list | 1 + content/xslt/src/xslt/txBufferingHandler.cpp | 117 ++++++++----------- content/xslt/src/xslt/txBufferingHandler.h | 7 +- content/xslt/src/xslt/txEXSLTFunctions.cpp | 5 +- content/xslt/src/xslt/txInstructions.cpp | 2 +- content/xslt/src/xslt/txRtfHandler.cpp | 2 +- content/xslt/src/xslt/txRtfHandler.h | 2 +- content/xslt/src/xslt/txUnknownHandler.cpp | 11 +- 9 files changed, 94 insertions(+), 85 deletions(-) create mode 100644 content/xslt/crashtests/603844.html diff --git a/content/xslt/crashtests/603844.html b/content/xslt/crashtests/603844.html new file mode 100644 index 000000000000..f576effdb6a8 --- /dev/null +++ b/content/xslt/crashtests/603844.html @@ -0,0 +1,32 @@ + + + + + + + + diff --git a/content/xslt/crashtests/crashtests.list b/content/xslt/crashtests/crashtests.list index ffe0e29a7f83..182f22486d5e 100644 --- a/content/xslt/crashtests/crashtests.list +++ b/content/xslt/crashtests/crashtests.list @@ -11,3 +11,4 @@ load 528488.xml load 528963.xml load 545927.html load 601543.html +load 603844.html diff --git a/content/xslt/src/xslt/txBufferingHandler.cpp b/content/xslt/src/xslt/txBufferingHandler.cpp index 5d0d15115e01..e250e0a445ce 100644 --- a/content/xslt/src/xslt/txBufferingHandler.cpp +++ b/content/xslt/src/xslt/txBufferingHandler.cpp @@ -359,122 +359,103 @@ txResultBuffer::addTransaction(txOutputTransaction* aTransaction) return NS_OK; } -struct Holder +static nsresult +flushTransaction(txOutputTransaction* aTransaction, + txAXMLEventHandler* aHandler, + nsAFlatString::const_char_iterator& aIter) { - txAXMLEventHandler** mHandler; - nsresult mResult; - nsAFlatString::const_char_iterator mIter; -}; - -static PRBool -flushTransaction(txOutputTransaction* aElement, Holder* aData) -{ - Holder* holder = aData; - txAXMLEventHandler* handler = *holder->mHandler; - txOutputTransaction* transaction = aElement; - - nsresult rv; - switch (transaction->mType) { + switch (aTransaction->mType) { case txOutputTransaction::eAttributeAtomTransaction: { txAttributeAtomTransaction* transaction = - static_cast(aElement); - rv = handler->attribute(transaction->mPrefix, - transaction->mLocalName, - transaction->mLowercaseLocalName, - transaction->mNsID, - transaction->mValue); - break; + static_cast(aTransaction); + return aHandler->attribute(transaction->mPrefix, + transaction->mLocalName, + transaction->mLowercaseLocalName, + transaction->mNsID, + transaction->mValue); } case txOutputTransaction::eAttributeTransaction: { txAttributeTransaction* attrTransaction = - static_cast(aElement); - rv = handler->attribute(attrTransaction->mPrefix, - attrTransaction->mLocalName, - attrTransaction->mNsID, - attrTransaction->mValue); - break; + static_cast(aTransaction); + return aHandler->attribute(attrTransaction->mPrefix, + attrTransaction->mLocalName, + attrTransaction->mNsID, + attrTransaction->mValue); } case txOutputTransaction::eCharacterTransaction: case txOutputTransaction::eCharacterNoOETransaction: { txCharacterTransaction* charTransaction = - static_cast(aElement); - nsAFlatString::const_char_iterator& start = - holder->mIter; + static_cast(aTransaction); + nsAFlatString::const_char_iterator& start = aIter; nsAFlatString::const_char_iterator end = start + charTransaction->mLength; - rv = handler->characters(Substring(start, end), - transaction->mType == - txOutputTransaction::eCharacterNoOETransaction); - start = end; - break; + aIter = end; + return aHandler->characters(Substring(start, end), + aTransaction->mType == + txOutputTransaction::eCharacterNoOETransaction); } case txOutputTransaction::eCommentTransaction: { txCommentTransaction* commentTransaction = - static_cast(aElement); - rv = handler->comment(commentTransaction->mValue); - break; + static_cast(aTransaction); + return aHandler->comment(commentTransaction->mValue); } case txOutputTransaction::eEndElementTransaction: { - rv = handler->endElement(); - break; + return aHandler->endElement(); } case txOutputTransaction::ePITransaction: { txPITransaction* piTransaction = - static_cast(aElement); - rv = handler->processingInstruction(piTransaction->mTarget, - piTransaction->mData); - break; + static_cast(aTransaction); + return aHandler->processingInstruction(piTransaction->mTarget, + piTransaction->mData); } case txOutputTransaction::eStartDocumentTransaction: { - rv = handler->startDocument(); - break; + return aHandler->startDocument(); } case txOutputTransaction::eStartElementAtomTransaction: { txStartElementAtomTransaction* transaction = - static_cast(aElement); - rv = handler->startElement(transaction->mPrefix, - transaction->mLocalName, - transaction->mLowercaseLocalName, - transaction->mNsID); - break; + static_cast(aTransaction); + return aHandler->startElement(transaction->mPrefix, + transaction->mLocalName, + transaction->mLowercaseLocalName, + transaction->mNsID); } case txOutputTransaction::eStartElementTransaction: { txStartElementTransaction* transaction = - static_cast(aElement); - rv = handler->startElement(transaction->mPrefix, - transaction->mLocalName, - transaction->mNsID); - break; + static_cast(aTransaction); + return aHandler->startElement(transaction->mPrefix, + transaction->mLocalName, + transaction->mNsID); + } + default: + { + NS_NOTREACHED("Unexpected transaction type"); } } - holder->mResult = rv; - - return NS_SUCCEEDED(rv); + return NS_ERROR_UNEXPECTED; } nsresult -txResultBuffer::flushToHandler(txAXMLEventHandler** aHandler) +txResultBuffer::flushToHandler(txAXMLEventHandler* aHandler) { - Holder data = { aHandler, NS_OK }; - mStringValue.BeginReading(data.mIter); + nsAFlatString::const_char_iterator iter; + mStringValue.BeginReading(iter); for (PRUint32 i = 0, len = mTransactions.Length(); i < len; ++i) { - if (!flushTransaction(mTransactions[i], &data)) { - break; - } + nsresult rv = flushTransaction(mTransactions[i], aHandler, iter); + NS_ENSURE_SUCCESS(rv, rv); } - return data.mResult; + return NS_OK; } txOutputTransaction* diff --git a/content/xslt/src/xslt/txBufferingHandler.h b/content/xslt/src/xslt/txBufferingHandler.h index 72c20dbc286e..26df8acc72b7 100644 --- a/content/xslt/src/xslt/txBufferingHandler.h +++ b/content/xslt/src/xslt/txBufferingHandler.h @@ -55,12 +55,7 @@ public: nsresult addTransaction(txOutputTransaction* aTransaction); - /** - * Flush the transactions to aHandler. Some handlers create a new handler - * and replace themselves with the new handler. The pointer that aHandler - * points to should be updated in that case. - */ - nsresult flushToHandler(txAXMLEventHandler** aHandler); + nsresult flushToHandler(txAXMLEventHandler* aHandler); txOutputTransaction* getLastTransaction(); diff --git a/content/xslt/src/xslt/txEXSLTFunctions.cpp b/content/xslt/src/xslt/txEXSLTFunctions.cpp index 6808a980a4f5..1589795b39ea 100644 --- a/content/xslt/src/xslt/txEXSLTFunctions.cpp +++ b/content/xslt/src/xslt/txEXSLTFunctions.cpp @@ -88,10 +88,7 @@ convertRtfToNode(txIEvalContext *aContext, txResultTreeFragment *aRtf) txOutputFormat format; txMozillaXMLOutput mozHandler(&format, domFragment, PR_TRUE); - txAXMLEventHandler* handler = &mozHandler; - rv = aRtf->flushToHandler(&handler); - NS_ASSERTION(handler == &mozHandler, - "This handler shouldn't have been replaced!"); + rv = aRtf->flushToHandler(&mozHandler); NS_ENSURE_SUCCESS(rv, rv); rv = mozHandler.closePrevious(PR_TRUE); diff --git a/content/xslt/src/xslt/txInstructions.cpp b/content/xslt/src/xslt/txInstructions.cpp index dcb45283a631..046890d12496 100644 --- a/content/xslt/src/xslt/txInstructions.cpp +++ b/content/xslt/src/xslt/txInstructions.cpp @@ -432,7 +432,7 @@ txCopyOf::execute(txExecutionState& aEs) txResultTreeFragment* rtf = static_cast (static_cast(exprRes)); - return rtf->flushToHandler(&aEs.mResultHandler); + return rtf->flushToHandler(aEs.mResultHandler); } default: { diff --git a/content/xslt/src/xslt/txRtfHandler.cpp b/content/xslt/src/xslt/txRtfHandler.cpp index 55efdbf6a01a..90deffc71bbb 100644 --- a/content/xslt/src/xslt/txRtfHandler.cpp +++ b/content/xslt/src/xslt/txRtfHandler.cpp @@ -80,7 +80,7 @@ double txResultTreeFragment::numberValue() return Double::toDouble(mBuffer->mStringValue); } -nsresult txResultTreeFragment::flushToHandler(txAXMLEventHandler** aHandler) +nsresult txResultTreeFragment::flushToHandler(txAXMLEventHandler* aHandler) { if (!mBuffer) { return NS_ERROR_FAILURE; diff --git a/content/xslt/src/xslt/txRtfHandler.h b/content/xslt/src/xslt/txRtfHandler.h index 9418e1aefaa0..4b38f3a3ad75 100644 --- a/content/xslt/src/xslt/txRtfHandler.h +++ b/content/xslt/src/xslt/txRtfHandler.h @@ -51,7 +51,7 @@ public: TX_DECL_EXPRRESULT - nsresult flushToHandler(txAXMLEventHandler** aHandler); + nsresult flushToHandler(txAXMLEventHandler* aHandler); void setNode(const txXPathNode* aNode) { diff --git a/content/xslt/src/xslt/txUnknownHandler.cpp b/content/xslt/src/xslt/txUnknownHandler.cpp index 3c889d7d6718..03d4516b641e 100644 --- a/content/xslt/src/xslt/txUnknownHandler.cpp +++ b/content/xslt/src/xslt/txUnknownHandler.cpp @@ -147,14 +147,17 @@ nsresult txUnknownHandler::createHandlerAndFlush(PRBool aHTMLRoot, format.mMethod = aHTMLRoot ? eHTMLOutput : eXMLOutput; } - txAXMLEventHandler *handler = nsnull; + nsAutoPtr handler; nsresult rv = mEs->mOutputHandlerFactory->createHandlerWith(&format, aName, aNsID, - &handler); + getter_Transfers(handler)); + NS_ENSURE_SUCCESS(rv, rv); + + rv = mBuffer->flushToHandler(handler); NS_ENSURE_SUCCESS(rv, rv); mEs->mOutputHandler = handler; - mEs->mResultHandler = handler; + mEs->mResultHandler = handler.forget(); - return mBuffer->flushToHandler(&handler); + return NS_OK; }