Make qmlecmascript:selfDeletingBinding pass

Expressions and bindings must not reference data following their
evalutation incase their object has been deleted.  To solve this, the
needed data is separated into a reference counted QmlExpressionData
and QmlBindingData object.

4 files changed, 182 insertions, 133 deletions

diff --git a/src/declarative/qml/qmlbinding.cpp b/src/declarative/qml/qmlbinding.cpp
index 454369b..3a34f46 100644
--- a/src/declarative/qml/qmlbinding.cpp
+++ b/src/declarative/qml/qmlbinding.cpp
@@ -57,11 +57,16 @@ QT_BEGIN_NAMESPACE
 
 QML_DEFINE_NOCREATE_TYPE(QmlBinding);
 
-QmlBindingPrivate::QmlBindingPrivate()
+QmlBindingData::QmlBindingData()
 : updating(false), enabled(false)
 {
 }
 
+QmlBindingPrivate::QmlBindingPrivate()
+: QmlExpressionPrivate(new QmlBindingData)
+{
+}
+
 QmlBinding::QmlBinding(void *data, QmlRefCount *rc, QObject *obj, QmlContext *ctxt, QObject *parent)
 : QmlExpression(ctxt, data, rc, obj, *new QmlBindingPrivate)
 {
@@ -81,7 +86,7 @@ QmlBinding::~QmlBinding()
 void QmlBinding::setTarget(const QmlMetaProperty &prop)
 {
     Q_D(QmlBinding);
-    d->property = prop;
+    d->bindingData()->property = prop;
 
     update();
 }
@@ -89,7 +94,7 @@ void QmlBinding::setTarget(const QmlMetaProperty &prop)
 QmlMetaProperty QmlBinding::property() const 
 {
    Q_D(const QmlBinding);
-   return d->property; 
+   return d->bindingData()->property; 
 }
 
 void QmlBinding::update()
@@ -99,45 +104,41 @@ void QmlBinding::update()
 #ifdef Q_ENABLE_PERFORMANCE_LOG
     QFxPerfTimer<QFxPerf::BindableValueUpdate> bu;
 #endif
-    if (!d->enabled)
+    QmlBindingData *data = d->bindingData();
+
+    if (!data->enabled)
         return;
 
-    if (!d->updating) {
-        d->updating = true;
+    data->addref();
 
-        if (d->property.propertyCategory() == QmlMetaProperty::Bindable) {
+    if (!data->updating) {
+        data->updating = true;
 
-            int idx = d->property.coreIndex();
+        if (data->property.propertyCategory() == QmlMetaProperty::Bindable) {
+
+            int idx = data->property.coreIndex();
             Q_ASSERT(idx != -1);
 
             void *a[1];
             QmlBinding *t = this;
             a[0] = (void *)&t;
-            QMetaObject::metacall(d->property.object(), 
+            QMetaObject::metacall(data->property.object(), 
                                   QMetaObject::WriteProperty,
                                   idx, a);
 
         } else {
 
             QVariant value = this->value();
-            if (value.type() == QVariant::String) {
-                QmlMetaType::StringConverter con = QmlMetaType::customStringConverter(d->property.propertyType());
-                if (con)
-                    value = con(value.toString());
-            }
-
-            if (d->property.propertyType() == QVariant::Vector3D &&
-                value.type() == QVariant::String) {
-                value = qVariantFromValue(QmlStringConverters::vector3DFromString(value.toString()));
-            }
-
-            d->property.write(value, QmlMetaProperty::Binding);
+            data->property.write(value, QmlMetaProperty::Binding);
         }
 
-        d->updating = false;
+        data->updating = false;
     } else {
-        qmlInfo(d->property.object()) << "Binding loop detected for property" << d->property.name();
+        qmlInfo(data->property.object()) << "Binding loop detected for property" 
+                                         << data->property.name();
     }
+
+    data->release();
 }
 
 void QmlBinding::valueChanged()
@@ -148,30 +149,30 @@ void QmlBinding::valueChanged()
 void QmlBinding::setEnabled(bool e)
 {
     Q_D(QmlBinding);
-    d->enabled = e;
+    d->bindingData()->enabled = e;
     setTrackChange(e);
 
+    QmlAbstractBinding::setEnabled(e);
+
     if (e) {
-        addToObject(d->property.object());
+        addToObject(d->bindingData()->property.object());
         update();
     } else {
         removeFromObject();
     }
-
-    QmlAbstractBinding::setEnabled(e);
 }
 
 int QmlBinding::propertyIndex()
 {
     Q_D(QmlBinding);
-    return d->property.coreIndex();
+    return d->bindingData()->property.coreIndex();
 }
 
 bool QmlBinding::enabled() const
 {
     Q_D(const QmlBinding);
 
-    return d->enabled;
+    return d->bindingData()->enabled;
 }
 
 QString QmlBinding::expression() const
diff --git a/src/declarative/qml/qmlbinding_p.h b/src/declarative/qml/qmlbinding_p.h
index 963e2c1..2c0c6b9 100644
--- a/src/declarative/qml/qmlbinding_p.h
+++ b/src/declarative/qml/qmlbinding_p.h
@@ -59,11 +59,10 @@
 
 QT_BEGIN_NAMESPACE
 
-class QmlBindingPrivate : public QmlExpressionPrivate
+class QmlBindingData : public QmlExpressionData
 {
-    Q_DECLARE_PUBLIC(QmlBinding)
 public:
-    QmlBindingPrivate();
+    QmlBindingData();
 
     bool updating:1;
     bool enabled:1;
@@ -71,6 +70,16 @@ public:
     QmlMetaProperty property;
 };
 
+class QmlBindingPrivate : public QmlExpressionPrivate
+{
+    Q_DECLARE_PUBLIC(QmlBinding)
+public:
+    QmlBindingPrivate();
+
+    QmlBindingData *bindingData() { return static_cast<QmlBindingData *>(data); }
+    const QmlBindingData *bindingData() const { return static_cast<const QmlBindingData *>(data); }
+};
+
 QT_END_NAMESPACE
 
 #endif // QMLBINDING_P_H
diff --git a/src/declarative/qml/qmlexpression.cpp b/src/declarative/qml/qmlexpression.cpp
index e1c7afe..23e1700 100644
--- a/src/declarative/qml/qmlexpression.cpp
+++ b/src/declarative/qml/qmlexpression.cpp
@@ -51,41 +51,63 @@ Q_DECLARE_METATYPE(QList<QObject *>);
 
 QT_BEGIN_NAMESPACE
 
-QmlExpressionPrivate::QmlExpressionPrivate()
+QmlExpressionData::QmlExpressionData()
 : expressionFunctionValid(false), expressionRewritten(false), me(0), 
   trackChange(true), line(-1), guardList(0), guardListLength(0)
 {
 }
 
+QmlExpressionData::~QmlExpressionData()
+{
+    if (guardList) { delete [] guardList; guardList = 0; }
+}
+
+QmlExpressionPrivate::QmlExpressionPrivate()
+: data(new QmlExpressionData)
+{
+    data->q = this;
+}
+
+QmlExpressionPrivate::QmlExpressionPrivate(QmlExpressionData *d)
+: data(d)
+{
+    data->q = this;
+}
+
+QmlExpressionPrivate::~QmlExpressionPrivate()
+{
+    if (data) { data->q = 0; data->release(); data = 0; }
+}
+
 void QmlExpressionPrivate::init(QmlContext *ctxt, const QString &expr, 
                                 QObject *me)
 {
-    expression = expr;
+    data->expression = expr;
 
-    QmlAbstractExpression::setContext(ctxt);
-    this->me = me;
+    data->QmlAbstractExpression::setContext(ctxt);
+    data->me = me;
 }
 
 void QmlExpressionPrivate::init(QmlContext *ctxt, void *expr, QmlRefCount *rc, 
                                 QObject *me)
 {
-    quint32 *data = (quint32 *)expr;
-    Q_ASSERT(*data == BasicScriptEngineData || 
-             *data == PreTransformedQtScriptData);
-    if (*data == BasicScriptEngineData) {
-        sse.load((const char *)(data + 1), rc);
+    quint32 *exprData = (quint32 *)expr;
+    Q_ASSERT(*exprData == BasicScriptEngineData || 
+             *exprData == PreTransformedQtScriptData);
+    if (*exprData == BasicScriptEngineData) {
+        data->sse.load((const char *)(exprData + 1), rc);
     } else {
         QmlCompiledData *dd = (QmlCompiledData *)rc;
 
-        expressionRewritten = true;
-        expression = QString::fromRawData((QChar *)(data + 3), data[2]);
+        data->expressionRewritten = true;
+        data->expression = QString::fromRawData((QChar *)(exprData + 3), exprData[2]);
 
-        int progIdx = *(data + 1);
+        int progIdx = *(exprData + 1);
         QmlEngine *engine = ctxt->engine();
         QmlEnginePrivate *ep = QmlEnginePrivate::get(engine);
         QScriptEngine *scriptEngine = QmlEnginePrivate::getScriptEngine(engine);
         if (!dd->programs.at(progIdx)) {
-            dd->programs[progIdx] = new QScriptProgram(scriptEngine->compile(expression));
+            dd->programs[progIdx] = new QScriptProgram(scriptEngine->compile(data->expression));
         }
 
         QmlContextPrivate *ctxtPriv = ctxt->d_func();
@@ -94,19 +116,14 @@ void QmlExpressionPrivate::init(QmlContext *ctxt, void *expr, QmlRefCount *rc,
         if (me)
             scriptContext->pushScope(ep->objectClass->newQObject(me));
 
-        expressionFunction = scriptEngine->evaluate(*dd->programs[progIdx]);
+        data->expressionFunction = scriptEngine->evaluate(*dd->programs[progIdx]);
 
-        expressionFunctionValid = true;
+        data->expressionFunctionValid = true;
         scriptEngine->popContext();
     }
 
-    QmlAbstractExpression::setContext(ctxt);
-    this->me = me;
-}
-
-QmlExpressionPrivate::~QmlExpressionPrivate()
-{
-    if (guardList) { delete [] guardList; guardList = 0; }
+    data->QmlAbstractExpression::setContext(ctxt);
+    data->me = me;
 }
 
 /*!
@@ -173,7 +190,7 @@ QmlExpression::~QmlExpression()
 QmlEngine *QmlExpression::engine() const
 {
     Q_D(const QmlExpression);
-    return d->context()?d->context()->engine():0;
+    return d->data->context()?d->data->context()->engine():0;
 }
 
 /*!
@@ -183,7 +200,7 @@ QmlEngine *QmlExpression::engine() const
 QmlContext *QmlExpression::context() const
 {
     Q_D(const QmlExpression);
-    return d->context();
+    return d->data->context();
 }
 
 /*!
@@ -192,10 +209,10 @@ QmlContext *QmlExpression::context() const
 QString QmlExpression::expression() const
 {
     Q_D(const QmlExpression);
-    if (d->sse.isValid())
-        return QLatin1String(d->sse.expression());
+    if (d->data->sse.isValid())
+        return QLatin1String(d->data->sse.expression());
     else
-        return d->expression;
+        return d->data->expression;
 }
 
 /*!
@@ -215,12 +232,12 @@ void QmlExpression::setExpression(const QString &expression)
 
     d->clearGuards();
 
-    d->expression = expression;
-    d->expressionFunctionValid = false;
-    d->expressionRewritten = false;
-    d->expressionFunction = QScriptValue();
+    d->data->expression = expression;
+    d->data->expressionFunctionValid = false;
+    d->data->expressionRewritten = false;
+    d->data->expressionFunction = QScriptValue();
 
-    d->sse.clear();
+    d->data->sse.clear();
 }
 
 QVariant QmlExpressionPrivate::evalSSE()
@@ -229,7 +246,7 @@ QVariant QmlExpressionPrivate::evalSSE()
     QFxPerfTimer<QFxPerf::BindValueSSE> perfsse;
 #endif
 
-    QVariant rv = sse.run(context(), me);
+    QVariant rv = data->sse.run(data->context(), data->me);
 
     return rv;
 }
@@ -240,8 +257,8 @@ QVariant QmlExpressionPrivate::evalQtScript(QObject *secondaryScope)
     QFxPerfTimer<QFxPerf::BindValueQt> perfqt;
 #endif
 
-    QmlContextPrivate *ctxtPriv = context()->d_func();
-    QmlEngine *engine = context()->engine();
+    QmlContextPrivate *ctxtPriv = data->context()->d_func();
+    QmlEngine *engine = data->context()->engine();
     QmlEnginePrivate *ep = QmlEnginePrivate::get(engine);
 
     if (secondaryScope)
@@ -250,28 +267,29 @@ QVariant QmlExpressionPrivate::evalQtScript(QObject *secondaryScope)
 
     QScriptEngine *scriptEngine = QmlEnginePrivate::getScriptEngine(engine);
 
-    if (!expressionFunctionValid) {
+    if (!data->expressionFunctionValid) {
 
         QScriptContext *scriptContext = scriptEngine->pushCleanContext();
         scriptContext->pushScope(ctxtPriv->scriptValue);
 
-        if (me)
-            scriptContext->pushScope(ep->objectClass->newQObject(me));
+        if (data->me)
+            scriptContext->pushScope(ep->objectClass->newQObject(data->me));
 
-        if (expressionRewritten) {
-            expressionFunction = scriptEngine->evaluate(expression, fileName, line);
+        if (data->expressionRewritten) {
+            data->expressionFunction = scriptEngine->evaluate(data->expression, 
+                                                              data->fileName, data->line);
         } else {
             QmlRewrite::RewriteBinding rewriteBinding;
 
-            const QString code = rewriteBinding(expression);
-            expressionFunction = scriptEngine->evaluate(code, fileName, line);
+            const QString code = rewriteBinding(data->expression);
+            data->expressionFunction = scriptEngine->evaluate(code, data->fileName, data->line);
         }
 
         scriptEngine->popContext();
-        expressionFunctionValid = true;
+        data->expressionFunctionValid = true;
     }
 
-    QScriptValue svalue = expressionFunction.call();
+    QScriptValue svalue = data->expressionFunction.call();
 
     if (scriptEngine->hasUncaughtException()) {
         if (scriptEngine->uncaughtException().isError()){
@@ -329,7 +347,7 @@ QVariant QmlExpressionPrivate::value(QObject *secondaryScope)
     Q_Q(QmlExpression);
 
     QVariant rv;
-    if (!q->engine() || (!sse.isValid() && expression.isEmpty()))
+    if (!q->engine() || (!data->sse.isValid() && data->expression.isEmpty()))
         return rv;
 
 #ifdef Q_ENABLE_PERFORMANCE_LOG
@@ -344,7 +362,11 @@ QVariant QmlExpressionPrivate::value(QObject *secondaryScope)
 
     ep->currentExpression = q;
 
-    if (sse.isValid()) {
+    // This object might be deleted during the eval
+    QmlExpressionData *localData = data;
+    localData->addref();
+
+    if (data->sse.isValid()) {
         rv = evalSSE();
     } else {
         rv = evalQtScript(secondaryScope);
@@ -352,12 +374,17 @@ QVariant QmlExpressionPrivate::value(QObject *secondaryScope)
 
     ep->currentExpression = lastCurrentExpression;
 
-    if ((!q->trackChange() || !ep->capturedProperties.count()) && guardList) {
-        clearGuards();
-    } else if(q->trackChange()) {
-        updateGuards(ep->capturedProperties);
+    // Check if we were deleted
+    if (localData->q) {
+        if ((!data->trackChange || !ep->capturedProperties.count()) && data->guardList) {
+            clearGuards();
+        } else if(data->trackChange) {
+            updateGuards(ep->capturedProperties);
+        }
     }
 
+    localData->release();
+
     lastCapturedProperties.copyAndClear(ep->capturedProperties);
 
     return rv;
@@ -381,7 +408,7 @@ QVariant QmlExpression::value()
 bool QmlExpression::isConstant() const
 {
     Q_D(const QmlExpression);
-    return !d->guardList;
+    return !d->data->guardList;
 }
 
 /*!
@@ -390,7 +417,7 @@ bool QmlExpression::isConstant() const
 bool QmlExpression::trackChange() const
 {
     Q_D(const QmlExpression);
-    return d->trackChange;
+    return d->data->trackChange;
 }
 
 /*!
@@ -411,7 +438,7 @@ bool QmlExpression::trackChange() const
 void QmlExpression::setTrackChange(bool trackChange)
 {
     Q_D(QmlExpression);
-    d->trackChange = trackChange;
+    d->data->trackChange = trackChange;
 }
 
 /*!
@@ -421,8 +448,8 @@ void QmlExpression::setTrackChange(bool trackChange)
 void QmlExpression::setSourceLocation(const QUrl &fileName, int line)
 {
     Q_D(QmlExpression);
-    d->fileName = fileName.toString();
-    d->line = line;
+    d->data->fileName = fileName.toString();
+    d->data->line = line;
 }
 
 /*!
@@ -434,7 +461,7 @@ void QmlExpression::setSourceLocation(const QUrl &fileName, int line)
 QObject *QmlExpression::scopeObject() const
 {
     Q_D(const QmlExpression);
-    return d->me;
+    return d->data->me;
 }
 
 /*! \internal */
@@ -452,16 +479,16 @@ void QmlExpressionPrivate::clearGuards()
         notifyIdx = 
             QmlExpression::staticMetaObject.indexOfMethod("__q_notify()");
 
-    for (int ii = 0; ii < guardListLength; ++ii) {
-        if (guardList[ii].data()) {
-            QMetaObject::disconnect(guardList[ii].data(), 
-                                    guardList[ii].notifyIndex, 
+    for (int ii = 0; ii < data->guardListLength; ++ii) {
+        if (data->guardList[ii].data()) {
+            QMetaObject::disconnect(data->guardList[ii].data(), 
+                                    data->guardList[ii].notifyIndex, 
                                     q, notifyIdx);
         }
     }
 
-    delete [] guardList; guardList = 0; 
-    guardListLength = 0;
+    delete [] data->guardList; data->guardList = 0; 
+    data->guardListLength = 0;
 }
 
 void QmlExpressionPrivate::updateGuards(const QPODVector<QmlEnginePrivate::CapturedProperty> &properties)
@@ -474,10 +501,10 @@ void QmlExpressionPrivate::updateGuards(const QPODVector<QmlEnginePrivate::Captu
         notifyIdx = 
             QmlExpression::staticMetaObject.indexOfMethod("__q_notify()");
 
-    SignalGuard *newGuardList = 0;
+    QmlExpressionData::SignalGuard *newGuardList = 0;
     
-    if (properties.count() != guardListLength)
-        newGuardList = new SignalGuard[properties.count()];
+    if (properties.count() != data->guardListLength)
+        newGuardList = new QmlExpressionData::SignalGuard[properties.count()];
 
     bool outputWarningHeader = false;
     int hit = 0;
@@ -485,20 +512,20 @@ void QmlExpressionPrivate::updateGuards(const QPODVector<QmlEnginePrivate::Captu
         const QmlEnginePrivate::CapturedProperty &property = properties.at(ii);
 
         bool needGuard = true;
-        if (ii >= guardListLength) {
+        if (ii >= data->guardListLength) {
             // New guard
-        } else if(guardList[ii].data() == property.object && 
-                  guardList[ii].notifyIndex == property.notifyIndex) {
+        } else if(data->guardList[ii].data() == property.object && 
+                  data->guardList[ii].notifyIndex == property.notifyIndex) {
             // Cache hit
-            if (!guardList[ii].isDuplicate || 
-                (guardList[ii].isDuplicate && hit == ii)) {
+            if (!data->guardList[ii].isDuplicate || 
+                (data->guardList[ii].isDuplicate && hit == ii)) {
                 needGuard = false;
                 ++hit;
             }
-        } else if(guardList[ii].data() && !guardList[ii].isDuplicate) {
+        } else if(data->guardList[ii].data() && !data->guardList[ii].isDuplicate) {
             // Cache miss
-            QMetaObject::disconnect(guardList[ii].data(), 
-                                    guardList[ii].notifyIndex, 
+            QMetaObject::disconnect(data->guardList[ii].data(), 
+                                    data->guardList[ii].notifyIndex, 
                                     q, notifyIdx);
         } 
         /* else {
@@ -507,9 +534,9 @@ void QmlExpressionPrivate::updateGuards(const QPODVector<QmlEnginePrivate::Captu
 
         if (needGuard) {
             if (!newGuardList) {
-                newGuardList = new SignalGuard[properties.count()];
+                newGuardList = new QmlExpressionData::SignalGuard[properties.count()];
                 for (int jj = 0; jj < ii; ++jj)
-                    newGuardList[jj] = guardList[jj];
+                    newGuardList[jj] = data->guardList[jj];
             }
 
             if (property.notifyIndex != -1) {
@@ -539,22 +566,22 @@ void QmlExpressionPrivate::updateGuards(const QPODVector<QmlEnginePrivate::Captu
                                      << "::" << metaProp.name();
             }
         } else if (newGuardList) {
-            newGuardList[ii] = guardList[ii];
+            newGuardList[ii] = data->guardList[ii];
         }
     }
 
-    for (int ii = properties.count(); ii < guardListLength; ++ii) {
-        if (guardList[ii].data() && !guardList[ii].isDuplicate) {
-            QMetaObject::disconnect(guardList[ii].data(), 
-                                    guardList[ii].notifyIndex, 
+    for (int ii = properties.count(); ii < data->guardListLength; ++ii) {
+        if (data->guardList[ii].data() && !data->guardList[ii].isDuplicate) {
+            QMetaObject::disconnect(data->guardList[ii].data(), 
+                                    data->guardList[ii].notifyIndex, 
                                     q, notifyIdx);
         }
     }
 
     if (newGuardList) {
-        if (guardList) delete [] guardList;
-        guardList = newGuardList;
-        guardListLength = properties.count();
+        if (data->guardList) delete [] data->guardList;
+        data->guardList = newGuardList;
+        data->guardListLength = properties.count();
     }
 }
 
diff --git a/src/declarative/qml/qmlexpression_p.h b/src/declarative/qml/qmlexpression_p.h
index 501e5d8..33016e6 100644
--- a/src/declarative/qml/qmlexpression_p.h
+++ b/src/declarative/qml/qmlexpression_p.h
@@ -79,23 +79,13 @@ private:
     QmlAbstractExpression  *m_nextExpression;
 };
 
-class QmlExpression;
-class QString;
-class QmlExpressionPrivate : public QObjectPrivate, public QmlAbstractExpression
+class QmlExpressionData : public QmlAbstractExpression, public QmlRefCount
 {
-    Q_DECLARE_PUBLIC(QmlExpression)
 public:
-    QmlExpressionPrivate();
-    ~QmlExpressionPrivate();
-
-    enum CompiledDataType {
-        BasicScriptEngineData = 1,
-        PreTransformedQtScriptData = 2
-    };
+    QmlExpressionData();
+    ~QmlExpressionData();
 
-
-    void init(QmlContext *, const QString &, QObject *);
-    void init(QmlContext *, void *, QmlRefCount *, QObject *);
+    QmlExpressionPrivate *q;
 
     QString expression;
     bool expressionFunctionValid:1;
@@ -109,10 +99,6 @@ public:
     QString fileName;
     int line;
 
-    QVariant value(QObject *secondaryScope = 0);
-    QVariant evalSSE();
-    QVariant evalQtScript(QObject *secondaryScope);
-
     struct SignalGuard : public QGuard<QObject> {
         SignalGuard() : isDuplicate(false), notifyIndex(-1) {}
 
@@ -132,6 +118,32 @@ public:
     };
     SignalGuard *guardList;
     int guardListLength;
+};
+
+class QmlExpression;
+class QString;
+class QmlExpressionPrivate : public QObjectPrivate
+{
+    Q_DECLARE_PUBLIC(QmlExpression)
+public:
+    QmlExpressionPrivate();
+    QmlExpressionPrivate(QmlExpressionData *);
+    ~QmlExpressionPrivate();
+
+    enum CompiledDataType {
+        BasicScriptEngineData = 1,
+        PreTransformedQtScriptData = 2
+    };
+
+    void init(QmlContext *, const QString &, QObject *);
+    void init(QmlContext *, void *, QmlRefCount *, QObject *);
+
+    QmlExpressionData *data;
+
+    QVariant value(QObject *secondaryScope = 0);
+    QVariant evalSSE();
+    QVariant evalQtScript(QObject *secondaryScope);
+
     void updateGuards(const QPODVector<QmlEnginePrivate::CapturedProperty> &properties);
     void clearGuards();