xcb: move clipboard code out of QXcbConnection::processXcbEvents

... to QXcbConnection::handleXcbEvent(), which is where it belongs.
This patch amends bc6f5b3ff61f4b1dea14084349702f2895feda66 (Sep, 2013).

And some other design cleanups.

Change-Id: Iefa0793c58de16a59d2294f38311e1e8dfa3035b
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

src/plugins/platforms/xcb/qxcbclipboard.cpp

@@ -153,114 +153,71 @@ private:
     QByteArray format_atoms;
 };
 
-namespace {
+QXcbClipboardTransaction::QXcbClipboardTransaction(QXcbClipboard *clipboard, xcb_window_t w,
-class INCRTransaction;
+                                               xcb_atom_t p, QByteArray d, xcb_atom_t t, int f)
-typedef QMap<xcb_window_t,INCRTransaction*> TransactionMap;
+    : m_clipboard(clipboard), m_window(w), m_property(p), m_data(d), m_target(t), m_format(f)
-static TransactionMap *transactions = 0;
-
-//#define INCR_DEBUG
-
-class INCRTransaction : public QObject
 {
-    Q_OBJECT
+    const quint32 values[] = { XCB_EVENT_MASK_PROPERTY_CHANGE };
-public:
+    xcb_change_window_attributes(m_clipboard->xcb_connection(), m_window,
-    INCRTransaction(QXcbConnection *c, xcb_window_t w, xcb_atom_t p,
+                                 XCB_CW_EVENT_MASK, values);
-                    QByteArray d, uint i, xcb_atom_t t, int f, int to) :
+
-        conn(c), win(w), property(p), data(d), increment(i),
+    m_abortTimerId = startTimer(m_clipboard->clipboardTimeout());
-        target(t), format(f), timeout(to), offset(0)
+}
-    {
+
-        const quint32 values[] = { XCB_EVENT_MASK_PROPERTY_CHANGE };
+QXcbClipboardTransaction::~QXcbClipboardTransaction()
-        xcb_change_window_attributes(conn->xcb_connection(), win,
+{
+    if (m_abortTimerId)
+        killTimer(m_abortTimerId);
+    m_abortTimerId = 0;
+    m_clipboard->removeTransaction(m_window);
+}
+
+bool QXcbClipboardTransaction::updateIncrementalProperty(const xcb_property_notify_event_t *event)
+{
+    if (event->atom != m_property || event->state != XCB_PROPERTY_DELETE)
+        return false;
+
+    // restart the timer
+    if (m_abortTimerId)
+        killTimer(m_abortTimerId);
+    m_abortTimerId = startTimer(m_clipboard->clipboardTimeout());
+
+    uint bytes_left = uint(m_data.size()) - m_offset;
+    if (bytes_left > 0) {
+        int increment = m_clipboard->increment();
+        uint bytes_to_send = qMin(uint(increment), bytes_left);
+
+        qCDebug(lcQpaClipboard, "sending %d bytes, %d remaining, transaction: %p)",
+                bytes_to_send, bytes_left - bytes_to_send, this);
+
+        uint32_t dataSize = bytes_to_send / (m_format / 8);
+        xcb_change_property(m_clipboard->xcb_connection(), XCB_PROP_MODE_REPLACE, m_window,
+                            m_property, m_target, m_format, dataSize, m_data.constData() + m_offset);
+        m_offset += bytes_to_send;
+    } else {
+        qCDebug(lcQpaClipboard, "transaction %p completed", this);
+
+        xcb_change_property(m_clipboard->xcb_connection(), XCB_PROP_MODE_REPLACE, m_window,
+                            m_property, m_target, m_format, 0, nullptr);
+
+        const quint32 values[] = { XCB_EVENT_MASK_NO_EVENT };
+        xcb_change_window_attributes(m_clipboard->xcb_connection(), m_window,
                                      XCB_CW_EVENT_MASK, values);
-        if (!transactions) {
+        delete this; // self destroy
-#ifdef INCR_DEBUG
-            qDebug("INCRTransaction: creating the TransactionMap");
-#endif
-            transactions = new TransactionMap;
-            conn->clipboard()->setProcessIncr(true);
-        }
-        transactions->insert(win, this);
-        abort_timer = startTimer(timeout);
     }
+    return true;
+}
 
-    ~INCRTransaction()
+
-    {
+void QXcbClipboardTransaction::timerEvent(QTimerEvent *ev)
-        if (abort_timer)
+{
-            killTimer(abort_timer);
+    if (ev->timerId() == m_abortTimerId) {
-        abort_timer = 0;
+        // this can happen when the X client we are sending data
-        transactions->remove(win);
+        // to decides to exit (normally or abnormally)
-        if (transactions->isEmpty()) {
+        qCDebug(lcQpaClipboard, "timed out while sending data to %p", this);
-#ifdef INCR_DEBUG
+        delete this; // self destroy
-            qDebug("INCRTransaction: no more INCR transactions left in the TransactionMap");
-#endif
-            delete transactions;
-            transactions = 0;
-            conn->clipboard()->setProcessIncr(false);
-        }
     }
-
+}
-    void updateIncrProperty(xcb_property_notify_event_t *event, bool &accepted)
-    {
-        xcb_connection_t *c = conn->xcb_connection();
-        if (event->atom == property && event->state == XCB_PROPERTY_DELETE) {
-            accepted = true;
-            // restart the timer
-            if (abort_timer)
-                killTimer(abort_timer);
-            abort_timer = startTimer(timeout);
-
-            unsigned int bytes_left = data.size() - offset;
-            if (bytes_left > 0) {
-                unsigned int bytes_to_send = qMin(increment, bytes_left);
-#ifdef INCR_DEBUG
-                qDebug("INCRTransaction: sending %d bytes, %d remaining (INCR transaction %p)",
-                       bytes_to_send, bytes_left - bytes_to_send, this);
-#endif
-                int dataSize = bytes_to_send / (format / 8);
-                xcb_change_property(c, XCB_PROP_MODE_REPLACE, win, property,
-                                    target, format, dataSize, data.constData() + offset);
-                offset += bytes_to_send;
-            } else {
-#ifdef INCR_DEBUG
-                qDebug("INCRTransaction: INCR transaction %p completed", this);
-#endif
-                xcb_change_property(c, XCB_PROP_MODE_REPLACE, win, property,
-                                    target, format, 0, (const void *)0);
-                const quint32 values[] = { XCB_EVENT_MASK_NO_EVENT };
-                xcb_change_window_attributes(conn->xcb_connection(), win,
-                                             XCB_CW_EVENT_MASK, values);
-                // self destroy
-                delete this;
-            }
-        }
-    }
-
-protected:
-    void timerEvent(QTimerEvent *ev) override
-    {
-        if (ev->timerId() == abort_timer) {
-            // this can happen when the X client we are sending data
-            // to decides to exit (normally or abnormally)
-#ifdef INCR_DEBUG
-            qDebug("INCRTransaction: Timed out while sending data to %p", this);
-#endif
-            delete this;
-        }
-    }
-
-private:
-    QXcbConnection *conn;
-    xcb_window_t win;
-    xcb_atom_t property;
-    QByteArray data;
-    uint increment;
-    xcb_atom_t target;
-    int format;
-    int timeout;
-    uint offset;
-    int abort_timer;
-};
-} // unnamed namespace
 
 const int QXcbClipboard::clipboard_timeout = 5000;
 
@@ -282,6 +239,9 @@ QXcbClipboard::QXcbClipboard(QXcbConnection *c)
         xcb_xfixes_select_selection_input_checked(xcb_connection(), m_owner, XCB_ATOM_PRIMARY, mask);
         xcb_xfixes_select_selection_input_checked(xcb_connection(), m_owner, atom(QXcbAtom::CLIPBOARD), mask);
     }
+
+    // change property protocol request is 24 bytes
+    m_increment = (xcb_get_maximum_request_length(xcb_connection()) * 4) - 24;
 }
 
 QXcbClipboard::~QXcbClipboard()
@@ -313,16 +273,17 @@ QXcbClipboard::~QXcbClipboard()
     delete m_clientClipboard[QClipboard::Selection];
 }
 
-void QXcbClipboard::incrTransactionPeeker(xcb_generic_event_t *ge, bool &accepted)
+bool QXcbClipboard::handlePropertyNotify(const xcb_generic_event_t *event)
 {
-    uint response_type = ge->response_type & ~0x80;
+    if (m_transactions.isEmpty() || event->response_type != XCB_PROPERTY_NOTIFY)
-    if (response_type == XCB_PROPERTY_NOTIFY) {
+        return false;
-        xcb_property_notify_event_t *event = (xcb_property_notify_event_t *)ge;
+
-        TransactionMap::Iterator it = transactions->find(event->window);
+    auto propertyNotify = reinterpret_cast<const xcb_property_notify_event_t *>(event);
-        if (it != transactions->end()) {
+    TransactionMap::Iterator it = m_transactions.find(propertyNotify->window);
-            (*it)->updateIncrProperty(event, accepted);
+    if (it == m_transactions.constEnd())
-        }
+        return false;
-    }
+
+    return (*it)->updateIncrementalProperty(propertyNotify);
 }
 
 xcb_window_t QXcbClipboard::getSelectionOwner(xcb_atom_t atom) const
@@ -522,19 +483,18 @@ xcb_atom_t QXcbClipboard::sendSelection(QMimeData *d, xcb_atom_t target, xcb_win
         // This 'bool' can be removed once there is a proper fix for QTBUG-32853
         if (m_clipboard_closing)
             allow_incr = false;
-        // X_ChangeProperty protocol request is 24 bytes
+
-        const int increment = (xcb_get_maximum_request_length(xcb_connection()) * 4) - 24;
+        if (data.size() > m_increment && allow_incr) {
-        if (data.size() > increment && allow_incr) {
             long bytes = data.size();
             xcb_change_property(xcb_connection(), XCB_PROP_MODE_REPLACE, window, property,
                                 atom(QXcbAtom::INCR), 32, 1, (const void *)&bytes);
-            new INCRTransaction(connection(), window, property, data, increment,
+            auto transaction = new QXcbClipboardTransaction(this, window, property, data, atomFormat, dataFormat);
-                                atomFormat, dataFormat, clipboard_timeout);
+            m_transactions.insert(window, transaction);
             return property;
         }
 
         // make sure we can perform the XChangeProperty in a single request
-        if (data.size() > increment)
+        if (data.size() > m_increment)
             return XCB_NONE; // ### perhaps use several XChangeProperty calls w/ PropModeAppend?
         int dataSize = data.size() / (dataFormat / 8);
         // use a single request to transfer data

src/plugins/platforms/xcb/qxcbclipboard.h

@@ -45,14 +45,41 @@
 #include <xcb/xcb.h>
 #include <xcb/xfixes.h>
 
+#include <QtCore/QObject>
+
 QT_BEGIN_NAMESPACE
 
 #ifndef QT_NO_CLIPBOARD
 
 class QXcbConnection;
 class QXcbScreen;
+class QXcbClipboard;
 class QXcbClipboardMime;
 
+class QXcbClipboardTransaction : public QObject
+{
+    Q_OBJECT
+public:
+    QXcbClipboardTransaction(QXcbClipboard *clipboard, xcb_window_t w, xcb_atom_t p,
+                           QByteArray d, xcb_atom_t t, int f);
+    ~QXcbClipboardTransaction();
+
+    bool updateIncrementalProperty(const xcb_property_notify_event_t *event);
+
+protected:
+    void timerEvent(QTimerEvent *ev) override;
+
+private:
+    QXcbClipboard *m_clipboard;
+    xcb_window_t m_window;
+    xcb_atom_t m_property;
+    QByteArray m_data;
+    xcb_atom_t m_target;
+    uint8_t m_format;
+    uint m_offset = 0;
+    int m_abortTimerId = 0;
+};
+
 class QXcbClipboard : public QXcbObject, public QPlatformClipboard
 {
 public:
@@ -81,13 +108,16 @@ public:
 
     QByteArray getDataInFormat(xcb_atom_t modeAtom, xcb_atom_t fmtatom);
 
-    void setProcessIncr(bool process) { m_incr_active = process; }
+    bool handlePropertyNotify(const xcb_generic_event_t *event);
-    bool processIncr() { return m_incr_active; }
-    void incrTransactionPeeker(xcb_generic_event_t *ge, bool &accepted);
 
     xcb_window_t getSelectionOwner(xcb_atom_t atom) const;
     QByteArray getSelection(xcb_atom_t selection, xcb_atom_t target, xcb_atom_t property, xcb_timestamp_t t = 0);
 
+    int increment() const { return m_increment; }
+    int clipboardTimeout() const { return clipboard_timeout; }
+
+    void removeTransaction(xcb_window_t window) { m_transactions.remove(window); }
+
 private:
     xcb_generic_event_t *waitForClipboardEvent(xcb_window_t window, int type, bool checkManager = false);
 
@@ -107,9 +137,12 @@ private:
 
     static const int clipboard_timeout;
 
-    bool m_incr_active = false;
+    int m_increment = 0;
     bool m_clipboard_closing = false;
     xcb_timestamp_t m_incr_receive_time = 0;
+
+    using TransactionMap = QMap<xcb_window_t, QXcbClipboardTransaction *>;
+    TransactionMap m_transactions;
 };
 
 #endif // QT_NO_CLIPBOARD

src/plugins/platforms/xcb/qxcbconnection.cpp

@@ -84,6 +84,7 @@ Q_LOGGING_CATEGORY(lcQpaEvents, "qt.qpa.events")
 Q_LOGGING_CATEGORY(lcQpaEventReader, "qt.qpa.events.reader")
 Q_LOGGING_CATEGORY(lcQpaPeeker, "qt.qpa.peeker")
 Q_LOGGING_CATEGORY(lcQpaKeyboard, "qt.qpa.xkeyboard")
+Q_LOGGING_CATEGORY(lcQpaClipboard, "qt.qpa.clipboard")
 Q_LOGGING_CATEGORY(lcQpaXDnd, "qt.qpa.xdnd")
 
 // this event type was added in libxcb 1.10,
@@ -651,6 +652,10 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
         break;
     case XCB_PROPERTY_NOTIFY:
     {
+#ifndef QT_NO_CLIPBOARD
+        if (m_clipboard->handlePropertyNotify(event))
+            break;
+#endif
         auto propertyNotify = reinterpret_cast<xcb_property_notify_event_t *>(event);
         if (propertyNotify->atom == atom(QXcbAtom::_NET_WORKAREA)) {
             QXcbVirtualDesktop *virtualDesktop = virtualDesktopForRootWindow(propertyNotify->window);
@@ -1010,14 +1015,6 @@ void QXcbConnection::processXcbEvents(QEventLoop::ProcessEventsFlags flags)
         if (compressEvent(event))
             continue;
 
-#ifndef QT_NO_CLIPBOARD
-        bool accepted = false;
-        if (clipboard()->processIncr())
-            clipboard()->incrTransactionPeeker(event, accepted);
-        if (accepted)
-            continue;
-#endif
-
         auto isWaitingFor = [=](PeekFunc peekFunc) {
             // These callbacks return true if the event is what they were
             // waiting for, remove them from the list in that case.

src/plugins/platforms/xcb/qxcbconnection.h

@@ -68,6 +68,7 @@ Q_DECLARE_LOGGING_CATEGORY(lcQpaScreen)
 Q_DECLARE_LOGGING_CATEGORY(lcQpaEvents)
 Q_DECLARE_LOGGING_CATEGORY(lcQpaPeeker)
 Q_DECLARE_LOGGING_CATEGORY(lcQpaKeyboard)
+Q_DECLARE_LOGGING_CATEGORY(lcQpaClipboard)
 Q_DECLARE_LOGGING_CATEGORY(lcQpaXDnd)
 Q_DECLARE_LOGGING_CATEGORY(lcQpaEventReader)