comphelper/source/container/embeddedobjectcontainer.cxx | 139 ++++++++-------- 1 file changed, 73 insertions(+), 66 deletions(-)
New commits: commit a3285627b2a5171bf97c57738f494ad7ae50c545 Author: Justin Luth <[email protected]> AuthorDate: Tue Jul 21 12:02:20 2020 +0300 Commit: Caolán McNamara <[email protected]> CommitDate: Thu Jul 30 13:25:58 2020 +0200 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 <[email protected]> Reviewed-by: Justin Luth <[email protected]> (cherry picked from commit 91d1cd8687c00a639cb4ecc5759254c946efe4c1) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99199 diff --git a/comphelper/source/container/embeddedobjectcontainer.cxx b/comphelper/source/container/embeddedobjectcontainer.cxx index 311edd303a86..cd3ab4736276 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 ) _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
