Simplify expression guard logic

Change-Id: I7d191bc8786452c5a1f14d024ff62d223adebd8b
Reviewed-by: Martin Jones <martin.jones@nokia.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

diff --git a/src/declarative/qml/qdeclarativeexpression.cpp b/src/declarative/qml/qdeclarativeexpression.cpp
index 27979af..d3ba92a 100644 (file)
--- a/src/declarative/qml/qdeclarativeexpression.cpp
+++ b/src/declarative/qml/qdeclarativeexpression.cpp
@@ -515,45 +515,18 @@ QDeclarativeJavaScriptExpression::GuardList::updateGuards(QDeclarativeJavaScript
 
         if (property.notifier != 0) {
 
-            if (!noChanges && guard.isConnected(property.notifier)) {
-                // Nothing to do
-
+            if (guard.isConnected(property.notifier)) {
+                 guard.cancelNotify();
             } else {
-                noChanges = false;
-
-                bool existing = false;
-                for (int jj = 0; !existing && jj < ii; ++jj) 
-                    if (endpoints[jj].isConnected(property.notifier)) 
-                        existing = true;
-
-                if (existing) {
-                    // duplicate
-                    guard.disconnect();
-                } else {
-                    guard.connect(property.notifier);
-                }
+                guard.connect(property.notifier);
             }
 
-
         } else if (property.notifyIndex != -1) {
 
-            if (!noChanges && guard.isConnected(property.object, property.notifyIndex)) {
-                // Nothing to do
-
-            } else {
-                noChanges = false;
-
-                bool existing = false;
-                for (int jj = 0; !existing && jj < ii; ++jj) 
-                    if (endpoints[jj].isConnected(property.object, property.notifyIndex)) 
-                        existing = true;
-
-                if (existing) {
-                    // duplicate
-                    guard.disconnect();
-                } else {
-                    guard.connect(property.object, property.notifyIndex);
-                }
+            if (guard.isConnected(property.object, property.notifyIndex)) {
+                guard.cancelNotify();
+            } else { 
+                guard.connect(property.object, property.notifyIndex);
             }
 
         } else {

diff --git a/src/declarative/qml/qdeclarativenotifier.cpp b/src/declarative/qml/qdeclarativenotifier.cpp
index a195280..842cbf0 100644 (file)
--- a/src/declarative/qml/qdeclarativenotifier.cpp
+++ b/src/declarative/qml/qdeclarativenotifier.cpp
@@ -48,14 +48,15 @@ void QDeclarativeNotifier::emitNotify(QDeclarativeNotifierEndpoint *endpoint)
 {
     QDeclarativeNotifierEndpoint **oldDisconnected = endpoint->disconnected;
     endpoint->disconnected = &endpoint;
+    endpoint->notifying = 1;
 
     if (endpoint->next)
         emitNotify(endpoint->next);
 
     if (endpoint) {
-        void *args[] = { 0 };
 
         Q_ASSERT(endpoint->callback);
+        
         endpoint->callback(endpoint);
 
         if (endpoint) 
@@ -63,6 +64,7 @@ void QDeclarativeNotifier::emitNotify(QDeclarativeNotifierEndpoint *endpoint)
     } 
 
     if (oldDisconnected) *oldDisconnected = endpoint;
+    else if (endpoint) endpoint->notifying = 0;
 }
 
 void QDeclarativeNotifierEndpoint::connect(QObject *source, int sourceSignal)
@@ -91,6 +93,7 @@ void QDeclarativeNotifierEndpoint::copyAndClear(QDeclarativeNotifierEndpoint &ot
     other.notifier = notifier;
     other.sourceSignal = sourceSignal;
     other.disconnected = disconnected;
+    other.notifying = notifying;
     if (other.disconnected) *other.disconnected = &other;
 
     if (next) {
@@ -104,6 +107,7 @@ void QDeclarativeNotifierEndpoint::copyAndClear(QDeclarativeNotifierEndpoint &ot
     next = 0;
     disconnected = 0;
     notifier = 0;
+    notifying = 0;
     sourceSignal = -1;
 }
 

diff --git a/src/declarative/qml/qdeclarativenotifier_p.h b/src/declarative/qml/qdeclarativenotifier_p.h
index 1d0d4fc..06c0ba0 100644 (file)
--- a/src/declarative/qml/qdeclarativenotifier_p.h
+++ b/src/declarative/qml/qdeclarativenotifier_p.h
@@ -80,6 +80,9 @@ public:
     inline void connect(QDeclarativeNotifier *);
     inline void disconnect();
 
+    inline bool isNotifying() const;
+    inline void cancelNotify();
+
     void copyAndClear(QDeclarativeNotifierEndpoint &other);
 
 private:
@@ -90,7 +93,8 @@ private:
         QDeclarativeNotifier *notifier;
         QObject *source;
     };
-    int sourceSignal;
+    unsigned int notifying : 1;
+    signed int sourceSignal : 31;
     QDeclarativeNotifierEndpoint **disconnected;
     QDeclarativeNotifierEndpoint  *next;
     QDeclarativeNotifierEndpoint **prev;
@@ -124,7 +128,7 @@ void QDeclarativeNotifier::notify()
 }
 
 QDeclarativeNotifierEndpoint::QDeclarativeNotifierEndpoint()
-: callback(0), notifier(0), sourceSignal(-1), disconnected(0), next(0), prev(0)
+: callback(0), notifier(0), notifying(0), sourceSignal(-1), disconnected(0), next(0), prev(0)
 {
 }
 
@@ -140,7 +144,7 @@ bool QDeclarativeNotifierEndpoint::isConnected()
 
 bool QDeclarativeNotifierEndpoint::isConnected(QObject *source, int sourceSignal)
 {
-    return sourceSignal != -1 && this->source == source && this->sourceSignal == sourceSignal;
+    return this->sourceSignal != -1 && this->source == source && this->sourceSignal == sourceSignal;
 }
 
 bool QDeclarativeNotifierEndpoint::isConnected(QDeclarativeNotifier *notifier)
@@ -168,9 +172,34 @@ void QDeclarativeNotifierEndpoint::disconnect()
     prev = 0;
     disconnected = 0;
     notifier = 0;
+    notifying = 0;
     sourceSignal = -1;
 }
 
+/*!
+Returns true if a notify is in progress.  This means that the signal or QDeclarativeNotifier
+that this endpoing is connected to has been triggered, but this endpoint's callback has not
+yet been called.
+
+An in progress notify can be cancelled by calling cancelNotify.
+*/
+bool QDeclarativeNotifierEndpoint::isNotifying() const
+{
+    return notifying == 1;
+}
+
+/*!
+Cancel any notifies that are in progress.
+*/
+void QDeclarativeNotifierEndpoint::cancelNotify() 
+{
+    notifying = 0;
+    if (disconnected) {
+        *disconnected = 0;
+        disconnected = 0;
+    }
+}
+
 QT_END_NAMESPACE
 
 #endif // QDECLARATIVENOTIFIER_P_H

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/doubleEvaluate.qml b/tests/auto/declarative/qdeclarativeecmascript/data/doubleEvaluate.qml
new file mode 100644 (file)
index 0000000..0532715
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/doubleEvaluate.qml
@@ -0,0 +1,6 @@
+import Qt.test 1.0
+
+WriteCounter {
+    property int x: 0
+    value: if (1) x + x
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp
index 5bd0287..513705d 100644 (file)
--- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.cpp
@@ -195,6 +195,7 @@ void registerTypes()
     qmlRegisterType<CircularReferenceHandle>("Qt.test", 1, 0, "CircularReferenceHandle");
 
     qmlRegisterType<MyDynamicCreationDestructionObject>("Qt.test", 1, 0, "MyDynamicCreationDestructionObject");
+    qmlRegisterType<WriteCounter>("Qt.test", 1, 0, "WriteCounter");
 }
 
 #include "testtypes.moc"

diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
index 5357a63..7684ddd 100644 (file)
--- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
+++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
@@ -1131,6 +1131,23 @@ private:
     int *m_dtorCount;
 };
 
+class WriteCounter : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(int value READ value WRITE setValue);
+public:
+    WriteCounter() : m_value(0), m_count(0) {}
+
+    int value() const { return m_value; }
+    void setValue(int v) { m_value = v; ++m_count; }
+
+    int count() const { return m_count; }
+
+private:
+    int m_value;
+    int m_count;
+};
+
 void registerTypes();
 
 #endif // TESTTYPES_H

diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 3feecfc..4f1cdcc 100644 (file)
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -207,6 +207,7 @@ private slots:
     void dynamicString();
     void include();
     void signalHandlers();
+    void doubleEvaluate();
 
     void callQtInvokables();
     void invokableObjectArg();
@@ -4763,6 +4764,23 @@ void tst_qdeclarativeecmascript::automaticSemicolon()
     QVERIFY(object != 0);
 }
 
+// Makes sure that a binding isn't double re-evaluated when it depends on the same variable twice
+void tst_qdeclarativeecmascript::doubleEvaluate()
+{
+    QDeclarativeComponent component(&engine, TEST_FILE("doubleEvaluate.qml"));
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+    WriteCounter *wc = qobject_cast<WriteCounter *>(object);
+    QVERIFY(wc != 0);
+    QCOMPARE(wc->count(), 1);
+
+    wc->setProperty("x", 9);
+
+    QCOMPARE(wc->count(), 2);
+
+    delete object;
+}
+
 QTEST_MAIN(tst_qdeclarativeecmascript)
 
 #include "tst_qdeclarativeecmascript.moc"