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.

1 files changed, 110 insertions, 83 deletions

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();
     }
 }