From 174b7a9a30dfb3e14c3fc671654ca2cea7ed501c Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 10 Oct 2016 18:01:43 +0200 Subject: [PATCH 1/5] Clients tests: Don't send leave events for destroyed surfaces Change-Id: Ia7dd13f629439b116f494ff8b7432020a65ea1df Reviewed-by: Paul Olav Tvete --- tests/auto/wayland/client/mockcompositor.cpp | 6 ++---- tests/auto/wayland/client/mockinput.cpp | 16 ++++++++++++++++ tests/auto/wayland/client/mockinput.h | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/auto/wayland/client/mockcompositor.cpp b/tests/auto/wayland/client/mockcompositor.cpp index 45a35ea7d96..2c5f2541fde 100644 --- a/tests/auto/wayland/client/mockcompositor.cpp +++ b/tests/auto/wayland/client/mockcompositor.cpp @@ -362,10 +362,8 @@ void Compositor::addSurface(Surface *surface) void Compositor::removeSurface(Surface *surface) { m_surfaces.removeOne(surface); - if (m_keyboard->focus() == surface) - m_keyboard->setFocus(0); - if (m_pointer->focus() == surface) - m_pointer->setFocus(0, QPoint()); + m_keyboard->handleSurfaceDestroyed(surface); + m_pointer->handleSurfaceDestroyed(surface); } } diff --git a/tests/auto/wayland/client/mockinput.cpp b/tests/auto/wayland/client/mockinput.cpp index fe06bf79bae..99acdd43a75 100644 --- a/tests/auto/wayland/client/mockinput.cpp +++ b/tests/auto/wayland/client/mockinput.cpp @@ -267,6 +267,14 @@ void Keyboard::setFocus(Surface *surface) m_focus = surface; } +void Keyboard::handleSurfaceDestroyed(Surface *surface) +{ + if (surface == m_focus) { + m_focusResource = nullptr; + m_focus = nullptr; + } +} + void Keyboard::sendKey(uint32_t key, uint32_t state) { if (m_focusResource) { @@ -314,6 +322,14 @@ void Pointer::setFocus(Surface *surface, const QPoint &pos) m_focus = surface; } +void Pointer::handleSurfaceDestroyed(Surface *surface) +{ + if (m_focus == surface) { + m_focus = nullptr; + m_focusResource = nullptr; + } +} + void Pointer::sendMotion(const QPoint &pos) { if (m_focusResource) diff --git a/tests/auto/wayland/client/mockinput.h b/tests/auto/wayland/client/mockinput.h index 031be2a4ad0..7e88ffb0fac 100644 --- a/tests/auto/wayland/client/mockinput.h +++ b/tests/auto/wayland/client/mockinput.h @@ -75,6 +75,7 @@ public: Surface *focus() const { return m_focus; } void setFocus(Surface *surface); + void handleSurfaceDestroyed(Surface *surface); void sendKey(uint32_t key, uint32_t state); @@ -97,6 +98,7 @@ public: Surface *focus() const { return m_focus; } void setFocus(Surface *surface, const QPoint &pos); + void handleSurfaceDestroyed(Surface *surface); void sendMotion(const QPoint &pos); void sendButton(uint32_t button, uint32_t state); From a33cc547055eb12c5efa82a6612cbf8793988b72 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 10 Oct 2016 17:52:15 +0200 Subject: [PATCH 2/5] Client: Remove windows from keyboard focus list when destroyed This fixes the undefined behavior in tst_WaylandClient::touchDrag and mouseDrag Note: The test still fails if run twice in a row, but it appears to be deterministic. Task-number: QTBUG-56187 Change-Id: Ib45d82224f004d1324f2ce4d6b7df05ee36c04f5 Reviewed-by: Paul Olav Tvete --- src/plugins/platforms/wayland/qwaylanddisplay.cpp | 6 ++++++ src/plugins/platforms/wayland/qwaylanddisplay_p.h | 3 ++- src/plugins/platforms/wayland/qwaylandwindow.cpp | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index d94afcab5c9..3fd2aa56515 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -422,6 +422,12 @@ void QWaylandDisplay::handleKeyboardFocusChanged(QWaylandInputDevice *inputDevic mLastKeyboardFocus = keyboardFocus; } +void QWaylandDisplay::handleWindowDestroyed(QWaylandWindow *window) +{ + if (mActiveWindows.contains(window)) + handleWindowDeactivated(window); +} + void QWaylandDisplay::handleWaylandSync() { // This callback is used to set the window activation because we may get an activate/deactivate diff --git a/src/plugins/platforms/wayland/qwaylanddisplay_p.h b/src/plugins/platforms/wayland/qwaylanddisplay_p.h index f4fb3fa56a8..2864b357d80 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay_p.h +++ b/src/plugins/platforms/wayland/qwaylanddisplay_p.h @@ -176,6 +176,7 @@ public: void handleWindowActivated(QWaylandWindow *window); void handleWindowDeactivated(QWaylandWindow *window); void handleKeyboardFocusChanged(QWaylandInputDevice *inputDevice); + void handleWindowDestroyed(QWaylandWindow *window); public slots: void blockingReadEvents(); @@ -217,7 +218,7 @@ private: uint32_t mLastInputSerial; QWaylandInputDevice *mLastInputDevice; QPointer mLastInputWindow; - QWaylandWindow *mLastKeyboardFocus; + QPointer mLastKeyboardFocus; QVector mActiveWindows; struct wl_callback *mSyncCallback; static const wl_callback_listener syncCallbackListener; diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 59c446bb300..2b7d81f8897 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -102,6 +102,8 @@ QWaylandWindow::QWaylandWindow(QWindow *window) QWaylandWindow::~QWaylandWindow() { + mDisplay->handleWindowDestroyed(this); + delete mWindowDecoration; if (isInitialized()) From 38503100519ca8d9716376f1d0fd9379ce26c27b Mon Sep 17 00:00:00 2001 From: Paul Olav Tvete Date: Tue, 11 Oct 2016 11:35:14 +0200 Subject: [PATCH 3/5] Don't try to deliver event to destroyed window Change-Id: If1c57250e2dc9e0d55767bbdfb15c3e3f5d9b333 Reviewed-by: Johan Helsing --- src/plugins/platforms/wayland/qwaylanddatadevice.cpp | 3 ++- src/plugins/platforms/wayland/qwaylanddatadevice_p.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddatadevice.cpp b/src/plugins/platforms/wayland/qwaylanddatadevice.cpp index 0ff1f19794a..d04c20bf4d6 100644 --- a/src/plugins/platforms/wayland/qwaylanddatadevice.cpp +++ b/src/plugins/platforms/wayland/qwaylanddatadevice.cpp @@ -183,7 +183,8 @@ void QWaylandDataDevice::data_device_enter(uint32_t serial, wl_surface *surface, void QWaylandDataDevice::data_device_leave() { - QWindowSystemInterface::handleDrag(m_dragWindow, 0, QPoint(), Qt::IgnoreAction); + if (m_dragWindow) + QWindowSystemInterface::handleDrag(m_dragWindow, 0, QPoint(), Qt::IgnoreAction); QDrag *drag = static_cast(QGuiApplicationPrivate::platformIntegration()->drag())->currentDrag(); if (!drag) { diff --git a/src/plugins/platforms/wayland/qwaylanddatadevice_p.h b/src/plugins/platforms/wayland/qwaylanddatadevice_p.h index 579cb3c7c7c..7daa9f0d3c8 100644 --- a/src/plugins/platforms/wayland/qwaylanddatadevice_p.h +++ b/src/plugins/platforms/wayland/qwaylanddatadevice_p.h @@ -53,6 +53,7 @@ // #include +#include #include #include @@ -106,7 +107,7 @@ private: QWaylandDisplay *m_display; QWaylandInputDevice *m_inputDevice; uint32_t m_enterSerial; - QWindow *m_dragWindow; + QPointer m_dragWindow; QPoint m_dragPoint; QScopedPointer m_dragOffer; QScopedPointer m_selectionOffer; From ac24d8341edc672b29623436a45232d8e4bd904e Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Thu, 29 Sep 2016 15:59:36 +0200 Subject: [PATCH 4/5] use modularized platformsupport modules Change-Id: I7883470e22deb089240d86df7dc2d625a107a53e Reviewed-by: Lars Knoll --- src/plugins/platforms/wayland/client.pro | 9 +++++++-- src/plugins/platforms/wayland/qwaylandintegration.cpp | 6 +++--- .../wayland/qwaylandwindowmanagerintegration_p.h | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/plugins/platforms/wayland/client.pro b/src/plugins/platforms/wayland/client.pro index fa20367451c..34955dfbbe2 100644 --- a/src/plugins/platforms/wayland/client.pro +++ b/src/plugins/platforms/wayland/client.pro @@ -2,13 +2,18 @@ TARGET = QtWaylandClient MODULE = waylandclient QT += core-private gui-private -QT_FOR_PRIVATE += platformsupport-private +QT_FOR_PRIVATE += service_support-private +QT_PRIVATE += fontdatabase_support-private eventdispatcher_support-private theme_support-private # We have a bunch of C code with casts, so we can't have this option QMAKE_CXXFLAGS_WARN_ON -= -Wcast-qual +# Prevent gold linker from crashing. +# This started happening when QtPlatformSupport was modularized. +use_gold_linker: CONFIG += no_linker_version_script + CONFIG -= precompile_header -CONFIG += link_pkgconfig qpa/genericunixfontdatabase wayland-scanner +CONFIG += link_pkgconfig wayland-scanner contains(QT_CONFIG, opengl) { DEFINES += QT_WAYLAND_GL_SUPPORT diff --git a/src/plugins/platforms/wayland/qwaylandintegration.cpp b/src/plugins/platforms/wayland/qwaylandintegration.cpp index 0e9eb44d65a..48517fce191 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration.cpp +++ b/src/plugins/platforms/wayland/qwaylandintegration.cpp @@ -50,9 +50,9 @@ #include "qwaylandwindowmanagerintegration_p.h" #include "qwaylandscreen_p.h" -#include "QtPlatformSupport/private/qgenericunixfontdatabase_p.h" -#include -#include +#include +#include +#include #include diff --git a/src/plugins/platforms/wayland/qwaylandwindowmanagerintegration_p.h b/src/plugins/platforms/wayland/qwaylandwindowmanagerintegration_p.h index 0e5f67ac10e..09a79d48dc8 100644 --- a/src/plugins/platforms/wayland/qwaylandwindowmanagerintegration_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindowmanagerintegration_p.h @@ -55,7 +55,7 @@ #include #include -#include +#include #include #include From fec8cd87e0555e763b8ef405a34afbcfaa9dd9f2 Mon Sep 17 00:00:00 2001 From: Giulio Camuffo Date: Mon, 10 Oct 2016 16:26:24 +0200 Subject: [PATCH 5/5] Fix memory leaks Several fields in QWaylandIntegration were never deleted, so use QScopedPointer to handle that. Also use QScopedPointer for all the heap allocated fields of the class. Change-Id: I4c33be4157a6e17abfa1610f84ef9a88afe5f38a Reviewed-by: Pier Luigi Fiorini Reviewed-by: Johan Helsing --- .../platforms/wayland/qwaylandintegration.cpp | 65 ++++++++----------- .../platforms/wayland/qwaylandintegration_p.h | 22 ++++--- 2 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandintegration.cpp b/src/plugins/platforms/wayland/qwaylandintegration.cpp index 48517fce191..3e263061019 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration.cpp +++ b/src/plugins/platforms/wayland/qwaylandintegration.cpp @@ -119,7 +119,6 @@ public: QWaylandIntegration::QWaylandIntegration() : mClientBufferIntegration(0) - , mShellIntegration(Q_NULLPTR) , mInputDeviceIntegration(Q_NULLPTR) , mFontDb(new QGenericUnixFontDatabase()) , mNativeInterface(new QWaylandNativeInterface(this)) @@ -131,16 +130,16 @@ QWaylandIntegration::QWaylandIntegration() , mShellIntegrationInitialized(false) { initializeInputDeviceIntegration(); - mDisplay = new QWaylandDisplay(this); - mClipboard = new QWaylandClipboard(mDisplay); - mDrag = new QWaylandDrag(mDisplay); + mDisplay.reset(new QWaylandDisplay(this)); + mClipboard.reset(new QWaylandClipboard(mDisplay.data())); + mDrag.reset(new QWaylandDrag(mDisplay.data())); QString icStr = QPlatformInputContextFactory::requested(); if (!icStr.isNull()) { mInputContext.reset(QPlatformInputContextFactory::create(icStr)); } else { //try to use the input context using the wl_text_input interface - QPlatformInputContext *ctx = new QWaylandInputContext(mDisplay); + QPlatformInputContext *ctx = new QWaylandInputContext(mDisplay.data()); mInputContext.reset(ctx); //use the traditional way for on screen keyboards for now @@ -153,18 +152,11 @@ QWaylandIntegration::QWaylandIntegration() QWaylandIntegration::~QWaylandIntegration() { - delete mDrag; - delete mClipboard; -#ifndef QT_NO_ACCESSIBILITY - delete mAccessibility; -#endif - delete mNativeInterface; - delete mDisplay; } QPlatformNativeInterface * QWaylandIntegration::nativeInterface() const { - return mNativeInterface; + return mNativeInterface.data(); } bool QWaylandIntegration::hasCapability(QPlatformIntegration::Capability cap) const @@ -217,27 +209,27 @@ QAbstractEventDispatcher *QWaylandIntegration::createEventDispatcher() const void QWaylandIntegration::initialize() { QAbstractEventDispatcher *dispatcher = QGuiApplicationPrivate::eventDispatcher; - QObject::connect(dispatcher, SIGNAL(aboutToBlock()), mDisplay, SLOT(flushRequests())); - QObject::connect(dispatcher, SIGNAL(awake()), mDisplay, SLOT(flushRequests())); + QObject::connect(dispatcher, SIGNAL(aboutToBlock()), mDisplay.data(), SLOT(flushRequests())); + QObject::connect(dispatcher, SIGNAL(awake()), mDisplay.data(), SLOT(flushRequests())); int fd = wl_display_get_fd(mDisplay->wl_display()); - QSocketNotifier *sn = new QSocketNotifier(fd, QSocketNotifier::Read, mDisplay); - QObject::connect(sn, SIGNAL(activated(int)), mDisplay, SLOT(flushRequests())); + QSocketNotifier *sn = new QSocketNotifier(fd, QSocketNotifier::Read, mDisplay.data()); + QObject::connect(sn, SIGNAL(activated(int)), mDisplay.data(), SLOT(flushRequests())); } QPlatformFontDatabase *QWaylandIntegration::fontDatabase() const { - return mFontDb; + return mFontDb.data(); } QPlatformClipboard *QWaylandIntegration::clipboard() const { - return mClipboard; + return mClipboard.data(); } QPlatformDrag *QWaylandIntegration::drag() const { - return mDrag; + return mDrag.data(); } QPlatformInputContext *QWaylandIntegration::inputContext() const @@ -263,7 +255,7 @@ QVariant QWaylandIntegration::styleHint(StyleHint hint) const #ifndef QT_NO_ACCESSIBILITY QPlatformAccessibility *QWaylandIntegration::accessibility() const { - return mAccessibility; + return mAccessibility.data(); } #endif @@ -274,7 +266,7 @@ QPlatformServices *QWaylandIntegration::services() const QWaylandDisplay *QWaylandIntegration::display() const { - return mDisplay; + return mDisplay.data(); } QStringList QWaylandIntegration::themeNames() const @@ -292,7 +284,7 @@ QWaylandClientBufferIntegration *QWaylandIntegration::clientBufferIntegration() if (!mClientBufferIntegrationInitialized) const_cast(this)->initializeClientBufferIntegration(); - return mClientBufferIntegration && mClientBufferIntegration->isValid() ? mClientBufferIntegration : 0; + return mClientBufferIntegration && mClientBufferIntegration->isValid() ? mClientBufferIntegration.data() : nullptr; } QWaylandServerBufferIntegration *QWaylandIntegration::serverBufferIntegration() const @@ -300,7 +292,7 @@ QWaylandServerBufferIntegration *QWaylandIntegration::serverBufferIntegration() if (!mServerBufferIntegrationInitialized) const_cast(this)->initializeServerBufferIntegration(); - return mServerBufferIntegration; + return mServerBufferIntegration.data(); } QWaylandShellIntegration *QWaylandIntegration::shellIntegration() const @@ -308,7 +300,7 @@ QWaylandShellIntegration *QWaylandIntegration::shellIntegration() const if (!mShellIntegrationInitialized) const_cast(this)->initializeShellIntegration(); - return mShellIntegration; + return mShellIntegration.data(); } void QWaylandIntegration::initializeClientBufferIntegration() @@ -334,10 +326,10 @@ void QWaylandIntegration::initializeClientBufferIntegration() QStringList keys = QWaylandClientBufferIntegrationFactory::keys(); if (keys.contains(targetKey)) { - mClientBufferIntegration = QWaylandClientBufferIntegrationFactory::create(targetKey, QStringList()); + mClientBufferIntegration.reset(QWaylandClientBufferIntegrationFactory::create(targetKey, QStringList())); } if (mClientBufferIntegration) - mClientBufferIntegration->initialize(mDisplay); + mClientBufferIntegration->initialize(mDisplay.data()); else qWarning("Failed to load client buffer integration: %s\n", qPrintable(targetKey)); } @@ -364,10 +356,10 @@ void QWaylandIntegration::initializeServerBufferIntegration() QStringList keys = QWaylandServerBufferIntegrationFactory::keys(); if (keys.contains(targetKey)) { - mServerBufferIntegration = QWaylandServerBufferIntegrationFactory::create(targetKey, QStringList()); + mServerBufferIntegration.reset(QWaylandServerBufferIntegrationFactory::create(targetKey, QStringList())); } if (mServerBufferIntegration) - mServerBufferIntegration->initialize(mDisplay); + mServerBufferIntegration->initialize(mDisplay.data()); else qWarning("Failed to load server buffer integration %s\n", qPrintable(targetKey)); } @@ -383,7 +375,7 @@ void QWaylandIntegration::initializeShellIntegration() QStringList keys = QWaylandShellIntegrationFactory::keys(); if (keys.contains(targetKey)) { qDebug("Using the '%s' shell integration", qPrintable(targetKey)); - mShellIntegration = QWaylandShellIntegrationFactory::create(targetKey, QStringList()); + mShellIntegration.reset(QWaylandShellIntegrationFactory::create(targetKey, QStringList())); } } else { QStringList preferredShells; @@ -393,15 +385,14 @@ void QWaylandIntegration::initializeShellIntegration() Q_FOREACH (QString preferredShell, preferredShells) { if (mDisplay->hasRegistryGlobal(preferredShell)) { - mShellIntegration = createShellIntegration(preferredShell); + mShellIntegration.reset(createShellIntegration(preferredShell)); break; } } } - if (!mShellIntegration || !mShellIntegration->initialize(mDisplay)) { - delete mShellIntegration; - mShellIntegration = Q_NULLPTR; + if (!mShellIntegration || !mShellIntegration->initialize(mDisplay.data())) { + mShellIntegration.reset(); qWarning("Failed to load shell integration %s", qPrintable(targetKey)); } } @@ -425,7 +416,7 @@ void QWaylandIntegration::initializeInputDeviceIntegration() QStringList keys = QWaylandInputDeviceIntegrationFactory::keys(); if (keys.contains(targetKey)) { - mInputDeviceIntegration = QWaylandInputDeviceIntegrationFactory::create(targetKey, QStringList()); + mInputDeviceIntegration.reset(QWaylandInputDeviceIntegrationFactory::create(targetKey, QStringList())); qDebug("Using the '%s' input device integration", qPrintable(targetKey)); } else { qWarning("Wayland inputdevice integration '%s' not found, using default", qPrintable(targetKey)); @@ -435,9 +426,9 @@ void QWaylandIntegration::initializeInputDeviceIntegration() QWaylandShellIntegration *QWaylandIntegration::createShellIntegration(const QString &interfaceName) { if (interfaceName == QLatin1Literal("wl_shell")) { - return new QWaylandWlShellIntegration(mDisplay); + return new QWaylandWlShellIntegration(mDisplay.data()); } else if (interfaceName == QLatin1Literal("xdg_shell")) { - return new QWaylandXdgShellIntegration(mDisplay); + return new QWaylandXdgShellIntegration(mDisplay.data()); } else { return Q_NULLPTR; } diff --git a/src/plugins/platforms/wayland/qwaylandintegration_p.h b/src/plugins/platforms/wayland/qwaylandintegration_p.h index 9a499022981..bd66f55a775 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration_p.h +++ b/src/plugins/platforms/wayland/qwaylandintegration_p.h @@ -54,6 +54,8 @@ #include #include +#include + QT_BEGIN_NAMESPACE namespace QtWaylandClient { @@ -113,10 +115,10 @@ public: virtual QWaylandShellIntegration *shellIntegration() const; protected: - QWaylandClientBufferIntegration *mClientBufferIntegration; - QWaylandServerBufferIntegration *mServerBufferIntegration; - QWaylandShellIntegration *mShellIntegration; - QWaylandInputDeviceIntegration *mInputDeviceIntegration; + QScopedPointer mClientBufferIntegration; + QScopedPointer mServerBufferIntegration; + QScopedPointer mShellIntegration; + QScopedPointer mInputDeviceIntegration; private: void initializeClientBufferIntegration(); @@ -125,14 +127,14 @@ private: void initializeInputDeviceIntegration(); QWaylandShellIntegration *createShellIntegration(const QString& interfaceName); - QPlatformFontDatabase *mFontDb; - QPlatformClipboard *mClipboard; - QPlatformDrag *mDrag; - QWaylandDisplay *mDisplay; - QPlatformNativeInterface *mNativeInterface; + QScopedPointer mFontDb; + QScopedPointer mClipboard; + QScopedPointer mDrag; + QScopedPointer mDisplay; + QScopedPointer mNativeInterface; QScopedPointer mInputContext; #ifndef QT_NO_ACCESSIBILITY - QPlatformAccessibility *mAccessibility; + QScopedPointer mAccessibility; #endif bool mClientBufferIntegrationInitialized; bool mServerBufferIntegrationInitialized;