[Libreoffice-commits] core.git: sw/source

[Mike Kaganski (via logerrit)]

 sw/source/core/unocore/unoframe.cxx |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

New commits:
commit 129f55097f926661f00919329fb28c6a85ecebbb
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Mon Apr 27 17:32:07 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Mon Apr 27 18:29:37 2020 +0200

    Don't insert embedded object twice
    
    During debugging of failure https://gerrit.libreoffice.org/c/core/+/92980,
    I saw that one of objects throws calling OCommonEmbeddedObject::getEntryName
    in EmbeddedObjectContainer::StoreAsChildren, because its m_bWaitSaveCompleted
    is true.
    
    The reason turns out that the object was already inserted, and thus its
    OCommonEmbeddedObject::storeAsEntry was called; but not yet its
    OCommonEmbeddedObject::saveCompleted. The double insertion happened in
    SwXFrame::attachToRange, where the object was first passed to
    InsertEmbeddedObject directly, and then was passed to InsertEmbObject,
    which called the former one again in this stack:
    
            comphelper.dll!comphelper::EmbeddedObjectContainer::InsertEmbeddedObject(const com::sun::star::uno::Reference<com::sun::star::embed::XEmbeddedObject> & xObj, rtl::OUString & rName) Line 514   C++
            swlo.dll!SwOLEObj::SetNode(SwOLENode * pNode) Line 884  C++
            swlo.dll!SwOLENode::SwOLENode(const SwNodeIndex & rWhere, const svt::EmbeddedObjectRef & xObj, SwGrfFormatColl * pGrfColl, const SwAttrSet * pAutoAttr) Line 220        C++
            swlo.dll!SwNodes::MakeOLENode(const SwNodeIndex & rWhere, const svt::EmbeddedObjectRef & xObj, SwGrfFormatColl * pGrfColl) Line 382     C++
            swlo.dll!sw::DocumentContentOperationsManager::InsertEmbObject(const SwPaM & rRg, const svt::EmbeddedObjectRef & xObj, SfxItemSet * pFlyAttrSet) Line 2914      C++
            swlo.dll!SwXFrame::attachToRange(const com::sun::star::uno::Reference<com::sun::star::text::XTextRange> & xTextRange, const SwPaM * pCopySource) Line 3028      C++
            ...
    
    Remove the redundant call, and put a comment on that.
    
    Change-Id: I680786a601ec454483e86c47ae07666d82c24146
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/92982
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/sw/source/core/unocore/unoframe.cxx b/sw/source/core/unocore/unoframe.cxx
index 760418f4be6a..99d0098fcf72 100644
--- a/sw/source/core/unocore/unoframe.cxx
+++ b/sw/source/core/unocore/unoframe.cxx
@@ -3014,15 +3014,12 @@ void SwXFrame::attachToRange(uno::Reference<text::XTextRange> const& xTextRange,
 
             pDoc->GetIDocumentUndoRedo().StartUndo(SwUndoId::INSERT, nullptr);
 
-            // Not sure if these setParent() and InsertEmbeddedObject() calls are really
-            // needed, it seems to work without, but logic from code elsewhere suggests
-            // they should be done.
-            SfxObjectShell& rPers = *pDoc->GetPersist();
-            uno::Reference < container::XChild > xChild( obj, uno::UNO_QUERY );
-            if ( xChild.is() )
-                xChild->setParent( rPers.GetModel() );
-            OUString rName;
-            rPers.GetEmbeddedObjectContainer().InsertEmbeddedObject( obj, rName );
+            // Do not call here container::XChild(obj)->setParent() and
+            // pDoc->GetPersist()->GetEmbeddedObjectContainer().InsertEmbeddedObject:
+            // they are called indirectly by pDoc->getIDocumentContentOperations().InsertEmbObject
+            // below. Calling them twice will add the same object twice to EmbeddedObjectContainer's
+            // pImpl->maNameToObjectMap, and then it will misbehave in
+            // EmbeddedObjectContainer::StoreAsChildren and SfxObjectShell::SaveCompletedChildren.
 
             SwFlyFrameFormat* pFrameFormat
                 = pDoc->getIDocumentContentOperations().InsertEmbObject(aPam, xObj, &aFrameSet);