QmlExpression API review

15 files changed, 45 insertions, 70 deletions

diff --git a/src/declarative/graphicsitems/qmlgraphicsvisualitemmodel.cpp b/src/declarative/graphicsitems/qmlgraphicsvisualitemmodel.cpp
index bfa9e9b..9c11f25 100644
--- a/src/declarative/graphicsitems/qmlgraphicsvisualitemmodel.cpp
+++ b/src/declarative/graphicsitems/qmlgraphicsvisualitemmodel.cpp
@@ -187,7 +187,6 @@ QVariant QmlGraphicsVisualItemModel::evaluate(int index, const QString &expressi
     QmlContext *ctxt = new QmlContext(ccontext);
     ctxt->addDefaultObject(d->children.at(index));
     QmlExpression e(ctxt, expression, objectContext);
-    e.setTrackChange(false);
     QVariant value = e.value();
     delete ctxt;
     return value;
@@ -1057,7 +1056,6 @@ QVariant QmlGraphicsVisualDataModel::evaluate(int index, const QString &expressi
         QmlGraphicsItem *item = qobject_cast<QmlGraphicsItem *>(nobj);
         if (item) {
             QmlExpression e(qmlContext(item), expression, objectContext);
-            e.setTrackChange(false);
             value = e.value();
         }
     } else {
@@ -1067,7 +1065,6 @@ QVariant QmlGraphicsVisualDataModel::evaluate(int index, const QString &expressi
         QmlGraphicsVisualDataModelData *data = new QmlGraphicsVisualDataModelData(index, this);
         ctxt->addDefaultObject(data);
         QmlExpression e(ctxt, expression, objectContext);
-        e.setTrackChange(false);
         value = e.value();
         delete data;
         delete ctxt;
diff --git a/src/declarative/qml/qmlbinding.cpp b/src/declarative/qml/qmlbinding.cpp
index 9f93fee..aeda28b 100644
--- a/src/declarative/qml/qmlbinding.cpp
+++ b/src/declarative/qml/qmlbinding.cpp
@@ -85,12 +85,14 @@ QmlBinding::QmlBinding(void *data, QmlRefCount *rc, QObject *obj, QmlContext *ct
 : QmlExpression(ctxt, data, rc, obj, url, lineNumber, *new QmlBindingPrivate)
 {
     setParent(parent);
+    setNotifyOnValueChanged(true);
 }
 
 QmlBinding::QmlBinding(const QString &str, QObject *obj, QmlContext *ctxt, QObject *parent)
 : QmlExpression(ctxt, str, obj, *new QmlBindingPrivate)
 {
     setParent(parent);
+    setNotifyOnValueChanged(true);
 }
 
 QmlBinding::~QmlBinding()
@@ -198,17 +200,17 @@ void QmlBinding::update(QmlMetaProperty::WriteFlags flags)
     data->release();
 }
 
-void QmlBinding::emitValueChanged()
+void QmlBindingPrivate::emitValueChanged()
 {
-    update();
-    // don't bother calling valueChanged()
+    Q_Q(QmlBinding);
+    q->update();
 }
 
 void QmlBinding::setEnabled(bool e, QmlMetaProperty::WriteFlags flags)
 {
     Q_D(QmlBinding);
     d->bindingData()->enabled = e;
-    setTrackChange(e);
+    setNotifyOnValueChanged(e);
 
     QmlAbstractBinding::setEnabled(e, flags);
 
diff --git a/src/declarative/qml/qmlbinding_p_p.h b/src/declarative/qml/qmlbinding_p_p.h
index e5a06fc..131bacc 100644
--- a/src/declarative/qml/qmlbinding_p_p.h
+++ b/src/declarative/qml/qmlbinding_p_p.h
@@ -82,6 +82,8 @@ public:
 
     QmlBindingData *bindingData() { return static_cast<QmlBindingData *>(data); }
     const QmlBindingData *bindingData() const { return static_cast<const QmlBindingData *>(data); }
+
+    virtual void emitValueChanged();
 };
 
 QT_END_NAMESPACE
diff --git a/src/declarative/qml/qmlboundsignal.cpp b/src/declarative/qml/qmlboundsignal.cpp
index a075899..db5fd61 100644
--- a/src/declarative/qml/qmlboundsignal.cpp
+++ b/src/declarative/qml/qmlboundsignal.cpp
@@ -124,7 +124,6 @@ QmlBoundSignal::QmlBoundSignal(QmlContext *ctxt, const QString &val,
     QMetaObject::connect(scope, m_signal.methodIndex(), this, evaluateIdx);
 
     m_expression = new QmlExpression(ctxt, val, scope);
-    m_expression->setTrackChange(false);
 }
 
 QmlBoundSignal::~QmlBoundSignal()
@@ -157,7 +156,7 @@ QmlExpression *QmlBoundSignal::setExpression(QmlExpression *e)
 {
     QmlExpression *rv = m_expression;
     m_expression = e;
-    if (m_expression) m_expression->setTrackChange(false);
+    if (m_expression) m_expression->setNotifyOnValueChanged(false);
     return rv;
 }
 
diff --git a/src/declarative/qml/qmlenginedebug.cpp b/src/declarative/qml/qmlenginedebug.cpp
index 2ab709a..973e5e5 100644
--- a/src/declarative/qml/qmlenginedebug.cpp
+++ b/src/declarative/qml/qmlenginedebug.cpp
@@ -408,14 +408,13 @@ void QmlEngineDebugServer::messageReceived(const QByteArray &message)
         QmlContext *context = qmlContext(object);
         QVariant result;
         if (object && context) {
-            QmlExpression *exprObj = new QmlExpression(context, expr, object);
+            QmlExpression exprObj(context, expr, object);
             bool undefined = false;
-            QVariant value = exprObj->value(&undefined);
+            QVariant value = exprObj.value(&undefined);
             if (undefined)
                 result = QLatin1String("<undefined>");
             else
                 result = valueContents(value);
-            delete exprObj;
         } else {
             result = QLatin1String("<unknown context>");
         }
diff --git a/src/declarative/qml/qmlexpression.cpp b/src/declarative/qml/qmlexpression.cpp
index 8f0c945..64b2d7f 100644
--- a/src/declarative/qml/qmlexpression.cpp
+++ b/src/declarative/qml/qmlexpression.cpp
@@ -70,7 +70,7 @@ bool QmlDelayedError::addError(QmlEnginePrivate *e)
 
 QmlExpressionData::QmlExpressionData()
 : q(0), dataRef(0), expressionFunctionValid(false), expressionRewritten(false), me(0), 
-  trackChange(true), isShared(false), line(-1), guardList(0), guardListLength(0)
+  trackChange(false), isShared(false), line(-1), guardList(0), guardListLength(0)
 {
 }
 
@@ -273,14 +273,6 @@ QString QmlExpression::expression() const
 }
 
 /*!
-    Clear the expression.
-*/
-void QmlExpression::clearExpression()
-{
-    setExpression(QString());
-}
-
-/*!
     Set the expression to \a expression.
 */
 void QmlExpression::setExpression(const QString &expression)
@@ -495,44 +487,34 @@ QVariant QmlExpression::value(bool *isUndefined)
 }
 
 /*!
-    Returns true if the expression results in a constant value.
-    QmlExpression::value() must have been invoked at least once before the
-    return from this method is valid.
- */
-bool QmlExpression::isConstant() const
-{
-    Q_D(const QmlExpression);
-    return !d->data->guardList;
-}
-
-/*!
-    Returns true if the changes are tracked in the expression's value.
+Returns true if the valueChanged() signal is emitted when the expression's evaluated
+value changes.
 */
-bool QmlExpression::trackChange() const
+bool QmlExpression::notifyOnValueChanged() const
 {
     Q_D(const QmlExpression);
     return d->data->trackChange;
 }
 
 /*!
-    Set whether changes are tracked in the expression's value to \a trackChange.
+Sets whether the valueChanged() signal is emitted when the expression's evaluated
+value changes.
 
-    If true, the QmlExpression will monitor properties involved in the
-    expression's evaluation, and call QmlExpression::valueChanged() if they have
-    changed.  This allows an application to ensure that any value associated
-    with the result of the expression remains up to date.
+If true, the QmlExpression will monitor properties involved in the expression's 
+evaluation, and emit QmlExpression::valueChanged() if they have changed.  This allows 
+an application to ensure that any value associated with the result of the expression 
+remains up to date.
 
-    If false, the QmlExpression will not montitor properties involved in the
-    expression's evaluation, and QmlExpression::valueChanged() will never be
-    called.  This is more efficient if an application wants a "one off"
-    evaluation of the expression.
+If false, the QmlExpression will not montitor properties involved in the expression's 
+evaluation, and QmlExpression::valueChanged() will never be emitted.  This is more efficient 
+if an application wants a "one off" evaluation of the expression.
 
-    By default, trackChange is true.
+By default, notifyOnChange is false.
 */
-void QmlExpression::setTrackChange(bool trackChange)
+void QmlExpression::setNotifyOnValueChanged(bool notifyOnChange)
 {
     Q_D(QmlExpression);
-    d->data->trackChange = trackChange;
+    d->data->trackChange = notifyOnChange;
 }
 
 /*!
@@ -618,7 +600,8 @@ QmlError QmlExpression::error() const
 /*! \internal */
 void QmlExpression::__q_notify()
 {
-    emitValueChanged();
+    Q_D(QmlExpression);
+    d->emitValueChanged();
 }
 
 void QmlExpressionPrivate::clearGuards()
@@ -765,13 +748,10 @@ void QmlExpressionPrivate::updateGuards(const QPODVector<QmlEnginePrivate::Captu
     calling QmlExpression::value()) before this signal will be emitted.
 */
 
-/*!
-    Subclasses can capture the emission of the valueChanged() signal by overriding
-    this function. They can choose whether to then call valueChanged().
-*/
-void QmlExpression::emitValueChanged()
+void QmlExpressionPrivate::emitValueChanged()
 {
-    emit valueChanged();
+    Q_Q(QmlExpression);
+    emit q->valueChanged();
 }
 
 QmlAbstractExpression::QmlAbstractExpression()
diff --git a/src/declarative/qml/qmlexpression.h b/src/declarative/qml/qmlexpression.h
index 428eefa..61374f2 100644
--- a/src/declarative/qml/qmlexpression.h
+++ b/src/declarative/qml/qmlexpression.h
@@ -70,12 +70,10 @@ public:
     QmlContext *context() const;
 
     QString expression() const;
-    void clearExpression();
-    virtual void setExpression(const QString &);
-    bool isConstant() const;
+    void setExpression(const QString &);
 
-    bool trackChange() const;
-    void setTrackChange(bool);
+    bool notifyOnValueChanged() const;
+    void setNotifyOnValueChanged(bool);
 
     QString sourceFile() const;
     int lineNumber() const;
@@ -87,15 +85,12 @@ public:
     void clearError();
     QmlError error() const;
 
-public Q_SLOTS:
     QVariant value(bool *isUndefined = 0);
 
 Q_SIGNALS:
     void valueChanged();
 
 protected:
-    virtual void emitValueChanged();
-
     QmlExpression(QmlContext *, const QString &, QObject *, 
                   QmlExpressionPrivate &dd);
     QmlExpression(QmlContext *, void *, QmlRefCount *rc, QObject *me, const QString &,
diff --git a/src/declarative/qml/qmlexpression_p.h b/src/declarative/qml/qmlexpression_p.h
index e52a199..e4bed05 100644
--- a/src/declarative/qml/qmlexpression_p.h
+++ b/src/declarative/qml/qmlexpression_p.h
@@ -177,6 +177,8 @@ public:
         return expr->q_func();
     }
 
+    virtual void emitValueChanged();
+
     static void exceptionToError(QScriptEngine *, QmlError &);
     static QScriptValue evalInObjectScope(QmlContext *, QObject *, const QString &);
     static QScriptValue evalInObjectScope(QmlContext *, QObject *, const QScriptProgram &);
diff --git a/src/declarative/qml/qmlwatcher.cpp b/src/declarative/qml/qmlwatcher.cpp
index 59503de..a8a94c5 100644
--- a/src/declarative/qml/qmlwatcher.cpp
+++ b/src/declarative/qml/qmlwatcher.cpp
@@ -154,6 +154,7 @@ bool QmlWatcher::addWatch(int id, quint32 objectId, const QString &expr)
     QmlContext *context = qmlContext(object);
     if (context) {
         QmlExpression *exprObj = new QmlExpression(context, expr, object);
+        exprObj->setNotifyOnValueChanged(true);
         QmlWatchProxy *proxy = new QmlWatchProxy(id, exprObj, objectId, this);
         exprObj->setParent(proxy);
         m_proxies[id].append(proxy);
diff --git a/src/declarative/util/qmlanimation.cpp b/src/declarative/util/qmlanimation.cpp
index c044f97..6dcce58 100644
--- a/src/declarative/util/qmlanimation.cpp
+++ b/src/declarative/util/qmlanimation.cpp
@@ -793,7 +793,6 @@ void QmlScriptActionPrivate::execute()
     const QString &str = scriptStr.script();
     if (!str.isEmpty()) {
         QmlExpression expr(scriptStr.context(), str, scriptStr.scopeObject());
-        expr.setTrackChange(false);
         expr.value();
     }
 }
diff --git a/src/declarative/util/qmlpropertychanges.cpp b/src/declarative/util/qmlpropertychanges.cpp
index 0ce3604..3abbadd 100644
--- a/src/declarative/util/qmlpropertychanges.cpp
+++ b/src/declarative/util/qmlpropertychanges.cpp
@@ -277,14 +277,12 @@ void QmlPropertyChangesPrivate::decode()
         QmlMetaProperty prop = property(name);      //### better way to check for signal property?
         if (prop.type() & QmlMetaProperty::SignalProperty) {
             QmlExpression *expression = new QmlExpression(qmlContext(q), data.toString(), object);
-            expression->setTrackChange(false);
             QmlReplaceSignalHandler *handler = new QmlReplaceSignalHandler;
             handler->property = prop;
             handler->expression = expression;
             signalReplacements << handler;
         } else if (isScript) {
             QmlExpression *expression = new QmlExpression(qmlContext(q), data.toString(), object);
-            expression->setTrackChange(false);
             expressions << qMakePair(name, expression);
         } else {
             properties << qMakePair(name, data);
diff --git a/src/declarative/util/qmlstateoperations.cpp b/src/declarative/util/qmlstateoperations.cpp
index bd1f5f0..fff8774 100644
--- a/src/declarative/util/qmlstateoperations.cpp
+++ b/src/declarative/util/qmlstateoperations.cpp
@@ -373,7 +373,6 @@ void QmlStateChangeScript::execute()
     const QString &script = d->script.script();
     if (!script.isEmpty()) {
         QmlExpression expr(d->script.context(), script, d->script.scopeObject());
-        expr.setTrackChange(false);
         expr.value();
     }
 }
diff --git a/tests/auto/declarative/qmlecmascript/testtypes.h b/tests/auto/declarative/qmlecmascript/testtypes.h
index 0af72cb..f511c29 100644
--- a/tests/auto/declarative/qmlecmascript/testtypes.h
+++ b/tests/auto/declarative/qmlecmascript/testtypes.h
@@ -173,17 +173,21 @@ QML_DECLARE_TYPE(MyQmlContainer);
 
 class MyExpression : public QmlExpression
 {
+    Q_OBJECT
 public:
     MyExpression(QmlContext *ctxt, const QString &expr)
         : QmlExpression(ctxt, expr, 0), changed(false)
     {
+        QObject::connect(this, SIGNAL(valueChanged()), this, SLOT(expressionValueChanged()));
+        setNotifyOnValueChanged(true);
     }
 
-    void emitValueChanged() {
+    bool changed;
+
+public slots:
+    void expressionValueChanged() {
         changed = true;
-        QmlExpression::emitValueChanged();
     }
-    bool changed;
 };
 
 
diff --git a/tests/auto/declarative/qmlgraphicslistview/tst_qmlgraphicslistview.cpp b/tests/auto/declarative/qmlgraphicslistview/tst_qmlgraphicslistview.cpp
index 13ed41d..0876520 100644
--- a/tests/auto/declarative/qmlgraphicslistview/tst_qmlgraphicslistview.cpp
+++ b/tests/auto/declarative/qmlgraphicslistview/tst_qmlgraphicslistview.cpp
@@ -1322,7 +1322,6 @@ T *tst_QmlGraphicsListView::findItem(QGraphicsObject *parent, const QString &obj
         if (mo.cast(item) && (objectName.isEmpty() || item->objectName() == objectName)) {
             if (index != -1) {
                 QmlExpression e(qmlContext(item), "index", item);
-                e.setTrackChange(false);
                 if (e.value().toInt() == index)
                     return static_cast<T*>(item);
             } else {
diff --git a/tests/auto/declarative/qmlgraphicspathview/tst_qmlgraphicspathview.cpp b/tests/auto/declarative/qmlgraphicspathview/tst_qmlgraphicspathview.cpp
index b986a64..62b3cfc 100644
--- a/tests/auto/declarative/qmlgraphicspathview/tst_qmlgraphicspathview.cpp
+++ b/tests/auto/declarative/qmlgraphicspathview/tst_qmlgraphicspathview.cpp
@@ -452,7 +452,6 @@ T *tst_QmlGraphicsPathView::findItem(QGraphicsObject *parent, const QString &obj
         if (mo.cast(item) && (objectName.isEmpty() || item->objectName() == objectName)) {
             if (index != -1) {
                 QmlExpression e(qmlContext(item), "index", item);
-                e.setTrackChange(false);
                 if (e.value().toInt() == index)
                     return static_cast<T*>(item);
             } else {