Add a function for QPA plugins to explicitly destroy QScreens

Previously QPlatformScreen was automatically deleting its QScreen
in ~QPlatformScreen(). That means that we cannot use QScreen's
methods when the screen is being removed, because doing so would
call virtual methods of QPlatformScreen. By that point the
QPlatformScreen subclass object does not exist anymore, and we
call the default implementation instead of the subclassed one, or
get a crash for the pure virtual methods. This happens for example
when removing a screen which contains a QWindow with some QML item
using QQuickScreenAttached.

This patch adds a QPlatformIntegration::destroyScreen() function,
which deletes the QScreen and later the QPlatformScreen.

~QPlatformScreen will still delete the QScreen if it was not deleted
with destroyScreen(), so code not ported to the new approach
will continue to work as before, with only a warning added.

Task-number: QTBUG-41141
Change-Id: Ie4a03dee08ceb4c3e94a81875411f6f723273fe1
Reviewed-by: Jørgen Lind <[email protected]>
Reviewed-by: Shawn Rutledge <[email protected]>

src/gui/kernel/qplatformintegration.cpp

@@ -439,7 +439,7 @@ QList<int> QPlatformIntegration::possibleKeys(const QKeyEvent *) const
   This adds the screen to QGuiApplication::screens(), and emits the
   QGuiApplication::screenAdded() signal.
 
-  The screen is automatically removed when the QPlatformScreen is destroyed.
+  The screen should be deleted by calling QPlatformIntegration::destroyScreen().
 */
 void QPlatformIntegration::screenAdded(QPlatformScreen *ps)
 {
@@ -449,6 +449,22 @@ void QPlatformIntegration::screenAdded(QPlatformScreen *ps)
     emit qGuiApp->screenAdded(screen);
 }
 
+/*!
+  Should be called by the implementation whenever a screen is removed.
+
+  This removes the screen from QGuiApplication::screens(), and deletes it.
+
+  Failing to call this and manually deleting the QPlatformScreen instead may
+  lead to a crash due to a pure virtual call.
+*/
+void QPlatformIntegration::destroyScreen(QPlatformScreen *screen)
+{
+    QGuiApplicationPrivate::screen_list.removeOne(screen->d_func()->screen);
+    delete screen->d_func()->screen;
+    screen->d_func()->screen = Q_NULLPTR;
+    delete screen;
+}
+
 QStringList QPlatformIntegration::themeNames() const
 {
     return QStringList();

src/gui/kernel/qplatformintegration.h

@@ -171,6 +171,7 @@ class Q_GUI_EXPORT QPlatformIntegration
 
 protected:
     void screenAdded(QPlatformScreen *screen);
+    void destroyScreen(QPlatformScreen *screen);
 };
 
 QT_END_NAMESPACE

src/gui/kernel/qplatformscreen.cpp

@@ -52,9 +52,11 @@ QPlatformScreen::QPlatformScreen()
 QPlatformScreen::~QPlatformScreen()
 {
     Q_D(QPlatformScreen);
-
-    QGuiApplicationPrivate::screen_list.removeOne(d->screen);
-    delete d->screen;
+    if (d->screen) {
+        qWarning("Manually deleting a QPlatformScreen. Call QPlatformIntegration::destroyScreen instead.");
+        QGuiApplicationPrivate::screen_list.removeOne(d->screen);
+        delete d->screen;
+    }
 }
 
 /*!

src/plugins/platforms/cocoa/qcocoaintegration.mm

@@ -337,7 +337,7 @@ static void initResources()
 
     // Delete screens in reverse order to avoid crash in case of multiple screens
     while (!mScreens.isEmpty()) {
-        delete mScreens.takeLast();
+        destroyScreen(mScreens.takeLast());
     }
 
     clearToolbars();
@@ -397,7 +397,7 @@ static void initResources()
     // Now the leftovers in remainingScreens are no longer current, so we can delete them.
     foreach (QCocoaScreen* screen, remainingScreens) {
         mScreens.removeOne(screen);
-        delete screen;
+        destroyScreen(screen);
     }
     // All screens in mScreens are siblings, because we ignored the mirrors.
     foreach (QCocoaScreen* screen, mScreens)

src/plugins/platforms/ios/qiosintegration.mm

@@ -120,7 +120,7 @@
     m_inputContext = 0;
 
     foreach (QScreen *screen, QGuiApplication::screens())
-        delete screen->handle();
+        destroyScreen(screen->handle());
 
     delete m_platformServices;
     m_platformServices = 0;

src/plugins/platforms/kms/qkmsintegration.cpp

@@ -74,7 +74,7 @@ QKmsIntegration::~QKmsIntegration()
         delete device;
     }
     foreach (QPlatformScreen *screen, m_screens) {
-        delete screen;
+        destroyScreen(screen);
     }
     delete m_fontDatabase;
     delete m_vtHandler;

src/plugins/platforms/linuxfb/qlinuxfbintegration.cpp

@@ -57,7 +57,7 @@ QLinuxFbIntegration::QLinuxFbIntegration(const QStringList &paramList)
 
 QLinuxFbIntegration::~QLinuxFbIntegration()
 {
-    delete m_primaryScreen;
+    destroyScreen(m_primaryScreen);
 }
 
 void QLinuxFbIntegration::initialize()

src/plugins/platforms/minimalegl/qminimaleglintegration.cpp

@@ -60,7 +60,7 @@ QMinimalEglIntegration::QMinimalEglIntegration()
 
 QMinimalEglIntegration::~QMinimalEglIntegration()
 {
-    delete mScreen;
+    destroyScreen(mScreen);
 }
 
 bool QMinimalEglIntegration::hasCapability(QPlatformIntegration::Capability cap) const

src/plugins/platforms/openwfd/qopenwfdintegration.cpp

@@ -133,3 +133,8 @@ void QOpenWFDIntegration::addScreen(QOpenWFDScreen *screen)
 {
     screenAdded(screen);
 }
+
+void QOpenWFDIntegration::destroyScreen(QOpenWFDScreen *screen)
+{
+    QPlatformIntegration::destroyScreen(screen);
+}

src/plugins/platforms/openwfd/qopenwfdintegration.h

@@ -63,6 +63,7 @@ class QOpenWFDIntegration : public QPlatformIntegration
     QPlatformPrinterSupport *printerSupport() const;
 
     void addScreen(QOpenWFDScreen *screen);
+    void destroyScreen(QOpenWFDScreen *screen);
 private:
     QList<QPlatformScreen *> mScreens;
     QList<QOpenWFDDevice *>mDevices;

src/plugins/platforms/openwfd/qopenwfdport.cpp

@@ -140,7 +140,7 @@ void QOpenWFDPort::detach()
     mAttached = false;
     mOn = false;
 
-    delete mScreen;
+    mDevice->integration()->destroyScreen(mScreen);
 
     wfdDestroyPipeline(mDevice->handle(),mPipeline);
     mPipelineId = WFD_INVALID_PIPELINE_ID;

src/plugins/platforms/qnx/qqnxintegration.cpp

@@ -554,7 +554,7 @@ void QQnxIntegration::removeDisplay(QQnxScreen *screen)
     Q_CHECK_PTR(screen);
     Q_ASSERT(m_screens.contains(screen));
     m_screens.removeAll(screen);
-    screen->deleteLater();
+    destroyScreen(screen);
 }
 
 void QQnxIntegration::destroyDisplays()

src/plugins/platforms/windows/qwindowsintegration.h

@@ -94,6 +94,7 @@ class QWindowsIntegration : public QPlatformIntegration
     static QWindowsIntegration *instance();
 
     inline void emitScreenAdded(QPlatformScreen *s) { screenAdded(s); }
+    inline void emitDestroyScreen(QPlatformScreen *s) { destroyScreen(s); }
 
     unsigned options() const;
 

src/plugins/platforms/windows/qwindowsscreen.cpp

@@ -462,7 +462,7 @@ void QWindowsScreenManager::removeScreen(int index)
         if (movedWindowCount)
             QWindowSystemInterface::flushWindowSystemEvents();
     }
-    delete m_screens.takeAt(index);
+    QWindowsIntegration::instance()->emitDestroyScreen(m_screens.takeAt(index));
 }
 
 /*!
@@ -497,4 +497,11 @@ bool QWindowsScreenManager::handleScreenChanges()
     return true;
 }
 
+void QWindowsScreenManager::clearScreens()
+{
+    // Delete screens in reverse order to avoid crash in case of multiple screens
+    while (!m_screens.isEmpty())
+        QWindowsIntegration::instance()->emitDestroyScreen(m_screens.takeLast());
+}
+
 QT_END_NAMESPACE

src/plugins/platforms/windows/qwindowsscreen.h

@@ -127,11 +127,7 @@ class QWindowsScreenManager
 
     QWindowsScreenManager();
 
-    inline void clearScreens() {
-        // Delete screens in reverse order to avoid crash in case of multiple screens
-        while (!m_screens.isEmpty())
-            delete m_screens.takeLast();
-    }
+    void clearScreens();
 
     bool handleScreenChanges();
     bool handleDisplayChange(WPARAM wParam, LPARAM lParam);

src/plugins/platforms/xcb/qxcbconnection.cpp

@@ -240,11 +240,12 @@ void QXcbConnection::updateScreens()
         ++xcbScreenNumber;
     } // for each xcb screen
 
+    QXcbIntegration *integration = static_cast<QXcbIntegration *>(QGuiApplicationPrivate::platformIntegration());
     // Now activeScreens is the complete set of screens which are active at this time.
     // Delete any existing screens which are not in activeScreens
     for (int i = m_screens.count() - 1; i >= 0; --i) {
         if (!activeScreens.contains(m_screens[i])) {
-            delete m_screens[i];
+            integration->destroyScreen(m_screens.at(i));
             m_screens.removeAt(i);
         }
     }
@@ -261,7 +262,7 @@ void QXcbConnection::updateScreens()
     // Now that they are in the right order, emit the added signals for new screens only
     foreach (QXcbScreen* screen, m_screens)
         if (newScreens.contains(screen))
-            ((QXcbIntegration*)QGuiApplicationPrivate::platformIntegration())->screenAdded(screen);
+            integration->screenAdded(screen);
 }
 
 QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, bool canGrabServer, const char *displayName)
@@ -400,9 +401,10 @@ QXcbConnection::~QXcbConnection()
 
     delete m_reader;
 
+    QXcbIntegration *integration = static_cast<QXcbIntegration *>(QGuiApplicationPrivate::platformIntegration());
     // Delete screens in reverse order to avoid crash in case of multiple screens
     while (!m_screens.isEmpty())
-        delete m_screens.takeLast();
+        integration->destroyScreen(m_screens.takeLast());
 
 #ifdef XCB_USE_XLIB
     XCloseDisplay((Display *)m_xlib_display);