tdf#81522 comphelper: just ignore disposed obj on save

Even if xObj.is(), it might be disposed, and then
getCurrentState() will raise an exception,
resulting in an ERRCODE_IO_GENERAL,
and failing to save the entire document.

Although it might not seem good to just ignore an error
on save, the general attitude in that function already
leans that way. In fact, the other ::StoreChildren()
wraps the entire for-loop in a try-catch.
Also, in this function a bad xPersist also breaks
out of the loop and returns a false.
So wrapping the entire xObj in an "ignore if errors"
try-catch seems reasonable to me in this instance.

The alternative is not to allow the user to save at all,
which is much worse, especially since there is nothing
he can do to fix the problem.

Change-Id: I0372245563735ae7ac7976bf7ce6060e57a5eb87
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99129
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Reviewed-by: Justin Luth <justin_luth@sil.org>

1 files changed, 73 insertions, 66 deletions

diff --git a/comphelper/source/container/embeddedobjectcontainer.cxx b/comphelper/source/container/embeddedobjectcontainer.cxx
index 2c3a6f937652..8e812823e044 100644
--- a/comphelper/source/container/embeddedobjectcontainer.cxx
+++ b/comphelper/source/container/embeddedobjectcontainer.cxx
@@ -1284,95 +1284,102 @@ bool EmbeddedObjectContainer::StoreChildren(bool _bOasisFormat,bool _bObjectsOnl
     const OUString* pEnd   = pIter + aNames.getLength();
     for(;pIter != pEnd;++pIter)
     {
-        uno::Reference < embed::XEmbeddedObject > xObj = GetEmbeddedObject( *pIter );
-        SAL_WARN_IF( !xObj.is(), "comphelper.container", "An empty entry in the embedded objects list!" );
-        if ( xObj.is() )
+        try
         {
-            sal_Int32 nCurState = xObj->getCurrentState();
-            if ( _bOasisFormat && nCurState != embed::EmbedStates::LOADED && nCurState != embed::EmbedStates::RUNNING )
+            uno::Reference < embed::XEmbeddedObject > xObj = GetEmbeddedObject( *pIter );
+            SAL_WARN_IF( !xObj.is(), "comphelper.container", "An empty entry in the embedded objects list!" );
+            if ( xObj.is() )
             {
-                // means that the object is active
-                // the image must be regenerated
-                OUString aMediaType;
-
-                // TODO/LATER: another aspect could be used
-                uno::Reference < io::XInputStream > xStream =
-                            GetGraphicReplacementStream(
-                                                        embed::Aspects::MSOLE_CONTENT,
-                                                        xObj,
-                                                        &aMediaType );
-                if ( xStream.is() )
+                sal_Int32 nCurState = xObj->getCurrentState();
+                if ( _bOasisFormat && nCurState != embed::EmbedStates::LOADED && nCurState != embed::EmbedStates::RUNNING )
                 {
-                    if ( !InsertGraphicStreamDirectly( xStream, *pIter, aMediaType ) )
-                        InsertGraphicStream( xStream, *pIter, aMediaType );
+                    // means that the object is active
+                    // the image must be regenerated
+                    OUString aMediaType;
+
+                    // TODO/LATER: another aspect could be used
+                    uno::Reference < io::XInputStream > xStream =
+                                GetGraphicReplacementStream(
+                                                            embed::Aspects::MSOLE_CONTENT,
+                                                            xObj,
+                                                            &aMediaType );
+                    if ( xStream.is() )
+                    {
+                        if ( !InsertGraphicStreamDirectly( xStream, *pIter, aMediaType ) )
+                            InsertGraphicStream( xStream, *pIter, aMediaType );
+                    }
                 }
-            }
 
-            // TODO/LATER: currently the object by default does not cache replacement image
-            // that means that if somebody loads SO7 document and store its objects using
-            // this method the images might be lost.
-            // Currently this method is only used on storing to alien formats, that means
-            // that SO7 documents storing does not use it, and all other filters are
-            // based on OASIS format. But if it changes the method must be fixed. The fix
-            // must be done only on demand since it can affect performance.
+                // TODO/LATER: currently the object by default does not cache replacement image
+                // that means that if somebody loads SO7 document and store its objects using
+                // this method the images might be lost.
+                // Currently this method is only used on storing to alien formats, that means
+                // that SO7 documents storing does not use it, and all other filters are
+                // based on OASIS format. But if it changes the method must be fixed. The fix
+                // must be done only on demand since it can affect performance.
 
-            uno::Reference< embed::XEmbedPersist > xPersist( xObj, uno::UNO_QUERY );
-            if ( xPersist.is() )
-            {
-                try
+                uno::Reference< embed::XEmbedPersist > xPersist( xObj, uno::UNO_QUERY );
+                if ( xPersist.is() )
                 {
-                    //TODO/LATER: only storing if changed!
-                    //xPersist->storeOwn(); //commented, i120168
-
-            // begin:all charts will be persisted as xml format on disk when saving, which is time consuming.
-                    // '_bObjectsOnly' mean we are storing to alien formats.
-                    //  'isStorageElement' mean current object is NOT a MS OLE format. (may also include in future), i120168
-                    if (_bObjectsOnly && (nCurState == embed::EmbedStates::LOADED || nCurState == embed::EmbedStates::RUNNING)
-                        && (pImpl->mxStorage->isStorageElement( *pIter ) ))
+                    try
                     {
-                        uno::Reference< util::XModifiable > xModifiable( xObj->getComponent(), uno::UNO_QUERY );
-                        if ( xModifiable.is() && xModifiable->isModified())
+                        //TODO/LATER: only storing if changed!
+                        //xPersist->storeOwn(); //commented, i120168
+
+                        // begin:all charts will be persisted as xml format on disk when saving, which is time consuming.
+                        // '_bObjectsOnly' mean we are storing to alien formats.
+                        //  'isStorageElement' mean current object is NOT a MS OLE format. (may also include in future), i120168
+                        if (_bObjectsOnly && (nCurState == embed::EmbedStates::LOADED || nCurState == embed::EmbedStates::RUNNING)
+                            && (pImpl->mxStorage->isStorageElement( *pIter ) ))
                         {
-                            xPersist->storeOwn();
+                            uno::Reference< util::XModifiable > xModifiable( xObj->getComponent(), uno::UNO_QUERY );
+                            if ( xModifiable.is() && xModifiable->isModified())
+                            {
+                                xPersist->storeOwn();
+                            }
+                            else
+                            {
+                                //do nothing. Embedded model is not modified, no need to persist.
+                            }
                         }
-                        else
+                        else //the embedded object is in active status, always store back it.
                         {
-                            //do nothing. Embedded model is not modified, no need to persist.
+                            xPersist->storeOwn();
                         }
+                        //end i120168
                     }
-                    else //the embedded object is in active status, always store back it.
+                    catch (const uno::Exception&)
                     {
-                        xPersist->storeOwn();
+                        // TODO/LATER: error handling
+                        bResult = false;
+                        break;
                     }
-                    //end i120168
                 }
-                catch (const uno::Exception&)
-                {
-                    // TODO/LATER: error handling
-                    bResult = false;
-                    break;
-                }
-            }
 
-            if ( !_bOasisFormat && !_bObjectsOnly )
-            {
-                // copy replacement images for linked objects
-                try
+                if ( !_bOasisFormat && !_bObjectsOnly )
                 {
-                    uno::Reference< embed::XLinkageSupport > xLink( xObj, uno::UNO_QUERY );
-                    if ( xLink.is() && xLink->isLink() )
+                    // copy replacement images for linked objects
+                    try
+                    {
+                        uno::Reference< embed::XLinkageSupport > xLink( xObj, uno::UNO_QUERY );
+                        if ( xLink.is() && xLink->isLink() )
+                        {
+                            OUString aMediaType;
+                            uno::Reference < io::XInputStream > xInStream = GetGraphicStream( xObj, &aMediaType );
+                            if ( xInStream.is() )
+                                InsertStreamIntoPicturesStorage_Impl( pImpl->mxStorage, xInStream, *pIter );
+                        }
+                    }
+                    catch (const uno::Exception&)
                     {
-                        OUString aMediaType;
-                        uno::Reference < io::XInputStream > xInStream = GetGraphicStream( xObj, &aMediaType );
-                        if ( xInStream.is() )
-                            InsertStreamIntoPicturesStorage_Impl( pImpl->mxStorage, xInStream, *pIter );
                     }
-                }
-                catch (const uno::Exception&)
-                {
                 }
             }
         }
+        catch (const uno::Exception&)
+        {
+            // TODO/LATER: error handling
+        }
     }
 
     if ( bResult && _bOasisFormat )