Fix for bug 603844 (Leak txUnknownHandler+ with transformToDocument(textnode)). r=sicking a=b:final
☠☠ backed out by cb06c76f79fb ☠ ☠
authorPeter Van der Beken <peterv@propagandism.org>
Thu, 02 Dec 2010 11:12:27 -0500
changeset 58490 40b718853f48f2b12802e6266004bc1ea35d28dd
parent 58487 57a6ec830cd465d84012835d109d5db561a14cd2
child 58491 feb478675a92f9d46beaeda01795f78440a7a1eb
child 58500 cb06c76f79fbc4d7b882fc672fb6e93551db3a9c
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewerssicking, b
bugs603844
milestone2.0b8pre
Fix for bug 603844 (Leak txUnknownHandler+ with transformToDocument(textnode)). r=sicking a=b:final
content/xslt/crashtests/603844.html
content/xslt/crashtests/crashtests.list
content/xslt/src/xslt/txBufferingHandler.cpp
content/xslt/src/xslt/txBufferingHandler.h
content/xslt/src/xslt/txEXSLTFunctions.cpp
content/xslt/src/xslt/txInstructions.cpp
content/xslt/src/xslt/txRtfHandler.cpp
content/xslt/src/xslt/txRtfHandler.h
content/xslt/src/xslt/txUnknownHandler.cpp
new file mode 100644
--- /dev/null
+++ 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
@@ -6,8 +6,9 @@ load 406106-1.html
 load 483444.xml
 load 485217.xml
 load 485286.xml
 load 528300.xml
 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
@@ -354,132 +354,113 @@ nsresult
 txResultBuffer::addTransaction(txOutputTransaction* aTransaction)
 {
     if (mTransactions.AppendElement(aTransaction) == nsnull) {
         return NS_ERROR_OUT_OF_MEMORY;
     }
     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*
 txResultBuffer::getLastTransaction()
 {
     PRInt32 last = mTransactions.Length() - 1;
     if (last < 0) {
         return nsnull;
--- a/content/xslt/src/xslt/txBufferingHandler.h
+++ b/content/xslt/src/xslt/txBufferingHandler.h
@@ -50,22 +50,17 @@ class txCharacterTransaction;
 
 class txResultBuffer
 {
 public:
     ~txResultBuffer();
 
     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();
 
     nsString mStringValue;
 
 private:
     nsTArray<txOutputTransaction*> mTransactions;
 };
--- a/content/xslt/src/xslt/txEXSLTFunctions.cpp
+++ b/content/xslt/src/xslt/txEXSLTFunctions.cpp
@@ -83,20 +83,17 @@ convertRtfToNode(txIEvalContext *aContex
     nsCOMPtr<nsIDOMDocumentFragment> domFragment;
     nsresult rv = NS_NewDocumentFragment(getter_AddRefs(domFragment),
                                          doc->NodeInfoManager());
     NS_ENSURE_SUCCESS(rv, rv);
 
     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);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // The txResultTreeFragment will own this.
     const txXPathNode* node = txXPathNativeNode::createXPathNode(domFragment,
                                                                  PR_TRUE);
--- a/content/xslt/src/xslt/txInstructions.cpp
+++ b/content/xslt/src/xslt/txInstructions.cpp
@@ -427,17 +427,17 @@ txCopyOf::execute(txExecutionState& aEs)
             }
             break;
         }
         case txAExprResult::RESULT_TREE_FRAGMENT:
         {
             txResultTreeFragment* rtf =
                 static_cast<txResultTreeFragment*>
                            (static_cast<txAExprResult*>(exprRes));
-            return rtf->flushToHandler(&aEs.mResultHandler);
+            return rtf->flushToHandler(aEs.mResultHandler);
         }
         default:
         {
             nsAutoString value;
             exprRes->stringValue(value);
             if (!value.IsEmpty()) {
                 return aEs.mResultHandler->characters(value, PR_FALSE);
             }
--- a/content/xslt/src/xslt/txRtfHandler.cpp
+++ b/content/xslt/src/xslt/txRtfHandler.cpp
@@ -75,17 +75,17 @@ double txResultTreeFragment::numberValue
 {
     if (!mBuffer) {
         return 0;
     }
 
     return Double::toDouble(mBuffer->mStringValue);
 }
 
-nsresult txResultTreeFragment::flushToHandler(txAXMLEventHandler** aHandler)
+nsresult txResultTreeFragment::flushToHandler(txAXMLEventHandler* aHandler)
 {
     if (!mBuffer) {
         return NS_ERROR_FAILURE;
     }
 
     return mBuffer->flushToHandler(aHandler);
 }
 
--- a/content/xslt/src/xslt/txRtfHandler.h
+++ b/content/xslt/src/xslt/txRtfHandler.h
@@ -46,17 +46,17 @@
 
 class txResultTreeFragment : public txAExprResult
 {
 public:
     txResultTreeFragment(nsAutoPtr<txResultBuffer>& aBuffer);
 
     TX_DECL_EXPRRESULT
 
-    nsresult flushToHandler(txAXMLEventHandler** aHandler);
+    nsresult flushToHandler(txAXMLEventHandler* aHandler);
 
     void setNode(const txXPathNode* aNode)
     {
         NS_ASSERTION(!mNode, "Already converted!");
 
         mNode = aNode;
     }
     const txXPathNode *getNode() const
--- a/content/xslt/src/xslt/txUnknownHandler.cpp
+++ b/content/xslt/src/xslt/txUnknownHandler.cpp
@@ -142,19 +142,22 @@ nsresult txUnknownHandler::createHandler
     NS_ENSURE_TRUE(mBuffer, NS_ERROR_NOT_INITIALIZED);
 
     txOutputFormat format;
     format.merge(*(mEs->mStylesheet->getOutputFormat()));
     if (format.mMethod == eMethodNotSet) {
         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;
 }