Avoid crash in QML app shutdown and actually send events in C++ app

The logic was such that in a C++ application the QNode::setParent()
function would bail out early in a C++ application when called from the
destructor of its parent object. This is because by the time the
child is being deleted the parent is in QObjectPrivate::deleteChildren
and therefore the QNode part of the object has already been destroyed.
This led to the cast in the parentNode() == parent to fail, thereby
exiting the functio early and never getting into
QNodePrivate::_q_setParentHelper().

In the case of a QML application, the parent has a dynamic metaobject
set by the QML engine. This resulted in the cast in QNode::setParent()
succeeding and we called into _q_setParentHelper(). The logic in here
resulted in a crash when called from a destructor because the child
had already been removed from its parent's list of children. Thus when
we called QObjectPrivate::setParentHelper(), this function ended up with
an index of -1 for the child in its child list (i.e. not found) and it
then tried to index into the children list with this index and we
then crashed.

The solution in this change is to not do the full logic in
QNode::setParent() and _q_setParentHelper(). Rather, we simply remove
the subtree at this node from the scene and we send node destruction
changes to the backend.

With this we avoid the crash of QML application shutdowns and we also
make sure to correctly send the node destruction changes even in the
case of a C++ Qt 3D application.

The backend does not yet get an opportunity to process these final
changes. This will be addressed in a follow up commit.

As a result of these changes many unit tests began crashing. This is
because the QNode dtor is now actually doing some work, rather than
bailing out of that work early when the parent is no longer a QNode.
This work involves mutating the QScene object which in the unit
tests did not live longer than the QNode's to which it was
associated with.

The unit tests have been adjusted to ensure that the arbiter and
scene objects remain alive longer than the QNodes they are being
used to test.

Task-number: QTBUG-42353
Change-Id: I197870f48fca30656bd85c4c51346d93403fba08
Reviewed-by: Kevin Ottens <kevin.ottens@kdab.com>

1 files changed, 2 insertions, 0 deletions

diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index 648bc4c34..e290ffe32 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -92,6 +92,7 @@ public:
     QMetaObject *m_typeInfo;
     QScene *m_scene;
     mutable QNodeId m_id;
+    QNodeId m_parentId; // Store this so we have it even in parent's QObject dtor
     bool m_blockNotifications;
     bool m_hasBackendNode;
     bool m_enabled;
@@ -101,6 +102,7 @@ public:
 
 private:
     void notifyCreationChange();
+    void notifyDestructionChangesAndRemoveFromScene();
     void _q_notifyCreationAndChildChanges();
     void _q_addChild(QNode *childNode);
     void _q_removeChild(QNode *childNode);