vcl/win/dtrans/WinClipboard.cxx |   35 +++++++++++++++++++++++------------
 vcl/win/dtrans/WinClipboard.hxx |    7 +++++--
 2 files changed, 28 insertions(+), 14 deletions(-)

New commits:
commit 2e0664015255ffc0f76a11a9cb254564b34de496
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Jul 14 22:01:59 2024 +0500
Commit:     Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Sun Jul 14 21:16:54 2024 +0200

    tdf#148647: make sure to update own content on Win clipboard change
    
    Before, only a new call to setContents, or destruction of the data
    object referenced from CWinClipboard, would force CWinClipboard to
    clean its m_pCurrentClipContent. A data object could be referenced
    elsewhere, and so not get destroyed on clipboard change; this kept
    the value of m_pCurrentClipContent, and all the following calls to
    getContents would still return that stuck own data object.
    
    This change makes CWinClipboard  store new own data object pointer
    into temporary m_pNewOwnClipContent.  In onClipboardContentChanged
    notification, the value is moved to m_pCurrentOwnClipContent. This
    ensures that following onClipboardContentChanged events will clear
    m_pCurrentOwnClipContent, preventing stuck clipboard content.
    
    Change-Id: Idd0a64a820d79bede9deceba8d5800b7fc61e66b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170459
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
    Tested-by: Jenkins

diff --git a/vcl/win/dtrans/WinClipboard.cxx b/vcl/win/dtrans/WinClipboard.cxx
index ccd3c6907078..3c9f57a1e250 100644
--- a/vcl/win/dtrans/WinClipboard.cxx
+++ b/vcl/win/dtrans/WinClipboard.cxx
@@ -60,7 +60,6 @@ CWinClipboard::CWinClipboard(const 
uno::Reference<uno::XComponentContext>& rxCon
                              const OUString& aClipboardName)
     : m_xContext(rxContext)
     , m_itsName(aClipboardName)
-    , m_pCurrentClipContent(nullptr)
 {
     // necessary to reassociate from
     // the static callback function
@@ -92,6 +91,12 @@ void CWinClipboard::disposing(std::unique_lock<std::mutex>& 
mutex)
 
 // XClipboard
 
+CXNotifyingDataObject* CWinClipboard::getOwnClipContent() const
+{
+    assert(!m_pCurrentOwnClipContent || !m_pNewOwnClipContent); // Both can be 
null, or only one set
+    return m_pCurrentOwnClipContent ? m_pCurrentOwnClipContent : 
m_pNewOwnClipContent;
+}
+
 // to avoid unnecessary traffic we check first if there is a clipboard
 // content which was set via setContent, in this case we don't need
 // to query the content from the clipboard, create a new wrapper object
@@ -110,12 +115,12 @@ css::uno::Reference<css::datatransfer::XTransferable> 
CWinClipboard::getContents
         throw lang::DisposedException("object is already disposed",
                                       static_cast<XClipboardEx*>(this));
 
-    assert(!m_pCurrentClipContent || !m_foreignContent); // Both can be null, 
or only one set
+    assert(!getOwnClipContent() || !m_foreignContent); // Both can be null, or 
only one set
 
     // use the shortcut or create a transferable from
     // system clipboard
-    if (nullptr != m_pCurrentClipContent)
-        return m_pCurrentClipContent->m_XTransferable;
+    if (auto pOwnClipContent = getOwnClipContent())
+        return pOwnClipContent->m_XTransferable;
 
     // Content cached?
     if (m_foreignContent.is())
@@ -175,18 +180,21 @@ void SAL_CALL CWinClipboard::setContents(
     IDataObjectPtr pIDataObj;
 
     m_foreignContent.clear();
+    m_pCurrentOwnClipContent = nullptr;
 
     if (xTransferable.is())
     {
-        m_pCurrentClipContent = new CXNotifyingDataObject(
+        // Store the new object's pointer to temporary m_pNewOwnClipContent, 
to be moved to
+        // m_pCurrentOwnClipContent in handleClipboardContentChanged.
+        m_pNewOwnClipContent = new CXNotifyingDataObject(
             CDTransObjFactory::createDataObjFromTransferable(m_xContext, 
xTransferable),
             xTransferable, xClipboardOwner, this);
 
-        pIDataObj = IDataObjectPtr(m_pCurrentClipContent);
+        pIDataObj = IDataObjectPtr(m_pNewOwnClipContent);
     }
     else
     {
-        m_pCurrentClipContent = nullptr;
+        m_pNewOwnClipContent = nullptr;
     }
 
     m_MtaOleClipboard.setClipboard(pIDataObj.get());
@@ -219,7 +227,7 @@ void SAL_CALL CWinClipboard::flushClipboard()
     // It may be possible to move the request to the clipboard STA thread by 
saving the
     // DataObject and call OleIsCurrentClipboard before flushing.
 
-    if (nullptr != m_pCurrentClipContent)
+    if (getOwnClipContent())
     {
         aGuard.unlock();
         m_MtaOleClipboard.flushClipboard();
@@ -272,13 +280,16 @@ void SAL_CALL CWinClipboard::removeClipboardListener(
     maClipboardListeners.removeInterface(aGuard, listener);
 }
 
-void CWinClipboard::clearCacheAndAllClipboardListener()
+void CWinClipboard::handleClipboardContentChanged()
 {
     std::unique_lock aGuard(m_aMutex);
     if (m_bDisposed)
         return;
 
     m_foreignContent.clear();
+    // If new own content assignment is pending, do it; otherwise, clear it.
+    // This makes sure that there will be no stuck clipboard content.
+    m_pCurrentOwnClipContent = std::exchange(m_pNewOwnClipContent, nullptr);
 
     if (!maClipboardListeners.getLength(aGuard))
         return;
@@ -351,8 +362,8 @@ void 
CWinClipboard::onReleaseDataObject(CXNotifyingDataObject* theCaller)
     // because an external source must be the clipboardowner now
     std::unique_lock aGuard(m_aMutex);
 
-    if (m_pCurrentClipContent == theCaller)
-        m_pCurrentClipContent = nullptr;
+    if (getOwnClipContent() == theCaller)
+        m_pCurrentOwnClipContent = m_pNewOwnClipContent = nullptr;
 }
 
 void CWinClipboard::registerClipboardViewer()
@@ -372,7 +383,7 @@ void WINAPI CWinClipboard::onClipboardContentChanged()
     }
 
     if (pWinClipboard)
-        pWinClipboard->clearCacheAndAllClipboardListener();
+        pWinClipboard->handleClipboardContentChanged();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/dtrans/WinClipboard.hxx b/vcl/win/dtrans/WinClipboard.hxx
index fcf7bf089cb6..83d01da65e05 100644
--- a/vcl/win/dtrans/WinClipboard.hxx
+++ b/vcl/win/dtrans/WinClipboard.hxx
@@ -57,12 +57,15 @@ class CWinClipboard final
     css::uno::Reference<css::uno::XComponentContext> m_xContext;
     const OUString m_itsName;
     CMtaOleClipboard m_MtaOleClipboard;
-    CXNotifyingDataObject* m_pCurrentClipContent;
+    CXNotifyingDataObject* m_pNewOwnClipContent = nullptr; // until 
onClipboardContentChanged
+    CXNotifyingDataObject* m_pCurrentOwnClipContent = nullptr;
     
com::sun::star::uno::Reference<com::sun::star::datatransfer::XTransferable> 
m_foreignContent;
     
comphelper::OInterfaceContainerHelper4<css::datatransfer::clipboard::XClipboardListener>
         maClipboardListeners;
 
-    void clearCacheAndAllClipboardListener();
+    CXNotifyingDataObject* getOwnClipContent() const;
+
+    void handleClipboardContentChanged();
     void onReleaseDataObject(CXNotifyingDataObject* theCaller);
 
     void registerClipboardViewer();

Reply via email to