Bug 203229: Use strings better by using existing stringobjects inside ExprResults if one exists.

r=Pike sr=peterv a=asa

content/xslt/src/xpath/txBooleanResult.cpp

@@ -62,6 +62,15 @@ void BooleanResult::stringValue(nsAString& str)  {
     else str.Append(NS_LITERAL_STRING("false"));
 } //-- toString
 
+nsAString*
+BooleanResult::stringValuePointer()
+{
+    // In theory we could set strings containing "true" and "false" somewhere,
+    // but most stylesheets never get the stringvalue of a bool so that won't
+    // really buy us anything.
+    return nsnull;
+}
+
 MBool BooleanResult::booleanValue() {
    return this->value;
 } //-- toBoolean

content/xslt/src/xpath/txExpr.h

@@ -517,11 +517,8 @@ private:
 class RelationalExpr : public Expr {
 
 public:
-
+    enum RelationalExprType {
-    //-- RelationalExpr Types
+        EQUAL,
-    //-- LF, changed from static const short to enum
-    enum _RelationalExprType {
-        EQUAL = 1,
         NOT_EQUAL,
         LESS_THAN,
         GREATER_THAN,
@@ -529,18 +526,17 @@ public:
         GREATER_OR_EQUAL
     };
 
-     RelationalExpr(Expr* leftExpr, Expr* rightExpr, short op);
+    RelationalExpr(Expr* aLeftExpr, Expr* aRightExpr, RelationalExprType aOp);
-     ~RelationalExpr();
 
     TX_DECL_EXPR;
 
 private:
-    short op;
+    PRBool compareResults(ExprResult* aLeft, ExprResult* aRight);
-    Expr* leftExpr;
-    Expr* rightExpr;
 
-    MBool compareResults(ExprResult* left, ExprResult* right);
+    nsAutoPtr<Expr> mLeftExpr;
-}; //-- RelationalExpr
+    nsAutoPtr<Expr> mRightExpr;
+    RelationalExprType mOp;
+};
 
 /**
  * VariableRefExpr

content/xslt/src/xpath/txExprResult.h

@@ -74,6 +74,12 @@ public:
     **/
     virtual void stringValue(nsAString& str) = 0;
 
+    /**
+     * Returns a pointer to the stringvalue if possible. Otherwise null is
+     * returned.
+     */
+    virtual nsAString* stringValuePointer() = 0;
+
     /**
      * Converts this ExprResult to a Boolean (MBool) value
      * @return the Boolean value
@@ -88,6 +94,15 @@ public:
 
 };
 
+#define TX_DECL_EXPRRESULT                                        \
+    virtual ExprResult* clone();                                  \
+    virtual short getResultType();                                \
+    virtual void stringValue(nsAString& str);                     \
+    virtual nsAString* stringValuePointer();                      \
+    virtual PRBool booleanValue();                                \
+    virtual double numberValue();                                 \
+
+
 class BooleanResult : public ExprResult {
 
 public:
@@ -95,11 +110,7 @@ public:
     BooleanResult();
     BooleanResult(MBool boolean);
 
-    virtual ExprResult* clone();
+    TX_DECL_EXPRRESULT
-    virtual short  getResultType();
-    virtual void   stringValue(nsAString& str);
-    virtual MBool  booleanValue();
-    virtual double numberValue();
 
 private:
     MBool value;
@@ -112,11 +123,7 @@ public:
     NumberResult();
     NumberResult(double dbl);
 
-    virtual ExprResult* clone();
+    TX_DECL_EXPRRESULT
-    virtual short  getResultType();
-    virtual void   stringValue(nsAString& str);
-    virtual MBool  booleanValue();
-    virtual double numberValue();
 
 private:
     double value;
@@ -129,11 +136,7 @@ public:
     StringResult();
     StringResult(const nsAString& str);
 
-    virtual ExprResult* clone();
+    TX_DECL_EXPRRESULT
-    virtual short  getResultType();
-    virtual void   stringValue(nsAString& str);
-    virtual MBool  booleanValue();
-    virtual double numberValue();
 
     nsString mValue;
 };

content/xslt/src/xpath/txNumberResult.cpp

@@ -63,6 +63,12 @@ void NumberResult::stringValue(nsAString& str)  {
     Double::toString(value, str);
 } //-- stringValue
 
+nsAString*
+NumberResult::stringValuePointer()
+{
+    return nsnull;
+}
+
 MBool NumberResult::booleanValue() {
   // OG+
   // As per the XPath spec, the boolean value of a number is true if and only if

content/xslt/src/xpath/txRelationalExpr.cpp

@@ -31,217 +31,160 @@
 #include "NodeSet.h"
 #include "XMLDOMUtils.h"
 
-  //------------------/
+RelationalExpr::RelationalExpr(Expr* aLeftExpr, Expr* aRightExpr,
- //- RelationalExpr -/
+                               RelationalExprType aOp)
-//------------------/
+    : mLeftExpr(aLeftExpr), mRightExpr(aRightExpr), mOp(aOp)
-
+{
-RelationalExpr::RelationalExpr(Expr* leftExpr, Expr* rightExpr, short op) {
+}
-    this->op = op;
-    this->leftExpr = leftExpr;
-    this->rightExpr = rightExpr;
-} //-- RelationalExpr
-
-RelationalExpr::~RelationalExpr() {
-    delete leftExpr;
-    delete rightExpr;
-} //-- ~RelationalExpr
 
 /**
  *  Compares the two ExprResults based on XPath 1.0 Recommendation (section 3.4)
-**/
+ */
-MBool RelationalExpr::compareResults(ExprResult* left, ExprResult* right) {
+PRBool
+RelationalExpr::compareResults(ExprResult* aLeft, ExprResult* aRight)
+{
+    short ltype = aLeft->getResultType();
+    short rtype = aRight->getResultType();
 
-
+    // Handle case for just Left NodeSet or Both NodeSets
-    short ltype = left->getResultType();
-    short rtype = right->getResultType();
-
-    MBool result = MB_FALSE;
-
-    //-- handle case for just Left NodeSet or Both NodeSets
     if (ltype == ExprResult::NODESET) {
         if (rtype == ExprResult::BOOLEAN) {
-            BooleanResult leftBool(left->booleanValue());
+            BooleanResult leftBool(aLeft->booleanValue());
-            return compareResults(&leftBool, right);
+            return compareResults(&leftBool, aRight);
         }
 
-        NodeSet* nodeSet = (NodeSet*)left;
+        NodeSet* nodeSet = NS_STATIC_CAST(NodeSet*, aLeft);
-        for ( int i = 0; i < nodeSet->size(); i++) {
+        StringResult strResult;
-                nsAutoString str;
+        int i;
-                Node* node = nodeSet->get(i);
+        for (i = 0; i < nodeSet->size(); ++i) {
-                XMLDOMUtils::getNodeValue(node, str);
+            Node* node = nodeSet->get(i);
-                StringResult strResult(str);
+            strResult.mValue.Truncate();
-                result = compareResults(&strResult, right);
+            XMLDOMUtils::getNodeValue(node, strResult.mValue);
-                if ( result ) break;
+            if (compareResults(&strResult, aRight)) {
+                return PR_TRUE;
+            }
         }
+
+        return PR_FALSE;
     }
-    //-- handle case for Just Right NodeSet
+
-    else if ( rtype == ExprResult::NODESET) {
+    // Handle case for Just Right NodeSet
+    if (rtype == ExprResult::NODESET) {
         if (ltype == ExprResult::BOOLEAN) {
-            BooleanResult rightBool(right->booleanValue());
+            BooleanResult rightBool(aRight->booleanValue());
-            return compareResults(left, &rightBool);
+            return compareResults(aLeft, &rightBool);
         }
 
-        NodeSet* nodeSet = (NodeSet*)right;
+        NodeSet* nodeSet = NS_STATIC_CAST(NodeSet*, aRight);
-        for ( int i = 0; i < nodeSet->size(); i++) {
+        StringResult strResult;
-                nsAutoString str;
+        int i;
-                Node* node = nodeSet->get(i);
+        for (i = 0; i < nodeSet->size(); ++i) {
-                XMLDOMUtils::getNodeValue(node, str);
+            Node* node = nodeSet->get(i);
-                StringResult strResult(str);
+            strResult.mValue.Truncate();
-                result = compareResults(left, &strResult);
+            XMLDOMUtils::getNodeValue(node, strResult.mValue);
-                if ( result ) break;
+            if (compareResults(aLeft, &strResult)) {
+                return PR_TRUE;
+            }
         }
+
+        return PR_FALSE;
     }
-    //-- neither NodeSet
-    else {
-        if ( op == NOT_EQUAL) {
 
-            if ((ltype == ExprResult::BOOLEAN)
+    // Neither is a NodeSet
-                    || (rtype == ExprResult::BOOLEAN)) {
+    if (mOp == EQUAL || mOp == NOT_EQUAL) {
-                result = (left->booleanValue() != right->booleanValue());
+        PRBool result;
-            }
+        nsAString *lString, *rString;
-            else if ((ltype == ExprResult::NUMBER) ||
+
-                            (rtype == ExprResult::NUMBER)) {
+        // If either is a bool, compare as bools.
-                double lval = left->numberValue();
+        if (ltype == ExprResult::BOOLEAN || rtype == ExprResult::BOOLEAN) {
-                double rval = right->numberValue();
+            result = aLeft->booleanValue() == aRight->booleanValue();
+        }
+
+        // If either is a number, compare as numbers.
+        else if (ltype == ExprResult::NUMBER || rtype == ExprResult::NUMBER) {
+            double lval = aLeft->numberValue();
+            double rval = aRight->numberValue();
 #if defined(XP_WIN) || defined(XP_OS2)
-                if (Double::isNaN(lval) || Double::isNaN(rval))
+            if (Double::isNaN(lval) || Double::isNaN(rval))
-                    result = MB_TRUE;
+                result = PR_FALSE;
-                else
+            else
-                    result = (lval != rval);
+                result = lval == rval;
 #else
-                result = (lval != rval);
+            result = lval == rval;
 #endif
+        }
+
+        // Otherwise compare as strings. Try to use the stringobject in
+        // StringResult if possible since that is a common case.
+        else if ((lString = aLeft->stringValuePointer())) {
+            if ((rString = aRight->stringValuePointer())) {
+                result = lString->Equals(*rString);
             }
             else {
-                nsAutoString lStr;
-                left->stringValue(lStr);
                 nsAutoString rStr;
-                right->stringValue(rStr);
+                aRight->stringValue(rStr);
-                result = !lStr.Equals(rStr);
+                result = rStr.Equals(*lString);
             }
         }
-        else if ( op == EQUAL) {
+        else if ((rString = aRight->stringValuePointer())) {
-
+            nsAutoString lStr;
-            if ((ltype == ExprResult::BOOLEAN)
+            aLeft->stringValue(lStr);
-                    || (rtype == ExprResult::BOOLEAN)) {
+            result = rString->Equals(lStr);
-                result = (left->booleanValue() == right->booleanValue());
-            }
-            else if ((ltype == ExprResult::NUMBER) ||
-                            (rtype == ExprResult::NUMBER)) {
-                double lval = left->numberValue();
-                double rval = right->numberValue();
-#if defined(XP_WIN) || defined(XP_OS2)
-                if (Double::isNaN(lval) || Double::isNaN(rval))
-                    result = MB_FALSE;
-                else
-                    result = (lval == rval);
-#else
-                result = (lval == rval);
-#endif
-            }
-            else {
-                nsAutoString lStr;
-                left->stringValue(lStr);
-                nsAutoString rStr;
-                right->stringValue(rStr);
-                result = lStr.Equals(rStr);
-            }
-
         }
         else {
-            double leftDbl = left->numberValue();
+            nsAutoString lStr, rStr;
-            double rightDbl = right->numberValue();
+            aLeft->stringValue(lStr);
-            switch( op ) {
+            aRight->stringValue(rStr);
-                case LESS_THAN:
+            result = lStr.Equals(rStr);
-#if defined(XP_WIN) || defined(XP_OS2)
-                    if (Double::isNaN(leftDbl) || Double::isNaN(rightDbl))
-                        result = MB_FALSE;
-                    else
-                        result = (leftDbl < rightDbl);
-#else
-                    result = (leftDbl < rightDbl);
-#endif
-                    break;
-                case LESS_OR_EQUAL:
-#if defined(XP_WIN) || defined(XP_OS2)
-                    if (Double::isNaN(leftDbl) || Double::isNaN(rightDbl))
-                        result = MB_FALSE;
-                    else
-                        result = (leftDbl <= rightDbl);
-#else
-                    result = (leftDbl <= rightDbl);
-#endif
-                    break;
-                case GREATER_THAN :
-#if defined(XP_WIN) || defined(XP_OS2)
-                    if (Double::isNaN(leftDbl) || Double::isNaN(rightDbl))
-                        result = MB_FALSE;
-                    else
-                        result = (leftDbl > rightDbl);
-#else
-                    result = (leftDbl > rightDbl);
-#endif
-                    break;
-                case GREATER_OR_EQUAL:
-#if defined(XP_WIN) || defined(XP_OS2)
-                    if (Double::isNaN(leftDbl) || Double::isNaN(rightDbl))
-                        result = MB_FALSE;
-                    else
-                        result = (leftDbl >= rightDbl);
-#else
-                    result = (leftDbl >= rightDbl);
-#endif
-                    break;
-            }
         }
-    }
-    return result;
-} //-- compareResult
 
-/**
+        return mOp == EQUAL ? result : !result;
- * Evaluates this Expr based on the given context node and processor state
+    }
- * @param context the context node for evaluation of this Expr
+
- * @param ps the ContextState containing the stack information needed
+    double leftDbl = aLeft->numberValue();
- * for evaluation
+    double rightDbl = aRight->numberValue();
- * @return the result of the evaluation
+#if defined(XP_WIN) || defined(XP_OS2)
-**/
+    if (Double::isNaN(leftDbl) || Double::isNaN(rightDbl))
-ExprResult* RelationalExpr::evaluate(txIEvalContext* aContext)
+        return PR_FALSE;
+#endif
+
+    switch (mOp) {
+        case LESS_THAN:
+            return leftDbl < rightDbl;
+
+        case LESS_OR_EQUAL:
+            return leftDbl <= rightDbl;
+
+        case GREATER_THAN :
+            return leftDbl > rightDbl;
+
+        case GREATER_OR_EQUAL:
+            return leftDbl >= rightDbl;
+    }
+
+    NS_NOTREACHED("We should have cought all cases");
+
+    return PR_FALSE;
+}
+
+ExprResult*
+RelationalExpr::evaluate(txIEvalContext* aContext)
 {
-    //-- get result of left expression
+    nsAutoPtr<ExprResult> lResult(mLeftExpr->evaluate(aContext));
-    ExprResult* lResult = 0;
+    NS_ENSURE_TRUE(lResult, nsnull);
-    if (leftExpr)
-        lResult = leftExpr->evaluate(aContext);
-    else
-        return new BooleanResult();
 
-    //-- get result of right expr
+    nsAutoPtr<ExprResult> rResult(mRightExpr->evaluate(aContext));
-    ExprResult* rResult = 0;
+    NS_ENSURE_TRUE(rResult, nsnull);
-    if (rightExpr)
-        rResult = rightExpr->evaluate(aContext);
-    else {
-        delete lResult;
-        return new BooleanResult();
-    }
-    BooleanResult* boolResult = new BooleanResult(compareResults(lResult, rResult));
-    delete lResult;
-    delete rResult;
-    return boolResult;
-} //-- evaluate
 
-/**
+    return new BooleanResult(compareResults(lResult, rResult));
- * Returns the String representation of this Expr.
+}
- * @param dest the String to use when creating the String
- * representation. The String representation will be appended to
- * any data in the destination String, to allow cascading calls to
- * other #toString() methods for Expressions.
- * @return the String representation of this Expr.
-**/
-void RelationalExpr::toString(nsAString& str) {
 
-    if ( leftExpr ) leftExpr->toString(str);
+void
-    else str.Append(NS_LITERAL_STRING("null"));
+RelationalExpr::toString(nsAString& str)
+{
+    mLeftExpr->toString(str);
 
-    switch ( op ) {
+    switch (mOp) {
         case NOT_EQUAL:
             str.Append(NS_LITERAL_STRING("!="));
             break;
@@ -262,8 +205,5 @@ void RelationalExpr::toString(nsAString& str) {
             break;
     }
 
-    if ( rightExpr ) rightExpr->toString(str);
+    mRightExpr->toString(str);
-    else str.Append(NS_LITERAL_STRING("null"));
+}
-
-} //-- toString
-

content/xslt/src/xpath/txStringResult.cpp

@@ -61,6 +61,12 @@ void StringResult::stringValue(nsAString& str)  {
     str.Append(mValue);
 } //-- stringValue
 
+nsAString*
+StringResult::stringValuePointer()
+{
+    return &mValue;
+}
+
 MBool StringResult::booleanValue() {
    return !mValue.IsEmpty();
 } //-- booleanValue

content/xslt/src/xslt/txInstructions.cpp

@@ -575,14 +575,18 @@ txLREAttribute::execute(txExecutionState& aEs)
         mLocalName->ToString(nodeName);
     }
 
-    ExprResult* exprRes = mValue->evaluate(aEs.getEvalContext());
+    nsAutoPtr<ExprResult> exprRes = mValue->evaluate(aEs.getEvalContext());
     NS_ENSURE_TRUE(exprRes, NS_ERROR_FAILURE);
 
-    nsAutoString value;
+    nsAString* value = exprRes->stringValuePointer();
-    exprRes->stringValue(value);
+    if (value) {
-    delete exprRes;
+        aEs.mResultHandler->attribute(nodeName, mNamespaceID, *value);
-
+    }
-    aEs.mResultHandler->attribute(nodeName, mNamespaceID, value);
+    else {
+        nsAutoString valueStr;
+        exprRes->stringValue(valueStr);
+        aEs.mResultHandler->attribute(nodeName, mNamespaceID, valueStr);
+    }
 
     return NS_OK;
 }
@@ -1040,15 +1044,23 @@ txValueOf::txValueOf(nsAutoPtr<Expr> aExpr, PRBool aDOE)
 nsresult
 txValueOf::execute(txExecutionState& aEs)
 {
-    ExprResult* exprRes = mExpr->evaluate(aEs.getEvalContext());
+    nsAutoPtr<ExprResult> exprRes = mExpr->evaluate(aEs.getEvalContext());
     NS_ENSURE_TRUE(exprRes, NS_ERROR_FAILURE);
 
-    nsAutoString value;
+    nsAString* value = exprRes->stringValuePointer();
-    exprRes->stringValue(value);
+    if (value) {
-    delete exprRes;
+        if (!value->IsEmpty()) {
-
+            aEs.mResultHandler->characters(*value, mDOE);
-    if (!value.IsEmpty()) {
+        }
-        aEs.mResultHandler->characters(value, mDOE);
     }
+    else {
+        nsAutoString valueStr;
+        exprRes->stringValue(valueStr);
+        if (!valueStr.IsEmpty()) {
+            aEs.mResultHandler->characters(valueStr, mDOE);
+        }
+    }
+
+
     return NS_OK;
 }

content/xslt/src/xslt/txRtfHandler.cpp

@@ -67,6 +67,11 @@ void txResultTreeFragment::stringValue(nsAString& aResult)
     aResult.Append(mBuffer->mStringValue);
 }
 
+nsAString* txResultTreeFragment::stringValuePointer()
+{
+    return mBuffer ? &mBuffer->mStringValue : nsnull;
+}
+
 PRBool txResultTreeFragment::booleanValue()
 {
     return PR_TRUE;

content/xslt/src/xslt/txRtfHandler.h

@@ -49,11 +49,7 @@ public:
     txResultTreeFragment(txResultBuffer* aBuffer);
     ~txResultTreeFragment();
 
-    virtual ExprResult* clone();
+    TX_DECL_EXPRRESULT
-    virtual short getResultType();
-    virtual void stringValue(nsAString& aResult);
-    virtual MBool booleanValue();
-    virtual double numberValue();
 
     nsresult flushToHandler(txAXMLEventHandler* aHandler);