Fix crash when connecting a new screen

In QWaylandWindow::virtualSiblings, don't include screens that have not been
added yet. I.e. QWaylandScreens for which QPlatformIntegration::screenAdded has
not yet been called.

There are two reasons why this crash wasn't covered by the
removePrimaryScreen() test. First of all, the mock output didn't send
wl_output.done events when updating the mode/geometry. These wayland events are
what causes QWindowSystemInterface::handleScreenGeometryChange() to be called
(where virtualSiblings are called).

Furthermore, virtualSiblings is only called when the geometry actually changes,
so add a new test that changes the screen geometry of the existing screen while
a new one is being added (i.e. moves it to the right).

Task-number: QTBUG-62044
Change-Id: I623fbf8799d21c6b9293e7120ded301277639cc6
Reviewed-by: David Edmundson <davidedmundson@kde.org>
Reviewed-by: Aleix Pol
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

6 files changed, 65 insertions, 4 deletions

diff --git a/src/client/qwaylandscreen.cpp b/src/client/qwaylandscreen.cpp
index fba755577..1c9ce23ba 100644
--- a/src/client/qwaylandscreen.cpp
+++ b/src/client/qwaylandscreen.cpp
@@ -138,8 +138,10 @@ QList<QPlatformScreen *> QWaylandScreen::virtualSiblings() const
     QList<QPlatformScreen *> list;
     const QList<QWaylandScreen*> screens = mWaylandDisplay->screens();
     list.reserve(screens.count());
-    foreach (QWaylandScreen *screen, screens)
-        list << screen;
+    for (QWaylandScreen *screen : qAsConst(screens)) {
+        if (screen->screen())
+            list << screen;
+    }
     return list;
 }
 
diff --git a/tests/auto/client/client/tst_client.cpp b/tests/auto/client/client/tst_client.cpp
index 981b8f129..e244de7f2 100644
--- a/tests/auto/client/client/tst_client.cpp
+++ b/tests/auto/client/client/tst_client.cpp
@@ -164,6 +164,7 @@ public slots:
 private slots:
     void primaryScreen();
     void screens();
+    void addScreenWithGeometryChange();
     void windowScreens();
     void removePrimaryScreen();
     void createDestroyWindow();
@@ -197,6 +198,29 @@ void tst_WaylandClient::screens()
     QTRY_COMPARE(QGuiApplication::screens().size(), 1);
 }
 
+//QTBUG-62044
+void tst_WaylandClient::addScreenWithGeometryChange()
+{
+    QTRY_COMPARE(QGuiApplication::screens().size(), 1);
+    const QRect oldGeometry = QGuiApplication::primaryScreen()->geometry();
+    compositor->sendAddOutput();
+
+    // Move the primary screen to the right
+    const QRect newGeometry(QPoint(screenSize.width(), 0), screenSize);
+    Q_ASSERT(oldGeometry != newGeometry);
+    compositor->sendOutputGeometry(compositor->output(0), newGeometry);
+
+    QTRY_COMPARE(QGuiApplication::screens().size(), 2);
+    QTRY_COMPARE(QGuiApplication::primaryScreen()->geometry(), newGeometry);
+
+    compositor->sendRemoveOutput(compositor->output(1));
+    QTRY_COMPARE(QGuiApplication::screens().size(), 1);
+
+    // Move the screen back
+    compositor->sendOutputGeometry(compositor->output(0), oldGeometry);
+    QTRY_COMPARE(QGuiApplication::primaryScreen()->geometry(), oldGeometry);
+}
+
 void tst_WaylandClient::windowScreens()
 {
     QSharedPointer<MockOutput> firstOutput;
@@ -255,6 +279,7 @@ void tst_WaylandClient::removePrimaryScreen()
     compositor->sendAddOutput();
 
     QTRY_COMPARE(QGuiApplication::screens().size(), 2);
+    QTRY_COMPARE(QGuiApplication::primaryScreen()->virtualSiblings().size(), 2);
     QScreen *secondaryScreen = QGuiApplication::screens().at(1);
     QVERIFY(secondaryScreen);
 
diff --git a/tests/auto/client/shared/mockcompositor.cpp b/tests/auto/client/shared/mockcompositor.cpp
index e7c6e90d2..bdeca4e23 100644
--- a/tests/auto/client/shared/mockcompositor.cpp
+++ b/tests/auto/client/shared/mockcompositor.cpp
@@ -196,6 +196,14 @@ void MockCompositor::sendRemoveOutput(const QSharedPointer<MockOutput> &output)
     processCommand(command);
 }
 
+void MockCompositor::sendOutputGeometry(const QSharedPointer<MockOutput> &output, const QRect &geometry)
+{
+    Command command = makeCommand(Impl::Compositor::sendOutputGeometry, m_compositor);
+    command.parameters << QVariant::fromValue(output);
+    command.parameters << QVariant::fromValue(geometry);
+    processCommand(command);
+}
+
 void MockCompositor::sendSurfaceEnter(const QSharedPointer<MockSurface> &surface, QSharedPointer<MockOutput> &output)
 {
     Command command = makeCommand(Impl::Compositor::sendSurfaceEnter, m_compositor);
diff --git a/tests/auto/client/shared/mockcompositor.h b/tests/auto/client/shared/mockcompositor.h
index 9258b1926..d4cf5f367 100644
--- a/tests/auto/client/shared/mockcompositor.h
+++ b/tests/auto/client/shared/mockcompositor.h
@@ -89,6 +89,7 @@ public:
     static void setOutputMode(void *compositor, const QList<QVariant> &parameters);
     static void sendAddOutput(void *data, const QList<QVariant> &parameters);
     static void sendRemoveOutput(void *data, const QList<QVariant> &parameters);
+    static void sendOutputGeometry(void *data, const QList<QVariant> &parameters);
     static void sendSurfaceEnter(void *data, const QList<QVariant> &parameters);
     static void sendSurfaceLeave(void *data, const QList<QVariant> &parameters);
 
@@ -181,6 +182,7 @@ public:
     void sendDataDeviceLeave(const QSharedPointer<MockSurface> &surface);
     void sendAddOutput();
     void sendRemoveOutput(const QSharedPointer<MockOutput> &output);
+    void sendOutputGeometry(const QSharedPointer<MockOutput> &output, const QRect &geometry);
     void sendSurfaceEnter(const QSharedPointer<MockSurface> &surface, QSharedPointer<MockOutput> &output);
     void sendSurfaceLeave(const QSharedPointer<MockSurface> &surface, QSharedPointer<MockOutput> &output);
     void waitForStartDrag();
diff --git a/tests/auto/client/shared/mockoutput.cpp b/tests/auto/client/shared/mockoutput.cpp
index 7d7b7413a..13e0524ad 100644
--- a/tests/auto/client/shared/mockoutput.cpp
+++ b/tests/auto/client/shared/mockoutput.cpp
@@ -54,6 +54,16 @@ void Compositor::sendRemoveOutput(void *data, const QList<QVariant> &parameters)
     delete output;
 }
 
+void Compositor::sendOutputGeometry(void *data, const QList<QVariant> &parameters)
+{
+    Compositor *compositor = static_cast<Compositor *>(data);
+    Q_ASSERT(compositor);
+    Output *output = resolveOutput(parameters.first());
+    Q_ASSERT(output);
+    QRect geometry = parameters.at(1).toRect();
+    output->sendGeometryAndMode(geometry);
+}
+
 void Compositor::setOutputMode(void *data, const QList<QVariant> &parameters)
 {
     Compositor *compositor = static_cast<Compositor *>(data);
@@ -74,16 +84,29 @@ Output::Output(wl_display *display, const QSize &resolution, const QPoint &posit
 
 void Output::setCurrentMode(const QSize &size)
 {
-    qDebug() << Q_FUNC_INFO << size;
     m_size = size;
-    for (Resource *resource : resourceMap())
+    for (Resource *resource : resourceMap()) {
+        sendCurrentMode(resource);
+        send_done(resource->handle);
+    }
+}
+
+void Output::sendGeometryAndMode(const QRect &geometry)
+{
+    m_size = geometry.size();
+    m_position = geometry.topLeft();
+    for (Resource *resource : resourceMap()) {
+        sendGeometry(resource);
         sendCurrentMode(resource);
+        send_done(resource->handle);
+    }
 }
 
 void Output::output_bind_resource(QtWaylandServer::wl_output::Resource *resource)
 {
     sendGeometry(resource);
     sendCurrentMode(resource);
+    send_done(resource->handle);
 }
 
 void Output::sendGeometry(Resource *resource)
diff --git a/tests/auto/client/shared/mockoutput.h b/tests/auto/client/shared/mockoutput.h
index d5a2bb56b..9f261d5d7 100644
--- a/tests/auto/client/shared/mockoutput.h
+++ b/tests/auto/client/shared/mockoutput.h
@@ -44,6 +44,7 @@ public:
 
     QSharedPointer<MockOutput> mockOutput() const { return m_mockOutput; }
     void setCurrentMode(const QSize &size);
+    void sendGeometryAndMode(const QRect &geometry);
 
 protected:
     void output_bind_resource(Resource *resource) override;