From 422838685c31d9b57133a8711bfd5db92095d96d Mon Sep 17 00:00:00 2001
From: Robin Burchell <robin.burchell@viroteck.net>
Date: Wed, 7 Sep 2016 14:34:03 +0200
Subject: [PATCH 084/352] xcb: Treat bitmap cursors differently from shaped
 cursors

QXcbCursor had a "cache" of cursor handles. Unfortunately, as QXcbCursor has its
lifetime tied to the screen, this cache grew unbounded whenever the cursor was
set: this could be witnessed worst when repeatedly setting the current cursor to
a different pixmap each time.

We fix this by keeping the cursor cache only for the "regular" shaped cursors
that are often shared between windows, working on the assumption that custom
cursors are generally specific only to a given window. This makes the lifetime
of the bitmap cursors much more clear: they are tied to that window, and when
the window is destroyed (or changes cursor), so too is the bitmap cursor
destroyed (if set).

Reported-by: Will Thompson <wjt@endlessm.com>
Change-Id: Ia558d858ff49e89cd5220344567203eb0267a133
Reviewed-by: Uli Schlachter <psychon@znc.in>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
---
 src/plugins/platforms/xcb/qxcbcursor.cpp | 24 +++++++++++++++++-------
 src/plugins/platforms/xcb/qxcbwindow.cpp | 22 +++++++++++++++++++---
 src/plugins/platforms/xcb/qxcbwindow.h   |  3 ++-
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/src/plugins/platforms/xcb/qxcbcursor.cpp b/src/plugins/platforms/xcb/qxcbcursor.cpp
index b321ed9..4646ced 100644
--- a/src/plugins/platforms/xcb/qxcbcursor.cpp
+++ b/src/plugins/platforms/xcb/qxcbcursor.cpp
@@ -353,17 +353,27 @@ void QXcbCursor::changeCursor(QCursor *cursor, QWindow *widget)
         return;
 
     xcb_cursor_t c = XCB_CURSOR_NONE;
+    bool isBitmapCursor = false;
+
     if (cursor) {
-        const QXcbCursorCacheKey key(*cursor);
-        CursorHash::iterator it = m_cursorHash.find(key);
-        if (it == m_cursorHash.end()) {
-            const Qt::CursorShape shape = cursor->shape();
-            it = m_cursorHash.insert(key, shape == Qt::BitmapCursor ? createBitmapCursor(cursor) : createFontCursor(shape));
+        const Qt::CursorShape shape = cursor->shape();
+        isBitmapCursor = shape == Qt::BitmapCursor;
+
+        if (!isBitmapCursor) {
+            const QXcbCursorCacheKey key(*cursor);
+            CursorHash::iterator it = m_cursorHash.find(key);
+            if (it == m_cursorHash.end()) {
+                it = m_cursorHash.insert(key, createFontCursor(shape));
+            }
+            c = it.value();
+        } else {
+            // Do not cache bitmap cursors, as otherwise they have unclear
+            // lifetime (we effectively leak xcb_cursor_t).
+            c = createBitmapCursor(cursor);
         }
-        c = it.value();
     }
 
-    w->setCursor(c);
+    w->setCursor(c, isBitmapCursor);
 }
 
 static int cursorIdForShape(int cshape)
diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp
index d46228c..5f402b6 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.cpp
+++ b/src/plugins/platforms/xcb/qxcbwindow.cpp
@@ -302,6 +302,7 @@ QXcbWindow::QXcbWindow(QWindow *window)
     , m_lastWindowStateEvent(-1)
     , m_syncState(NoSyncNeeded)
     , m_pendingSyncRequest(0)
+    , m_currentBitmapCursor(XCB_CURSOR_NONE)
 {
     setConnection(xcbScreen()->connection());
 }
@@ -620,6 +621,9 @@ void QXcbWindow::create()
 
 QXcbWindow::~QXcbWindow()
 {
+    if (m_currentBitmapCursor != XCB_CURSOR_NONE) {
+        xcb_free_cursor(xcb_connection(), m_currentBitmapCursor);
+    }
     if (window()->type() != Qt::ForeignWindow)
         destroy();
     else {
@@ -2665,10 +2669,22 @@ bool QXcbWindow::setMouseGrabEnabled(bool grab)
     return result;
 }
 
-void QXcbWindow::setCursor(xcb_cursor_t cursor)
+void QXcbWindow::setCursor(xcb_cursor_t cursor, bool isBitmapCursor)
 {
-    xcb_change_window_attributes(xcb_connection(), m_window, XCB_CW_CURSOR, &cursor);
-    xcb_flush(xcb_connection());
+    xcb_connection_t *conn = xcb_connection();
+
+    xcb_change_window_attributes(conn, m_window, XCB_CW_CURSOR, &cursor);
+    xcb_flush(conn);
+
+    if (m_currentBitmapCursor != XCB_CURSOR_NONE) {
+        xcb_free_cursor(conn, m_currentBitmapCursor);
+    }
+
+    if (isBitmapCursor) {
+        m_currentBitmapCursor = cursor;
+    } else {
+        m_currentBitmapCursor = XCB_CURSOR_NONE;
+    }
 }
 
 void QXcbWindow::windowEvent(QEvent *event)
diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h
index b8bcf44..f2b6904 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.h
+++ b/src/plugins/platforms/xcb/qxcbwindow.h
@@ -97,7 +97,7 @@ public:
     bool setKeyboardGrabEnabled(bool grab) Q_DECL_OVERRIDE;
     bool setMouseGrabEnabled(bool grab) Q_DECL_OVERRIDE;
 
-    void setCursor(xcb_cursor_t cursor);
+    void setCursor(xcb_cursor_t cursor, bool isBitmapCursor);
 
     QSurfaceFormat format() const Q_DECL_OVERRIDE;
 
@@ -263,6 +263,7 @@ protected:
     SyncState m_syncState;
 
     QXcbSyncWindowRequest *m_pendingSyncRequest;
+    xcb_cursor_t m_currentBitmapCursor;
 };
 
 QT_END_NAMESPACE
-- 
2.9.3