Fix for bug 603844 (Leak txUnknownHandler+ with transformToDocument(textnode)). r=sicking a=b:final

2010-12-02 11:12:27 -05:00 · 2010-12-02 11:12:27 -05:00 · 6b75d0f84f
--- a/content/xslt/crashtests/603844.html
+++ b/content/xslt/crashtests/603844.html
@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<script>
+
+function boom()
+{
+  var frame = document.createElementNS("http://www.w3.org/1999/xhtml", "iframe");
+  frame.onload = y; 
+  frame.src = "data:text/plain,0"; 
+  document.body.appendChild(frame);
+  frameDoc = frame.contentDocument;
+
+  function y()
+  {
+    frameDoc.removeChild(frameDoc.documentElement);
+
+    var xp = new XSLTProcessor;
+    xp.importStylesheet(frameDoc);
+    try {
+      xp.transformToDocument(frameDoc.createTextNode('x'));
+    } catch(e) { }
+
+    document.documentElement.removeAttribute("class");
+  }
+}
+
+</script>
+</head>
+
+<body onload="boom();"></body>
+</html>
--- 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
--- 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<txAttributeAtomTransaction*>(aElement);
-            rv = handler->attribute(transaction->mPrefix,
-                                    transaction->mLocalName,
-                                    transaction->mLowercaseLocalName,
-                                    transaction->mNsID,
-                                    transaction->mValue);
-            break;
+                static_cast<txAttributeAtomTransaction*>(aTransaction);
+            return aHandler->attribute(transaction->mPrefix,
+                                       transaction->mLocalName,
+                                       transaction->mLowercaseLocalName,
+                                       transaction->mNsID,
+                                       transaction->mValue);
        }
        case txOutputTransaction::eAttributeTransaction:
        {
            txAttributeTransaction* attrTransaction =
-                static_cast<txAttributeTransaction*>(aElement);
-            rv = handler->attribute(attrTransaction->mPrefix,
-                                    attrTransaction->mLocalName,
-                                    attrTransaction->mNsID,
-                                    attrTransaction->mValue);
-            break;
+                static_cast<txAttributeTransaction*>(aTransaction);
+            return aHandler->attribute(attrTransaction->mPrefix,
+                                       attrTransaction->mLocalName,
+                                       attrTransaction->mNsID,
+                                       attrTransaction->mValue);
        }
        case txOutputTransaction::eCharacterTransaction:
        case txOutputTransaction::eCharacterNoOETransaction:
        {
            txCharacterTransaction* charTransaction =
-                static_cast<txCharacterTransaction*>(aElement);
-            nsAFlatString::const_char_iterator& start =
-                holder->mIter;
+                static_cast<txCharacterTransaction*>(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<txCommentTransaction*>(aElement);
-            rv = handler->comment(commentTransaction->mValue);
-            break;
+                static_cast<txCommentTransaction*>(aTransaction);
+            return aHandler->comment(commentTransaction->mValue);
        }
        case txOutputTransaction::eEndElementTransaction:
        {
-            rv = handler->endElement();
-            break;
+            return aHandler->endElement();
        }
        case txOutputTransaction::ePITransaction:
        {
            txPITransaction* piTransaction =
-                static_cast<txPITransaction*>(aElement);
-            rv = handler->processingInstruction(piTransaction->mTarget,
-                                                piTransaction->mData);
-            break;
+                static_cast<txPITransaction*>(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<txStartElementAtomTransaction*>(aElement);
-            rv = handler->startElement(transaction->mPrefix,
-                                       transaction->mLocalName,
-                                       transaction->mLowercaseLocalName,
-                                       transaction->mNsID);
-            break;
+                static_cast<txStartElementAtomTransaction*>(aTransaction);
+            return aHandler->startElement(transaction->mPrefix,
+                                          transaction->mLocalName,
+                                          transaction->mLowercaseLocalName,
+                                          transaction->mNsID);
        }
        case txOutputTransaction::eStartElementTransaction:
        {
            txStartElementTransaction* transaction =
-                static_cast<txStartElementTransaction*>(aElement);
-            rv = handler->startElement(transaction->mPrefix,
-                                       transaction->mLocalName,
-                                       transaction->mNsID);
-            break;
+                static_cast<txStartElementTransaction*>(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*
--- 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();

--- 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);
--- 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<txResultTreeFragment*>
                           (static_cast<txAExprResult*>(exprRes));
-            return rtf->flushToHandler(&aEs.mResultHandler);
+            return rtf->flushToHandler(aEs.mResultHandler);
        }
        default:
        {
--- 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;
--- 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)
    {
--- 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<txAXMLEventHandler> 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;
 }