xcb: rewrite auto-repeat key detection logic

It's unclear what the original code was doing. It relied on 'm_release'
which could never be 'false' (ref. QTBUG-69679). It was subtracting event
times and comparing with arbitrary '10'. On X11 auto-repeat keys can be
detected by checking time and keycode of the current release event and
the next event in the queue. If an event is an auto-repeat, then next event
in the queue will be a key press with matching time and keycode.

Verified that auto-repeat was unreliable in Qt 4 as well. With this patch
auto-repeat works as expected.

Added support for Xlib's XPeekEvent in our XCB implementation QXcbConnection::checkEvent():

"The XPeekEvent() function returns the first event from the event queue,
but it does not remove the event from the queue."

Sneaking in one variable renaming: "string" -> "text", to match the QKeyEvent::text().

Task-number: QTBUG-57335
Task-number: QTBUG-69679
Change-Id: I0a23f138287f57eaaecf1a009bd939e7e0e23269
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 26 insertions, 92 deletions

diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
index 24719a6c31..697b509bf0 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.h
+++ b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -441,7 +441,7 @@ public:
     QXcbWindow *platformWindowFromId(xcb_window_t id);
 
     template<typename Functor>
-    inline xcb_generic_event_t *checkEvent(Functor &&filter);
+    inline xcb_generic_event_t *checkEvent(Functor &&filter, bool removeFromQueue = true);
 
     typedef bool (*PeekFunc)(QXcbConnection *, xcb_generic_event_t *);
     void addPeekFunc(PeekFunc f);
@@ -735,14 +735,15 @@ Q_DECLARE_TYPEINFO(QXcbConnection::TabletData, Q_MOVABLE_TYPE);
 #endif
 
 template<typename Functor>
-xcb_generic_event_t *QXcbConnection::checkEvent(Functor &&filter)
+xcb_generic_event_t *QXcbConnection::checkEvent(Functor &&filter, bool removeFromQueue)
 {
     QXcbEventArray *eventqueue = m_reader->lock();
 
     for (int i = 0; i < eventqueue->size(); ++i) {
         xcb_generic_event_t *event = eventqueue->at(i);
         if (event && filter(event, event->response_type & ~0x80)) {
-            (*eventqueue)[i] = nullptr;
+            if (removeFromQueue)
+                (*eventqueue)[i] = nullptr;
             m_reader->unlock();
             return event;
         }
diff --git a/src/plugins/platforms/xcb/qxcbkeyboard.cpp b/src/plugins/platforms/xcb/qxcbkeyboard.cpp
index 37c8def6cc..40100c3a62 100644
--- a/src/plugins/platforms/xcb/qxcbkeyboard.cpp
+++ b/src/plugins/platforms/xcb/qxcbkeyboard.cpp
@@ -1472,60 +1472,6 @@ void QXcbKeyboard::resolveMaskConflicts()
     }
 }
 
-class KeyChecker
-{
-public:
-    KeyChecker(xcb_window_t window, xcb_keycode_t code, xcb_timestamp_t time, quint16 state)
-        : m_window(window)
-        , m_code(code)
-        , m_time(time)
-        , m_state(state)
-        , m_error(false)
-        , m_release(true)
-    {
-    }
-
-    bool operator() (xcb_generic_event_t *ev, int type)
-    {
-        if (m_error || !ev)
-            return false;
-
-        if (type != XCB_KEY_PRESS && type != XCB_KEY_RELEASE)
-            return false;
-
-        xcb_key_press_event_t *event = (xcb_key_press_event_t *)ev;
-
-        if (event->event != m_window || event->detail != m_code || event->state != m_state) {
-            m_error = true;
-            return false;
-        }
-
-        if (type == XCB_KEY_PRESS) {
-            m_error = !m_release || event->time - m_time > 10;
-            return !m_error;
-        }
-
-        if (m_release) {
-            m_error = true;
-            return false;
-        }
-
-        m_release = true;
-        m_time = event->time;
-
-        return false;
-    }
-
-private:
-    xcb_window_t m_window;
-    xcb_keycode_t m_code;
-    xcb_timestamp_t m_time;
-    quint16 m_state;
-
-    bool m_error;
-    bool m_release;
-};
-
 void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type, xcb_keycode_t code,
                                   quint16 state, xcb_timestamp_t time, bool fromSendEvent)
 {
@@ -1539,7 +1485,6 @@ void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type,
     if (type == QEvent::KeyPress)
         targetWindow->updateNetWmUserTime(time);
 
-
     ScopedXKBState sendEventState;
     if (fromSendEvent) {
         // Have a temporary keyboard state filled in from state
@@ -1557,7 +1502,7 @@ void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type,
     struct xkb_state *xkbState = fromSendEvent ? sendEventState.get() : m_xkbState.get();
 
     xcb_keysym_t sym = xkb_state_key_get_one_sym(xkbState, code);
-    QString string = lookupString(xkbState, code);
+    QString text = lookupString(xkbState, code);
 
     Qt::KeyboardModifiers modifiers = translateModifiers(state);
     if (sym >= XKB_KEY_KP_Space && sym <= XKB_KEY_KP_9)
@@ -1582,55 +1527,42 @@ void QXcbKeyboard::handleKeyEvent(xcb_window_t sourceWindow, QEvent::Type type,
     int qtcode = keysymToQtKey(latinKeysym != XKB_KEY_NoSymbol ? latinKeysym : sym,
                                modifiers, xkbState, code);
 
-    bool isAutoRepeat = false;
     if (type == QEvent::KeyPress) {
-        if (m_autorepeat_code == code) {
-            isAutoRepeat = true;
-            m_autorepeat_code = 0;
-        }
+        if (m_isAutoRepeat && m_autoRepeatCode != code)
+            // Some other key was pressed while we are auto-repeating on a different key.
+            m_isAutoRepeat = false;
     } else {
-        // look ahead for auto-repeat
-        KeyChecker checker(source->xcb_window(), code, time, state);
-        xcb_generic_event_t *event = connection()->checkEvent(checker);
-        if (event) {
-            isAutoRepeat = true;
-            free(event);
-        }
-        m_autorepeat_code = isAutoRepeat ? code : 0;
+        m_isAutoRepeat = false;
+        // Look at the next event in the queue to see if we are auto-repeating.
+        connection()->checkEvent([this, time, code](xcb_generic_event_t *event, int type) {
+            if (type == XCB_KEY_PRESS) {
+                auto keyPress = reinterpret_cast<xcb_key_press_event_t *>(event);
+                m_isAutoRepeat = keyPress->time == time && keyPress->detail == code;
+                if (m_isAutoRepeat)
+                    m_autoRepeatCode = code;
+            }
+            return true;
+        }, false /* removeFromQueue */);
     }
 
     bool filtered = false;
-    QPlatformInputContext *inputContext = QGuiApplicationPrivate::platformIntegration()->inputContext();
-    if (inputContext) {
-        QKeyEvent event(type, qtcode, modifiers, code, sym, state, string, isAutoRepeat, string.length());
+    if (auto inputContext = QGuiApplicationPrivate::platformIntegration()->inputContext()) {
+        QKeyEvent event(type, qtcode, modifiers, code, sym, state, text, m_isAutoRepeat, text.size());
         event.setTimestamp(time);
         filtered = inputContext->filterEvent(&event);
     }
 
-    QWindow *window = targetWindow->window();
     if (!filtered) {
+        QWindow *window = targetWindow->window();
 #ifndef QT_NO_CONTEXTMENU
         if (type == QEvent::KeyPress && qtcode == Qt::Key_Menu) {
             const QPoint globalPos = window->screen()->handle()->cursor()->pos();
             const QPoint pos = window->mapFromGlobal(globalPos);
             QWindowSystemInterface::handleContextMenuEvent(window, false, pos, globalPos, modifiers);
         }
-#endif // QT_NO_CONTEXTMENU
+#endif
         QWindowSystemInterface::handleExtendedKeyEvent(window, time, type, qtcode, modifiers,
-                                                       code, sym, state, string, isAutoRepeat);
-    }
-
-    if (isAutoRepeat && type == QEvent::KeyRelease) {
-        // since we removed it from the event queue using checkEvent we need to send the key press here
-        filtered = false;
-        if (inputContext) {
-            QKeyEvent event(QEvent::KeyPress, qtcode, modifiers, code, sym, state, string, isAutoRepeat, string.length());
-            event.setTimestamp(time);
-            filtered = inputContext->filterEvent(&event);
-        }
-        if (!filtered)
-            QWindowSystemInterface::handleExtendedKeyEvent(window, time, QEvent::KeyPress, qtcode, modifiers,
-                                                           code, sym, state, string, isAutoRepeat);
+                                                       code, sym, state, text, m_isAutoRepeat);
     }
 }
 
diff --git a/src/plugins/platforms/xcb/qxcbkeyboard.h b/src/plugins/platforms/xcb/qxcbkeyboard.h
index ab926eab84..95915fb2e6 100644
--- a/src/plugins/platforms/xcb/qxcbkeyboard.h
+++ b/src/plugins/platforms/xcb/qxcbkeyboard.h
@@ -109,7 +109,8 @@ protected:
 
 private:
     bool m_config = false;
-    xcb_keycode_t m_autorepeat_code = 0;
+    bool m_isAutoRepeat = false;
+    xcb_keycode_t m_autoRepeatCode = 0;
 
     struct _mod_masks {
         uint alt;