From 313081c0703c66918e95640c74cd57312a1e8bba Mon Sep 17 00:00:00 2001 From: Jan-Marek Glogowski Date: Wed, 25 Mar 2020 18:33:29 +0100 Subject: [PATCH] tdf#131533 Qt5 defer dropping clipboard ownership This is maybe a Qt bug. Calling QClipboard::setMimeData from Qt5Clipboard::setContents after a previous call to QClipboard::clear results in a clipboard ownership failure. In a terminal you'll see: "QXcbClipboard::setMimeData: Cannot set X11 selection owner". Calling Application::Reschedule() after the clear() doesn't help. The result is a de-sync between the LO's clipboard state and the real clipboard state, which will lead to a crash on LO shutdown. I'm not sure this fix is correct. Maybe this could also be handled by some X11 flush operation. But it's the only working solution I could find: don't clear, if LO re-claims the ownership later. I tried to reproduce the ownership error by modifying the Qt fridgemagnets example, adding some QClipboard::clear and QClipboard::setMimeData calls to the drop handling, but couldn't reproduce. Maybe the dynamic Qt5MimeData object is also involved. Change-Id: I32b6575a78a4b10a2e2b7b189303ab3a40dc69ca Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90990 Tested-by: Jenkins Reviewed-by: Michael Weghorn Reviewed-by: Jan-Marek Glogowski --- vcl/inc/qt5/Qt5Clipboard.hxx | 6 ++++++ vcl/qt5/Qt5Clipboard.cxx | 23 +++++++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/vcl/inc/qt5/Qt5Clipboard.hxx b/vcl/inc/qt5/Qt5Clipboard.hxx index 122184b44942..b99534f59039 100644 --- a/vcl/inc/qt5/Qt5Clipboard.hxx +++ b/vcl/inc/qt5/Qt5Clipboard.hxx @@ -41,6 +41,8 @@ class Qt5Clipboard final // has to be set, if LO changes the QClipboard itself, so it won't instantly lose // ownership by it's self-triggered QClipboard::changed handler bool m_bOwnClipboardChange; + // true, if LO really wants to give up clipboard ownership + bool m_bDoClear; // if not empty, this holds the setContents provided XTransferable or a Qt5ClipboardTransferable css::uno::Reference m_aContents; @@ -55,6 +57,10 @@ class Qt5Clipboard final private Q_SLOTS: void handleChanged(QClipboard::Mode mode); + void handleClearClipboard(); + +signals: + void clearClipboard(); public: // factory function to construct only valid Qt5Clipboard objects by name diff --git a/vcl/qt5/Qt5Clipboard.cxx b/vcl/qt5/Qt5Clipboard.cxx index cadedbfd327e..8720cfe44310 100644 --- a/vcl/qt5/Qt5Clipboard.cxx +++ b/vcl/qt5/Qt5Clipboard.cxx @@ -30,11 +30,16 @@ Qt5Clipboard::Qt5Clipboard(const OUString& aModeString, const QClipboard::Mode a , m_aClipboardName(aModeString) , m_aClipboardMode(aMode) , m_bOwnClipboardChange(false) + , m_bDoClear(false) { assert(isSupported(m_aClipboardMode)); // DirectConnection guarantees the changed slot runs in the same thread as the QClipboard connect(QApplication::clipboard(), &QClipboard::changed, this, &Qt5Clipboard::handleChanged, Qt::DirectConnection); + + // explicitly queue an event, so we can eventually ignore it + connect(this, &Qt5Clipboard::clearClipboard, this, &Qt5Clipboard::handleClearClipboard, + Qt::QueuedConnection); } css::uno::Reference Qt5Clipboard::create(const OUString& aModeString) @@ -98,6 +103,13 @@ css::uno::Reference Qt5Clipboard::getContents( return m_aContents; } +void Qt5Clipboard::handleClearClipboard() +{ + if (!m_bDoClear) + return; + QApplication::clipboard()->clear(m_aClipboardMode); +} + void Qt5Clipboard::setContents( const css::uno::Reference& xTrans, const css::uno::Reference& xClipboardOwner) @@ -110,15 +122,18 @@ void Qt5Clipboard::setContents( m_aContents = xTrans; m_aOwner = xClipboardOwner; - m_bOwnClipboardChange = true; - if (m_aContents.is()) + m_bDoClear = !m_aContents.is(); + if (!m_bDoClear) + { + m_bOwnClipboardChange = true; QApplication::clipboard()->setMimeData(new Qt5MimeData(m_aContents), m_aClipboardMode); + m_bOwnClipboardChange = false; + } else { assert(!m_aOwner.is()); - QApplication::clipboard()->clear(m_aClipboardMode); + Q_EMIT clearClipboard(); } - m_bOwnClipboardChange = false; aGuard.clear();