[Libreoffice-commits] core.git: Branch 'libreoffice-7-1' - embeddedobj/source

Armin Le Grand (Allotropia) (via logerrit) logerrit at kemper.freedesktop.org
Thu Apr 8 19:07:41 UTC 2021


 embeddedobj/source/commonembedding/embedobj.cxx    |    8 +-
 embeddedobj/source/commonembedding/miscobj.cxx     |   68 +++++++++++++++++-
 embeddedobj/source/commonembedding/persistence.cxx |   78 +++++++++++++++------
 embeddedobj/source/commonembedding/visobj.cxx      |    8 +-
 embeddedobj/source/inc/commonembobj.hxx            |    7 +
 5 files changed, 141 insertions(+), 28 deletions(-)

New commits:
commit aeca771f8a8e4c77008e921eebcdb5784003e419
Author:     Armin Le Grand (Allotropia) <armin.le.grand at me.com>
AuthorDate: Thu Apr 8 10:30:15 2021 +0200
Commit:     Adolfo Jayme Barrientos <fitojb at ubuntu.com>
CommitDate: Thu Apr 8 21:07:05 2021 +0200

    tdf#141529 sync linked OLE save(s) with hosting file save(s)
    
    Before deactivating a linked OLE saved it and overwrote the
    original, even when not saving the hosting document at all.
    This is not intuitive from user perspective and may lead
    to unexpected data loss (of the OLE content).
    Reported case was especially about closing the hosted document
    without saving in the understandable believe that that way the
    changed OLE will not be changed on external medium.
    Added mechanism for linked OLE to hold data in a hidden local
    temp file, synching/writing back on hosting file save. Most
    complicated was adapting the 'break link' case and ensuring
    other cases to work, but looks good now from my POV
    
    Change-Id: I7f63d667820b2d9725abc598a9dd7360be1f8d5a
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113793
    Tested-by: Armin Le Grand <Armin.Le.Grand at me.com>
    Reviewed-by: Armin Le Grand <Armin.Le.Grand at me.com>
    (cherry picked from commit f4c66a2272d890eaa7a6d7429e5bfa7bdb1f7162)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113762
    Tested-by: Jenkins
    Reviewed-by: Adolfo Jayme Barrientos <fitojb at ubuntu.com>

diff --git a/embeddedobj/source/commonembedding/embedobj.cxx b/embeddedobj/source/commonembedding/embedobj.cxx
index 669ba884b055..ffa2a0789be0 100644
--- a/embeddedobj/source/commonembedding/embedobj.cxx
+++ b/embeddedobj/source/commonembedding/embedobj.cxx
@@ -100,6 +100,12 @@ void OCommonEmbeddedObject::Deactivate()
     {
         try {
             xClientSite->saveObject();
+
+            // tdf#141529 take note that an eventually used linked file
+            // got changed/saved/written and that we need to copy it back if the
+            // hosting file/document gets saved
+            if(m_aLinkTempFile.is())
+                m_bLinkTempFileChanged = true;
         }
         catch( const embed::ObjectSaveVetoException& )
         {
@@ -168,7 +174,7 @@ void OCommonEmbeddedObject::SwitchStateTo_Impl( sal_Int32 nNextState )
             // after the object reaches the running state the cloned size is not necessary any more
             m_bHasClonedSize = false;
 
-            if ( m_bIsLink )
+            if ( m_bIsLinkURL )
             {
                 m_xDocHolder->SetComponent( LoadLink_Impl(), m_bReadOnly );
             }
diff --git a/embeddedobj/source/commonembedding/miscobj.cxx b/embeddedobj/source/commonembedding/miscobj.cxx
index ea2a3cb3c110..3d0cbb98b839 100644
--- a/embeddedobj/source/commonembedding/miscobj.cxx
+++ b/embeddedobj/source/commonembedding/miscobj.cxx
@@ -27,6 +27,10 @@
 #include <com/sun/star/lang/NoSupportException.hpp>
 #include <com/sun/star/beans/NamedValue.hpp>
 
+#include <com/sun/star/ucb/SimpleFileAccess.hpp>
+#include <com/sun/star/io/TempFile.hpp>
+#include <comphelper/storagehelper.hxx>
+
 #include <cppuhelper/queryinterface.hxx>
 #include <cppuhelper/interfacecontainer.h>
 #include <comphelper/mimeconfighelper.hxx>
@@ -52,8 +56,10 @@ OCommonEmbeddedObject::OCommonEmbeddedObject( const uno::Reference< uno::XCompon
 , m_bEmbeddedScriptSupport( true )
 , m_bDocumentRecoverySupport( true )
 , m_bWaitSaveCompleted( false )
-, m_bIsLink( false )
+, m_bIsLinkURL( false )
+, m_bLinkTempFileChanged( false )
 , m_bLinkHasPassword( false )
+, m_aLinkTempFile( )
 , m_bHasClonedSize( false )
 , m_nClonedMapUnit( 0 )
 {
@@ -78,8 +84,10 @@ OCommonEmbeddedObject::OCommonEmbeddedObject(
 , m_bEmbeddedScriptSupport( true )
 , m_bDocumentRecoverySupport( true )
 , m_bWaitSaveCompleted( false )
-, m_bIsLink( true )
+, m_bIsLinkURL( true )
+, m_bLinkTempFileChanged( false )
 , m_bLinkHasPassword( false )
+, m_aLinkTempFile( )
 , m_bHasClonedSize( false )
 , m_nClonedMapUnit( 0 )
 {
@@ -214,7 +222,61 @@ void OCommonEmbeddedObject::LinkInit_Impl(
         m_bReadOnly = aExportFilterName != m_aLinkFilterName;
     }
 
-    m_aDocMediaDescriptor = GetValuableArgs_Impl( aMediaDescr, false );
+    if(m_bIsLinkURL && !m_bReadOnly)
+    {
+        // tdf#141529 we have a linked OLE object. To prevent the original OLE
+        // data to be changed each time the OLE gets changed (at deactivate), copy it to
+        // a temporary file. That file will be changed on activated OLE changes then.
+        // The moment the original gets changed itself will now be associated with the
+        // file/document embedding the OLE being changed (see other additions to the
+        // task-ID above)
+        //
+        // open OLE original data as read input file
+        uno::Reference< ucb::XSimpleFileAccess3 > xTempAccess( ucb::SimpleFileAccess::create( m_xContext ) );
+        uno::Reference< io::XInputStream > xInStream( xTempAccess->openFileRead( m_aLinkURL ) );
+
+        if(xInStream.is())
+        {
+            // create temporary file
+            m_aLinkTempFile = io::TempFile::create(m_xContext);
+
+            if(m_aLinkTempFile.is())
+            {
+                // completely copy content of original OLE data
+                uno::Reference < io::XOutputStream > xTempOut = m_aLinkTempFile->getOutputStream();
+                ::comphelper::OStorageHelper::CopyInputToOutput( xInStream, xTempOut );
+                xTempOut->flush();
+                xTempOut->closeOutput();
+
+                // reset flag m_bLinkTempFileChanged, so it will also work for multiple
+                // save op's of the containing file/document
+                m_bLinkTempFileChanged = false;
+            }
+        }
+    }
+
+    if(m_aLinkTempFile.is())
+    {
+        uno::Sequence< beans::PropertyValue > aAlternativeMediaDescr(aMediaDescr.getLength());
+
+        for ( sal_Int32 a(0); a < aMediaDescr.getLength(); a++ )
+        {
+            const beans::PropertyValue& rSource(aMediaDescr[a]);
+            beans::PropertyValue& rDestination(aAlternativeMediaDescr[a]);
+
+            rDestination.Name = rSource.Name;
+            if(rSource.Name == "URL")
+                rDestination.Value <<= m_aLinkTempFile->getUri();
+            else
+                rDestination.Value = rSource.Value;
+        }
+
+        m_aDocMediaDescriptor = GetValuableArgs_Impl( aAlternativeMediaDescr, false );
+    }
+    else
+    {
+        m_aDocMediaDescriptor = GetValuableArgs_Impl( aMediaDescr, false );
+    }
 
     uno::Reference< frame::XDispatchProviderInterceptor > xDispatchInterceptor;
     for ( beans::PropertyValue const & prop : aObjectDescr )
diff --git a/embeddedobj/source/commonembedding/persistence.cxx b/embeddedobj/source/commonembedding/persistence.cxx
index bf0bcdf2bbf2..d21eb297360a 100644
--- a/embeddedobj/source/commonembedding/persistence.cxx
+++ b/embeddedobj/source/commonembedding/persistence.cxx
@@ -47,6 +47,9 @@
 #include <com/sun/star/beans/IllegalTypeException.hpp>
 #include <com/sun/star/chart2/XChartDocument.hpp>
 
+#include <com/sun/star/ucb/SimpleFileAccess.hpp>
+#include <com/sun/star/io/XTruncate.hpp>
+
 #include <comphelper/fileformat.h>
 #include <comphelper/storagehelper.hxx>
 #include <comphelper/mimeconfighelper.hxx>
@@ -254,7 +257,7 @@ void OCommonEmbeddedObject::SwitchOwnPersistence( const uno::Reference< embed::X
     m_aEntryName = aNewName;
 
     // the linked document should not be switched
-    if ( !m_bIsLink )
+    if ( !m_bIsLinkURL )
     {
         uno::Reference< document::XStorageBasedDocument > xDoc( m_xDocHolder->GetComponent(), uno::UNO_QUERY );
         if ( xDoc.is() )
@@ -369,10 +372,16 @@ uno::Reference< util::XCloseable > OCommonEmbeddedObject::LoadLink_Impl()
 
     sal_Int32 nLen = 2;
     uno::Sequence< beans::PropertyValue > aArgs( nLen );
+
     aArgs[0].Name = "URL";
-    aArgs[0].Value <<= m_aLinkURL;
+    if(m_aLinkTempFile.is())
+        aArgs[0].Value <<= m_aLinkTempFile->getUri();
+    else
+        aArgs[0].Value <<= m_aLinkURL;
+
     aArgs[1].Name = "FilterName";
     aArgs[1].Value <<= m_aLinkFilterName;
+
     if ( m_bLinkHasPassword )
     {
         aArgs.realloc( ++nLen );
@@ -843,7 +852,7 @@ uno::Reference< util::XCloseable > OCommonEmbeddedObject::CreateTempDocFromLink_
 {
     uno::Reference< util::XCloseable > xResult;
 
-    SAL_WARN_IF( !m_bIsLink, "embeddedobj.common", "The object is not a linked one!" );
+    SAL_WARN_IF( !m_bIsLinkURL, "embeddedobj.common", "The object is not a linked one!" );
 
     uno::Sequence< beans::PropertyValue > aTempMediaDescr;
 
@@ -895,7 +904,12 @@ uno::Reference< util::XCloseable > OCommonEmbeddedObject::CreateTempDocFromLink_
     {
         aTempMediaDescr.realloc( 2 );
         aTempMediaDescr[0].Name = "URL";
-        aTempMediaDescr[0].Value <<= m_aLinkURL;
+
+        // tdf#141529 use URL of the linked TempFile if it exists
+        aTempMediaDescr[0].Value <<= m_aLinkTempFile.is()
+            ? m_aLinkTempFile->getUri()
+            : m_aLinkURL;
+
         aTempMediaDescr[1].Name = "FilterName";
         aTempMediaDescr[1].Value <<= m_aLinkFilterName;
     }
@@ -968,8 +982,8 @@ void SAL_CALL OCommonEmbeddedObject::setPersistentEntry(
 
     // for now support of this interface is required to allow breaking of links and converting them to normal embedded
     // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used )
-    // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" );
-    if ( m_bIsLink )
+    // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" );
+    if ( m_bIsLinkURL )
     {
         m_aEntryName = sEntName;
         return;
@@ -1147,8 +1161,8 @@ void SAL_CALL OCommonEmbeddedObject::storeToEntry( const uno::Reference< embed::
 
     // for now support of this interface is required to allow breaking of links and converting them to normal embedded
     // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used )
-    // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" );
-    if ( m_bIsLink )
+    // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" );
+    if ( m_bIsLinkURL )
         return;
 
     OSL_ENSURE( m_xParentStorage.is() && m_xObjectStorage.is(), "The object has no valid persistence!" );
@@ -1275,10 +1289,33 @@ void SAL_CALL OCommonEmbeddedObject::storeAsEntry( const uno::Reference< embed::
 
     // for now support of this interface is required to allow breaking of links and converting them to normal embedded
     // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used )
-    // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" );
-    if ( m_bIsLink )
+    // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" );
+    if ( m_bIsLinkURL )
     {
         m_aNewEntryName = sEntName;
+
+        if(m_aLinkTempFile.is() && m_bLinkTempFileChanged)
+        {
+            // tdf#141529 if we have a changed copy of the original OLE data we now
+            // need to write it back 'over' the original OLE data
+            uno::Reference < ucb::XSimpleFileAccess2 > xFileAccess(ucb::SimpleFileAccess::create( m_xContext ));
+            uno::Reference < io::XInputStream > xTempIn = m_aLinkTempFile->getInputStream();
+
+            // This is *needed* since OTempFileService calls OTempFileService::readBytes which
+            // ensures the SvStream mpStream gets/is opened, *but* also sets the mnCachedPos from
+            // OTempFileService which still points to the end-of-file (from write-cc'ing).
+            uno::Reference < io::XSeekable > xSeek( xTempIn, uno::UNO_QUERY_THROW );
+            xSeek->seek(0);
+
+            xFileAccess->writeFile(m_aLinkURL, xTempIn);
+
+            // Do *not* close input, that would remove the temporary file too early
+            // xTempIn->closeInput();
+
+            // reset flag m_bLinkTempFileChanged
+            m_bLinkTempFileChanged = false;
+        }
+
         return;
     }
 
@@ -1407,8 +1444,8 @@ void SAL_CALL OCommonEmbeddedObject::saveCompleted( sal_Bool bUseNew )
 
     // for now support of this interface is required to allow breaking of links and converting them to normal embedded
     // objects, so the persist name must be handled correctly ( althowgh no real persist entry is used )
-    // OSL_ENSURE( !m_bIsLink, "This method implementation must not be used for links!" );
-    if ( m_bIsLink )
+    // OSL_ENSURE( !m_bIsLinkURL, "This method implementation must not be used for links!" );
+    if ( m_bIsLinkURL )
     {
         if ( bUseNew )
             m_aEntryName = m_aNewEntryName;
@@ -1542,7 +1579,7 @@ void SAL_CALL OCommonEmbeddedObject::storeOwn()
     if ( !m_xDocHolder->GetComponent().is() )
         throw uno::RuntimeException();
 
-    if ( m_bIsLink )
+    if ( m_bIsLinkURL )
     {
         // TODO: just store the document to its location
         uno::Reference< frame::XStorable > xStorable( m_xDocHolder->GetComponent(), uno::UNO_QUERY_THROW );
@@ -1650,7 +1687,7 @@ void SAL_CALL OCommonEmbeddedObject::reload(
                     "The object waits for saveCompleted() call!",
                     static_cast< ::cppu::OWeakObject* >(this) );
 
-    if ( m_bIsLink )
+    if ( m_bIsLinkURL )
     {
         // reload of the link
         OUString aOldLinkFilter = m_aLinkFilterName;
@@ -1719,7 +1756,7 @@ void SAL_CALL OCommonEmbeddedObject::reload(
         if ( prop.Name == "ReadOnly" )
             prop.Value >>= m_bReadOnly;
 
-    if ( bOldReadOnlyValue == m_bReadOnly || m_bIsLink )
+    if ( bOldReadOnlyValue == m_bReadOnly || m_bIsLinkURL )
         return;
 
     // close own storage
@@ -1751,7 +1788,7 @@ void SAL_CALL OCommonEmbeddedObject::breakLink( const uno::Reference< embed::XSt
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
-    if (!m_bIsLink || m_nObjectState == -1)
+    if (!m_bIsLinkURL || m_nObjectState == -1)
     {
         // it must be a linked initialized object
         throw embed::WrongStateException(
@@ -1820,7 +1857,10 @@ void SAL_CALL OCommonEmbeddedObject::breakLink( const uno::Reference< embed::XSt
     else if ( m_nObjectState == embed::EmbedStates::ACTIVE )
         m_xDocHolder->Show();
 
-    m_bIsLink = false;
+    // tdf#141529 reset all stuff involved in linked state, including
+    // the OLE content copied to the temp file
+    m_bIsLinkURL = false;
+    m_aLinkTempFile.clear();
     m_aLinkFilterName.clear();
     m_aLinkURL.clear();
 }
@@ -1832,7 +1872,7 @@ sal_Bool SAL_CALL  OCommonEmbeddedObject::isLink()
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
-    return m_bIsLink;
+    return m_bIsLinkURL;
 }
 
 
@@ -1842,7 +1882,7 @@ OUString SAL_CALL OCommonEmbeddedObject::getLinkURL()
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
-    if ( !m_bIsLink )
+    if ( !m_bIsLinkURL )
         throw embed::WrongStateException(
                     "The object is not a link object!",
                     static_cast< ::cppu::OWeakObject* >(this) );
diff --git a/embeddedobj/source/commonembedding/visobj.cxx b/embeddedobj/source/commonembedding/visobj.cxx
index 6306ecfad0cf..851457609e7d 100644
--- a/embeddedobj/source/commonembedding/visobj.cxx
+++ b/embeddedobj/source/commonembedding/visobj.cxx
@@ -55,7 +55,7 @@ void SAL_CALL OCommonEmbeddedObject::setVisualAreaSize( sal_Int64 nAspect, const
         changeState( embed::EmbedStates::RUNNING );
 
         // the links should be switched back to loaded state for now to avoid locking problems
-        bBackToLoaded = m_bIsLink;
+        bBackToLoaded = m_bIsLinkURL;
     }
 
     bool bSuccess = m_xDocHolder->SetExtent( nAspect, aSize );
@@ -88,7 +88,7 @@ awt::Size SAL_CALL OCommonEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
         changeState( embed::EmbedStates::RUNNING );
 
         // the links should be switched back to loaded state for now to avoid locking problems
-        bBackToLoaded = m_bIsLink;
+        bBackToLoaded = m_bIsLinkURL;
     }
 
     awt::Size aResult;
@@ -128,7 +128,7 @@ sal_Int32 SAL_CALL OCommonEmbeddedObject::getMapUnit( sal_Int64 nAspect )
         changeState( embed::EmbedStates::RUNNING );
 
         // the links should be switched back to loaded state for now to avoid locking problems
-        bBackToLoaded = m_bIsLink;
+        bBackToLoaded = m_bIsLinkURL;
     }
 
     sal_Int32 nResult = m_xDocHolder->GetMapUnit( nAspect );
@@ -172,7 +172,7 @@ embed::VisualRepresentation SAL_CALL OCommonEmbeddedObject::getPreferredVisualRe
             setVisualAreaSize(nAspect, aOrigSize);
 
         // the links should be switched back to loaded state for now to avoid locking problems
-        bBackToLoaded = m_bIsLink;
+        bBackToLoaded = m_bIsLinkURL;
     }
 
     SAL_WARN_IF( !m_xDocHolder->GetComponent().is(), "embeddedobj.common", "Running or Active object has no component!" );
diff --git a/embeddedobj/source/inc/commonembobj.hxx b/embeddedobj/source/inc/commonembobj.hxx
index 2294b0d7e915..1b020f430855 100644
--- a/embeddedobj/source/inc/commonembobj.hxx
+++ b/embeddedobj/source/inc/commonembobj.hxx
@@ -32,6 +32,7 @@
 #include <com/sun/star/awt/Rectangle.hpp>
 #include <com/sun/star/util/XCloseable.hpp>
 #include <com/sun/star/chart2/XDefaultSizeTransmitter.hpp>
+#include <com/sun/star/io/XTempFile.hpp>
 #include <cppuhelper/weak.hxx>
 #include <rtl/ref.hxx>
 #include <map>
@@ -132,7 +133,8 @@ protected:
     css::awt::Rectangle m_aOwnRectangle;
     css::awt::Rectangle m_aClipRectangle;
 
-    bool m_bIsLink;
+    bool m_bIsLinkURL;
+    bool m_bLinkTempFileChanged;
 
     // embedded object related stuff
     OUString m_aEntryName;
@@ -146,6 +148,9 @@ protected:
     bool        m_bLinkHasPassword;
     OUString m_aLinkPassword;
 
+    // tdf#141529 hold a cc of a linked OLE
+    css::uno::Reference < css::io::XTempFile > m_aLinkTempFile;
+
     css::uno::Reference< css::uno::XInterface > m_xParent;
 
     bool m_bHasClonedSize; // the object has cached size


More information about the Libreoffice-commits mailing list