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();