xcb: Use functors in QXcbConnection::checkEvent()

... to check for buffered events. This makes the code less verbose and
easier to read. Changed the filter signature to pass an event type in addition
to the actual event, for the convenience of API user. And do not pass worthless
nullptr-s to the filter.

The only reason why KeyChecker from qxcbkeyboard.cpp was not converted
to lambda expression is that the code looks suspicious - KeyChecker::m_release
default value is 'true' and I don't see where it would ever be assigned
'false' (ref. QTBUG-69679) and the code is known to be buggy (ref. QTBUG-57335).
Those issues are out-of-scope for this patch.

Change-Id: If2fdd60fbb93eb983f3c9ad616aaf04834fede9f
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Mikhail Svetkin <mikhail.svetkin@qt.io>

8 files changed, 80 insertions, 178 deletions

diff --git a/src/plugins/platforms/xcb/qxcbclipboard.cpp b/src/plugins/platforms/xcb/qxcbclipboard.cpp
index 24eb13326c..7b1cca4070 100644
--- a/src/plugins/platforms/xcb/qxcbclipboard.cpp
+++ b/src/plugins/platforms/xcb/qxcbclipboard.cpp
@@ -807,73 +807,44 @@ bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property,
     return ok;
 }
 
-
-namespace
+xcb_generic_event_t *QXcbClipboard::waitForClipboardEvent(xcb_window_t window, int type, int timeout, bool checkManager)
 {
-    class Notify {
-    public:
-        Notify(xcb_window_t win, int t)
-            : window(win), type(t) {}
-        xcb_window_t window;
-        int type;
-        bool checkEvent(xcb_generic_event_t *event) const {
-            if (!event)
-                return false;
-            int t = event->response_type & 0x7f;
-            if (t != type)
+    QElapsedTimer timer;
+    timer.start();
+    do {
+        auto e = connection()->checkEvent([window, type](xcb_generic_event_t *event, int eventType) {
+            if (eventType != type)
                 return false;
-            if (t == XCB_PROPERTY_NOTIFY) {
-                xcb_property_notify_event_t *pn = (xcb_property_notify_event_t *)event;
-                if (pn->window == window)
+            if (eventType == XCB_PROPERTY_NOTIFY) {
+                auto propertyNotify = reinterpret_cast<xcb_property_notify_event_t *>(event);
+                if (propertyNotify->window == window)
                     return true;
-            } else if (t == XCB_SELECTION_NOTIFY) {
-                xcb_selection_notify_event_t *sn = (xcb_selection_notify_event_t *)event;
-                if (sn->requestor == window)
+            } else if (eventType == XCB_SELECTION_NOTIFY) {
+                auto selectionNotify = reinterpret_cast<xcb_selection_notify_event_t *>(event);
+                if (selectionNotify->requestor == window)
                     return true;
             }
             return false;
-        }
-    };
-    class ClipboardEvent {
-    public:
-        ClipboardEvent(QXcbConnection *c)
-        { clipboard = c->internAtom("CLIPBOARD"); }
-        xcb_atom_t clipboard;
-        bool checkEvent(xcb_generic_event_t *e) const {
-            if (!e)
-                return false;
-            int type = e->response_type & 0x7f;
-            if (type == XCB_SELECTION_REQUEST) {
-                xcb_selection_request_event_t *sr = (xcb_selection_request_event_t *)e;
-                return sr->selection == XCB_ATOM_PRIMARY || sr->selection == clipboard;
-            } else if (type == XCB_SELECTION_CLEAR) {
-                xcb_selection_clear_event_t *sc = (xcb_selection_clear_event_t *)e;
-                return sc->selection == XCB_ATOM_PRIMARY || sc->selection == clipboard;
-            }
-            return false;
-        }
-    };
-}
-
-xcb_generic_event_t *QXcbClipboard::waitForClipboardEvent(xcb_window_t win, int type, int timeout, bool checkManager)
-{
-    QElapsedTimer timer;
-    timer.start();
-    do {
-        Notify notify(win, type);
-        xcb_generic_event_t *e = connection()->checkEvent(notify);
-        if (e)
+        });
+        if (e) // found the waited for event
             return e;
 
         if (checkManager) {
             auto reply = Q_XCB_REPLY(xcb_get_selection_owner, xcb_connection(), atom(QXcbAtom::CLIPBOARD_MANAGER));
             if (!reply || reply->owner == XCB_NONE)
-                return 0;
+                return nullptr;
         }
 
         // process other clipboard events, since someone is probably requesting data from us
-        ClipboardEvent clipboard(connection());
-        e = connection()->checkEvent(clipboard);
+        auto clipboardAtom = connection()->internAtom("CLIPBOARD");
+        e = connection()->checkEvent([clipboardAtom](xcb_generic_event_t *event, int type) {
+            xcb_atom_t selection = XCB_ATOM_NONE;
+            if (type == XCB_SELECTION_REQUEST)
+                selection = reinterpret_cast<xcb_selection_request_event_t *>(event)->selection;
+            else if (type == XCB_SELECTION_CLEAR)
+                selection = reinterpret_cast<xcb_selection_clear_event_t *>(event)->selection;
+            return selection == XCB_ATOM_PRIMARY || selection == clipboardAtom;
+        });
         if (e) {
             connection()->handleXcbEvent(e);
             free(e);
@@ -885,7 +856,7 @@ xcb_generic_event_t *QXcbClipboard::waitForClipboardEvent(xcb_window_t win, int
         QThread::msleep(50);
     } while (timer.elapsed() < timeout);
 
-    return 0;
+    return nullptr;
 }
 
 QByteArray QXcbClipboard::clipboardReadIncrementalProperty(xcb_window_t win, xcb_atom_t property, int nbytes, bool nullterm)
diff --git a/src/plugins/platforms/xcb/qxcbclipboard.h b/src/plugins/platforms/xcb/qxcbclipboard.h
index bfeae13e10..8d0df39ac8 100644
--- a/src/plugins/platforms/xcb/qxcbclipboard.h
+++ b/src/plugins/platforms/xcb/qxcbclipboard.h
@@ -89,7 +89,7 @@ public:
     QByteArray getSelection(xcb_atom_t selection, xcb_atom_t target, xcb_atom_t property, xcb_timestamp_t t = 0);
 
 private:
-    xcb_generic_event_t *waitForClipboardEvent(xcb_window_t win, int type, int timeout, bool checkManager = false);
+    xcb_generic_event_t *waitForClipboardEvent(xcb_window_t window, int type, int timeout, bool checkManager = false);
 
     xcb_atom_t sendTargetsSelection(QMimeData *d, xcb_window_t window, xcb_atom_t property);
     xcb_atom_t sendSelection(QMimeData *d, xcb_atom_t target, xcb_window_t window, xcb_atom_t property);
diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp
index c14f3f3703..1a5d9d1667 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection.cpp
@@ -1484,54 +1484,35 @@ void QXcbConnection::sendConnectionEvent(QXcbAtom::Atom a, uint id)
     xcb_flush(xcb_connection());
 }
 
-namespace
-{
-    class PropertyNotifyEvent {
-    public:
-        PropertyNotifyEvent(xcb_window_t win, xcb_atom_t property)
-            : window(win), type(XCB_PROPERTY_NOTIFY), atom(property) {}
-        xcb_window_t window;
-        int type;
-        xcb_atom_t atom;
-        bool checkEvent(xcb_generic_event_t *event) const {
-            if (!event)
-                return false;
-            if ((event->response_type & ~0x80) != type) {
-                return false;
-            } else {
-                xcb_property_notify_event_t *pn = reinterpret_cast<xcb_property_notify_event_t *>(event);
-                if ((pn->window == window) && (pn->atom == atom))
-                    return true;
-            }
-            return false;
-        }
-    };
-}
-
 xcb_timestamp_t QXcbConnection::getTimestamp()
 {
     // send a dummy event to myself to get the timestamp from X server.
-    xcb_window_t root_win = rootWindow();
-    xcb_change_property(xcb_connection(), XCB_PROP_MODE_APPEND, root_win, atom(QXcbAtom::CLIP_TEMPORARY),
-                        XCB_ATOM_INTEGER, 32, 0, NULL);
+    xcb_window_t window = rootWindow();
+    xcb_atom_t dummyAtom = atom(QXcbAtom::CLIP_TEMPORARY);
+    xcb_change_property(xcb_connection(), XCB_PROP_MODE_APPEND, window, dummyAtom,
+                        XCB_ATOM_INTEGER, 32, 0, nullptr);
 
     connection()->flush();
-    PropertyNotifyEvent checker(root_win, atom(QXcbAtom::CLIP_TEMPORARY));
 
-    xcb_generic_event_t *event = 0;
+    xcb_generic_event_t *event = nullptr;
     // lets keep this inside a loop to avoid a possible race condition, where
     // reader thread has not yet had the time to acquire the mutex in order
     // to add the new set of events to its event queue
     while (!event) {
         connection()->sync();
-        event = checkEvent(checker);
+        event = checkEvent([window, dummyAtom](xcb_generic_event_t *event, int type) {
+            if (type != XCB_PROPERTY_NOTIFY)
+                return false;
+            auto propertyNotify = reinterpret_cast<xcb_property_notify_event_t *>(event);
+            return propertyNotify->window == window && propertyNotify->atom == dummyAtom;
+        });
     }
 
     xcb_property_notify_event_t *pn = reinterpret_cast<xcb_property_notify_event_t *>(event);
     xcb_timestamp_t timestamp = pn->time;
     free(event);
 
-    xcb_delete_property(xcb_connection(), root_win, atom(QXcbAtom::CLIP_TEMPORARY));
+    xcb_delete_property(xcb_connection(), window, dummyAtom);
 
     return timestamp;
 }
diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
index 0b31e9c3e7..24719a6c31 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.h
+++ b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -440,8 +440,8 @@ public:
     QXcbWindowEventListener *windowEventListenerFromId(xcb_window_t id);
     QXcbWindow *platformWindowFromId(xcb_window_t id);
 
-    template<typename T>
-    inline xcb_generic_event_t *checkEvent(T &checker);
+    template<typename Functor>
+    inline xcb_generic_event_t *checkEvent(Functor &&filter);
 
     typedef bool (*PeekFunc)(QXcbConnection *, xcb_generic_event_t *);
     void addPeekFunc(PeekFunc f);
@@ -734,21 +734,21 @@ Q_DECLARE_TYPEINFO(QXcbConnection::TabletData, Q_MOVABLE_TYPE);
 #endif
 #endif
 
-template<typename T>
-xcb_generic_event_t *QXcbConnection::checkEvent(T &checker)
+template<typename Functor>
+xcb_generic_event_t *QXcbConnection::checkEvent(Functor &&filter)
 {
     QXcbEventArray *eventqueue = m_reader->lock();
 
     for (int i = 0; i < eventqueue->size(); ++i) {
         xcb_generic_event_t *event = eventqueue->at(i);
-        if (checker.checkEvent(event)) {
-            (*eventqueue)[i] = 0;
+        if (event && filter(event, event->response_type & ~0x80)) {
+            (*eventqueue)[i] = nullptr;
             m_reader->unlock();
             return event;
         }
     }
     m_reader->unlock();
-    return 0;
+    return nullptr;
 }
 
 class QXcbConnectionGrabber
diff --git a/src/plugins/platforms/xcb/qxcbdrag.cpp b/src/plugins/platforms/xcb/qxcbdrag.cpp
index 0d72da0701..2b8e507f30 100644
--- a/src/plugins/platforms/xcb/qxcbdrag.cpp
+++ b/src/plugins/platforms/xcb/qxcbdrag.cpp
@@ -781,12 +781,11 @@ namespace
     public:
         ClientMessageScanner(xcb_atom_t a) : atom(a) {}
         xcb_atom_t atom;
-        bool checkEvent(xcb_generic_event_t *event) const {
-            if (!event)
+        bool operator() (xcb_generic_event_t *event, int type) const {
+            if (type != XCB_CLIENT_MESSAGE)
                 return false;
-            if ((event->response_type & 0x7f) != XCB_CLIENT_MESSAGE)
-                return false;
-            return ((xcb_client_message_event_t *)event)->type == atom;
+            auto clientMessage = reinterpret_cast<xcb_client_message_event_t *>(event);
+            return clientMessage->type == atom;
         }
     };
 }
@@ -794,12 +793,11 @@ namespace
 void QXcbDrag::handlePosition(QPlatformWindow * w, const xcb_client_message_event_t *event)
 {
     xcb_client_message_event_t *lastEvent = const_cast<xcb_client_message_event_t *>(event);
-    xcb_generic_event_t *nextEvent;
     ClientMessageScanner scanner(atom(QXcbAtom::XdndPosition));
-    while ((nextEvent = connection()->checkEvent(scanner))) {
+    while (auto nextEvent = connection()->checkEvent(scanner)) {
         if (lastEvent != event)
             free(lastEvent);
-        lastEvent = (xcb_client_message_event_t *)nextEvent;
+        lastEvent = reinterpret_cast<xcb_client_message_event_t *>(nextEvent);
     }
 
     handle_xdnd_position(w, lastEvent);
diff --git a/src/plugins/platforms/xcb/qxcbkeyboard.cpp b/src/plugins/platforms/xcb/qxcbkeyboard.cpp
index 5a2dac4a5a..37c8def6cc 100644
--- a/src/plugins/platforms/xcb/qxcbkeyboard.cpp
+++ b/src/plugins/platforms/xcb/qxcbkeyboard.cpp
@@ -1485,12 +1485,11 @@ public:
     {
     }
 
-    bool checkEvent(xcb_generic_event_t *ev)
+    bool operator() (xcb_generic_event_t *ev, int type)
     {
         if (m_error || !ev)
             return false;
 
-        int type = ev->response_type & ~0x80;
         if (type != XCB_KEY_PRESS && type != XCB_KEY_RELEASE)
             return false;
 
@@ -1517,9 +1516,6 @@ public:
         return false;
     }
 
-    bool release() const { return m_release; }
-    xcb_timestamp_t time() const { return m_time; }
-
 private:
     xcb_window_t m_window;
     xcb_keycode_t m_code;
diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp
index e313e2f648..9c589771d0 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.cpp
+++ b/src/plugins/platforms/xcb/qxcbwindow.cpp
@@ -1809,53 +1809,6 @@ bool QXcbWindow::requestSystemTrayWindowDock()
     return true;
 }
 
-class ExposeCompressor
-{
-public:
-    ExposeCompressor(xcb_window_t window, QRegion *region)
-        : m_window(window)
-        , m_region(region)
-        , m_pending(true)
-    {
-    }
-
-    bool checkEvent(xcb_generic_event_t *event)
-    {
-        if (!event)
-            return false;
-        if ((event->response_type & ~0x80) != XCB_EXPOSE)
-            return false;
-        xcb_expose_event_t *expose = (xcb_expose_event_t *)event;
-        if (expose->window != m_window)
-            return false;
-        if (expose->count == 0)
-            m_pending = false;
-        *m_region |= QRect(expose->x, expose->y, expose->width, expose->height);
-        return true;
-    }
-
-    bool pending() const
-    {
-        return m_pending;
-    }
-
-private:
-    xcb_window_t m_window;
-    QRegion *m_region;
-    bool m_pending;
-};
-
-bool QXcbWindow::compressExposeEvent(QRegion &exposeRegion)
-{
-    ExposeCompressor compressor(m_window, &exposeRegion);
-    xcb_generic_event_t *filter = 0;
-    do {
-        filter = connection()->checkEvent(compressor);
-        free(filter);
-    } while (filter);
-    return compressor.pending();
-}
-
 bool QXcbWindow::handleGenericEvent(xcb_generic_event_t *event, long *result)
 {
     return QWindowSystemInterface::handleNativeEvent(window(),
@@ -1867,9 +1820,24 @@ bool QXcbWindow::handleGenericEvent(xcb_generic_event_t *event, long *result)
 void QXcbWindow::handleExposeEvent(const xcb_expose_event_t *event)
 {
     QRect rect(event->x, event->y, event->width, event->height);
-
     m_exposeRegion |= rect;
-    bool pending = compressExposeEvent(m_exposeRegion);
+
+    bool pending = true;
+    xcb_generic_event_t *e = nullptr;
+    do { // compress expose events
+        e = connection()->checkEvent([this, &pending](xcb_generic_event_t *event, int type) {
+            if (type != XCB_EXPOSE)
+                return false;
+            auto expose = reinterpret_cast<xcb_expose_event_t *>(event);
+            if (expose->window != m_window)
+                return false;
+            if (expose->count == 0)
+                pending = false;
+            m_exposeRegion |= QRect(expose->x, expose->y, expose->width, expose->height);
+            return true;
+        });
+        free(e);
+    } while (e);
 
     // if count is non-zero there are more expose events pending
     if (event->count == 0 || !pending) {
@@ -2162,24 +2130,6 @@ static bool ignoreEnterEvent(quint8 mode, quint8 detail, QXcbConnection *conn =
             || detail == XCB_NOTIFY_DETAIL_NONLINEAR_VIRTUAL);
 }
 
-class EnterEventChecker
-{
-public:
-    bool checkEvent(xcb_generic_event_t *event)
-    {
-        if (!event)
-            return false;
-        if ((event->response_type & ~0x80) != XCB_ENTER_NOTIFY)
-            return false;
-
-        xcb_enter_notify_event_t *enter = (xcb_enter_notify_event_t *)event;
-        if (ignoreEnterEvent(enter->mode, enter->detail))
-            return false;
-
-        return true;
-    }
-};
-
 void QXcbWindow::handleEnterNotifyEvent(int event_x, int event_y, int root_x, int root_y,
                                         quint8 mode, quint8 detail, xcb_timestamp_t timestamp)
 {
@@ -2206,9 +2156,17 @@ void QXcbWindow::handleLeaveNotifyEvent(int root_x, int root_y,
     if (ignoreLeaveEvent(mode, detail, connection()) || connection()->mousePressWindow())
         return;
 
-    EnterEventChecker checker;
-    xcb_enter_notify_event_t *enter = (xcb_enter_notify_event_t *)connection()->checkEvent(checker);
-    QXcbWindow *enterWindow = enter ? connection()->platformWindowFromId(enter->event) : 0;
+    // check if enter event is buffered
+    auto event = connection()->checkEvent([](xcb_generic_event_t *event, int type) {
+        if (type != XCB_ENTER_NOTIFY)
+            return false;
+        auto enter = reinterpret_cast<xcb_enter_notify_event_t *>(event);
+        if (ignoreEnterEvent(enter->mode, enter->detail))
+            return false;
+        return true;
+    });
+    auto enter = reinterpret_cast<xcb_enter_notify_event_t *>(event);
+    QXcbWindow *enterWindow = enter ? connection()->platformWindowFromId(enter->event) : nullptr;
 
     if (enterWindow) {
         QPoint local(enter->event_x, enter->event_y);
diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h
index 047ee2eae9..128c6e126d 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.h
+++ b/src/plugins/platforms/xcb/qxcbwindow.h
@@ -217,8 +217,6 @@ protected:
     void doFocusIn();
     void doFocusOut();
 
-    bool compressExposeEvent(QRegion &exposeRegion);
-
     void handleButtonPressEvent(int event_x, int event_y, int root_x, int root_y,
                                 int detail, Qt::KeyboardModifiers modifiers, xcb_timestamp_t timestamp,
                                 QEvent::Type type, Qt::MouseEventSource source = Qt::MouseEventNotSynthesized);