Get rid of the signal expression rewriter

This replaces the entire rewriter with more or less:

expressionToEval = "(function(<named params here>) { " + expr + " } )"

This also fixes crashes at run-time when the signal rewriter was
executed from the loader thread and tried to use a v4 identifier
hash with the same engine that's also in the main thread.

Change-Id: Ib1e4927d330706a593411fbff64ed3da1e23d0e0
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

1 files changed, 66 insertions, 95 deletions

diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp
index e79aaefe1e..80a06e9761 100644
--- a/src/qml/qml/qqmlboundsignal.cpp
+++ b/src/qml/qml/qqmlboundsignal.cpp
@@ -50,7 +50,6 @@
 #include "qqml.h"
 #include "qqmlcontext.h"
 #include "qqmlglobal_p.h"
-#include "qqmlrewrite_p.h"
 #include <private/qqmlprofilerservice_p.h>
 #include <private/qv8debugservice_p.h>
 #include "qqmlinfo.h"
@@ -68,45 +67,21 @@ static QQmlJavaScriptExpression::VTable QQmlBoundSignalExpression_jsvtable = {
 };
 
 QQmlBoundSignalExpression::QQmlBoundSignalExpression(QObject *target, int index,
-                                                     QQmlContextData *ctxt, QObject *scope, const QByteArray &expression,
-                                                     bool isRewritten, const QString &fileName, quint16 line, quint16 column)
-    : QQmlJavaScriptExpression(&QQmlBoundSignalExpression_jsvtable),
-      m_fileName(fileName),
-      m_line(line),
-      m_column(column),
-      m_parameterCountForJS(-1),
-      m_target(target),
-      m_index(index),
-      m_expressionFunctionValid(false),
-      m_expressionFunctionRewritten(isRewritten),
-      m_invalidParameterName(false)
-{
-    init(ctxt, scope);
-    if (isRewritten)
-        m_expressionUtf8 = expression;
-    else
-        m_expression = QString::fromUtf8(expression);
-}
-
-QQmlBoundSignalExpression::QQmlBoundSignalExpression(QObject *target, int index,
                                                      QQmlContextData *ctxt, QObject *scope, const QString &expression,
-                                                     bool isRewritten, const QString &fileName, quint16 line, quint16 column)
+                                                     const QString &fileName, quint16 line, quint16 column,
+                                                     const QString &handlerName)
     : QQmlJavaScriptExpression(&QQmlBoundSignalExpression_jsvtable),
       m_fileName(fileName),
       m_line(line),
       m_column(column),
-      m_parameterCountForJS(-1),
       m_target(target),
       m_index(index),
       m_expressionFunctionValid(false),
-      m_expressionFunctionRewritten(isRewritten),
       m_invalidParameterName(false)
 {
     init(ctxt, scope);
-    if (isRewritten)
-        m_expressionUtf8 = expression.toUtf8();
-    else
-        m_expression = expression;
+    m_handlerName = handlerName;
+    m_expression = expression;
 }
 
 void QQmlBoundSignalExpression::init(QQmlContextData *ctxt, QObject *scope)
@@ -139,8 +114,6 @@ QString QQmlBoundSignalExpression::expression() const
     if (m_expressionFunctionValid) {
         Q_ASSERT (context() && engine());
         return m_v8function.value().toQString();
-    } else if (!m_expressionUtf8.isEmpty()) {
-        return QString::fromUtf8(m_expressionUtf8);
     } else {
         return m_expression;
     }
@@ -164,47 +137,49 @@ void QQmlBoundSignalExpression::evaluate(void **a)
             //TODO: look at using the property cache here (as in the compiler)
             //      for further optimization
             QMetaMethod signal = QMetaObjectPrivate::signal(m_target->metaObject(), m_index);
-            QQmlRewrite::RewriteSignalHandler rewriter(ep->v4engine());
 
             QString expression;
-            bool ok = true;
-
-            if (m_expressionFunctionRewritten) {
-                expression = QString::fromUtf8(m_expressionUtf8);
-
-                //if we need parameters, and the rewrite doesn't include them,
-                //create and insert the parameter string now
-                if (m_parameterCountForJS == -1 && signal.parameterCount()) {
-                    const QString &parameters = rewriter.createParameterString(signal.parameterNames(),
-                                                                               ep->v8engine()->illegalNames());
-                    int index = expression.indexOf(QLatin1Char('('), 1);
-                    Q_ASSERT(index > -1);
-                    expression.insert(index + 1, parameters);
 
-                    setParameterCountForJS(rewriter.parameterCountForJS());
+            expression = QStringLiteral("(function ");
+            expression += m_handlerName;
+            expression += QLatin1Char('(');
+
+            QString error;
+
+            bool unnamedParameter = false;
+            const QV4::IdentifierHash<bool> &illegalNames = ep->v8engine()->illegalNames();
+
+            const QList<QByteArray> parameters = signal.parameterNames();
+            for (int i = 0; i < parameters.count(); ++i) {
+                if (i > 0)
+                    expression += QLatin1Char(',');
+                const QByteArray &param = parameters.at(i);
+                if (param.isEmpty())
+                    unnamedParameter = true;
+                else if (unnamedParameter) {
+                    error = QCoreApplication::translate("QQmlRewrite", "Signal uses unnamed parameter followed by named parameter.");
+                    break;
+                } else if (illegalNames.contains(param)) {
+                    error = QCoreApplication::translate("QQmlRewrite", "Signal parameter \"%1\" hides global variable.").arg(QString::fromUtf8(param));
+                    break;
                 }
-
-                m_expressionUtf8.clear();
-            } else {
-                //expression is still in its original form, so perform a full rewrite
-                expression = rewriter(m_expression, QString()/*no name hint available*/, &ok,
-                                      signal.parameterNames(),
-                                      ep->v8engine()->illegalNames());
-                setParameterCountForJS(rewriter.parameterCountForJS());
-                m_expression.clear();
+                expression += QString::fromUtf8(param);
             }
 
-            if (rewriter.hasParameterError()) {
-                qmlInfo(scopeObject()) << rewriter.parameterError();
+            if (!error.isEmpty()) {
+                qmlInfo(scopeObject()) << error;
                 m_invalidParameterName = true;
                 ep->dereferenceScarceResources();
                 return;
             }
 
-            if (ok) {
-                m_v8function = evalFunction(context(), scopeObject(), expression,
-                                            m_fileName, m_line, &m_v8qmlscope);
-            }
+            expression += QStringLiteral(") { ");
+            expression += m_expression;
+            expression += QStringLiteral(" })");
+            m_expression.clear();
+
+            m_v8function = evalFunction(context(), scopeObject(), expression,
+                                        m_fileName, m_line, &m_v8qmlscope);
 
             if (m_v8function.isEmpty() || m_v8function.value().isNull()) {
                 ep->dereferenceScarceResources();
@@ -214,42 +189,38 @@ void QQmlBoundSignalExpression::evaluate(void **a)
             m_expressionFunctionValid = true;
         }
 
-        if (!hasParameterInfo()) {
-            QQmlJavaScriptExpression::evaluate(context(), m_v8function.value(), 0);
-        } else {
-            QV8Engine *engine = ep->v8engine();
-            QVarLengthArray<int, 9> dummy;
-            //TODO: lookup via signal index rather than method index as an optimization
-            int methodIndex = QMetaObjectPrivate::signal(m_target->metaObject(), m_index).methodIndex();
-            int *argsTypes = QQmlPropertyCache::methodParameterTypes(m_target, methodIndex, dummy, 0);
-            int argCount = argsTypes ? m_parameterCountForJS : 0;
-
-            QVarLengthArray<QV4::Value, 9> args(argCount);
-
-            for (int ii = 0; ii < argCount; ++ii) {
-                int type = argsTypes[ii + 1];
-                //### ideally we would use metaTypeToJS, however it currently gives different results
-                //    for several cases (such as QVariant type and QObject-derived types)
-                //args[ii] = engine->metaTypeToJS(type, a[ii + 1]);
-                if (type == QMetaType::QVariant) {
-                    args[ii] = engine->fromVariant(*((QVariant *)a[ii + 1]));
-                } else if (type == QMetaType::Int) {
-                    //### optimization. Can go away if we switch to metaTypeToJS, or be expanded otherwise
-                    args[ii] = QV4::Value::fromInt32(*reinterpret_cast<const int*>(a[ii + 1]));
-                } else if (type == qMetaTypeId<QQmlV4Handle>()) {
-                    args[ii] = reinterpret_cast<QQmlV4Handle *>(a[ii + 1])->toValue();
-                } else if (ep->isQObject(type)) {
-                    if (!*reinterpret_cast<void* const *>(a[ii + 1]))
-                        args[ii] = QV4::Value::nullValue();
-                    else
-                        args[ii] = QV4::QObjectWrapper::wrap(ep->v4engine(), *reinterpret_cast<QObject* const *>(a[ii + 1]));
-                } else {
-                    args[ii] = engine->fromVariant(QVariant(type, a[ii + 1]));
-                }
+        QV8Engine *engine = ep->v8engine();
+        QVarLengthArray<int, 9> dummy;
+        //TODO: lookup via signal index rather than method index as an optimization
+        int methodIndex = QMetaObjectPrivate::signal(m_target->metaObject(), m_index).methodIndex();
+        int *argsTypes = QQmlPropertyCache::methodParameterTypes(m_target, methodIndex, dummy, 0);
+        int argCount = argsTypes ? *argsTypes : 0;
+
+        QVarLengthArray<QV4::Value, 9> args(argCount);
+
+        for (int ii = 0; ii < argCount; ++ii) {
+            int type = argsTypes[ii + 1];
+            //### ideally we would use metaTypeToJS, however it currently gives different results
+            //    for several cases (such as QVariant type and QObject-derived types)
+            //args[ii] = engine->metaTypeToJS(type, a[ii + 1]);
+            if (type == QMetaType::QVariant) {
+                args[ii] = engine->fromVariant(*((QVariant *)a[ii + 1]));
+            } else if (type == QMetaType::Int) {
+                //### optimization. Can go away if we switch to metaTypeToJS, or be expanded otherwise
+                args[ii] = QV4::Value::fromInt32(*reinterpret_cast<const int*>(a[ii + 1]));
+            } else if (type == qMetaTypeId<QQmlV4Handle>()) {
+                args[ii] = reinterpret_cast<QQmlV4Handle *>(a[ii + 1])->toValue();
+            } else if (ep->isQObject(type)) {
+                if (!*reinterpret_cast<void* const *>(a[ii + 1]))
+                    args[ii] = QV4::Value::nullValue();
+                else
+                    args[ii] = QV4::QObjectWrapper::wrap(ep->v4engine(), *reinterpret_cast<QObject* const *>(a[ii + 1]));
+            } else {
+                args[ii] = engine->fromVariant(QVariant(type, a[ii + 1]));
             }
-
-            QQmlJavaScriptExpression::evaluate(context(), m_v8function.value(), argCount, args.data(), 0);
         }
+
+        QQmlJavaScriptExpression::evaluate(context(), m_v8function.value(), argCount, args.data(), 0);
     }
     ep->dereferenceScarceResources(); // "release" scarce resources if top-level expression evaluation is complete.
 }