Improve the x11 timestamp handling

Summary:
So far KWin only updated the x11 timestamp if the new timestamp is larger
than the existing one. While this is a useful thing it creates problems
when the 32 bit msec based time stamp wraps around which happens after
running an X server for 49 days. After the timestamp wrapped around KWin
would not update the timestamp any more and thus some calls might fail.
Most prominent victims are keyboard and pointer grab which fails as the
timestamp is either larger than the server timestamp or smaller than the
last grab timestamp.

Another problem related to timestamp handling is KWin getting broken by
wrong timestamps sent by applications. A prominent example is clusterssh
which used to send a timestamp as unix time which is larger than the
x timestamp and thus our timestamp gets too large.

This change addresses these problems by allowing to reset the timestamp.
This is only used from updateXTime (which is normally invoked before we
do things like grabKeyboard). Thus we make QX11Info::getTimestamp the
ultimate trusted source for timestamps.

BUG: 377901
BUG: 348569
FIXED-IN: 5.8.7

Test Plan: As I cannot wait 50 days: unit tests for the two conditions added.

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D5704

autotests/CMakeLists.txt

@@ -310,3 +310,14 @@ target_link_libraries(testScreenEdges
 
 add_test(kwin_testScreenEdges testScreenEdges)
 ecm_mark_as_test(testScreenEdges)
+
+########################################################
+# Test X11 TimestampUpdate
+########################################################
+add_executable(testX11TimestampUpdate test_x11_timestamp_update.cpp)
+target_link_libraries(testX11TimestampUpdate
+    Qt5::Test
+    kwin
+)
+add_test(kwin-testX11TimestampUpdate testX11TimestampUpdate)
+ecm_mark_as_test(testX11TimestampUpdate)

autotests/test_x11_timestamp_update.cpp

@@ -0,0 +1,123 @@
+/********************************************************************
+KWin - the KDE window manager
+This file is part of the KDE project.
+
+Copyright (C) 2017 Martin Gräßlin <mgraesslin@kde.org>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+
+#include <QTest>
+#include <QX11Info>
+
+#include "main.h"
+#include "utils.h"
+
+namespace KWin
+{
+
+class X11TestApplication : public Application
+{
+    Q_OBJECT
+public:
+    X11TestApplication(int &argc, char **argv);
+    virtual ~X11TestApplication();
+
+protected:
+    void performStartup() override;
+
+};
+
+X11TestApplication::X11TestApplication(int &argc, char **argv)
+    : Application(OperationModeX11, argc, argv)
+{
+    setX11Connection(QX11Info::connection());
+    setX11RootWindow(QX11Info::appRootWindow());
+}
+
+X11TestApplication::~X11TestApplication()
+{
+}
+
+void X11TestApplication::performStartup()
+{
+}
+
+}
+
+class X11TimestampUpdateTest : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void testGrabAfterServerTime();
+    void testBeforeLastGrabTime();
+};
+
+void X11TimestampUpdateTest::testGrabAfterServerTime()
+{
+    // this test tries to grab the X keyboard with a timestamp in future
+    // that should fail, but after updating the X11 timestamp, it should
+    // work again
+    KWin::updateXTime();
+    QCOMPARE(KWin::grabXKeyboard(), true);
+    KWin::ungrabXKeyboard();
+
+    // now let's change the timestamp
+    KWin::kwinApp()->setX11Time(KWin::xTime() + 5 * 60 * 1000);
+
+    // now grab keyboard should fail
+    QCOMPARE(KWin::grabXKeyboard(), false);
+
+    // let's update timestamp, now it should work again
+    KWin::updateXTime();
+    QCOMPARE(KWin::grabXKeyboard(), true);
+    KWin::ungrabXKeyboard();
+}
+
+void X11TimestampUpdateTest::testBeforeLastGrabTime()
+{
+    // this test tries to grab the X keyboard with a timestamp before the
+    // last grab time on the server. That should fail, but after updating the X11
+    // timestamp it should work again
+
+    // first set the grab timestamp
+    KWin::updateXTime();
+    QCOMPARE(KWin::grabXKeyboard(), true);
+    KWin::ungrabXKeyboard();
+
+    // now go to past
+    const auto timestamp = KWin::xTime();
+    KWin::kwinApp()->setX11Time(KWin::xTime() - 5 * 60 * 1000, KWin::Application::TimestampUpdate::Always);
+    QCOMPARE(KWin::xTime(), timestamp - 5 * 60 * 1000);
+
+    // now grab keyboard should fail
+    QCOMPARE(KWin::grabXKeyboard(), false);
+
+    // let's update timestamp, now it should work again
+    KWin::updateXTime();
+    QVERIFY(KWin::xTime() >= timestamp);
+    QCOMPARE(KWin::grabXKeyboard(), true);
+    KWin::ungrabXKeyboard();
+}
+
+int main(int argc, char *argv[])
+{
+    setenv("QT_QPA_PLATFORM", "xcb", true);
+    KWin::X11TestApplication app(argc, argv);
+    app.setAttribute(Qt::AA_Use96Dpi, true);
+    X11TimestampUpdateTest tc;
+    return QTest::qExec(&tc, argc, argv);
+}
+
+#include "test_x11_timestamp_update.moc"

main.h

@@ -104,8 +104,12 @@ public:
     xcb_timestamp_t x11Time() const {
         return m_x11Time;
     }
-    void setX11Time(xcb_timestamp_t timestamp) {
-        if (timestamp > m_x11Time) {
+    enum class TimestampUpdate {
+        OnlyIfLarger,
+        Always
+    };
+    void setX11Time(xcb_timestamp_t timestamp, TimestampUpdate force = TimestampUpdate::OnlyIfLarger) {
+        if (timestamp > m_x11Time || force == TimestampUpdate::Always) {
             m_x11Time = timestamp;
         }
     }

utils.cpp

@@ -84,7 +84,7 @@ void updateXTime()
     // NOTE: QX11Info::getTimestamp does not yet search the event queue as the old
     // solution did. This means there might be regressions currently. See the
     // documentation above on how it should be done properly.
-    kwinApp()->setX11Time(QX11Info::getTimestamp());
+    kwinApp()->setX11Time(QX11Info::getTimestamp(), Application::TimestampUpdate::Always);
 }
 
 static int server_grab_count = 0;

utils.h

@@ -143,12 +143,12 @@ MaximizeMode operator^(MaximizeMode m1, MaximizeMode m2)
 
 template <typename T> using ScopedCPointer = QScopedPointer<T, QScopedPointerPodDeleter>;
 
-void updateXTime();
+void KWIN_EXPORT updateXTime();
 void grabXServer();
 void ungrabXServer();
 bool grabbedXServer();
-bool grabXKeyboard(xcb_window_t w = rootWindow());
-void ungrabXKeyboard();
+bool KWIN_EXPORT grabXKeyboard(xcb_window_t w = rootWindow());
+void KWIN_EXPORT ungrabXKeyboard();
 
 /**
  * Small helper class which performs @link grabXServer in the ctor and