xcb: cleanup QXcbConnection::handleXcbEvent()

- the usage of 'handled' variable was a mess.
- remove "generic" from API names. The naming was probably influenced
  too much from underlying C API (xcb_generic_event_t):
  - handleGenericEvent() -> handleNativeEvent() to be consistent with QWindow::nativeEvent().
    - dropped unnecessary 'long *result' from signature. It is useful only on MS Windows.
  - genericEventFilterType() -> nativeEventType(), it *is* an event type, not a filter type.
- XCB_CLIENT_MESSAGE was not passed to QWindow::nativeEvent(), which is done via
  HANDLE_PLATFORM_WINDOW_EVENT.
- minor: added some 'auto's where it makes sense and improved some variable names.

Change-Id: Id1c9896054e2dbd9a79bacd88394149c8cf2cdea
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

src/plugins/platforms/xcb/gl_integrations/xcb_glx/qxcbglxintegration.cpp

@@ -163,9 +163,9 @@ bool QXcbGlxIntegration::handleXcbEvent(xcb_generic_event_t *event, uint respons
                 // Unlock the display before calling the native event filter
                 XUnlockDisplay(xdisplay);
                 locked = false;
-                QByteArray genericEventFilterType = m_connection->nativeInterface()->genericEventFilterType();
+                auto eventType = m_connection->nativeInterface()->nativeEventType();
                 long result = 0;
-                handled = dispatcher->filterNativeEvent(genericEventFilterType, &ev, &result);
+                handled = dispatcher->filterNativeEvent(eventType, &ev, &result);
             }
 #endif
         }

src/plugins/platforms/xcb/qxcbconnection.cpp

@@ -733,10 +733,10 @@ QXcbWindow *QXcbConnection::platformWindowFromId(xcb_window_t id)
 
 #define HANDLE_PLATFORM_WINDOW_EVENT(event_t, windowMember, handler) \
 { \
-    event_t *e = reinterpret_cast<event_t *>(event); \
+    auto e = reinterpret_cast<event_t *>(event); \
     if (QXcbWindowEventListener *eventListener = windowEventListenerFromId(e->windowMember))  { \
-        handled = eventListener->handleGenericEvent(event, &result); \
+        if (eventListener->handleNativeEvent(event)) \
-        if (!handled) \
+            return; \
         eventListener->handler(e); \
     } \
 } \
@@ -744,10 +744,10 @@ break;
 
 #define HANDLE_KEYBOARD_EVENT(event_t, handler) \
 { \
-    event_t *e = reinterpret_cast<event_t *>(event); \
+    auto e = reinterpret_cast<event_t *>(event); \
     if (QXcbWindowEventListener *eventListener = windowEventListenerFromId(e->event)) { \
-        handled = eventListener->handleGenericEvent(event, &result); \
+        if (eventListener->handleNativeEvent(event)) \
-        if (!handled) \
+            return; \
         m_keyboard->handler(e); \
     } \
 } \
@@ -974,7 +974,7 @@ void QXcbConnection::handleXcbError(xcb_generic_error_t *error)
 {
     long result = 0;
     QAbstractEventDispatcher* dispatcher = QAbstractEventDispatcher::instance();
-    if (dispatcher && dispatcher->filterNativeEvent(m_nativeInterface->genericEventFilterType(), error, &result))
+    if (dispatcher && dispatcher->filterNativeEvent(m_nativeInterface->nativeEventType(), error, &result))
         return;
 
     printXcbError("QXcbConnection: XCB error", error);
@@ -1066,47 +1066,53 @@ namespace {
 
 void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
 {
-    long result = 0;
+    if (Q_UNLIKELY(lcQpaEvents().isDebugEnabled()))
+        printXcbEvent(lcQpaEvents(), "Event", event);
+
+    long result = 0; // Used only by MS Windows
     QAbstractEventDispatcher* dispatcher = QAbstractEventDispatcher::instance();
-    bool handled = dispatcher && dispatcher->filterNativeEvent(m_nativeInterface->genericEventFilterType(), event, &result);
+    bool handledByNativeEventFilter = dispatcher && dispatcher->filterNativeEvent(
+                m_nativeInterface->nativeEventType(), event, &result);
+    if (handledByNativeEventFilter)
+        return;
 
     uint response_type = event->response_type & ~0x80;
 
-    if (!handled) {
+    bool handled = true;
     switch (response_type) {
     case XCB_EXPOSE:
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_expose_event_t, window, handleExposeEvent);
-
     case XCB_BUTTON_PRESS: {
-            xcb_button_press_event_t *ev = (xcb_button_press_event_t *)event;
+        auto ev = reinterpret_cast<xcb_button_press_event_t *>(event);
         m_keyboard->updateXKBStateFromCore(ev->state);
         // the event explicitly contains the state of the three first buttons,
         // the rest we need to manage ourselves
         m_buttonState = (m_buttonState & ~0x7) | translateMouseButtons(ev->state);
         setButtonState(translateMouseButton(ev->detail), true);
         if (Q_UNLIKELY(lcQpaXInputEvents().isDebugEnabled()))
-                qCDebug(lcQpaXInputEvents, "legacy mouse press, button %d state %X", ev->detail, static_cast<unsigned int>(m_buttonState));
+            qCDebug(lcQpaXInputEvents, "legacy mouse press, button %d state %X",
+                    ev->detail, static_cast<unsigned int>(m_buttonState));
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_button_press_event_t, event, handleButtonPressEvent);
     }
     case XCB_BUTTON_RELEASE: {
-            xcb_button_release_event_t *ev = (xcb_button_release_event_t *)event;
+        auto ev = reinterpret_cast<xcb_button_release_event_t *>(event);
         m_keyboard->updateXKBStateFromCore(ev->state);
         m_buttonState = (m_buttonState & ~0x7) | translateMouseButtons(ev->state);
         setButtonState(translateMouseButton(ev->detail), false);
         if (Q_UNLIKELY(lcQpaXInputEvents().isDebugEnabled()))
-                qCDebug(lcQpaXInputEvents, "legacy mouse release, button %d state %X", ev->detail, static_cast<unsigned int>(m_buttonState));
+            qCDebug(lcQpaXInputEvents, "legacy mouse release, button %d state %X",
+                    ev->detail, static_cast<unsigned int>(m_buttonState));
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_button_release_event_t, event, handleButtonReleaseEvent);
     }
     case XCB_MOTION_NOTIFY: {
-            xcb_motion_notify_event_t *ev = (xcb_motion_notify_event_t *)event;
+        auto ev = reinterpret_cast<xcb_motion_notify_event_t *>(event);
         m_keyboard->updateXKBStateFromCore(ev->state);
         m_buttonState = (m_buttonState & ~0x7) | translateMouseButtons(ev->state);
         if (Q_UNLIKELY(lcQpaXInputEvents().isDebugEnabled()))
-                qCDebug(lcQpaXInputEvents, "legacy mouse move %d,%d button %d state %X", ev->event_x, ev->event_y,
+            qCDebug(lcQpaXInputEvents, "legacy mouse move %d,%d button %d state %X",
-                        ev->detail, static_cast<unsigned int>(m_buttonState));
+                    ev->event_x, ev->event_y, ev->detail, static_cast<unsigned int>(m_buttonState));
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_motion_notify_event_t, event, handleMotionNotifyEvent);
     }
-
     case XCB_CONFIGURE_NOTIFY:
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_configure_notify_event_t, event, handleConfigureNotifyEvent);
     case XCB_MAP_NOTIFY:
@@ -1115,9 +1121,20 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_unmap_notify_event_t, event, handleUnmapNotifyEvent);
     case XCB_DESTROY_NOTIFY:
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_destroy_notify_event_t, event, handleDestroyNotifyEvent);
-        case XCB_CLIENT_MESSAGE:
+    case XCB_CLIENT_MESSAGE: {
-            handleClientMessageEvent((xcb_client_message_event_t *)event);
+        auto clientMessage = reinterpret_cast<xcb_client_message_event_t *>(event);
-            break;
+        if (clientMessage->format != 32)
+            return;
+#if QT_CONFIG(draganddrop)
+        if (clientMessage->type == atom(QXcbAtom::XdndStatus))
+            drag()->handleStatus(clientMessage);
+        else if (clientMessage->type == atom(QXcbAtom::XdndFinished))
+            drag()->handleFinished(clientMessage);
+#endif
+        if (m_systemTrayTracker && clientMessage->type == atom(QXcbAtom::MANAGER))
+            m_systemTrayTracker->notifyManagerClientMessageEvent(clientMessage);
+        HANDLE_PLATFORM_WINDOW_EVENT(xcb_client_message_event_t, window, handleClientMessageEvent);
+    }
     case XCB_ENTER_NOTIFY:
 #if QT_CONFIG(xcb_xinput)
         if (hasXInput2() && !xi2MouseEventsDisabled())
@@ -1129,7 +1146,7 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
         if (hasXInput2() && !xi2MouseEventsDisabled())
             break;
 #endif
-            m_keyboard->updateXKBStateFromCore(((xcb_leave_notify_event_t *)event)->state);
+        m_keyboard->updateXKBStateFromCore(reinterpret_cast<xcb_leave_notify_event_t *>(event)->state);
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_leave_notify_event_t, event, handleLeaveNotifyEvent);
     case XCB_FOCUS_IN:
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_focus_in_event_t, event, handleFocusInEvent);
@@ -1137,13 +1154,13 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
         HANDLE_PLATFORM_WINDOW_EVENT(xcb_focus_out_event_t, event, handleFocusOutEvent);
     case XCB_KEY_PRESS:
     {
-            xcb_key_press_event_t *kp = (xcb_key_press_event_t *)event;
+        auto keyPress = reinterpret_cast<xcb_key_press_event_t *>(event);
-            m_keyboard->updateXKBStateFromCore(kp->state);
+        m_keyboard->updateXKBStateFromCore(keyPress->state);
-            setTime(kp->time);
+        setTime(keyPress->time);
         HANDLE_KEYBOARD_EVENT(xcb_key_press_event_t, handleKeyPressEvent);
     }
     case XCB_KEY_RELEASE:
-            m_keyboard->updateXKBStateFromCore(((xcb_key_release_event_t *)event)->state);
+        m_keyboard->updateXKBStateFromCore(reinterpret_cast<xcb_key_release_event_t *>(event)->state);
         HANDLE_KEYBOARD_EVENT(xcb_key_release_event_t, handleKeyReleaseEvent);
     case XCB_MAPPING_NOTIFY:
         m_keyboard->updateKeymap(reinterpret_cast<xcb_mapping_notify_event_t *>(event));
@@ -1151,16 +1168,16 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
     case XCB_SELECTION_REQUEST:
     {
 #if QT_CONFIG(draganddrop) || QT_CONFIG(clipboard)
-            xcb_selection_request_event_t *sr = reinterpret_cast<xcb_selection_request_event_t *>(event);
+        auto selectionRequest = reinterpret_cast<xcb_selection_request_event_t *>(event);
 #endif
 #if QT_CONFIG(draganddrop)
-            if (sr->selection == atom(QXcbAtom::XdndSelection))
+        if (selectionRequest->selection == atom(QXcbAtom::XdndSelection))
-                m_drag->handleSelectionRequest(sr);
+            m_drag->handleSelectionRequest(selectionRequest);
         else
 #endif
         {
 #ifndef QT_NO_CLIPBOARD
-                m_clipboard->handleSelectionRequest(sr);
+            m_clipboard->handleSelectionRequest(selectionRequest);
 #endif
         }
         break;
@@ -1170,17 +1187,15 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
 #ifndef QT_NO_CLIPBOARD
         m_clipboard->handleSelectionClearRequest(reinterpret_cast<xcb_selection_clear_event_t *>(event));
 #endif
-            handled = true;
         break;
     case XCB_SELECTION_NOTIFY:
         setTime((reinterpret_cast<xcb_selection_notify_event_t *>(event))->time);
-            handled = false;
         break;
     case XCB_PROPERTY_NOTIFY:
     {
-            xcb_property_notify_event_t *pn = reinterpret_cast<xcb_property_notify_event_t *>(event);
+        auto propertyNotify = reinterpret_cast<xcb_property_notify_event_t *>(event);
-            if (pn->atom == atom(QXcbAtom::_NET_WORKAREA)) {
+        if (propertyNotify->atom == atom(QXcbAtom::_NET_WORKAREA)) {
-                QXcbVirtualDesktop *virtualDesktop = virtualDesktopForRootWindow(pn->window);
+            QXcbVirtualDesktop *virtualDesktop = virtualDesktopForRootWindow(propertyNotify->window);
             if (virtualDesktop)
                 virtualDesktop->updateWorkArea();
         } else {
@@ -1196,45 +1211,40 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
         break;
 #endif
     default:
-            handled = false;
+        handled = false; // event type not recognized
         break;
     }
-    }
 
-    if (!handled) {
+    if (handled)
+        return;
+
+    handled = true;
     if (has_xfixes && response_type == xfixes_first_event + XCB_XFIXES_SELECTION_NOTIFY) {
-            xcb_xfixes_selection_notify_event_t *notify_event = reinterpret_cast<xcb_xfixes_selection_notify_event_t *>(event);
+        auto notify_event = reinterpret_cast<xcb_xfixes_selection_notify_event_t *>(event);
         setTime(notify_event->timestamp);
 #ifndef QT_NO_CLIPBOARD
         m_clipboard->handleXFixesSelectionRequest(notify_event);
 #endif
         for (QXcbVirtualDesktop *virtualDesktop : qAsConst(m_virtualDesktops))
             virtualDesktop->handleXFixesSelectionNotify(notify_event);
-
-            handled = true;
     } else if (has_randr_extension && response_type == xrandr_first_event + XCB_RANDR_NOTIFY) {
         updateScreens(reinterpret_cast<xcb_randr_notify_event_t *>(event));
-            handled = true;
     } else if (has_randr_extension && response_type == xrandr_first_event + XCB_RANDR_SCREEN_CHANGE_NOTIFY) {
-            xcb_randr_screen_change_notify_event_t *change_event = reinterpret_cast<xcb_randr_screen_change_notify_event_t *>(event);
+        auto change_event = reinterpret_cast<xcb_randr_screen_change_notify_event_t *>(event);
-            if (auto *virtualDesktop = virtualDesktopForRootWindow(change_event->root))
+        if (auto virtualDesktop = virtualDesktopForRootWindow(change_event->root))
             virtualDesktop->handleScreenChange(change_event);
-
-            handled = true;
 #if QT_CONFIG(xkb)
     } else if (response_type == xkb_first_event) { // https://bugs.freedesktop.org/show_bug.cgi?id=51295
-            _xkb_event *xkb_event = reinterpret_cast<_xkb_event *>(event);
+        auto xkb_event = reinterpret_cast<_xkb_event *>(event);
         if (xkb_event->any.deviceID == m_keyboard->coreDeviceId()) {
             switch (xkb_event->any.xkbType) {
                 // XkbNewKkdNotify and XkbMapNotify together capture all sorts of keymap
                 // updates (e.g. xmodmap, xkbcomp, setxkbmap), with minimal redundent recompilations.
                 case XCB_XKB_STATE_NOTIFY:
                     m_keyboard->updateXKBState(&xkb_event->state_notify);
-                        handled = true;
                     break;
                 case XCB_XKB_MAP_NOTIFY:
                     m_keyboard->updateKeymap();
-                        handled = true;
                     break;
                 case XCB_XKB_NEW_KEYBOARD_NOTIFY: {
                     xcb_xkb_new_keyboard_notify_event_t *ev = &xkb_event->new_keyboard_notify;
@@ -1247,16 +1257,15 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
             }
         }
 #endif
-        }
+    } else {
+        handled = false; // event type still not recognized
     }
 
-    if (!handled && m_glIntegration)
+    if (handled)
-        handled = m_glIntegration->handleXcbEvent(event, response_type);
+        return;
 
-#if 0
+    if (m_glIntegration)
-    if (Q_UNLIKELY(lcQpaEvents().isDebugEnabled()))
+        m_glIntegration->handleXcbEvent(event, response_type);
-        printXcbEvent(lcQpaEvents(), handled ? "Handled" : "Unhandled", event);
-#endif
 }
 
 void QXcbConnection::addPeekFunc(PeekFunc f)
@@ -1761,28 +1770,6 @@ void QXcbConnection::processXcbEvents()
     xcb_flush(xcb_connection());
 }
 
-void QXcbConnection::handleClientMessageEvent(const xcb_client_message_event_t *event)
-{
-    if (event->format != 32)
-        return;
-
-#if QT_CONFIG(draganddrop)
-    if (event->type == atom(QXcbAtom::XdndStatus)) {
-        drag()->handleStatus(event);
-    } else if (event->type == atom(QXcbAtom::XdndFinished)) {
-        drag()->handleFinished(event);
-    }
-#endif
-    if (m_systemTrayTracker && event->type == atom(QXcbAtom::MANAGER))
-        m_systemTrayTracker->notifyManagerClientMessageEvent(event);
-
-    QXcbWindow *window = platformWindowFromId(event->window);
-    if (!window)
-        return;
-
-    window->handleClientMessageEvent(event);
-}
-
 static const char * xcb_atomnames = {
     // window-manager <-> client protocols
     "WM_PROTOCOLS\0"

src/plugins/platforms/xcb/qxcbconnection.h

@@ -335,7 +335,7 @@ class QXcbWindowEventListener
 {
 public:
     virtual ~QXcbWindowEventListener() {}
-    virtual bool handleGenericEvent(xcb_generic_event_t *, long *) { return false; }
+    virtual bool handleNativeEvent(xcb_generic_event_t *) { return false; }
 
     virtual void handleExposeEvent(const xcb_expose_event_t *) {}
     virtual void handleClientMessageEvent(const xcb_client_message_event_t *) {}
@@ -553,7 +553,6 @@ private:
     void initializeXShape();
     void initializeXKB();
     void initializeXSync();
-    void handleClientMessageEvent(const xcb_client_message_event_t *event);
     QXcbScreen* findScreenForCrtc(xcb_window_t rootWindow, xcb_randr_crtc_t crtc) const;
     QXcbScreen* findScreenForOutput(xcb_window_t rootWindow, xcb_randr_output_t output) const;
     QXcbVirtualDesktop* virtualDesktopForRootWindow(xcb_window_t rootWindow) const;

src/plugins/platforms/xcb/qxcbconnection_xi2.cpp

@@ -581,8 +581,7 @@ void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
     }
 
     if (eventListener) {
-        long result = 0;
+        if (eventListener->handleNativeEvent(reinterpret_cast<xcb_generic_event_t *>(event)))
-        if (eventListener->handleGenericEvent(reinterpret_cast<xcb_generic_event_t *>(event), &result))
             return;
     }
 

src/plugins/platforms/xcb/qxcbnativeinterface.cpp

@@ -92,8 +92,7 @@ static int resourceType(const QByteArray &key)
     return int(result - names);
 }
 
-QXcbNativeInterface::QXcbNativeInterface() :
+QXcbNativeInterface::QXcbNativeInterface()
-    m_genericEventFilterType(QByteArrayLiteral("xcb_generic_event_t"))
 {
 }
 

src/plugins/platforms/xcb/qxcbnativeinterface.h

@@ -98,7 +98,7 @@ public:
 
     QFunctionPointer platformFunction(const QByteArray &function) const override;
 
-    inline const QByteArray &genericEventFilterType() const { return m_genericEventFilterType; }
+    inline const QByteArray &nativeEventType() const { return m_nativeEventType; }
 
     void *displayForWindow(QWindow *window);
     void *connectionForWindow(QWindow *window);
@@ -131,7 +131,7 @@ signals:
     void systemTrayWindowChanged(QScreen *screen);
 
 private:
-    const QByteArray m_genericEventFilterType;
+    const QByteArray m_nativeEventType = QByteArrayLiteral("xcb_generic_event_t");
 
     xcb_atom_t m_sysTraySelectionAtom = XCB_ATOM_NONE;
 

src/plugins/platforms/xcb/qxcbwindow.cpp

@@ -1807,12 +1807,11 @@ bool QXcbWindow::requestSystemTrayWindowDock()
     return true;
 }
 
-bool QXcbWindow::handleGenericEvent(xcb_generic_event_t *event, long *result)
+bool QXcbWindow::handleNativeEvent(xcb_generic_event_t *event)
 {
-    return QWindowSystemInterface::handleNativeEvent(window(),
+    auto eventType = connection()->nativeInterface()->nativeEventType();
-                                                     connection()->nativeInterface()->genericEventFilterType(),
+    long result = 0; // Used only by MS Windows
-                                                     event,
+    return QWindowSystemInterface::handleNativeEvent(window(), eventType, event, &result);
-                                                     result);
 }
 
 void QXcbWindow::handleExposeEvent(const xcb_expose_event_t *event)

src/plugins/platforms/xcb/qxcbwindow.h

@@ -121,7 +121,7 @@ public:
     QImage::Format imageFormat() const { return m_imageFormat; }
     bool imageNeedsRgbSwap() const { return m_imageRgbSwap; }
 
-    bool handleGenericEvent(xcb_generic_event_t *event, long *result)  override;
+    bool handleNativeEvent(xcb_generic_event_t *event)  override;
 
     void handleExposeEvent(const xcb_expose_event_t *event) override;
     void handleClientMessageEvent(const xcb_client_message_event_t *event) override;