tdf#88555 sw: reduce usage of dynamic_cast in SwFormatsModifyBase::Contains

This is a bad idea as the function is sometimes used to check if a
SwFormat has been deleted, which can happen in Undo.

Replace with ContainsFormat() and IsAlive(), and don't require a
non-const SwFormat parameter so that the dynamic_cast using one
isn't called all the time but only called once where it's safe.

Change-Id: Ib74b79629f5c8ed432a912ada5974a6d816e7d31

6 files changed, 22 insertions, 14 deletions

diff --git a/sw/inc/docary.hxx b/sw/inc/docary.hxx
index 10e41d68f901..a63bb34cc307 100644
--- a/sw/inc/docary.hxx
+++ b/sw/inc/docary.hxx
@@ -23,6 +23,7 @@
 #include <vector>
 #include <set>
 #include <algorithm>
+#include <type_traits>
 #include <o3tl/sorted_vector.hxx>
 
 #include <boost/multi_index_container.hpp>
@@ -135,7 +136,8 @@ public:
         return it == end() ? SIZE_MAX : it - begin();
     }
 
-    bool Contains(Value const& p) const
+    /// check that given format is still alive (i.e. contained here)
+    bool IsAlive(typename std::remove_pointer<Value>::type const*const p) const
         { return std::find(begin(), end(), p) != end(); }
 
     static void dumpAsXml(struct _xmlTextWriter* /*pWriter*/) {};
@@ -158,9 +160,12 @@ public:
 
     size_t GetPos(const SwFormat *p) const
         { return SwVectorModifyBase<Value>::GetPos( static_cast<Value>( const_cast<SwFormat*>( p ) ) ); }
-    bool Contains(const SwFormat *p) const {
-        Value p2 = dynamic_cast<Value>(const_cast<SwFormat*>(p));
-        return p2 != nullptr && SwVectorModifyBase<Value>::Contains(p2);
+
+    /// check if given format is contained here
+    /// @precond pFormat must not have been deleted
+    bool ContainsFormat(SwFormat const*const pFormat) const {
+        Value p = dynamic_cast<Value>(const_cast<SwFormat*>(pFormat));
+        return p != nullptr && SwVectorModifyBase<Value>::IsAlive(p);
     }
 };
 
diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx
index 286d2269b0b2..f414b3a6a226 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -800,11 +800,13 @@ SwNumRule& SwNumRule::CopyNumRule( SwDoc* pDoc, const SwNumRule& rNumRule )
     {
         Set( n, rNumRule.maFormats[ n ] );
         if( maFormats[ n ] && maFormats[ n ]->GetCharFormat() &&
-            !pDoc->GetCharFormats()->Contains( maFormats[n]->GetCharFormat() ))
+            !pDoc->GetCharFormats()->IsAlive(maFormats[n]->GetCharFormat()))
+        {
             // If we copy across different Documents, then copy the
             // corresponding CharFormat into the new Document.
             maFormats[n]->SetCharFormat( pDoc->CopyCharFormat( *maFormats[n]->
                                         GetCharFormat() ) );
+        }
     }
     meRuleType = rNumRule.meRuleType;
     msName = rNumRule.msName;
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index 4b3ce11653f4..93c25f85bc5c 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -228,7 +228,7 @@ void SwHistorySetText::SetInDoc( SwDoc* pDoc, bool )
     if ( RES_TXTATR_CHARFMT == m_pAttr->Which() )
     {
         // ask the Doc if the CharFormat still exists
-        if ( !pDoc->GetCharFormats()->Contains( (static_cast<SwFormatCharFormat&>(*m_pAttr)).GetCharFormat() ) )
+        if (!pDoc->GetCharFormats()->IsAlive((static_cast<SwFormatCharFormat&>(*m_pAttr)).GetCharFormat()))
             return; // do not set, format does not exist
     }
 
@@ -522,12 +522,12 @@ void SwHistoryChangeFormatColl::SetInDoc( SwDoc* pDoc, bool )
     {
         if ( SwNodeType::Text == m_nNodeType )
         {
-            if (pDoc->GetTextFormatColls()->Contains( static_cast<SwTextFormatColl * const>(m_pColl) ))
+            if (pDoc->GetTextFormatColls()->IsAlive(static_cast<SwTextFormatColl * const>(m_pColl)))
             {
                 pContentNd->ChgFormatColl( m_pColl );
             }
         }
-        else if (pDoc->GetGrfFormatColls()->Contains( static_cast<SwGrfFormatColl * const>(m_pColl) ))
+        else if (pDoc->GetGrfFormatColls()->IsAlive(static_cast<SwGrfFormatColl * const>(m_pColl)))
         {
             pContentNd->ChgFormatColl( m_pColl );
         }
diff --git a/sw/source/core/undo/unattr.cxx b/sw/source/core/undo/unattr.cxx
index f631dc00706b..058edab78750 100644
--- a/sw/source/core/undo/unattr.cxx
+++ b/sw/source/core/undo/unattr.cxx
@@ -141,7 +141,7 @@ void SwUndoFormatAttr::Init()
                 m_nNodeIndex = pTable->GetTabSortBoxes()[ 0 ]->GetSttNd()
                                ->FindTableNode()->GetIndex();
             }
-        } else if ( pDoc->GetSections().Contains( m_pFormat )) {
+        } else if (pDoc->GetSections().ContainsFormat(m_pFormat)) {
             m_nNodeIndex = m_pFormat->GetContent().GetContentIdx()->GetIndex();
         } else if ( dynamic_cast< SwTableBoxFormat* >( m_pFormat ) !=  nullptr ) {
             SwTableBox * pTableBox = SwIterator<SwTableBox,SwFormat>( *m_pFormat ).First();
diff --git a/sw/source/core/undo/untblk.cxx b/sw/source/core/undo/untblk.cxx
index 3ce8864999e8..9c5ac1eb8ec0 100644
--- a/sw/source/core/undo/untblk.cxx
+++ b/sw/source/core/undo/untblk.cxx
@@ -230,7 +230,7 @@ void SwUndoInserts::UndoImpl(::sw::UndoRedoContext & rContext)
 
             pTextNode->ResetAllAttr();
 
-            if (rDoc.GetTextFormatColls()->Contains(pTextFormatColl))
+            if (rDoc.GetTextFormatColls()->IsAlive(pTextFormatColl))
                 pTextFormatColl = static_cast<SwTextFormatColl*>(pTextNode->ChgFormatColl( pTextFormatColl )) ;
 
             pHistory->SetTmpEnd( nSetPos );
@@ -269,7 +269,7 @@ void SwUndoInserts::RedoImpl(::sw::UndoRedoContext & rContext)
         rPam.Exchange();
     }
 
-    if( pDoc->GetTextFormatColls()->Contains( pTextFormatColl ))
+    if (pDoc->GetTextFormatColls()->IsAlive(pTextFormatColl))
     {
         SwTextNode* pTextNd = rPam.GetMark()->nNode.GetNode().GetTextNode();
         if( pTextNd )
@@ -277,8 +277,8 @@ void SwUndoInserts::RedoImpl(::sw::UndoRedoContext & rContext)
     }
     pTextFormatColl = pSavTextFormatColl;
 
-    if( pLastNdColl && pDoc->GetTextFormatColls()->Contains( pLastNdColl ) &&
-        rPam.GetPoint()->nNode != rPam.GetMark()->nNode )
+    if (pLastNdColl && pDoc->GetTextFormatColls()->IsAlive(pLastNdColl)
+        && rPam.GetPoint()->nNode != rPam.GetMark()->nNode)
     {
         SwTextNode* pTextNd = rPam.GetPoint()->nNode.GetNode().GetTextNode();
         if( pTextNd )
diff --git a/sw/source/filter/basflt/shellio.cxx b/sw/source/filter/basflt/shellio.cxx
index a487e0b05b2b..d2fa62bf0672 100644
--- a/sw/source/filter/basflt/shellio.cxx
+++ b/sw/source/filter/basflt/shellio.cxx
@@ -226,7 +226,8 @@ sal_uLong SwReader::Read( const Reader& rOptions )
             {
                 SwFrameFormat* pFrameFormat = (*mxDoc->GetSpzFrameFormats())[ n ];
                 const SwFormatAnchor& rAnchor = pFrameFormat->GetAnchor();
-                if( !aFlyFrameArr.Contains( pFrameFormat) )
+                // ok, here IsAlive is a misnomer...
+                if (!aFlyFrameArr.IsAlive(pFrameFormat))
                 {
                     SwPosition const*const pFrameAnchor(
                             rAnchor.GetContentAnchor());