[PATCH libreoffice-4-0] fdo#61016: sw::marks::MarkManager: delay deletion of text fi...

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/2466

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/66/2466/1

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
(cherry picked from commit 5bf6797e81d54177508d3eb370e8242e3b58a3f8)
---
M sw/inc/IDocumentMarkAccess.hxx
M sw/source/core/doc/docbm.cxx
M sw/source/core/inc/MarkManager.hxx
3 files changed, 50 insertions(+), 14 deletions(-)



diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index 9b4bbb1..21e2d0c 100644
--- a/sw/inc/IDocumentMarkAccess.hxx
+++ b/sw/inc/IDocumentMarkAccess.hxx
@@ -57,6 +57,11 @@
         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 @@
             @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 76e9cfc..afd9fc2 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -644,16 +644,24 @@
             }
         }
 
-        // 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 @@
 #endif
     }
 
-    void MarkManager::deleteMark(const const_iterator_t ppMark)
+    struct LazyTextFieldmarkDeleter : public IDocumentMarkAccess::ILazyDeleter
     {
-        if(ppMark == m_vMarks.end()) return;
+        ::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)
+    {
+        ::boost::shared_ptr<ILazyDeleter> ret;
+        if (ppMark == m_vMarks.end()) return ret;
 
         switch(IDocumentMarkAccess::GetType(**ppMark))
         {
@@ -690,7 +715,10 @@
                 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 @@
         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 2c2bf85..d6dcdb7 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -55,7 +55,8 @@
             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();
 

-- 
To view, visit https://gerrit.libreoffice.org/2466
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9139b174f8a518a551a3ca8520396202c306abcf
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Michael Stahl <mstahl@redhat.com>