QWindow: Remove "_q_foreignWinId" dynamic property

The platform plugins reading this out of the QWindow was a layering
violation, and propagates the notion that a window can shape shift
into representing a new native handle, while none of the platform
plugins support this.

A foreign QWindow is created via the factory function fromWinId(),
at which point we can pass the WId all the way to the platform
plugin as function arguments, where the platform will create a
corresponding platform-window.

The platform window can then answer the question of whether or
not it's representing a foreign window, which determines a few
behavioral changes here and there, as well as supplying the
native window handle back for QWindow::winId();

[ChangeLog][QtGui][QWindow] The "_q_foreignWinId" dynamic property
is no longer set nor read.

[ChangeLog][QtGui][QPA] The function createForeignWindow() has been
added to QPlatormIntegration and is now responsible for creating
foreign windows. The function isForeignWindow() in QPlatformWindow
has been added, and platforms should implement this to return true
for windows created by createForeignWindow().

Task-number: QTBUG-58383
Change-Id: If84142f95172f62b9377eb5d2a4d792cad36010b
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@qt.io>

18 files changed, 106 insertions, 64 deletions

diff --git a/src/gui/kernel/qplatformintegration.h b/src/gui/kernel/qplatformintegration.h
index 466b3348bd..0449e0b4c8 100644
--- a/src/gui/kernel/qplatformintegration.h
+++ b/src/gui/kernel/qplatformintegration.h
@@ -108,6 +108,7 @@ public:
 
     virtual QPlatformPixmap *createPlatformPixmap(QPlatformPixmap::PixelType type) const;
     virtual QPlatformWindow *createPlatformWindow(QWindow *window) const = 0;
+    virtual QPlatformWindow *createForeignWindow(QWindow *, WId) const { return 0; }
     virtual QPlatformBackingStore *createPlatformBackingStore(QWindow *window) const = 0;
 #ifndef QT_NO_OPENGL
     virtual QPlatformOpenGLContext *createPlatformOpenGLContext(QOpenGLContext *context) const;
diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp
index 9d41a72072..8e98958949 100644
--- a/src/gui/kernel/qwindow.cpp
+++ b/src/gui/kernel/qwindow.cpp
@@ -424,7 +424,7 @@ void QWindowPrivate::setTopLevelScreen(QScreen *newScreen, bool recreate)
     }
 }
 
-void QWindowPrivate::create(bool recursive)
+void QWindowPrivate::create(bool recursive, WId nativeHandle)
 {
     Q_Q(QWindow);
     if (platformWindow)
@@ -433,8 +433,10 @@ void QWindowPrivate::create(bool recursive)
     if (q->parent())
         q->parent()->create();
 
-    platformWindow = QGuiApplicationPrivate::platformIntegration()->createPlatformWindow(q);
-    Q_ASSERT(platformWindow || q->type() == Qt::ForeignWindow);
+    QPlatformIntegration *platformIntegration = QGuiApplicationPrivate::platformIntegration();
+    platformWindow = nativeHandle ? platformIntegration->createForeignWindow(q, nativeHandle)
+        : platformIntegration->createPlatformWindow(q);
+    Q_ASSERT(platformWindow);
 
     if (!platformWindow) {
         qWarning() << "Failed to create platform window for" << q << "with flags" << q->flags();
@@ -629,9 +631,6 @@ WId QWindow::winId() const
 {
     Q_D(const QWindow);
 
-    if (type() == Qt::ForeignWindow)
-        return WId(property("_q_foreignWinId").value<WId>());
-
     if(!d->platformWindow)
         const_cast<QWindow *>(this)->create();
 
@@ -865,7 +864,12 @@ void QWindow::setFlags(Qt::WindowFlags flags)
 Qt::WindowFlags QWindow::flags() const
 {
     Q_D(const QWindow);
-    return d->windowFlags;
+    Qt::WindowFlags flags = d->windowFlags;
+
+    if (d->platformWindow && d->platformWindow->isForeignWindow())
+        flags |= Qt::ForeignWindow;
+
+    return flags;
 }
 
 /*!
@@ -2584,13 +2588,13 @@ QWindow *QWindow::fromWinId(WId id)
     }
 
     QWindow *window = new QWindow;
-    window->setFlags(Qt::ForeignWindow);
-    window->setProperty("_q_foreignWinId", QVariant::fromValue(id));
-    window->create();
+    qt_window_private(window)->create(false, id);
+
     if (!window->handle()) {
         delete window;
         return nullptr;
     }
+
     return window;
 }
 
diff --git a/src/gui/kernel/qwindow_p.h b/src/gui/kernel/qwindow_p.h
index 272401453a..59016e4551 100644
--- a/src/gui/kernel/qwindow_p.h
+++ b/src/gui/kernel/qwindow_p.h
@@ -136,7 +136,7 @@ public:
     void updateSiblingPosition(SiblingPosition);
 
     bool windowRecreationRequired(QScreen *newScreen) const;
-    void create(bool recursive);
+    void create(bool recursive, WId nativeHandle = 0);
     void destroy();
     void setTopLevelScreen(QScreen *newScreen, bool recreate);
     void connectToScreen(QScreen *topLevelScreen);
diff --git a/src/plugins/platforms/android/qandroidplatformforeignwindow.cpp b/src/plugins/platforms/android/qandroidplatformforeignwindow.cpp
index 8c1f0ea8d2..1c920c0af9 100644
--- a/src/plugins/platforms/android/qandroidplatformforeignwindow.cpp
+++ b/src/plugins/platforms/android/qandroidplatformforeignwindow.cpp
@@ -45,12 +45,11 @@
 
 QT_BEGIN_NAMESPACE
 
-QAndroidPlatformForeignWindow::QAndroidPlatformForeignWindow(QWindow *window)
+QAndroidPlatformForeignWindow::QAndroidPlatformForeignWindow(QWindow *window, WId nativeHandle)
     : QAndroidPlatformWindow(window),
       m_surfaceId(-1)
 {
-    const WId wId = window->property("_q_foreignWinId").value<WId>();
-    m_view = reinterpret_cast<jobject>(wId);
+    m_view = reinterpret_cast<jobject>(nativeHandle);
     if (m_view.isValid())
         QtAndroid::setViewVisibility(m_view.object(), false);
 }
diff --git a/src/plugins/platforms/android/qandroidplatformforeignwindow.h b/src/plugins/platforms/android/qandroidplatformforeignwindow.h
index d42c36dcee..af1eee5499 100644
--- a/src/plugins/platforms/android/qandroidplatformforeignwindow.h
+++ b/src/plugins/platforms/android/qandroidplatformforeignwindow.h
@@ -49,7 +49,7 @@ QT_BEGIN_NAMESPACE
 class QAndroidPlatformForeignWindow : public QAndroidPlatformWindow
 {
 public:
-    explicit QAndroidPlatformForeignWindow(QWindow *window);
+    explicit QAndroidPlatformForeignWindow(QWindow *window, WId nativeHandle);
     ~QAndroidPlatformForeignWindow();
     void lower() override;
     void raise() override;
@@ -57,6 +57,7 @@ public:
     void setVisible(bool visible) override;
     void applicationStateChanged(Qt::ApplicationState state) override;
     void setParent(const QPlatformWindow *window) override;
+    bool isForeignWindow() const override { return true; }
 
 private:
     int m_surfaceId;
diff --git a/src/plugins/platforms/android/qandroidplatformintegration.cpp b/src/plugins/platforms/android/qandroidplatformintegration.cpp
index 173431208f..73aa9d0e8a 100644
--- a/src/plugins/platforms/android/qandroidplatformintegration.cpp
+++ b/src/plugins/platforms/android/qandroidplatformintegration.cpp
@@ -284,10 +284,13 @@ QPlatformWindow *QAndroidPlatformIntegration::createPlatformWindow(QWindow *wind
 {
     if (!QtAndroid::activity())
         return nullptr;
-    if (window->type() == Qt::ForeignWindow)
-        return new QAndroidPlatformForeignWindow(window);
-    else
-        return new QAndroidPlatformOpenGLWindow(window, m_eglDisplay);
+
+    return new QAndroidPlatformOpenGLWindow(window, m_eglDisplay);
+}
+
+QPlatformWindow *QAndroidPlatformIntegration::createForeignWindow(QWindow *window, WId nativeHandle) const
+{
+    return new QAndroidPlatformForeignWindow(window, nativeHandle);
 }
 
 QAbstractEventDispatcher *QAndroidPlatformIntegration::createEventDispatcher() const
diff --git a/src/plugins/platforms/android/qandroidplatformintegration.h b/src/plugins/platforms/android/qandroidplatformintegration.h
index 2337801250..be10c3d161 100644
--- a/src/plugins/platforms/android/qandroidplatformintegration.h
+++ b/src/plugins/platforms/android/qandroidplatformintegration.h
@@ -78,6 +78,7 @@ public:
     bool hasCapability(QPlatformIntegration::Capability cap) const override;
 
     QPlatformWindow *createPlatformWindow(QWindow *window) const override;
+    QPlatformWindow *createForeignWindow(QWindow *window, WId nativeHandle) const override;
     QPlatformBackingStore *createPlatformBackingStore(QWindow *window) const override;
     QPlatformOpenGLContext *createPlatformOpenGLContext(QOpenGLContext *context) const override;
     QAbstractEventDispatcher *createEventDispatcher() const override;
diff --git a/src/plugins/platforms/cocoa/qcocoaintegration.h b/src/plugins/platforms/cocoa/qcocoaintegration.h
index a3c375881d..ecdd20c4dc 100644
--- a/src/plugins/platforms/cocoa/qcocoaintegration.h
+++ b/src/plugins/platforms/cocoa/qcocoaintegration.h
@@ -126,6 +126,7 @@ public:
 
     bool hasCapability(QPlatformIntegration::Capability cap) const Q_DECL_OVERRIDE;
     QPlatformWindow *createPlatformWindow(QWindow *window) const Q_DECL_OVERRIDE;
+    QPlatformWindow *createForeignWindow(QWindow *window, WId nativeHandle) const Q_DECL_OVERRIDE;
 #ifndef QT_NO_OPENGL
     QPlatformOpenGLContext *createPlatformOpenGLContext(QOpenGLContext *context) const Q_DECL_OVERRIDE;
 #endif
diff --git a/src/plugins/platforms/cocoa/qcocoaintegration.mm b/src/plugins/platforms/cocoa/qcocoaintegration.mm
index 291d39ea9d..91f408e5c2 100644
--- a/src/plugins/platforms/cocoa/qcocoaintegration.mm
+++ b/src/plugins/platforms/cocoa/qcocoaintegration.mm
@@ -524,6 +524,11 @@ QPlatformWindow *QCocoaIntegration::createPlatformWindow(QWindow *window) const
     return new QCocoaWindow(window);
 }
 
+QPlatformWindow *QCocoaIntegration::createForeignWindow(QWindow *window, WId nativeHandle) const
+{
+    return new QCocoaWindow(window, nativeHandle);
+}
+
 #ifndef QT_NO_OPENGL
 QPlatformOpenGLContext *QCocoaIntegration::createPlatformOpenGLContext(QOpenGLContext *context) const
 {
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.h b/src/plugins/platforms/cocoa/qcocoawindow.h
index f60597fe42..8e2c441465 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.h
+++ b/src/plugins/platforms/cocoa/qcocoawindow.h
@@ -156,7 +156,7 @@ class QCocoaWindow : public QObject, public QPlatformWindow
 {
     Q_OBJECT
 public:
-    QCocoaWindow(QWindow *tlw);
+    QCocoaWindow(QWindow *tlw, WId nativeHandle = 0);
     ~QCocoaWindow();
 
     void setGeometry(const QRect &rect) Q_DECL_OVERRIDE;
@@ -186,6 +186,8 @@ public:
     QMargins frameMargins() const Q_DECL_OVERRIDE;
     QSurfaceFormat format() const Q_DECL_OVERRIDE;
 
+    bool isForeignWindow() const Q_DECL_OVERRIDE;
+
     void requestActivateWindow() Q_DECL_OVERRIDE;
 
     WId winId() const Q_DECL_OVERRIDE;
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 60cababba4..2dd151e324 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -433,7 +433,7 @@ Q_CONSTRUCTOR_FUNCTION(qRegisterNotificationCallbacks)
 
 const int QCocoaWindow::NoAlertRequest = -1;
 
-QCocoaWindow::QCocoaWindow(QWindow *tlw)
+QCocoaWindow::QCocoaWindow(QWindow *tlw, WId nativeHandle)
     : QPlatformWindow(tlw)
     , m_view(nil)
     , m_nsWindow(0)
@@ -470,8 +470,8 @@ QCocoaWindow::QCocoaWindow(QWindow *tlw)
 
     QMacAutoReleasePool pool;
 
-    if (tlw->type() == Qt::ForeignWindow) {
-        m_view = (NSView *)WId(tlw->property("_q_foreignWinId").value<WId>());
+    if (nativeHandle) {
+        m_view = reinterpret_cast<NSView *>(nativeHandle);
         [m_view retain];
     } else {
         m_view = [[QNSView alloc] initWithCocoaWindow:this];
@@ -563,6 +563,11 @@ void QCocoaWindow::setGeometry(const QRect &rectIn)
     setCocoaGeometry(rect);
 }
 
+bool QCocoaWindow::isForeignWindow() const
+{
+    return ![m_view isKindOfClass:[QNSView class]];
+}
+
 QRect QCocoaWindow::geometry() const
 {
     // QWindows that are embedded in a NSView hiearchy may be considered
diff --git a/src/plugins/platforms/windows/qwindowsintegration.cpp b/src/plugins/platforms/windows/qwindowsintegration.cpp
index 5a3020387a..316f93e3ac 100644
--- a/src/plugins/platforms/windows/qwindowsintegration.cpp
+++ b/src/plugins/platforms/windows/qwindowsintegration.cpp
@@ -304,24 +304,6 @@ QPlatformWindow *QWindowsIntegration::createPlatformWindow(QWindow *window) cons
         return result;
     }
 
-    if (window->type() == Qt::ForeignWindow) {
-        const HWND hwnd = reinterpret_cast<HWND>(window->winId());
-        if (!IsWindow(hwnd)) {
-           qWarning("Windows QPA: Invalid foreign window ID %p.", hwnd);
-           return nullptr;
-        }
-        QWindowsForeignWindow *result = new QWindowsForeignWindow(window, hwnd);
-        const QRect obtainedGeometry = result->geometry();
-        QScreen *screen = Q_NULLPTR;
-        if (const QPlatformScreen *pScreen = result->screenForGeometry(obtainedGeometry))
-            screen = pScreen->screen();
-        if (screen && screen != window->screen())
-            window->setScreen(screen);
-        qCDebug(lcQpaWindows) << "Foreign window:" << window << showbase << hex
-            << result->winId() << noshowbase << dec << obtainedGeometry << screen;
-        return result;
-    }
-
     QWindowsWindowData requested;
     requested.flags = window->flags();
     requested.geometry = QHighDpi::toNativePixels(window->geometry(), window);
@@ -365,6 +347,25 @@ QPlatformWindow *QWindowsIntegration::createPlatformWindow(QWindow *window) cons
     return result;
 }
 
+QPlatformWindow *QWindowsIntegration::createForeignWindow(QWindow *window, WId nativeHandle) const
+{
+    const HWND hwnd = reinterpret_cast<HWND>(nativeHandle);
+    if (!IsWindow(hwnd)) {
+       qWarning("Windows QPA: Invalid foreign window ID %p.", hwnd);
+       return nullptr;
+    }
+    QWindowsForeignWindow *result = new QWindowsForeignWindow(window, hwnd);
+    const QRect obtainedGeometry = result->geometry();
+    QScreen *screen = Q_NULLPTR;
+    if (const QPlatformScreen *pScreen = result->screenForGeometry(obtainedGeometry))
+        screen = pScreen->screen();
+    if (screen && screen != window->screen())
+        window->setScreen(screen);
+    qCDebug(lcQpaWindows) << "Foreign window:" << window << showbase << hex
+        << result->winId() << noshowbase << dec << obtainedGeometry << screen;
+    return result;
+}
+
 // Overridden to return a QWindowsDirect2DWindow in Direct2D plugin.
 QWindowsWindow *QWindowsIntegration::createPlatformWindowHelper(QWindow *window, const QWindowsWindowData &data) const
 {
diff --git a/src/plugins/platforms/windows/qwindowsintegration.h b/src/plugins/platforms/windows/qwindowsintegration.h
index 7647b0f4a6..607203829f 100644
--- a/src/plugins/platforms/windows/qwindowsintegration.h
+++ b/src/plugins/platforms/windows/qwindowsintegration.h
@@ -73,6 +73,7 @@ public:
     bool hasCapability(QPlatformIntegration::Capability cap) const override;
 
     QPlatformWindow *createPlatformWindow(QWindow *window) const override;
+    QPlatformWindow *createForeignWindow(QWindow *window, WId nativeHandle) const override;
 #ifndef QT_NO_OPENGL
     QPlatformOpenGLContext *createPlatformOpenGLContext(QOpenGLContext *context) const override;
     QOpenGLContext::OpenGLModuleType openGLModuleType() override;
diff --git a/src/plugins/platforms/windows/qwindowswindow.h b/src/plugins/platforms/windows/qwindowswindow.h
index 10cfc18dff..e541b110a6 100644
--- a/src/plugins/platforms/windows/qwindowswindow.h
+++ b/src/plugins/platforms/windows/qwindowswindow.h
@@ -171,6 +171,7 @@ public:
     void raise() override { raise_sys(); }
     void lower() override { lower_sys(); }
     void setWindowTitle(const QString &title) override { setWindowTitle_sys(title); }
+    bool isForeignWindow() const override { return true; }
 
 protected:
     HWND handle() const override { return m_hwnd; }
diff --git a/src/plugins/platforms/xcb/qxcbintegration.cpp b/src/plugins/platforms/xcb/qxcbintegration.cpp
index 47c85fce18..40858b39e0 100644
--- a/src/plugins/platforms/xcb/qxcbintegration.cpp
+++ b/src/plugins/platforms/xcb/qxcbintegration.cpp
@@ -214,6 +214,27 @@ QPlatformWindow *QXcbIntegration::createPlatformWindow(QWindow *window) const
     return xcbWindow;
 }
 
+class QXcbForeignWindow : public QXcbWindow
+{
+public:
+    QXcbForeignWindow(QWindow *window, WId nativeHandle)
+        : QXcbWindow(window) { m_window = nativeHandle; }
+    ~QXcbForeignWindow() {}
+    bool isForeignWindow() const override { return true; }
+
+protected:
+    // No-ops
+    void create() override {}
+    void destroy() override {}
+};
+
+QPlatformWindow *QXcbIntegration::createForeignWindow(QWindow *window, WId nativeHandle) const
+{
+    QXcbWindow *xcbWindow = new QXcbForeignWindow(window, nativeHandle);
+    xcbWindow->create();
+    return xcbWindow;
+}
+
 #ifndef QT_NO_OPENGL
 QPlatformOpenGLContext *QXcbIntegration::createPlatformOpenGLContext(QOpenGLContext *context) const
 {
diff --git a/src/plugins/platforms/xcb/qxcbintegration.h b/src/plugins/platforms/xcb/qxcbintegration.h
index f8034f436f..baa5c9d835 100644
--- a/src/plugins/platforms/xcb/qxcbintegration.h
+++ b/src/plugins/platforms/xcb/qxcbintegration.h
@@ -61,6 +61,7 @@ public:
     ~QXcbIntegration();
 
     QPlatformWindow *createPlatformWindow(QWindow *window) const override;
+    QPlatformWindow *createForeignWindow(QWindow *window, WId nativeHandle) const override;
 #ifndef QT_NO_OPENGL
     QPlatformOpenGLContext *createPlatformOpenGLContext(QOpenGLContext *context) const override;
 #endif
diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp
index fd23179075..f7c36ec386 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.cpp
+++ b/src/plugins/platforms/xcb/qxcbwindow.cpp
@@ -341,11 +341,6 @@ enum {
 
 void QXcbWindow::create()
 {
-    if (window()->type() == Qt::ForeignWindow) {
-        m_window = window()->winId();
-        return;
-    }
-
     destroy();
 
     m_windowState = Qt::WindowNoState;
@@ -590,9 +585,10 @@ QXcbWindow::~QXcbWindow()
     if (m_currentBitmapCursor != XCB_CURSOR_NONE) {
         xcb_free_cursor(xcb_connection(), m_currentBitmapCursor);
     }
-    if (window()->type() != Qt::ForeignWindow)
-        destroy();
-    else {
+
+    destroy();
+
+    if (isForeignWindow()) {
         if (connection()->mouseGrabber() == this)
             connection()->setMouseGrabber(Q_NULLPTR);
         if (connection()->mousePressWindow() == this)
diff --git a/src/widgets/widgets/qmaccocoaviewcontainer_mac.mm b/src/widgets/widgets/qmaccocoaviewcontainer_mac.mm
index 8e565ecfe0..610df2125b 100644
--- a/src/widgets/widgets/qmaccocoaviewcontainer_mac.mm
+++ b/src/widgets/widgets/qmaccocoaviewcontainer_mac.mm
@@ -44,6 +44,7 @@
 #include <QtGui/QWindow>
 #include <qpa/qplatformnativeinterface.h>
 #include <private/qwidget_p.h>
+#include <private/qwindow_p.h>
 
 /*!
     \class QMacCocoaViewContainer
@@ -117,7 +118,9 @@ QMacCocoaViewContainerPrivate::~QMacCocoaViewContainerPrivate()
 QMacCocoaViewContainer::QMacCocoaViewContainer(NSView *view, QWidget *parent)
    : QWidget(*new QMacCocoaViewContainerPrivate, parent, 0)
 {
+    // Ensures that we have a QWindow, even if we're not a top level widget
     setAttribute(Qt::WA_NativeWindow);
+
     setCocoaView(view);
 }
 
@@ -149,23 +152,19 @@ void QMacCocoaViewContainer::setCocoaView(NSView *view)
     [view retain];
     d->nsview = view;
 
-    QWindow *window = windowHandle();
+    // Get rid of QWindow completely, and re-create a new vanilla one, which
+    // we will then re-configure to be a foreign window.
+    destroy();
+    create();
 
-    // Note that we only set the flag on the QWindow, and not the QWidget.
-    // These two are not in sync, so from a QWidget standpoint the widget
-    // is not a Window, and hence will be shown when the parent widget is
-    // shown, like all QWidget children.
-    window->setFlags(Qt::ForeignWindow);
-    window->setProperty("_q_foreignWinId", view ? WId(view) : QVariant());
-
-    // Destroying the platform window implies hiding the window, and we
-    // also lose the geometry information that the platform window kept,
-    // and fall back to the stale QWindow geometry, so we update the two
-    // based on the widget visibility and geometry, which is up to date.
+    // Can't use QWindow::fromWinId() here due to QWidget controlling its
+    // QWindow, and can't use QWidget::createWindowContainer() due to the
+    // QMacCocoaViewContainer class being public API instead of a factory
+    // function.
+    QWindow *window = windowHandle();
     window->destroy();
-    window->setVisible(isVisible());
-    window->setGeometry(geometry());
-    window->create();
+    qt_window_private(window)->create(false, WId(view));
+    Q_ASSERT(window->handle());
 
     [oldView release];
 }