Ensure node creation changes are sent before using in list properties

This completes the fix for out of order event delivery related to
creation changes. We now ensure that QNodes used as values in singular
and list properties are fully constructed on the backend before they
are referenced in properties of other nodes.

Also added a check to not recurse into sending too many changes
when adding a child node.

Written with Svenn-Arne Dragly.

Task-number: Task-number: QTBUG-65956
Change-Id: I1470e0f685c81d1277ac04ad985ec1b76f1c27c0
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

2 files changed, 19 insertions, 2 deletions

diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index 09bd597e6..738ce6144 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -74,6 +74,7 @@ QNodePrivate::QNodePrivate()
     , m_blockNotifications(false)
     , m_hasBackendNode(false)
     , m_enabled(true)
+    , m_notifiedParent(false)
     , m_defaultPropertyTrackMode(QNode::TrackFinalValues)
     , m_propertyChangesSetup(false)
     , m_signals(this)
@@ -210,13 +211,19 @@ void QNodePrivate::_q_addChild(QNode *childNode)
     Q_ASSERT(childNode);
     Q_ASSERT_X(childNode->parent() == q_func(), Q_FUNC_INFO,  "not a child of this node");
 
+    // Have we already notified the parent about its new child? If so, bail out
+    // early so that we do not send more than one new child event to the backend
+    QNodePrivate *childD = QNodePrivate::get(childNode);
+    if (childD->m_notifiedParent == true)
+        return;
+
     // Store our id as the parentId in the child so that even if the child gets
     // removed from the scene as part of the destruction of the parent, when the
     // parent's children are deleted in the QObject dtor, we still have access to
     // the parentId. If we didn't store this, we wouldn't have access at that time
     // because the parent would then only be a QObject, the QNode part would have
     // been destroyed already.
-    QNodePrivate::get(childNode)->m_parentId = m_id;
+    childD->m_parentId = m_id;
 
     if (!m_scene)
         return;
@@ -224,6 +231,11 @@ void QNodePrivate::_q_addChild(QNode *childNode)
     // We need to send a QPropertyNodeAddedChange to the backend
     // to notify the backend that we have a new child
     if (m_changeArbiter != nullptr) {
+        // Flag that we have notified the parent. We do this immediately before
+        // creating the change because that recurses back into this function and
+        // we need to catch that to avoid sending more than one new child event
+        // to the backend.
+        childD->m_notifiedParent = true;
         const auto change = QPropertyNodeAddedChangePtr::create(m_id, childNode);
         change->setPropertyName("children");
         notifyObservers(change);
@@ -299,6 +311,9 @@ void QNodePrivate::_q_setParentHelper(QNode *parent)
             notifyDestructionChangesAndRemoveFromScene();
     }
 
+    // Flag that we need to notify any new parent
+    m_notifiedParent = false;
+
     // Basically QObject::setParent but for QObjectPrivate
     QObjectPrivate::setParent_helper(parent);
     QNode *newParentNode = q->parentNode();
diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index ad9d2376e..87a0226f1 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -100,6 +100,7 @@ public:
     bool m_blockNotifications;
     bool m_hasBackendNode;
     bool m_enabled;
+    bool m_notifiedParent;
     QNode::PropertyTrackingMode m_defaultPropertyTrackMode;
     QHash<QString, QNode::PropertyTrackingMode> m_trackedPropertiesOverrides;
 
@@ -137,10 +138,11 @@ public:
 
     static const QMetaObject *findStaticMetaObject(const QMetaObject *metaObject);
 
+    void _q_postConstructorInit();
+
 private:
     void notifyCreationChange();
     void notifyDestructionChangesAndRemoveFromScene();
-    void _q_postConstructorInit();
     void _q_addChild(QNode *childNode);
     void _q_removeChild(QNode *childNode);
     void _q_setParentHelper(QNode *parent);