[Libreoffice-commits] core.git: Branch 'libreoffice-6-4' - vcl/inc vcl/qt5

[Jan-Marek Glogowski (via logerrit)]

 vcl/inc/qt5/Qt5Clipboard.hxx |    6 ++++++
 vcl/qt5/Qt5Clipboard.cxx     |   23 +++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

New commits:
commit dbe45b7930dc027cf6d143dbb34390c469808e10
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Wed Mar 25 18:33:29 2020 +0100
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Tue Mar 31 02:56:36 2020 +0200

    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 <m.weghorn at posteo.de>
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit 313081c0703c66918e95640c74cd57312a1e8bba)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90963

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<css::datatransfer::XTransferable> 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<css::uno::XInterface> Qt5Clipboard::create(const OUString& aModeString)
@@ -98,6 +103,13 @@ css::uno::Reference<css::datatransfer::XTransferable> Qt5Clipboard::getContents(
     return m_aContents;
 }
 
+void Qt5Clipboard::handleClearClipboard()
+{
+    if (!m_bDoClear)
+        return;
+    QApplication::clipboard()->clear(m_aClipboardMode);
+}
+
 void Qt5Clipboard::setContents(
     const css::uno::Reference<css::datatransfer::XTransferable>& xTrans,
     const css::uno::Reference<css::datatransfer::clipboard::XClipboardOwner>& 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();