From 5e5235e0e621f782fbec4ccb3cd340ea0ce5f917 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 30 Oct 2019 08:59:49 +0100 Subject: [PATCH 1/4] Client tests: Fix flaky deadlock Sometimes the test would wait indefinitely in the compositor constructor, while also waiting in the compositor thread for m_ready to become true. m_ready is set to true in applicationInitialized(), which is supposed to be called from the client thread after QGuiApplication has been created (and also after the MockCompositor constructor has returned). I.e. the problem is that the wake in MockCompositor::run may sometimes happen before the MockCompositor::MockCompositor starts waiting. Move the wake inside the pre-initialized compositor loop. Essentially waking every 20 ms until the application is initialized. Fixes: QTBUG-66570 Change-Id: Ia5eba5d08ce4d1d3eeca99eae6cfa7d9d4fd5a0b Reviewed-by: Paul Olav Tvete --- tests/auto/wayland/shared_old/mockcompositor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auto/wayland/shared_old/mockcompositor.cpp b/tests/auto/wayland/shared_old/mockcompositor.cpp index 0dfaef5eaa6..96dc059e794 100644 --- a/tests/auto/wayland/shared_old/mockcompositor.cpp +++ b/tests/auto/wayland/shared_old/mockcompositor.cpp @@ -312,9 +312,9 @@ void *MockCompositor::run(void *data) Impl::Compositor compositor(controller); controller->m_compositor = &compositor; - controller->m_waitCondition.wakeOne(); while (!controller->m_ready) { + controller->m_waitCondition.wakeOne(); controller->dispatchCommands(); compositor.dispatchEvents(20); } From 027f48b7d949ce2de688f82c8af6ea5323615f35 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 1 Nov 2019 10:14:20 +0100 Subject: [PATCH 2/4] Add basic client test for subsurfaces Change-Id: I1ef21287933a2afccad989f47e4fe59329b6f537 Reviewed-by: Paul Olav Tvete --- tests/auto/wayland/shared/coreprotocol.h | 25 +++++++++++++++++++++- tests/auto/wayland/shared/mockcompositor.h | 1 + tests/auto/wayland/surface/tst_surface.cpp | 18 ++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/auto/wayland/shared/coreprotocol.h b/tests/auto/wayland/shared/coreprotocol.h index 5cef476c8f6..f92842e025d 100644 --- a/tests/auto/wayland/shared/coreprotocol.h +++ b/tests/auto/wayland/shared/coreprotocol.h @@ -163,6 +163,16 @@ protected: } }; +class Subsurface : public QObject, public QtWaylandServer::wl_subsurface +{ + Q_OBJECT +public: + explicit Subsurface(wl_client *client, int id, int version) + : QtWaylandServer::wl_subsurface(client, id, version) + { + } +}; + class SubCompositor : public Global, public QtWaylandServer::wl_subcompositor { Q_OBJECT @@ -170,7 +180,20 @@ public: explicit SubCompositor(CoreCompositor *compositor, int version = 1) : QtWaylandServer::wl_subcompositor(compositor->m_display, version) {} - // TODO + QVector m_subsurfaces; + +signals: + void subsurfaceCreated(Subsurface *subsurface); + +protected: + void subcompositor_get_subsurface(Resource *resource, uint32_t id, ::wl_resource *surface, ::wl_resource *parent) override + { + QTRY_VERIFY(parent); + QTRY_VERIFY(surface); + auto *subsurface = new Subsurface(resource->client(), id, resource->version()); + m_subsurfaces.append(subsurface); // TODO: clean up? + emit subsurfaceCreated(subsurface); + } }; struct OutputMode { diff --git a/tests/auto/wayland/shared/mockcompositor.h b/tests/auto/wayland/shared/mockcompositor.h index f5264ccf611..f17c934261e 100644 --- a/tests/auto/wayland/shared/mockcompositor.h +++ b/tests/auto/wayland/shared/mockcompositor.h @@ -50,6 +50,7 @@ public: // Convenience functions Output *output(int i = 0) { return getAll().value(i, nullptr); } Surface *surface(int i = 0) { return get()->m_surfaces.value(i, nullptr); } + Subsurface *subSurface(int i = 0) { return get()->m_subsurfaces.value(i, nullptr); } XdgSurface *xdgSurface(int i = 0) { return get()->m_xdgSurfaces.value(i, nullptr); } XdgToplevel *xdgToplevel(int i = 0) { return get()->toplevel(i); } XdgPopup *xdgPopup(int i = 0) { return get()->popup(i); } diff --git a/tests/auto/wayland/surface/tst_surface.cpp b/tests/auto/wayland/surface/tst_surface.cpp index dddff0866f0..451fbac75b6 100644 --- a/tests/auto/wayland/surface/tst_surface.cpp +++ b/tests/auto/wayland/surface/tst_surface.cpp @@ -41,6 +41,8 @@ private slots: void waitForFrameCallbackRaster(); void waitForFrameCallbackGl(); void negotiateShmFormat(); + + void createSubsurface(); }; void tst_surface::createDestroySurface() @@ -154,5 +156,21 @@ void tst_surface::negotiateShmFormat() }); } +void tst_surface::createSubsurface() +{ + QRasterWindow window; + window.resize(64, 64); + window.show(); + QCOMPOSITOR_TRY_VERIFY(xdgToplevel()); + exec([=] { xdgToplevel()->sendCompleteConfigure(); }); + QCOMPOSITOR_TRY_VERIFY(xdgSurface()->m_committedConfigureSerial); + + QRasterWindow subWindow; + subWindow.setParent(&window); + subWindow.resize(64, 64); + subWindow.show(); + QCOMPOSITOR_TRY_VERIFY(subSurface()); +} + QCOMPOSITOR_TEST_MAIN(tst_surface) #include "tst_surface.moc" From 8d409b0952c0d62a4001d0b448fa2bd5d55a120b Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 1 Nov 2019 11:24:26 +0100 Subject: [PATCH 3/4] Client: Fix crash when showing a child window with a hidden parent [ChangeLog][QPA plugin] Fixed a crash when showing a window with a hidden parent. Now we just avoid creating the subsurface, so nothing is shown. Seems to be the same behavior as on xcb. Fixes: QTBUG-79674 Change-Id: Ia46fcd9a0da5aad4704816a41515cb1e128ac65f Reviewed-by: Paul Olav Tvete --- src/plugins/platforms/wayland/qwaylanddisplay.cpp | 4 ++++ src/plugins/platforms/wayland/qwaylandwindow.cpp | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index 78524f6fcaf..27e38ccf7cc 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -109,6 +109,10 @@ struct ::wl_region *QWaylandDisplay::createRegion(const QRegion &qregion) return nullptr; } + // Make sure we don't pass NULL surfaces to libwayland (crashes) + Q_ASSERT(parent->object()); + Q_ASSERT(window->object()); + return mSubCompositor->get_subsurface(window->object(), parent->object()); } diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 8d34afd1f11..7098568b4e5 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -124,9 +124,10 @@ void QWaylandWindow::initWindow() if (shouldCreateSubSurface()) { Q_ASSERT(!mSubSurfaceWindow); - QWaylandWindow *p = static_cast(QPlatformWindow::parent()); - if (::wl_subsurface *ss = mDisplay->createSubSurface(this, p)) { - mSubSurfaceWindow = new QWaylandSubSurface(this, p, ss); + auto *parent = static_cast(QPlatformWindow::parent()); + if (parent->object()) { + if (::wl_subsurface *subsurface = mDisplay->createSubSurface(this, parent)) + mSubSurfaceWindow = new QWaylandSubSurface(this, parent, subsurface); } } else if (shouldCreateShellSurface()) { Q_ASSERT(!mShellSurface); From 41216bedd4a5eba1b12dacdad2c6368ac8978c77 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 1 Nov 2019 10:16:43 +0100 Subject: [PATCH 4/4] Add client test for subsurface with hidden parent Task-number: QTBUG-79674 Change-Id: I451ee4423dee511f41070498a61167912920c086 Reviewed-by: Paul Olav Tvete --- tests/auto/wayland/surface/tst_surface.cpp | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/auto/wayland/surface/tst_surface.cpp b/tests/auto/wayland/surface/tst_surface.cpp index 451fbac75b6..f9dcec58117 100644 --- a/tests/auto/wayland/surface/tst_surface.cpp +++ b/tests/auto/wayland/surface/tst_surface.cpp @@ -42,7 +42,9 @@ private slots: void waitForFrameCallbackGl(); void negotiateShmFormat(); + // Subsurfaces void createSubsurface(); + void createSubsurfaceForHiddenParent(); }; void tst_surface::createDestroySurface() @@ -172,5 +174,26 @@ void tst_surface::createSubsurface() QCOMPOSITOR_TRY_VERIFY(subSurface()); } +// Used to cause a crash in libwayland (QTBUG-79674) +void tst_surface::createSubsurfaceForHiddenParent() +{ + QRasterWindow window; + window.resize(64, 64); + window.show(); + QCOMPOSITOR_TRY_VERIFY(xdgToplevel()); + exec([=] { xdgToplevel()->sendCompleteConfigure(); }); + QCOMPOSITOR_TRY_VERIFY(xdgSurface()->m_committedConfigureSerial); + + window.hide(); + + QRasterWindow subWindow; + subWindow.setParent(&window); + subWindow.resize(64, 64); + subWindow.show(); + + // Make sure the client doesn't quit before it has a chance to crash + xdgPingAndWaitForPong(); +} + QCOMPOSITOR_TEST_MAIN(tst_surface) #include "tst_surface.moc"