fdo#61016: sw::marks::MarkManager: delay deletion of text fieldmarks

There is a STL assertion in deleteMarks because the ReleaseDoc call will
recursively call into deleteMark again and delete marks that are in the
vector vMarksToDelete up the stack.

Change-Id: I9139b174f8a518a551a3ca8520396202c306abcf

3 files changed, 50 insertions, 14 deletions

diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index 9b4bbb1d351b..21e2d0c88cbb 100644
--- a/sw/inc/IDocumentMarkAccess.hxx
+++ b/sw/inc/IDocumentMarkAccess.hxx
@@ -57,6 +57,11 @@ class IDocumentMarkAccess
         typedef container_t::const_iterator const_iterator_t;
         typedef container_t::const_reverse_iterator const_reverse_iterator_t;
 
+        /// To avoid recursive calls of deleteMark, the removal of dummy
+        /// characters of fieldmarks has to be delayed; this is the baseclass
+        /// that can be subclassed for that purpose.
+        struct ILazyDeleter { virtual ~ILazyDeleter() { } };
+
         /** Generates a new mark in the document for a certain selection.
 
            @param rPaM
@@ -168,7 +173,8 @@ class IDocumentMarkAccess
             @param ppMark
             [in] an iterator pointing to the Mark to be deleted.
         */
-        virtual void deleteMark(const IDocumentMarkAccess::const_iterator_t ppMark) =0;
+        virtual ::boost::shared_ptr<ILazyDeleter>
+            deleteMark(const IDocumentMarkAccess::const_iterator_t ppMark) =0;
 
         /** Deletes a mark.
 
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index e809b7c1874c..4d06724e25d0 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -644,16 +644,24 @@ namespace sw { namespace mark
             }
         }
 
-        // we just remembered the iterators to delete, so we do not need to search
-        // for the boost::shared_ptr<> (the entry in m_vMarks) again
-        // reverse iteration, since erasing an entry invalidates iterators
-        // behind it (the iterators in vMarksToDelete are sorted)
-        for(vector<const_iterator_t>::reverse_iterator pppMark = vMarksToDelete.rbegin();
-            pppMark != vMarksToDelete.rend();
-            ++pppMark)
         {
-            deleteMark(*pppMark);
-        }
+            // fdo#61016 delay the deletion of the fieldmark characters
+            // to prevent that from deleting the marks on that position
+            // which would invalidate the iterators in vMarksToDelete
+            vector< ::boost::shared_ptr<ILazyDeleter> > vDelay;
+            vDelay.reserve(vMarksToDelete.size());
+            // we just remembered the iterators to delete, so we do not need to
+            // search for the boost::shared_ptr<> (the entry in m_vMarks) again.
+            // reverse iteration, since erasing an entry invalidates iterators
+            // behind it (the iterators in vMarksToDelete are sorted)
+            for (vector<const_iterator_t>::reverse_iterator pppMark
+                    = vMarksToDelete.rbegin();
+                pppMark != vMarksToDelete.rend();
+                ++pppMark)
+            {
+                vDelay.push_back(deleteMark(*pppMark));
+            }
+        } // scope to kill vDelay
         if(isSortingNeeded)
             sortMarks();
 #if 0
@@ -662,9 +670,26 @@ namespace sw { namespace mark
 #endif
     }
 
-    void MarkManager::deleteMark(const const_iterator_t ppMark)
+    struct LazyTextFieldmarkDeleter : public IDocumentMarkAccess::ILazyDeleter
+    {
+        ::boost::shared_ptr<IMark> const m_pTextFieldmark;
+        SwDoc *const m_pDoc;
+        LazyTextFieldmarkDeleter(
+                ::boost::shared_ptr<IMark> const& pMark, SwDoc *const pDoc)
+            : m_pTextFieldmark(pMark), m_pDoc(pDoc)
+        { }
+        virtual ~LazyTextFieldmarkDeleter()
+        {
+            dynamic_cast<TextFieldmark*>(m_pTextFieldmark.get())
+                ->ReleaseDoc(m_pDoc);
+        }
+    };
+
+    ::boost::shared_ptr<IDocumentMarkAccess::ILazyDeleter>
+        MarkManager::deleteMark(const const_iterator_t ppMark)
     {
-        if(ppMark == m_vMarks.end()) return;
+        ::boost::shared_ptr<ILazyDeleter> ret;
+        if (ppMark == m_vMarks.end()) return ret;
 
         switch(IDocumentMarkAccess::GetType(**ppMark))
         {
@@ -690,7 +715,10 @@ namespace sw { namespace mark
                 m_vFieldmarks.erase(ppFieldmark);
                 sw::mark::TextFieldmark* pTextFieldmark = dynamic_cast<sw::mark::TextFieldmark*>(ppMark->get());
                 if (pTextFieldmark)
-                    pTextFieldmark->ReleaseDoc(m_pDoc);
+                {
+                    ret.reset(
+                        new LazyTextFieldmarkDeleter(*ppMark, m_pDoc));
+                }
                 break;
             }
             case IDocumentMarkAccess::NAVIGATOR_REMINDER:
@@ -721,6 +749,7 @@ namespace sw { namespace mark
         pMark_t xHoldPastErase = *aI;
         m_aMarkNamesSet.erase(ppMark->get()->GetName());
         m_vMarks.erase(aI);
+        return ret;
     }
 
     void MarkManager::deleteMark(const IMark* const pMark)
diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx
index 2f17661c8c3f..cf19e6037049 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -55,7 +55,8 @@ namespace sw {
             virtual void deleteMarks(const SwNodeIndex& rStt, const SwNodeIndex& rEnd, ::std::vector< ::sw::mark::SaveBookmark>* pSaveBkmk, const SwIndex* pSttIdx, const SwIndex* pEndIdx);
 
             // deleters
-            virtual void deleteMark(const const_iterator_t ppMark);
+            virtual ::boost::shared_ptr<ILazyDeleter>
+                deleteMark(const const_iterator_t ppMark);
             virtual void deleteMark(const ::sw::mark::IMark* const pMark);
             virtual void clearAllMarks();