Fix for bug 667315 (Make unknown handler forward to new handler). r=sicking.

2011-07-01 12:55:05 +02:00 · 2011-07-01 12:55:05 +02:00 · 7e94825de1
--- a/content/xslt/crashtests/667315.xml
+++ b/content/xslt/crashtests/667315.xml
@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="#stylesheet"?>
+<!DOCTYPE root [
+  <!ATTLIST xsl:stylesheet id ID #IMPLIED>
+]>
+<root>
+<xsl:stylesheet id="stylesheet" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
+  <xsl:variable name="var"><p>a</p></xsl:variable>
+  <xsl:template match="/"><xsl:copy-of select="$var" /></xsl:template>
+</xsl:stylesheet>
+</root>
--- a/content/xslt/crashtests/crashtests.list
+++ b/content/xslt/crashtests/crashtests.list
@ -13,3 +13,4 @@ load 545927.html
 load 601543.html
 load 603844.html
 load 602115.html
+load 667315.xml
--- a/content/xslt/src/xslt/txExecutionState.h
+++ b/content/xslt/src/xslt/txExecutionState.h
@ -163,6 +163,7 @@ public:

    txAXMLEventHandler* mOutputHandler;
    txAXMLEventHandler* mResultHandler;
+    nsAutoPtr<txAXMLEventHandler> mObsoleteHandler;
    txAOutputHandlerFactory* mOutputHandlerFactory;

    nsAutoPtr<txVariableMap> mTemplateParams;
--- a/content/xslt/src/xslt/txUnknownHandler.cpp
+++ b/content/xslt/src/xslt/txUnknownHandler.cpp
@ -43,7 +43,8 @@
 #include "txAtoms.h"

 txUnknownHandler::txUnknownHandler(txExecutionState* aEs)
-    : mEs(aEs)
+    : mEs(aEs),
+      mFlushed(PR_FALSE)
 {
    MOZ_COUNT_CTOR_INHERITED(txUnknownHandler, txBufferingHandler);
 }
@ -53,92 +54,145 @@ txUnknownHandler::~txUnknownHandler()
    MOZ_COUNT_DTOR_INHERITED(txUnknownHandler, txBufferingHandler);
 }

+nsresult
+txUnknownHandler::attribute(nsIAtom* aPrefix, nsIAtom* aLocalName,
+                            nsIAtom* aLowercaseLocalName, PRInt32 aNsID,
+                            const nsString& aValue)
+{
+    return mFlushed ?
+           mEs->mResultHandler->attribute(aPrefix, aLocalName,
+                                          aLowercaseLocalName, aNsID, aValue) :
+           txBufferingHandler::attribute(aPrefix, aLocalName,
+                                         aLowercaseLocalName, aNsID, aValue);
+}
+
+nsresult
+txUnknownHandler::attribute(nsIAtom* aPrefix, const nsSubstring& aLocalName,
+                            const PRInt32 aNsID, const nsString& aValue)
+{
+    return mFlushed ?
+           mEs->mResultHandler->attribute(aPrefix, aLocalName, aNsID, aValue) :
+           txBufferingHandler::attribute(aPrefix, aLocalName, aNsID, aValue);
+}
+
+nsresult
+txUnknownHandler::characters(const nsSubstring& aData, PRBool aDOE)
+{
+    return mFlushed ?
+           mEs->mResultHandler->characters(aData, aDOE) :
+           txBufferingHandler::characters(aData, aDOE);
+}
+
+nsresult
+txUnknownHandler::comment(const nsString& aData)
+{
+    return mFlushed ?
+           mEs->mResultHandler->comment(aData) :
+           txBufferingHandler::comment(aData);
+}
+
 nsresult
 txUnknownHandler::endDocument(nsresult aResult)
 {
-    if (NS_FAILED(aResult)) {
-        return NS_OK;
+    if (!mFlushed) {
+        if (NS_FAILED(aResult)) {
+            return NS_OK;
+        }
+
+        // This is an unusual case, no output method has been set and we
+        // didn't create a document element. Switching to XML output mode
+        // anyway.
+
+        // Make sure that mEs->mResultHandler == this is true, otherwise we'll
+        // leak mEs->mResultHandler in createHandlerAndFlush.
+        NS_ASSERTION(mEs->mResultHandler == this,
+                     "We're leaking mEs->mResultHandler.");
+
+        nsresult rv = createHandlerAndFlush(PR_FALSE, EmptyString(),
+                                            kNameSpaceID_None);
+        NS_ENSURE_SUCCESS(rv, rv);
    }

-    // This is an unusual case, no output method has been set and we
-    // didn't create a document element. Switching to XML output mode
-    // anyway.
+    return mEs->mResultHandler->endDocument(aResult);
+}

-    // Make sure that mEs->mResultHandler == this is true, otherwise we'll
-    // leak mEs->mResultHandler in createHandlerAndFlush and we'll crash on
-    // the last line (delete this).
-    NS_ASSERTION(mEs->mResultHandler == this,
-                 "We're leaking mEs->mResultHandler and are going to crash.");
+nsresult
+txUnknownHandler::endElement()
+{
+    return mFlushed ?
+           mEs->mResultHandler->endElement() :
+           txBufferingHandler::endElement();
+}

-    nsresult rv = createHandlerAndFlush(PR_FALSE, EmptyString(),
-                                        kNameSpaceID_None);
-    NS_ENSURE_SUCCESS(rv, rv);
+nsresult
+txUnknownHandler::processingInstruction(const nsString& aTarget,
+                                        const nsString& aData)
+{
+    return mFlushed ?
+           mEs->mResultHandler->processingInstruction(aTarget, aData) :
+           txBufferingHandler::processingInstruction(aTarget, aData);
+}

-    rv = mEs->mResultHandler->endDocument(aResult);
-
-    delete this;
-
-    return rv;
+nsresult
+txUnknownHandler::startDocument()
+{
+    return mFlushed ?
+           mEs->mResultHandler->startDocument() :
+           txBufferingHandler::startDocument();
 }

 nsresult
 txUnknownHandler::startElement(nsIAtom* aPrefix, nsIAtom* aLocalName,
                               nsIAtom* aLowercaseLocalName, PRInt32 aNsID)
 {
-    // Make sure that mEs->mResultHandler == this is true, otherwise we'll
-    // leak mEs->mResultHandler in createHandlerAndFlush and we may crash
-    // later on trying to delete this handler again.
-    NS_ASSERTION(mEs->mResultHandler == this,
-                 "We're leaking mEs->mResultHandler.");
+    if (!mFlushed) {
+        // Make sure that mEs->mResultHandler == this is true, otherwise we'll
+        // leak mEs->mResultHandler in createHandlerAndFlush.
+        NS_ASSERTION(mEs->mResultHandler == this,
+                     "We're leaking mEs->mResultHandler.");

-    nsCOMPtr<nsIAtom> owner;
-    if (!aLowercaseLocalName) {
-        owner = TX_ToLowerCaseAtom(aLocalName);
-        NS_ENSURE_TRUE(owner, NS_ERROR_OUT_OF_MEMORY);
-        
-        aLowercaseLocalName = owner;
+        nsCOMPtr<nsIAtom> owner;
+        if (!aLowercaseLocalName) {
+            owner = TX_ToLowerCaseAtom(aLocalName);
+            NS_ENSURE_TRUE(owner, NS_ERROR_OUT_OF_MEMORY);
+
+            aLowercaseLocalName = owner;
+        }
+
+        PRBool htmlRoot = aNsID == kNameSpaceID_None && !aPrefix &&
+                          aLowercaseLocalName == txHTMLAtoms::html;
+
+        // Use aLocalName and not aLowercaseLocalName in case the output
+        // handler cares about case. For eHTMLOutput the handler will hardcode
+        // to 'html' anyway.
+        nsresult rv = createHandlerAndFlush(htmlRoot,
+                                            nsDependentAtomString(aLocalName),
+                                            aNsID);
+        NS_ENSURE_SUCCESS(rv, rv);
    }

-    PRBool htmlRoot = aNsID == kNameSpaceID_None && !aPrefix &&
-                      aLowercaseLocalName == txHTMLAtoms::html;
-
-    // Use aLocalName and not aLowercaseLocalName in case the output
-    // handler cares about case. For eHTMLOutput the handler will hardcode
-    // to 'html' anyway.
-    nsresult rv = createHandlerAndFlush(htmlRoot,
-                                        nsDependentAtomString(aLocalName),
-                                        aNsID);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    rv = mEs->mResultHandler->startElement(aPrefix, aLocalName,
-                                           aLowercaseLocalName, aNsID);
-
-    delete this;
-
-    return rv;
+    return mEs->mResultHandler->startElement(aPrefix, aLocalName,
+                                             aLowercaseLocalName, aNsID);
 }

 nsresult
 txUnknownHandler::startElement(nsIAtom* aPrefix, const nsSubstring& aLocalName,
                               const PRInt32 aNsID)
 {
-    // Make sure that mEs->mResultHandler == this is true, otherwise we'll
-    // leak mEs->mResultHandler in createHandlerAndFlush and we may crash
-    // later on trying to delete this handler again.
-    NS_ASSERTION(mEs->mResultHandler == this,
-                 "We're leaking mEs->mResultHandler.");
+    if (!mFlushed) {
+        // Make sure that mEs->mResultHandler == this is true, otherwise we'll
+        // leak mEs->mResultHandler in createHandlerAndFlush.
+        NS_ASSERTION(mEs->mResultHandler == this,
+                     "We're leaking mEs->mResultHandler.");

-    PRBool htmlRoot = aNsID == kNameSpaceID_None && !aPrefix &&
-                      aLocalName.Equals(NS_LITERAL_STRING("html"),
-                                        txCaseInsensitiveStringComparator());
-    nsresult rv = createHandlerAndFlush(htmlRoot, aLocalName, aNsID);
-    NS_ENSURE_SUCCESS(rv, rv);
+        PRBool htmlRoot = aNsID == kNameSpaceID_None && !aPrefix &&
+                          aLocalName.Equals(NS_LITERAL_STRING("html"),
+                                            txCaseInsensitiveStringComparator());
+        nsresult rv = createHandlerAndFlush(htmlRoot, aLocalName, aNsID);
+        NS_ENSURE_SUCCESS(rv, rv);
+    }

-    rv = mEs->mResultHandler->startElement(aPrefix, aLocalName, aNsID);
-
-    delete this;
-
-    return rv;
+    return mEs->mResultHandler->startElement(aPrefix, aLocalName, aNsID);
 }

 nsresult txUnknownHandler::createHandlerAndFlush(PRBool aHTMLRoot,
@ -159,11 +213,18 @@ nsresult txUnknownHandler::createHandlerAndFlush(PRBool aHTMLRoot,
                                                                getter_Transfers(handler));
    NS_ENSURE_SUCCESS(rv, rv);

-    rv = mBuffer->flushToHandler(handler);
-    NS_ENSURE_SUCCESS(rv, rv);
-
    mEs->mOutputHandler = handler;
    mEs->mResultHandler = handler.forget();
+    // Let the executionstate delete us. We need to stay alive because we might
+    // need to forward hooks to mEs->mResultHandler if someone is currently
+    // flushing a buffer to mEs->mResultHandler.
+    mEs->mObsoleteHandler = this;

-    return NS_OK;
+    mFlushed = PR_TRUE;
+
+    // Let go of out buffer as soon as we're done flushing it, we're not going
+    // to need it anymore from this point on (all hooks get forwarded to
+    // mEs->mResultHandler.
+    nsAutoPtr<txResultBuffer> buffer(mBuffer);
+    return buffer->flushToHandler(mEs->mResultHandler);
 }
--- a/content/xslt/src/xslt/txUnknownHandler.h
+++ b/content/xslt/src/xslt/txUnknownHandler.h
@ -50,11 +50,7 @@ public:
    txUnknownHandler(txExecutionState* aEs);
    virtual ~txUnknownHandler();

-    nsresult endDocument(nsresult aResult);
-    nsresult startElement(nsIAtom* aPrefix, nsIAtom* aName,
-                          nsIAtom* aLowercaseName, PRInt32 aNsID);
-    nsresult startElement(nsIAtom* aPrefix, const nsSubstring& aLocalName,
-                          const PRInt32 aNsID);
+    TX_DECL_TXAXMLEVENTHANDLER

 private:
    nsresult createHandlerAndFlush(PRBool aHTMLRoot,
@ -67,6 +63,10 @@ private:
     * The right fix may need a txOutputFormat here.
     */
    txExecutionState* mEs;
+
+    // If mFlushed is true then we've replaced mEs->mResultHandler with a
+    // different handler and we should forward to that handler.
+    PRBool mFlushed;
 };

 #endif /* txUnknownHandler_h___ */
--- a/content/xslt/tests/mochitest/Makefile.in
+++ b/content/xslt/tests/mochitest/Makefile.in
@ -55,6 +55,7 @@ _TEST_FILES = 	test_bug319374.xhtml \
 		test_bug566629.html \
 		test_bug566629.xhtml \
 		test_bug603159.html \
+		test_bug667315.html \
 		test_exslt_regex.html \
 		$(NULL)

--- a/content/xslt/tests/mochitest/test_bug667315.html
+++ b/content/xslt/tests/mochitest/test_bug667315.html
@ -0,0 +1,47 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=667315
+-->
+<head>
+  <title>Test for Bug 667315</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=667315">Mozilla Bug 667315</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 667315 **/
+
+var style =
+  '<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" ' +
+                  'version="1.0">' +
+    '<xsl:variable name="var">' +
+      '<html><p>a</p></html>' +
+    '</xsl:variable>' +
+    '<xsl:template match="/">' +
+      '<xsl:copy-of select="$var" />' +
+    '</xsl:template>' +
+  '</xsl:stylesheet>';
+var styleDoc = new DOMParser().parseFromString (style, "text/xml");
+
+var data = '<root/>';
+var originalDoc = new DOMParser().parseFromString(data, "text/xml");
+
+var processor = new XSLTProcessor();
+processor.importStylesheet(styleDoc);
+
+var doc = processor.transformToDocument(originalDoc);
+ok(doc instanceof HTMLDocument, "should have switched to html output method");
+
+</script>
+</pre>
+</body>
+</html>