QWindow: Persist foreign winId to support destroy/create cycles

We used to set a private _q_foreignWinId property on QWindow when
creating foreign windows, and this was the mechanism which we then
passed the foreign winId to the platform plugin.

With c585802e946d97e7d177ea334a162dc7bc286b84 this was removed,
since we now were passing the winId through via explicit QPA
APIs, and since 0c6911e5cde24c45d6f2c08b6e71064bdd1eccfa removed
the ability to explicitly destroy() a foreign window.

But when closing a QWindow, we destroy both the window itself,
and all its children, including foreign windows. In this case
we still want to support recreating the foreign window, for
example when the parent window is shown again. To enable this
we restore the _q_foreignWinId private property, but keep
the limitation of not being able to explicitly destroy a
foreign window.

Pick-to: 6.7 6.5
Fixes: QTBUG-124160
Change-Id: Ia885ba9f043e64fb21eedd2b4c344679726f1b5c
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>

src/gui/kernel/qwindow.cpp

@@ -521,7 +521,9 @@ void QWindowPrivate::setTopLevelScreen(QScreen *newScreen, bool recreate)
     }
 }
 
-void QWindowPrivate::create(bool recursive, WId nativeHandle)
+static constexpr auto kForeignWindowId = "_q_foreignWinId";
+
+void QWindowPrivate::create(bool recursive)
 {
     Q_Q(QWindow);
     if (platformWindow)
@@ -549,6 +551,8 @@ void QWindowPrivate::create(bool recursive, WId nativeHandle)
             setTopLevelScreen(screen, false);
     }
 
+    const WId nativeHandle = q->property(kForeignWindowId).value<WId>();
+
     QPlatformIntegration *platformIntegration = QGuiApplicationPrivate::platformIntegration();
     platformWindow = nativeHandle ? platformIntegration->createForeignWindow(q, nativeHandle)
         : platformIntegration->createPlatformWindow(q);
@@ -2059,6 +2063,16 @@ void QWindowPrivate::destroy()
         QObject *object = childrenWindows.at(i);
         if (object->isWindowType()) {
             QWindow *w = static_cast<QWindow*>(object);
+            auto *childPlatformWindow = w->handle();
+            if (!childPlatformWindow)
+                continue;
+
+            // Decouple the foreign window from this window,
+            // so that destroying our native handle doesn't
+            // bring down the foreign window as well.
+            if (childPlatformWindow->isForeignWindow())
+                childPlatformWindow->setParent(nullptr);
+
             qt_window_private(w)->destroy();
         }
     }
@@ -2986,7 +3000,11 @@ QWindow *QWindow::fromWinId(WId id)
     }
 
     QWindow *window = new QWindow;
-    qt_window_private(window)->create(false, id);
+
+    // Persist the winId in a private property so that we
+    // can recreate the window after being destroyed.
+    window->setProperty(kForeignWindowId, id);
+    window->create();
 
     if (!window->handle()) {
         delete window;

src/gui/kernel/qwindow_p.h

@@ -66,7 +66,7 @@ public:
     void updateSiblingPosition(SiblingPosition);
 
     bool windowRecreationRequired(QScreen *newScreen) const;
-    void create(bool recursive, WId nativeHandle = 0);
+    void create(bool recursive);
     void destroy();
     void setTopLevelScreen(QScreen *newScreen, bool recreate);
     void connectToScreen(QScreen *topLevelScreen);

src/plugins/platforms/ios/qioswindow.mm

@@ -55,7 +55,8 @@ QIOSWindow::QIOSWindow(QWindow *window, WId nativeHandle)
 
     connect(qGuiApp, &QGuiApplication::applicationStateChanged, this, &QIOSWindow::applicationStateChanged);
 
-    setParent(QPlatformWindow::parent());
+    if (QPlatformWindow::parent())
+        setParent(QPlatformWindow::parent());
 
     if (!isForeignWindow()) {
         // Resolve default window geometry in case it was not set before creating the

src/plugins/platforms/windows/qwindowswindow.cpp

@@ -1357,6 +1357,8 @@ QWindowsForeignWindow::QWindowsForeignWindow(QWindow *window, HWND hwnd)
     , m_hwnd(hwnd)
     , m_topLevelStyle(0)
 {
+    if (QPlatformWindow::parent())
+        setParent(QPlatformWindow::parent());
 }
 
 void QWindowsForeignWindow::setParent(const QPlatformWindow *newParentWindow)

src/plugins/platforms/xcb/qxcbwindow.cpp

@@ -522,6 +522,10 @@ QXcbForeignWindow::QXcbForeignWindow(QWindow *window, WId nativeHandle)
         QRect nativeGeometry(geometry->x, geometry->y, geometry->width, geometry->height);
         QPlatformWindow::setGeometry(nativeGeometry);
     }
+
+    // And reparent, if we have a parent already
+    if (QPlatformWindow::parent())
+        setParent(QPlatformWindow::parent());
 }
 
 QXcbForeignWindow::~QXcbForeignWindow()

tests/auto/gui/kernel/qwindow/tst_foreignwindow.cpp

@@ -26,6 +26,9 @@ private slots:
 
     void embedForeignWindow();
     void embedInForeignWindow();
+
+    void destroyExplicitly();
+    void destroyWhenParentIsDestroyed();
 };
 
 void tst_ForeignWindow::fromWinId()
@@ -138,5 +141,50 @@ void tst_ForeignWindow::embedInForeignWindow()
     }
 }
 
+void tst_ForeignWindow::destroyExplicitly()
+{
+    NativeWindow nativeWindow;
+    QVERIFY(nativeWindow);
+
+    std::unique_ptr<QWindow> foreignWindow(QWindow::fromWinId(nativeWindow));
+    QVERIFY(foreignWindow->handle());
+
+    // Explicitly destroying a foreign window is a no-op, as
+    // the documentation claims that it "releases the native
+    // platform resources associated with this window.", which
+    // is not technically true for foreign windows.
+    auto *windowHandleBeforeDestroy = foreignWindow->handle();
+    foreignWindow->destroy();
+    QCOMPARE(foreignWindow->handle(), windowHandleBeforeDestroy);
+}
+
+void tst_ForeignWindow::destroyWhenParentIsDestroyed()
+{
+    QWindow parentWindow;
+
+    NativeWindow nativeWindow;
+    QVERIFY(nativeWindow);
+
+    std::unique_ptr<QWindow> foreignWindow(QWindow::fromWinId(nativeWindow));
+    foreignWindow->setParent(&parentWindow);
+    QTRY_COMPARE(nativeWindow.parentWinId(), parentWindow.winId());
+
+    // Reparenting into a window will result in creating it
+    QVERIFY(parentWindow.handle());
+
+    // Destroying the parent window of the foreign window results
+    // in destroying the foreign window as well, as the foreign
+    // window no longer has a parent it can be embedded in.
+    QVERIFY(foreignWindow->handle());
+    parentWindow.destroy();
+    QVERIFY(!foreignWindow->handle());
+
+    // But the foreign window can be recreated again, and will
+    // continue to be a native child of the parent window.
+    foreignWindow->create();
+    QVERIFY(foreignWindow->handle());
+    QTRY_COMPARE(nativeWindow.parentWinId(), parentWindow.winId());
+}
+
 #include <tst_foreignwindow.moc>
 QTEST_MAIN(tst_ForeignWindow)

tests/shared/nativewindow.h

@@ -238,7 +238,7 @@ WId NativeWindow::parentWinId() const
     xcb_query_tree_reply_t *tree = xcb_query_tree_reply(
         connection, xcb_query_tree(connection, m_handle), nullptr);
     const auto cleanup = qScopeGuard([&]{ free(tree); });
-    return tree->parent;
+    return tree ? tree->parent : 0;
 }
 
 bool NativeWindow::isParentOf(WId childWinId)