From 6a78bd45fea399f76c0d45bc3b34c2a29001a85b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Wed, 4 Jan 2017 17:53:46 +0100
Subject: [PATCH] Fix syncing of geometry to internal window when using ::move

Summary:
The OnScreenNotification gets moved during its placement. This update
of the geometry did not get synced to the QWindow's geometry as it
should.

The reason for this is the rather special implementation of
AbstractClient::move. This just modifies the geom variable and does not
call into setGeometry at all. Instead a virtual doMove method is invoked
to inform the platform specific client implementation about the move of
the window. For a "normal" ShellClient that is fine - there is nothing
to do.

This change adds an implementation of doMove in ShellClient to sync the
geometry to the internal window. This is not yet enough to fix the
geometry of the OnScreenNotification. It gets placed with a
GeometryUpdatesBlocker in place causing the doMove never to be called.
Instead setGeometry is invoked again when the geometry updates get
unblocked. So far ShellClient did not do anything in this case. This
change ensures that at least the doSetGeometry method is performed again.

Test Plan: Added test case and manual testing of OnScreenNotification

Reviewers: #kwin, #plasma_on_wayland

Subscribers: plasma-devel, kwin

Tags: #plasma_on_wayland, #kwin

Differential Revision: https://phabricator.kde.org/D3963
---
 autotests/integration/internal_window.cpp | 35 +++++++++++++++++++++++
 shell_client.cpp                          | 29 +++++++++++++------
 shell_client.h                            |  2 ++
 3 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/autotests/integration/internal_window.cpp b/autotests/integration/internal_window.cpp
index b1b2072311..45be338486 100644
--- a/autotests/integration/internal_window.cpp
+++ b/autotests/integration/internal_window.cpp
@@ -58,6 +58,7 @@ private Q_SLOTS:
     void testKeyboardTriggersLeave();
     void testTouch();
     void testOpacity();
+    void testMove();
 };
 
 class HelperWindow : public QRasterWindow
@@ -518,6 +519,40 @@ void InternalWindowTest::testOpacity()
     QCOMPARE(internalClient->opacity(), 0.75);
 }
 
+void InternalWindowTest::testMove()
+{
+    QSignalSpy clientAddedSpy(waylandServer(), &WaylandServer::shellClientAdded);
+    QVERIFY(clientAddedSpy.isValid());
+    HelperWindow win;
+    win.setOpacity(0.5);
+    win.setGeometry(0, 0, 100, 100);
+    win.show();
+    QVERIFY(clientAddedSpy.wait());
+    QCOMPARE(clientAddedSpy.count(), 1);
+    auto internalClient = clientAddedSpy.first().first().value<ShellClient*>();
+    QVERIFY(internalClient);
+    QCOMPARE(internalClient->geometry(), QRect(0, 0, 100, 100));
+
+    // normal move should be synced
+    internalClient->move(5, 10);
+    QCOMPARE(internalClient->geometry(), QRect(5, 10, 100, 100));
+    QCOMPARE(win.geometry(), QRect(5, 10, 100, 100));
+    // another move should also be synced
+    internalClient->move(10, 20);
+    QCOMPARE(internalClient->geometry(), QRect(10, 20, 100, 100));
+    QCOMPARE(win.geometry(), QRect(10, 20, 100, 100));
+
+    // now move with a Geometry update blocker
+    {
+        GeometryUpdatesBlocker blocker(internalClient);
+        internalClient->move(5, 10);
+        // not synced!
+        QCOMPARE(win.geometry(), QRect(10, 20, 100, 100));
+    }
+    // after destroying the blocker it should be synced
+    QCOMPARE(win.geometry(), QRect(5, 10, 100, 100));
+}
+
 }
 
 WAYLANDTEST_MAIN(KWin::InternalWindowTest)
diff --git a/shell_client.cpp b/shell_client.cpp
index 56ea20a6dc..172f10e2a4 100644
--- a/shell_client.cpp
+++ b/shell_client.cpp
@@ -482,7 +482,7 @@ void ShellClient::setGeometry(int x, int y, int w, int h, ForceGeometry_t force)
 
 void ShellClient::doSetGeometry(const QRect &rect)
 {
-    if (geom == rect) {
+    if (geom == rect && pendingGeometryUpdate() == PendingGeometryNone) {
         return;
     }
     if (!m_unmapped) {
@@ -499,19 +499,32 @@ void ShellClient::doSetGeometry(const QRect &rect)
     if (!m_unmapped) {
         addWorkspaceRepaint(visibleRect());
     }
-    if (m_internalWindow) {
-        const QRect windowRect = QRect(geom.topLeft() + QPoint(borderLeft(), borderTop()),
-                                       geom.size() - QSize(borderLeft() + borderRight(), borderTop() + borderBottom()));
-        if (m_internalWindow->geometry() != windowRect) {
-            m_internalWindow->setGeometry(windowRect);
-        }
-    }
+    syncGeometryToInternalWindow();
     if (hasStrut()) {
         workspace()->updateClientArea();
     }
     emit geometryShapeChanged(this, old);
 }
 
+void ShellClient::doMove(int x, int y)
+{
+    Q_UNUSED(x)
+    Q_UNUSED(y)
+    syncGeometryToInternalWindow();
+}
+
+void ShellClient::syncGeometryToInternalWindow()
+{
+    if (!m_internalWindow) {
+        return;
+    }
+    const QRect windowRect = QRect(geom.topLeft() + QPoint(borderLeft(), borderTop()),
+                                    geom.size() - QSize(borderLeft() + borderRight(), borderTop() + borderBottom()));
+    if (m_internalWindow->geometry() != windowRect) {
+        m_internalWindow->setGeometry(windowRect);
+    }
+}
+
 QByteArray ShellClient::windowRole() const
 {
     return QByteArray();
diff --git a/shell_client.h b/shell_client.h
index 32b612a801..6c3f38eb86 100644
--- a/shell_client.h
+++ b/shell_client.h
@@ -150,6 +150,7 @@ protected:
     bool isWaitingForMoveResizeSync() const override;
     bool acceptsFocus() const override;
     void doMinimize() override;
+    void doMove(int x, int y) override;
 
 private Q_SLOTS:
     void clientFullScreenChanged(bool fullScreen);
@@ -166,6 +167,7 @@ private:
     void createWindowId();
     void findInternalWindow();
     void updateInternalWindowGeometry();
+    void syncGeometryToInternalWindow();
     void updateIcon();
     void markAsMapped();
     void setTransient();