If Loader loads Window, set its transient parent to the Loader's window

This makes the Item { Loader { sourceComponent: Window { } } } case
consistent with the Item { Window { } } case: the inner Window is
transient for the outer Window.

It works even if the Loader's Window has a visible: true declaration:
in that case, until now, the Loader's Window would become visible
at component creation time, before the outer Item became visible.
So the test to check whether it had a transient parent did not work.

We now change the delayed-visibility mechanism in QQuickWindowQmlImpl
to wait for the parent Item to acquire a window of its own rather
than waiting for the transient-parent-if-any to become visible.
It should still take care of all the old cases too, e.g. in the
Window { Window { } } case, the inner Window's QObject parent is actually
the QQuickRootItem.  (Amends 701255c76f671f100338a799f0194bf10e26c9d1)

[ChangeLog][QtQuick][QQuickWindow] When a Window is declared inside
another Item or Window, the window will not be created until
the parent window is created.  This allows it to have the correct
transientParent and be managed as a transient window.

Task-number: QTBUG-52944
Change-Id: Iaf4aafbd696f6e8dd0eec1d02db8bd181483bd07
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

6 files changed, 166 insertions, 4 deletions

diff --git a/src/quick/items/qquickloader.cpp b/src/quick/items/qquickloader.cpp
index 414103c63e..a3e8379368 100644
--- a/src/quick/items/qquickloader.cpp
+++ b/src/quick/items/qquickloader.cpp
@@ -674,6 +674,13 @@ void QQuickLoaderPrivate::incubatorStateChanged(QQmlIncubator::Status status)
     if (status == QQmlIncubator::Ready) {
         object = incubator->object();
         item = qmlobject_cast<QQuickItem*>(object);
+        if (!item) {
+            QQuickWindow *window = qmlobject_cast<QQuickWindow*>(object);
+            if (window) {
+                qCDebug(lcTransient) << window << "is transient for" << q->window();
+                window->setTransientParent(q->window());
+            }
+        }
         emit q->itemChanged();
         initResize();
         incubator->clear();
@@ -818,6 +825,18 @@ void QQuickLoader::componentComplete()
     }
 }
 
+void QQuickLoader::itemChange(QQuickItem::ItemChange change, const QQuickItem::ItemChangeData &value)
+{
+    if (change == ItemSceneChange) {
+        QQuickWindow *loadedWindow = qmlobject_cast<QQuickWindow *>(item());
+        if (loadedWindow) {
+            qCDebug(lcTransient) << loadedWindow << "is transient for" << value.window;
+            loadedWindow->setTransientParent(value.window);
+        }
+    }
+    QQuickItem::itemChange(change, value);
+}
+
 /*!
     \qmlsignal QtQuick::Loader::loaded()
 
diff --git a/src/quick/items/qquickloader_p.h b/src/quick/items/qquickloader_p.h
index db171dcd1e..8f0a9980b9 100644
--- a/src/quick/items/qquickloader_p.h
+++ b/src/quick/items/qquickloader_p.h
@@ -107,6 +107,7 @@ Q_SIGNALS:
 protected:
     void geometryChanged(const QRectF &newGeometry, const QRectF &oldGeometry) Q_DECL_OVERRIDE;
     void componentComplete() Q_DECL_OVERRIDE;
+    void itemChange(ItemChange change, const ItemChangeData &value) override;
 
 private:
     void setSource(const QUrl &sourceUrl, bool needsClear);
diff --git a/src/quick/items/qquickwindowmodule.cpp b/src/quick/items/qquickwindowmodule.cpp
index 1981ed2f70..e702cc5a3e 100644
--- a/src/quick/items/qquickwindowmodule.cpp
+++ b/src/quick/items/qquickwindowmodule.cpp
@@ -123,7 +123,13 @@ void QQuickWindowQmlImpl::componentComplete()
 {
     Q_D(QQuickWindowQmlImpl);
     d->complete = true;
-    if (transientParent() && !transientParent()->isVisible()) {
+    QQuickItem *itemParent = qmlobject_cast<QQuickItem *>(QObject::parent());
+    if (itemParent && !itemParent->window()) {
+        qCDebug(lcTransient) << "window" << title() << "has invisible Item parent" << itemParent << "transientParent"
+                             << transientParent() << "declared visibility" << d->visibility << "; delaying show";
+        connect(itemParent, &QQuickItem::windowChanged, this,
+                &QQuickWindowQmlImpl::setWindowVisibility, Qt::QueuedConnection);
+    } else if (transientParent() && !transientParent()->isVisible()) {
         connect(transientParent(), &QQuickWindow::visibleChanged, this,
                 &QQuickWindowQmlImpl::setWindowVisibility, Qt::QueuedConnection);
     } else {
@@ -137,9 +143,10 @@ void QQuickWindowQmlImpl::setWindowVisibility()
     if (transientParent() && !transientParent()->isVisible())
         return;
 
-    if (sender()) {
-        disconnect(transientParent(), &QWindow::visibleChanged, this,
-                   &QQuickWindowQmlImpl::setWindowVisibility);
+    if (QQuickItem *senderItem = qmlobject_cast<QQuickItem *>(sender())) {
+        disconnect(senderItem, &QQuickItem::windowChanged, this, &QQuickWindowQmlImpl::setWindowVisibility);
+    } else if (sender()) {
+        disconnect(transientParent(), &QWindow::visibleChanged, this, &QQuickWindowQmlImpl::setWindowVisibility);
     }
 
     // We have deferred window creation until we have the full picture of what
diff --git a/tests/auto/quick/qquickloader/data/itemLoaderItemWindow.qml b/tests/auto/quick/qquickloader/data/itemLoaderItemWindow.qml
new file mode 100644
index 0000000000..d4c5daecab
--- /dev/null
+++ b/tests/auto/quick/qquickloader/data/itemLoaderItemWindow.qml
@@ -0,0 +1,27 @@
+import QtQuick 2.0
+import QtQuick.Window 2.1
+
+Item {
+    width: 400
+    height: 400
+    objectName: "root Item"
+
+    Loader {
+        sourceComponent: Rectangle {
+            objectName: "yellow rectangle"
+            x: 50; y: 50; width: 300; height: 300
+            color: "yellow"
+            Window {
+                objectName: "red transient Window"
+                width: 100
+                height: 100
+                visible: true // makes it harder, because it wants to become visible before root has a window
+                color: "red"
+                title: "red"
+                flags: Qt.Dialog
+                onVisibilityChanged: console.log("visibility " + visibility)
+                onVisibleChanged: console.log("visible " + visible)
+            }
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickloader/data/itemLoaderWindow.qml b/tests/auto/quick/qquickloader/data/itemLoaderWindow.qml
new file mode 100644
index 0000000000..69421448e0
--- /dev/null
+++ b/tests/auto/quick/qquickloader/data/itemLoaderWindow.qml
@@ -0,0 +1,22 @@
+import QtQuick 2.0
+import QtQuick.Window 2.1
+
+Item {
+    width: 400
+    height: 400
+    objectName: "root Item"
+
+    Loader {
+        sourceComponent: Window {
+            objectName: "red transient Window"
+            width: 100
+            height: 100
+            visible: true // makes it harder, because it wants to become visible before root has a window
+            color: "red"
+            title: "red"
+            flags: Qt.Dialog
+            onVisibilityChanged: console.log("visibility " + visibility)
+            onVisibleChanged: console.log("visible " + visible)
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickloader/tst_qquickloader.cpp b/tests/auto/quick/qquickloader/tst_qquickloader.cpp
index 521388c5fa..582ba2aabc 100644
--- a/tests/auto/quick/qquickloader/tst_qquickloader.cpp
+++ b/tests/auto/quick/qquickloader/tst_qquickloader.cpp
@@ -32,11 +32,15 @@
 #include <QtQml/qqmlengine.h>
 #include <QtQml/qqmlcomponent.h>
 #include <QtQml/qqmlincubator.h>
+#include <QtQuick/qquickview.h>
 #include <private/qquickloader_p.h>
+#include <private/qquickwindowmodule_p.h>
 #include "testhttpserver.h"
 #include "../../shared/util.h"
 #include "../shared/geometrytestutil.h"
 
+Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests")
+
 class SlowComponent : public QQmlComponent
 {
     Q_OBJECT
@@ -114,6 +118,8 @@ private slots:
     void parented();
     void sizeBound();
     void QTBUG_30183();
+    void transientWindow();
+    void nestedTransientWindow();
 
     void sourceComponentGarbageCollection();
 
@@ -1207,6 +1213,86 @@ void tst_QQuickLoader::QTBUG_30183()
     delete loader;
 }
 
+void tst_QQuickLoader::transientWindow() // QTBUG-52944
+{
+    QQuickView view;
+    view.setSource(testFileUrl("itemLoaderWindow.qml"));
+    QQuickItem *root = qobject_cast<QQuickItem*>(view.rootObject());
+    QVERIFY(root);
+    QQuickLoader *loader = root->findChild<QQuickLoader *>();
+    QVERIFY(loader);
+    QTRY_COMPARE(loader->status(), QQuickLoader::Ready);
+    QQuickWindowQmlImpl *loadedWindow = qobject_cast<QQuickWindowQmlImpl *>(loader->item());
+    QVERIFY(loadedWindow);
+    QCOMPARE(loadedWindow->visibility(), QWindow::Hidden);
+
+    QElapsedTimer timer;
+    qint64 viewVisibleTime = -1;
+    qint64 loadedWindowVisibleTime = -1;
+    connect(&view, &QWindow::visibleChanged,
+            [&viewVisibleTime, &timer]() { viewVisibleTime = timer.elapsed(); } );
+    connect(loadedWindow, &QQuickWindowQmlImpl::visibilityChanged,
+            [&loadedWindowVisibleTime, &timer]() { loadedWindowVisibleTime = timer.elapsed(); } );
+    timer.start();
+    view.show();
+
+    QTest::qWaitForWindowExposed(&view);
+    QTRY_VERIFY(loadedWindowVisibleTime >= 0);
+    QVERIFY(viewVisibleTime >= 0);
+
+    // now that we're sure they are both visible, which one became visible first?
+    qCDebug(lcTests) << "transient Window became visible" << (loadedWindowVisibleTime - viewVisibleTime) << "ms after the root Item";
+    QVERIFY((loadedWindowVisibleTime - viewVisibleTime) >= 0);
+
+    QWindowList windows = QGuiApplication::topLevelWindows();
+    QTRY_COMPARE(windows.size(), 2);
+
+    // TODO Ideally we would now close the outer window and make sure the transient window closes too.
+    // It works during manual testing because of QWindowPrivate::maybeQuitOnLastWindowClosed()
+    // but quitting an autotest doesn't make sense.
+}
+
+void tst_QQuickLoader::nestedTransientWindow() // QTBUG-52944
+{
+    QQuickView view;
+    view.setSource(testFileUrl("itemLoaderItemWindow.qml"));
+    QQuickItem *root = qobject_cast<QQuickItem*>(view.rootObject());
+    QVERIFY(root);
+    QQuickLoader *loader = root->findChild<QQuickLoader *>();
+    QVERIFY(loader);
+    QTRY_COMPARE(loader->status(), QQuickLoader::Ready);
+    QQuickItem *loadedItem = qobject_cast<QQuickItem *>(loader->item());
+    QVERIFY(loadedItem);
+    QQuickWindowQmlImpl *loadedWindow = loadedItem->findChild<QQuickWindowQmlImpl *>();
+    QVERIFY(loadedWindow);
+    QCOMPARE(loadedWindow->visibility(), QWindow::Hidden);
+
+    QElapsedTimer timer;
+    qint64 viewVisibleTime = -1;
+    qint64 loadedWindowVisibleTime = -1;
+    connect(&view, &QWindow::visibleChanged,
+            [&viewVisibleTime, &timer]() { viewVisibleTime = timer.elapsed(); } );
+    connect(loadedWindow, &QQuickWindowQmlImpl::visibilityChanged,
+            [&loadedWindowVisibleTime, &timer]() { loadedWindowVisibleTime = timer.elapsed(); } );
+    timer.start();
+    view.show();
+
+    QTest::qWaitForWindowExposed(&view);
+    QTRY_VERIFY(loadedWindowVisibleTime >= 0);
+    QVERIFY(viewVisibleTime >= 0);
+
+    // now that we're sure they are both visible, which one became visible first?
+    qCDebug(lcTests) << "transient Window became visible" << (loadedWindowVisibleTime - viewVisibleTime) << "ms after the root Item";
+    QVERIFY((loadedWindowVisibleTime - viewVisibleTime) >= 0);
+
+    QWindowList windows = QGuiApplication::topLevelWindows();
+    QTRY_COMPARE(windows.size(), 2);
+
+    // TODO Ideally we would now close the outer window and make sure the transient window closes too.
+    // It works during manual testing because of QWindowPrivate::maybeQuitOnLastWindowClosed()
+    // but quitting an autotest doesn't make sense.
+}
+
 void tst_QQuickLoader::sourceComponentGarbageCollection()
 {
     QQmlComponent component(&engine, testFileUrl("sourceComponentGarbageCollection.qml"));