Fix async issues related to JS / QML object lifetimes

libraries/ui/src/QmlWebWindowClass.cpp

@@ -31,19 +31,18 @@ static const char* const URL_PROPERTY = "source";
 // Method called by Qt scripts to create a new web window in the overlay
 QScriptValue QmlWebWindowClass::constructor(QScriptContext* context, QScriptEngine* engine) {
     return QmlWindowClass::internalConstructor("QmlWebWindow.qml", context, engine,
-        [&](QObject* object) { return new QmlWebWindowClass(object); });
+        [&](QObject* object) {  return new QmlWebWindowClass(object);  });
 }
 
 QmlWebWindowClass::QmlWebWindowClass(QObject* qmlWindow) : QmlWindowClass(qmlWindow) {
 }
 
 
-// FIXME remove.
-void QmlWebWindowClass::handleNavigation(const QString& url) {
-}
-
 QString QmlWebWindowClass::getURL() const {
     QVariant result = DependencyManager::get<OffscreenUi>()->returnFromUiThread([&]()->QVariant {
+        if (_qmlWindow.isNull()) {
+            return QVariant();
+        }
         return _qmlWindow->property(URL_PROPERTY);
     });
     return result.toString();
@@ -54,6 +53,8 @@ extern QString fixupHifiUrl(const QString& urlString);
 
 void QmlWebWindowClass::setURL(const QString& urlString) {
     DependencyManager::get<OffscreenUi>()->executeOnUiThread([=] {
-        _qmlWindow->setProperty(URL_PROPERTY, fixupHifiUrl(urlString));
+        if (!_qmlWindow.isNull()) {
+            _qmlWindow->setProperty(URL_PROPERTY, fixupHifiUrl(urlString));
+        }
     });
 }

libraries/ui/src/QmlWebWindowClass.h

@@ -26,9 +26,6 @@ public slots:
 
 signals:
     void urlChanged();
-
-private slots:
-    void handleNavigation(const QString& url);
 };
 
 #endif

libraries/ui/src/QmlWindowClass.cpp

@@ -214,7 +214,7 @@ QmlWindowClass::QmlWindowClass(QObject* qmlWindow)
 {
     qDebug() << "Created window with ID " << _windowId;
     Q_ASSERT(_qmlWindow);
-    Q_ASSERT(dynamic_cast<const QQuickItem*>(_qmlWindow));
+    Q_ASSERT(dynamic_cast<const QQuickItem*>(_qmlWindow.data()));
     // Forward messages received from QML on to the script
     connect(_qmlWindow, SIGNAL(sendToScript(QVariant)), this, SIGNAL(fromQml(const QVariant&)), Qt::QueuedConnection);
 }
@@ -240,7 +240,7 @@ QQuickItem* QmlWindowClass::asQuickItem() const {
     if (_toolWindow) {
         return DependencyManager::get<OffscreenUi>()->getToolWindow();
     }
-    return dynamic_cast<QQuickItem*>(_qmlWindow);
+    return _qmlWindow.isNull() ? nullptr : dynamic_cast<QQuickItem*>(_qmlWindow.data());
 }
 
 void QmlWindowClass::setVisible(bool visible) {
@@ -260,32 +260,34 @@ void QmlWindowClass::setVisible(bool visible) {
 
 bool QmlWindowClass::isVisible() const {
     // The tool window itself has special logic based on whether any tabs are enabled
-    if (_toolWindow) {
-        auto targetTab = dynamic_cast<QQuickItem*>(_qmlWindow);
-        return DependencyManager::get<OffscreenUi>()->returnFromUiThread([&] {
-            return QVariant::fromValue(targetTab->isEnabled());
-        }).toBool();
-    } else {
-        QQuickItem* targetWindow = asQuickItem();
-        return DependencyManager::get<OffscreenUi>()->returnFromUiThread([&] {
-            return QVariant::fromValue(targetWindow->isVisible());
-        }).toBool();
-    }
+    return DependencyManager::get<OffscreenUi>()->returnFromUiThread([&] {
+        if (_qmlWindow.isNull()) {
+            return QVariant::fromValue(false);
+        }
+        if (_toolWindow) {
+            return QVariant::fromValue(dynamic_cast<QQuickItem*>(_qmlWindow.data())->isEnabled());
+        } else {
+            return QVariant::fromValue(asQuickItem()->isVisible());
+        }
+    }).toBool();
 }
 
 glm::vec2 QmlWindowClass::getPosition() const {
-    QQuickItem* targetWindow = asQuickItem();
     QVariant result = DependencyManager::get<OffscreenUi>()->returnFromUiThread([&]()->QVariant {
-        return targetWindow->position();
+        if (_qmlWindow.isNull()) {
+            return QVariant(QPointF(0, 0));
+        }
+        return asQuickItem()->position();
     });
     return toGlm(result.toPointF());
 }
 
 
 void QmlWindowClass::setPosition(const glm::vec2& position) {
-    QQuickItem* targetWindow = asQuickItem();
     DependencyManager::get<OffscreenUi>()->executeOnUiThread([=] {
-        targetWindow->setPosition(QPointF(position.x, position.y));
+        if (!_qmlWindow.isNull()) {
+            asQuickItem()->setPosition(QPointF(position.x, position.y));
+        }
     });
 }
 
@@ -299,17 +301,21 @@ glm::vec2 toGlm(const QSizeF& size) {
 }
 
 glm::vec2 QmlWindowClass::getSize() const {
-    QQuickItem* targetWindow = asQuickItem();
     QVariant result = DependencyManager::get<OffscreenUi>()->returnFromUiThread([&]()->QVariant {
+        if (_qmlWindow.isNull()) {
+            return QVariant(QSizeF(0, 0));
+        }
+        QQuickItem* targetWindow = asQuickItem();
         return QSizeF(targetWindow->width(), targetWindow->height());
     });
     return toGlm(result.toSizeF());
 }
 
 void QmlWindowClass::setSize(const glm::vec2& size) {
-    QQuickItem* targetWindow = asQuickItem();
     DependencyManager::get<OffscreenUi>()->executeOnUiThread([=] {
-        targetWindow->setSize(QSizeF(size.x, size.y));
+        if (!_qmlWindow.isNull()) {
+            asQuickItem()->setSize(QSizeF(size.x, size.y));
+        }
     });
 }
 
@@ -318,9 +324,10 @@ void QmlWindowClass::setSize(int width, int height) {
 }
 
 void QmlWindowClass::setTitle(const QString& title) {
-    QQuickItem* targetWindow = asQuickItem();
     DependencyManager::get<OffscreenUi>()->executeOnUiThread([=] {
-        targetWindow->setProperty(TITLE_PROPERTY, title);
+        if (!_qmlWindow.isNull()) {
+            asQuickItem()->setProperty(TITLE_PROPERTY, title);
+        }
     });
 }
 
@@ -345,7 +352,12 @@ void QmlWindowClass::hasClosed() {
 }
 
 void QmlWindowClass::raise() {
-    QMetaObject::invokeMethod(asQuickItem(), "raise", Qt::QueuedConnection);
+    auto offscreenUi = DependencyManager::get<OffscreenUi>();
+    offscreenUi->executeOnUiThread([=] {
+        if (!_qmlWindow.isNull()) {
+            QMetaObject::invokeMethod(asQuickItem(), "raise", Qt::DirectConnection);
+        }
+    });
 }
 
 #include "QmlWindowClass.moc"

libraries/ui/src/QmlWindowClass.h

@@ -10,11 +10,13 @@
 #define hifi_ui_QmlWindowClass_h
 
 #include <QtCore/QObject>
-#include <GLMHelpers.h>
+#include <QtCore/QPointer>
 #include <QtScript/QScriptValue>
 #include <QtQuick/QQuickItem>
 #include <QtWebChannel/QWebChannelAbstractTransport>
 
+#include <GLMHelpers.h>
+
 class QScriptEngine;
 class QScriptContext;
 class QmlWindowClass;
@@ -38,14 +40,13 @@ private:
     const QmlWindowClass* _webWindow { nullptr };
     QWebSocket *_socket { nullptr };
 };
-
 // FIXME refactor this class to be a QQuickItem derived type and eliminate the needless wrapping 
 class QmlWindowClass : public QObject {
     Q_OBJECT
     Q_PROPERTY(QObject* eventBridge READ getEventBridge CONSTANT)
     Q_PROPERTY(int windowId READ getWindowId CONSTANT)
-    Q_PROPERTY(glm::vec2 position READ getPosition WRITE setPosition)
-    Q_PROPERTY(glm::vec2 size READ getSize WRITE setSize)
+    Q_PROPERTY(glm::vec2 position READ getPosition WRITE setPosition NOTIFY positionChanged)
+    Q_PROPERTY(glm::vec2 size READ getSize WRITE setSize NOTIFY sizeChanged)
     Q_PROPERTY(bool visible READ isVisible WRITE setVisible NOTIFY visibilityChanged)
 
 public:
@@ -64,11 +65,8 @@ public slots:
     glm::vec2 getSize() const;
     void setSize(const glm::vec2& size);
     void setSize(int width, int height);
-
     void setTitle(const QString& title);
 
-
-    // Ugh.... do not want to do
     Q_INVOKABLE void raise();
     Q_INVOKABLE void close();
     Q_INVOKABLE int getWindowId() const { return _windowId; };
@@ -79,6 +77,8 @@ public slots:
 
 signals:
     void visibilityChanged(bool visible);  // Tool window
+    void positionChanged();
+    void sizeChanged();
     void moved(glm::vec2 position);
     void resized(QSizeF size);
     void closed();
@@ -104,7 +104,7 @@ protected:
     // for tool window panes in QML
     bool _toolWindow { false };
     const int _windowId;
-    QObject* _qmlWindow;
+    QPointer<QObject> _qmlWindow;
     QString _source;
 };