From 657ff7f6f882d8392fadbbe1cfed66034e34f758 Mon Sep 17 00:00:00 2001
From: Alexander Ivash <elderorb@gmail.com>
Date: Mon, 22 Oct 2018 23:29:39 +0300
Subject: [PATCH] eliminate signal, introduce deletion list

---
 libraries/qml/src/qml/OffscreenSurface.cpp  | 10 +++-------
 libraries/qml/src/qml/impl/SharedObject.cpp | 14 +++++++++++---
 libraries/qml/src/qml/impl/SharedObject.h   |  6 +++---
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/libraries/qml/src/qml/OffscreenSurface.cpp b/libraries/qml/src/qml/OffscreenSurface.cpp
index 37c3ae7a37..1f1d50ed1f 100644
--- a/libraries/qml/src/qml/OffscreenSurface.cpp
+++ b/libraries/qml/src/qml/OffscreenSurface.cpp
@@ -389,13 +389,9 @@ void OffscreenSurface::finishQmlLoad(QQmlComponent* qmlComponent,
         }
         // manually control children items lifetime
         QQmlEngine::setObjectOwnership(newObject, QQmlEngine::CppOwnership);
-        // some objects we are going to delete might be already deleted (children of parent we already deleted) so use QPointer to track lifetime
-        QPointer<QObject> trackedObject(newObject);
-        connect(_sharedObject, &SharedObject::onBeforeDestroyed, [trackedObject]() {
-            if (trackedObject) {
-                delete trackedObject.data();
-            }
-        });
+
+        // add object to the manual deletion list
+        _sharedObject->addToDeletionList(newObject);
 
         newObject->setParent(parent);
         newItem->setParentItem(parent);
diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp
index 7612a54173..1f41fefb02 100644
--- a/libraries/qml/src/qml/impl/SharedObject.cpp
+++ b/libraries/qml/src/qml/impl/SharedObject.cpp
@@ -15,6 +15,7 @@
 #include <QtQml/QQmlEngine>
 
 #include <QtGui/QOpenGLContext>
+#include <QPointer>
 
 #include <NumericalConstants.h>
 #include <shared/NsightHelpers.h>
@@ -79,9 +80,6 @@ SharedObject::SharedObject() {
 
 
 SharedObject::~SharedObject() {
-
-    emit onBeforeDestroyed();
-
     // After destroy returns, the rendering thread should be gone
     destroy();
     // _renderTimer is created with `this` as the parent, so need no explicit destruction
@@ -98,6 +96,11 @@ SharedObject::~SharedObject() {
     }
 #endif
 
+    // already deleted objects will be reset to null by QPointer so it should be safe just iterate here
+    for (auto qmlObject : _deletionList) {
+        delete qmlObject;
+    }
+
     if (_rootItem) {
         delete _rootItem;
         _rootItem = nullptr;
@@ -414,6 +417,11 @@ bool SharedObject::fetchTexture(TextureAndFence& textureAndFence) {
     return true;
 }
 
+void hifi::qml::impl::SharedObject::addToDeletionList(QObject * object)
+{
+    _deletionList.append(QPointer<QObject>(object));
+}
+
 void SharedObject::setProxyWindow(QWindow* window) {
 #ifndef DISABLE_QML
     _proxyWindow = window;
diff --git a/libraries/qml/src/qml/impl/SharedObject.h b/libraries/qml/src/qml/impl/SharedObject.h
index 57f4756e79..ce9fcd46d2 100644
--- a/libraries/qml/src/qml/impl/SharedObject.h
+++ b/libraries/qml/src/qml/impl/SharedObject.h
@@ -66,9 +66,7 @@ public:
     void resume();
     bool isPaused() const;
     bool fetchTexture(TextureAndFence& textureAndFence);
-
-signals:
-    void onBeforeDestroyed();
+    void addToDeletionList(QObject* object);
 
 private:
     bool event(QEvent* e) override;
@@ -93,6 +91,8 @@ private:
     void onAboutToQuit();
     void updateTextureAndFence(const TextureAndFence& newTextureAndFence);
 
+    QList<QPointer<QObject>> _deletionList;
+
     // Texture management
     TextureAndFence _latestTextureAndFence{ 0, 0 };
     QQuickItem* _item{ nullptr };