Bug 1389406 - XSLT — nested xsl:apply-imports doesn't work in some cases. r=erahm.

Use separate classes for parameter and variable maps, and refcount the class for parameter
maps, since a parameter map can be stored in multiple places.

--HG--
extra : rebase_source : 56cc22349a8e03925e3d7446528dd70456117f52

dom/tests/reftest/bug1389406-01.xsl

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xsl:stylesheet version="1.1"
+  xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+>
+
+<xsl:import href="bug1389406-02.xsl"/>
+
+<xsl:template mode="title" match="*">
+  <xsl:value-of select="."/>
+  <xsl:apply-imports />
+</xsl:template>
+
+<xsl:template match="text()|@*"></xsl:template>
+
+</xsl:stylesheet>

dom/tests/reftest/bug1389406-02.xsl

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xsl:stylesheet version="1.0"
+  xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+>
+
+<xsl:template mode="title" match="*">
+  <xsl:text> — TEST CASE</xsl:text>
+  <xsl:apply-imports/>
+</xsl:template>
+
+<xsl:template match="/">
+  <h3 xmlns="http://www.w3.org/1999/xhtml"><xsl:apply-templates select="title" mode="title"/></h3>
+</xsl:template>
+
+</xsl:stylesheet>

dom/tests/reftest/bug1389406-ref.xml

@@ -0,0 +1,2 @@
+<?xml version="1.0"?>
+<h3 xmlns="http://www.w3.org/1999/xhtml">Firefox bug — TEST CASEFirefox bug</h3>

dom/tests/reftest/bug1389406.xml

@@ -0,0 +1,3 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="bug1389406-01.xsl"?>
+<title>Firefox bug</title>

dom/tests/reftest/reftest.list

@@ -11,3 +11,4 @@ fuzzy-if(skiaContent,1,5) == bug559996.html bug559996-ref.html
 == bug592366-1.xhtml bug592366-ref.xhtml
 == bug592366-2.xhtml bug592366-ref.xhtml
 == bug798068.xhtml bug798068-ref.xhtml
+== bug1389406.xml bug1389406-ref.xml

dom/xslt/xslt/txExecutionState.cpp

@@ -90,11 +90,6 @@ txExecutionState::~txExecutionState()
         delete (txAXMLEventHandler*)handlerIter.next();
     }
 
-    txStackIterator paramIter(&mParamStack);
-    while (paramIter.hasNext()) {
-        delete (txVariableMap*)paramIter.next();
-    }
-
     delete mInitialEvalContext;
 }
 
@@ -381,7 +376,7 @@ txExecutionState::popResultHandler()
 void
 txExecutionState::pushTemplateRule(txStylesheet::ImportFrame* aFrame,
                                    const txExpandedName& aMode,
-                                   txVariableMap* aParams)
+                                   txParameterMap* aParams)
 {
     TemplateRule* rule = mTemplateRules.AppendElement();
     rule->mFrame = aFrame;
@@ -521,23 +516,19 @@ txExecutionState::removeVariable(const txExpandedName& aName)
     mLocalVariables->removeVariable(aName);
 }
 
-nsresult
-txExecutionState::pushParamMap(txVariableMap* aParams)
+void
+txExecutionState::pushParamMap(txParameterMap* aParams)
 {
-    nsresult rv = mParamStack.push(mTemplateParams);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    mTemplateParams.forget();
+    mParamStack.AppendElement(mTemplateParams.forget());
     mTemplateParams = aParams;
-
-    return NS_OK;
 }
 
-txVariableMap*
+already_AddRefed<txParameterMap>
 txExecutionState::popParamMap()
 {
-    txVariableMap* oldParams = mTemplateParams.forget();
-    mTemplateParams = (txVariableMap*)mParamStack.pop();
+    RefPtr<txParameterMap> oldParams = mTemplateParams.forget();
+    mTemplateParams = mParamStack.LastElement();
+    mParamStack.RemoveElementAt(mParamStack.Length() - 1);
 
-    return oldParams;
+    return oldParams.forget();
 }

dom/xslt/xslt/txExecutionState.h

@@ -88,7 +88,7 @@ public:
         txStylesheet::ImportFrame* mFrame;
         int32_t mModeNsId;
         nsCOMPtr<nsIAtom> mModeLocalName;
-        txVariableMap* mParams;
+        RefPtr<txParameterMap> mParams;
     };
 
     // Stack functions
@@ -108,10 +108,10 @@ public:
     txAXMLEventHandler* popResultHandler();
     void pushTemplateRule(txStylesheet::ImportFrame* aFrame,
                           const txExpandedName& aMode,
-                          txVariableMap* aParams);
+                          txParameterMap* aParams);
     void popTemplateRule();
-    nsresult pushParamMap(txVariableMap* aParams);
-    txVariableMap* popParamMap();
+    void pushParamMap(txParameterMap* aParams);
+    already_AddRefed<txParameterMap> popParamMap();
 
     // state-getting functions
     txIEvalContext* getEvalContext();
@@ -145,7 +145,7 @@ public:
     nsAutoPtr<txAXMLEventHandler> mObsoleteHandler;
     txAOutputHandlerFactory* mOutputHandlerFactory;
 
-    nsAutoPtr<txVariableMap> mTemplateParams;
+    RefPtr<txParameterMap> mTemplateParams;
 
     RefPtr<txStylesheet> mStylesheet;
 
@@ -155,7 +155,7 @@ private:
     txStack mEvalContextStack;
     nsTArray<bool> mBoolStack;
     txStack mResultHandlerStack;
-    txStack mParamStack;
+    nsTArray<RefPtr<txParameterMap>> mParamStack;
     txInstruction* mNextInstruction;
     txVariableMap* mLocalVariables;
     txVariableMap mGlobalVariableValues;

dom/xslt/xslt/txInstructions.cpp

@@ -39,7 +39,16 @@ txApplyDefaultElementTemplate::execute(txExecutionState& aEs)
 }
 
 nsresult
-txApplyImports::execute(txExecutionState& aEs)
+txApplyImportsEnd::execute(txExecutionState& aEs)
+{
+    aEs.popTemplateRule();
+    RefPtr<txParameterMap> paramMap = aEs.popParamMap();
+
+    return NS_OK;
+}
+
+nsresult
+txApplyImportsStart::execute(txExecutionState& aEs)
 {
     txExecutionState::TemplateRule* rule = aEs.getCurrentTemplateRule();
     // The frame is set to null when there is no current template rule, or
@@ -50,23 +59,22 @@ txApplyImports::execute(txExecutionState& aEs)
         return NS_ERROR_XSLT_EXECUTION_FAILURE;
     }
 
-    nsresult rv = aEs.pushParamMap(rule->mParams);
-    NS_ENSURE_SUCCESS(rv, rv);
+    aEs.pushParamMap(rule->mParams);
 
     txStylesheet::ImportFrame* frame = 0;
     txExpandedName mode(rule->mModeNsId, rule->mModeLocalName);
     txInstruction* templ;
-    rv = aEs.mStylesheet->findTemplate(aEs.getEvalContext()->getContextNode(),
-                                       mode, &aEs, rule->mFrame, &templ,
-                                       &frame);
+    nsresult rv = aEs.mStylesheet->findTemplate(aEs.getEvalContext()->getContextNode(),
+                                                mode, &aEs, rule->mFrame, &templ,
+                                                &frame);
     NS_ENSURE_SUCCESS(rv, rv);
 
     aEs.pushTemplateRule(frame, mode, rule->mParams);
 
     rv = aEs.runTemplate(templ);
-
-    aEs.popTemplateRule();
-    aEs.popParamMap();
+    if (NS_FAILED(rv)) {
+      aEs.popTemplateRule();
+    }
 
     return rv;
 }
@@ -573,7 +581,7 @@ txNumber::execute(txExecutionState& aEs)
 nsresult
 txPopParams::execute(txExecutionState& aEs)
 {
-    delete aEs.popParamMap();
+    RefPtr<txParameterMap> paramMap = aEs.popParamMap();
 
     return NS_OK;
 }
@@ -694,7 +702,8 @@ txPushNullTemplateRule::execute(txExecutionState& aEs)
 nsresult
 txPushParams::execute(txExecutionState& aEs)
 {
-    return aEs.pushParamMap(nullptr);
+    aEs.pushParamMap(nullptr);
+    return NS_OK;
 }
 
 nsresult
@@ -760,8 +769,7 @@ txSetParam::execute(txExecutionState& aEs)
 {
     nsresult rv = NS_OK;
     if (!aEs.mTemplateParams) {
-        aEs.mTemplateParams = new txVariableMap;
-        NS_ENSURE_TRUE(aEs.mTemplateParams, NS_ERROR_OUT_OF_MEMORY);
+        aEs.mTemplateParams = new txParameterMap;
     }
 
     RefPtr<txAExprResult> exprRes;

dom/xslt/xslt/txInstructions.h

@@ -47,7 +47,13 @@ public:
     TX_DECL_TXINSTRUCTION
 };
 
-class txApplyImports : public txInstruction
+class txApplyImportsEnd : public txInstruction
+{
+public:
+    TX_DECL_TXINSTRUCTION
+};
+
+class txApplyImportsStart : public txInstruction
 {
 public:
     TX_DECL_TXINSTRUCTION

dom/xslt/xslt/txStylesheetCompileHandlers.cpp

@@ -1312,7 +1312,8 @@ txFnText(const nsAString& aStr, txStylesheetCompilerState& aState)
 /*
   xsl:apply-imports
 
-  txApplyImports
+  txApplyImportsStart
+  txApplyImportsEnd
 */
 static nsresult
 txFnStartApplyImports(int32_t aNamespaceID,
@@ -1324,7 +1325,11 @@ txFnStartApplyImports(int32_t aNamespaceID,
 {
     nsresult rv = NS_OK;
 
-    nsAutoPtr<txInstruction> instr(new txApplyImports);
+    nsAutoPtr<txInstruction> instr(new txApplyImportsStart);
+    rv = aState.addInstruction(Move(instr));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    instr = new txApplyImportsEnd;
     rv = aState.addInstruction(Move(instr));
     NS_ENSURE_SUCCESS(rv, rv);
 

dom/xslt/xslt/txVariableMap.h

@@ -11,33 +11,59 @@
 #include "txExprResult.h"
 #include "txExpandedNameMap.h"
 
-class txVariableMap {
+/**
+ * Map that maps from expanded name to an expression result value. This is just a base
+ * class, use txVariableMap or txParameterMap instead.
+ */
+class txVariableMapBase {
 public:
-    txVariableMap();
-    ~txVariableMap();
-
     nsresult bindVariable(const txExpandedName& aName, txAExprResult* aValue);
 
     void getVariable(const txExpandedName& aName, txAExprResult** aResult);
 
     void removeVariable(const txExpandedName& aName);
 
-private:
+protected:
+    txVariableMapBase()
+    {}
+    ~txVariableMapBase();
+
     txExpandedNameMap<txAExprResult> mMap;
 };
 
+/**
+ * Map for mapping from expanded name to variable values. This is not refcounted, so
+ * owners need to be careful to clean this up.
+ */
+class txVariableMap : public txVariableMapBase {
+public:
+    txVariableMap()
+      : txVariableMapBase()
+    {
+        MOZ_COUNT_CTOR(txVariableMap);
+    }
+    ~txVariableMap()
+    {
+        MOZ_COUNT_DTOR(txVariableMap);
+    }
+};
+
+/**
+ * Map for mapping from expanded name to parameter values. This is refcounted, so multiple
+ * owners can hold a reference.
+ */
+class txParameterMap : public txVariableMapBase {
+public:
+    NS_INLINE_DECL_REFCOUNTING(txParameterMap)
+
+private:
+    ~txParameterMap()
+    {}
+};
 
 inline
-txVariableMap::txVariableMap()
+txVariableMapBase::~txVariableMapBase()
 {
-    MOZ_COUNT_CTOR(txVariableMap);
-}
-
-inline
-txVariableMap::~txVariableMap()
-{
-    MOZ_COUNT_DTOR(txVariableMap);
-
     txExpandedNameMap<txAExprResult>::iterator iter(mMap);
     while (iter.next()) {
         txAExprResult* res = iter.value();
@@ -46,7 +72,7 @@ txVariableMap::~txVariableMap()
 }
 
 inline nsresult
-txVariableMap::bindVariable(const txExpandedName& aName, txAExprResult* aValue)
+txVariableMapBase::bindVariable(const txExpandedName& aName, txAExprResult* aValue)
 {
     NS_ASSERTION(aValue, "can't add null-variables to a txVariableMap");
     nsresult rv = mMap.add(aName, aValue);
@@ -60,7 +86,7 @@ txVariableMap::bindVariable(const txExpandedName& aName, txAExprResult* aValue)
 }
 
 inline void
-txVariableMap::getVariable(const txExpandedName& aName, txAExprResult** aResult)
+txVariableMapBase::getVariable(const txExpandedName& aName, txAExprResult** aResult)
 {
     *aResult = mMap.get(aName);
     if (*aResult) {
@@ -69,7 +95,7 @@ txVariableMap::getVariable(const txExpandedName& aName, txAExprResult** aResult)
 }
 
 inline void
-txVariableMap::removeVariable(const txExpandedName& aName)
+txVariableMapBase::removeVariable(const txExpandedName& aName)
 {
     txAExprResult* var = mMap.remove(aName);
     NS_IF_RELEASE(var);