Fix visual parent ownership with scenes that create windows

Commit 39540124dd0900e0c99dcda8c0ebdf4f3cea8d5e introduced the concept that
a visual parent marks its children, by recursively marking the children of
the root item in a QQuickView. This allowed for the removal of an ugly hack
in QtQuick Controls.

Unfortunately that fix is incomplete in the sense that it makes the incorrect
assumption that a QQuickView is always used. The use-case in the bug report is
to have child items inside a QtQuick.Window (a regular ApplicationWindow in
fact). That window - implemented by QQuickWindowQmlImpl - also needs to mark
its children, so this patch introduces the use of the same GC marking helper
class (which now operates on a QQuickWindow instead of a QQuickViewPrivate).

Task-number: QTBUG-37711
Change-Id: Id788e84dbb041ac8ba6ff23dc4ef56f6fe9e465a
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

5 files changed, 90 insertions, 8 deletions

diff --git a/src/quick/items/qquickview.cpp b/src/quick/items/qquickview.cpp
index 82c1b3a6ae..72c66448b4 100644
--- a/src/quick/items/qquickview.cpp
+++ b/src/quick/items/qquickview.cpp
@@ -58,16 +58,16 @@ QT_BEGIN_NAMESPACE
 
 DEFINE_OBJECT_VTABLE(QQuickRootItemMarker);
 
-QQuickRootItemMarker::QQuickRootItemMarker(QQuickViewPrivate *view)
-    : QV4::Object(QQmlEnginePrivate::getV4Engine(view->engine.data()))
-    , view(view)
+QQuickRootItemMarker::QQuickRootItemMarker(QQmlEngine *engine, QQuickWindow *window)
+    : QV4::Object(QQmlEnginePrivate::getV4Engine(engine))
+    , window(window)
 {
     setVTable(staticVTable());
 }
 
 void QQuickRootItemMarker::markObjects(QV4::Managed *that, QV4::ExecutionEngine *e)
 {
-    QQuickItem *root = static_cast<QQuickRootItemMarker*>(that)->view->root;
+    QQuickItem *root = static_cast<QQuickRootItemMarker*>(that)->window->contentItem();
     if (root) {
         QQuickItemPrivate *rootPrivate = QQuickItemPrivate::get(root);
         rootPrivate->markObjects(e);
@@ -91,7 +91,7 @@ void QQuickViewPrivate::init(QQmlEngine* e)
     {
         QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(engine.data());
         QV4::Scope scope(v4);
-        QV4::Scoped<QQuickRootItemMarker> v(scope, new (v4->memoryManager) QQuickRootItemMarker(this));
+        QV4::Scoped<QQuickRootItemMarker> v(scope, new (v4->memoryManager) QQuickRootItemMarker(engine.data(), q));
         rootItemMarker = v;
     }
 
diff --git a/src/quick/items/qquickview_p.h b/src/quick/items/qquickview_p.h
index 74e40081e9..dfc8c21d0b 100644
--- a/src/quick/items/qquickview_p.h
+++ b/src/quick/items/qquickview_p.h
@@ -106,7 +106,7 @@ struct QQuickRootItemMarker : public QV4::Object
 {
     V4_OBJECT
 
-    QQuickRootItemMarker(QQuickViewPrivate *view);
+    QQuickRootItemMarker(QQmlEngine *engine, QQuickWindow *window);
 
     static void destroy(Managed *that)
     {
@@ -115,7 +115,7 @@ struct QQuickRootItemMarker : public QV4::Object
 
     static void markObjects(Managed *that, QV4::ExecutionEngine *e);
 
-    QQuickViewPrivate *view;
+    QQuickWindow *window;
 };
 
 QT_END_NAMESPACE
diff --git a/src/quick/items/qquickwindowmodule.cpp b/src/quick/items/qquickwindowmodule.cpp
index 3bd62d21db..d06fff8c3e 100644
--- a/src/quick/items/qquickwindowmodule.cpp
+++ b/src/quick/items/qquickwindowmodule.cpp
@@ -41,6 +41,7 @@
 
 #include "qquickwindowmodule_p.h"
 #include "qquickscreen_p.h"
+#include "qquickview_p.h"
 #include <QtQuick/QQuickWindow>
 #include <QtCore/QCoreApplication>
 #include <QtQml/QQmlEngine>
@@ -91,12 +92,19 @@ Q_SIGNALS:
 
 protected:
     void classBegin() {
+        QQmlEngine* e = qmlEngine(this);
         //Give QQuickView behavior when created from QML with QQmlApplicationEngine
         if (QCoreApplication::instance()->property("__qml_using_qqmlapplicationengine") == QVariant(true)) {
-            QQmlEngine* e = qmlEngine(this);
             if (e && !e->incubationController())
                 e->setIncubationController(incubationController());
         }
+        Q_ASSERT(e);
+        {
+            QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(e);
+            QV4::Scope scope(v4);
+            QV4::ScopedObject v(scope, new (v4->memoryManager) QQuickRootItemMarker(e, this));
+            rootItemMarker = v;
+        }
     }
 
     void componentComplete() {
@@ -158,6 +166,7 @@ private:
     bool m_complete;
     bool m_visible;
     Visibility m_visibility;
+    QV4::PersistentValue rootItemMarker;
 };
 
 void QQuickWindowModule::defineModule()
diff --git a/tests/auto/quick/qquickitem/data/visualParentOwnershipWindow.qml b/tests/auto/quick/qquickitem/data/visualParentOwnershipWindow.qml
new file mode 100644
index 0000000000..30fc844d7a
--- /dev/null
+++ b/tests/auto/quick/qquickitem/data/visualParentOwnershipWindow.qml
@@ -0,0 +1,15 @@
+import QtQuick 2.0
+import QtQuick.Window 2.1
+
+Window {
+    Component {
+        id: factory
+        Item {}
+    }
+
+    property Item keepAliveProperty;
+
+    function createItemWithoutParent() {
+        return factory.createObject(/*parent*/ null);
+    }
+}
diff --git a/tests/auto/quick/qquickitem/tst_qquickitem.cpp b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
index a1d9d0c73b..6f11986e20 100644
--- a/tests/auto/quick/qquickitem/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
@@ -169,6 +169,7 @@ private slots:
     void acceptedMouseButtons();
 
     void visualParentOwnership();
+    void visualParentOwnershipWindow();
 
 private:
 
@@ -1821,6 +1822,63 @@ void tst_qquickitem::visualParentOwnership()
     }
 }
 
+void tst_qquickitem::visualParentOwnershipWindow()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl("visualParentOwnershipWindow.qml"));
+
+    QQuickWindow *window = qobject_cast<QQuickWindow*>(component.create());
+    QVERIFY(window);
+    QQuickItem *root = window->contentItem();
+
+    QVariant newObject;
+    {
+        QVERIFY(QMetaObject::invokeMethod(window, "createItemWithoutParent", Q_RETURN_ARG(QVariant, newObject)));
+        QPointer<QQuickItem> newItem = qvariant_cast<QQuickItem*>(newObject);
+        QVERIFY(!newItem.isNull());
+
+        QVERIFY(!newItem->parent());
+        QVERIFY(!newItem->parentItem());
+
+        newItem->setParentItem(root);
+
+        gc(engine);
+
+        QVERIFY(!newItem.isNull());
+        newItem->setParentItem(0);
+
+        gc(engine);
+        QVERIFY(newItem.isNull());
+    }
+    {
+        QVERIFY(QMetaObject::invokeMethod(window, "createItemWithoutParent", Q_RETURN_ARG(QVariant, newObject)));
+        QPointer<QQuickItem> firstItem = qvariant_cast<QQuickItem*>(newObject);
+        QVERIFY(!firstItem.isNull());
+
+        firstItem->setParentItem(root);
+
+        QVERIFY(QMetaObject::invokeMethod(window, "createItemWithoutParent", Q_RETURN_ARG(QVariant, newObject)));
+        QPointer<QQuickItem> secondItem = qvariant_cast<QQuickItem*>(newObject);
+        QVERIFY(!firstItem.isNull());
+
+        secondItem->setParentItem(firstItem);
+
+        gc(engine);
+
+        delete firstItem;
+
+        window->setProperty("keepAliveProperty", newObject);
+
+        gc(engine);
+        QVERIFY(!secondItem.isNull());
+
+        window->setProperty("keepAliveProperty", QVariant());
+
+        gc(engine);
+        QVERIFY(secondItem.isNull());
+    }
+}
+
 QTEST_MAIN(tst_qquickitem)
 
 #include "tst_qquickitem.moc"