[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - comphelper/source
Justin Luth (via logerrit)
logerrit at kemper.freedesktop.org
Thu Jul 30 11:26:32 UTC 2020
comphelper/source/container/embeddedobjectcontainer.cxx | 139 ++++++++--------
1 file changed, 73 insertions(+), 66 deletions(-)
New commits:
commit a3285627b2a5171bf97c57738f494ad7ae50c545
Author: Justin Luth <justin.luth at collabora.com>
AuthorDate: Tue Jul 21 12:02:20 2020 +0300
Commit: Caolán McNamara <caolanm at redhat.com>
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 <caolanm at redhat.com>
Reviewed-by: Justin Luth <justin_luth at sil.org>
(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 )
More information about the Libreoffice-commits
mailing list