sw: fix assert with frames anchored in redlines on rhbz490395-1.odt

SwRangeRedline::Show() will move nodes around in the nodes-array, which
means that using SwNodeIndex as a key in a map that has a lifetime
not limited by the stack is a bad idea, as the map will become unsorted.

Remove SwFrmFmtAnchorMap from SwDoc and replace it with new
SwNode::m_pAnchoredFlys to do the same mapping.

(regression from 738fb2ad)

Change-Id: I396d92b9d0b2045e98bad6d0b374303cd4e62b59

sw/inc/doc.hxx

@@ -103,7 +103,6 @@ class SwFmt;
 class SwFmtINetFmt;
 class SwFmtRefMark;
 class SwFrmFmt;
-class SwFrmFmtAnchorMap;
 class SwFrmFmts;
 class SwFtnIdxs;
 class SwFtnInfo;
@@ -300,7 +299,6 @@ class SW_DLLPUBLIC SwDoc :
     SwGrfFmtColl    *mpDfltGrfFmtColl;
 
     SwFrmFmts       *mpFrmFmtTbl;        //< Format table
-    SwFrmFmtAnchorMap *mpFrmFmtAnchorMap;
     SwCharFmts      *mpCharFmtTbl;
     SwFrmFmts       *mpSpzFrmFmtTbl;
     SwSectionFmts   *mpSectionFmtTbl;
@@ -808,9 +806,6 @@ public:
     const SwCharFmt *GetDfltCharFmt() const { return mpDfltCharFmt;}
           SwCharFmt *GetDfltCharFmt()       { return mpDfltCharFmt;}
 
-    const SwFrmFmtAnchorMap* GetFrmFmtAnchorMap() const { return mpFrmFmtAnchorMap; }
-    SwFrmFmtAnchorMap* GetFrmFmtAnchorMap() { return mpFrmFmtAnchorMap; }
-
     // @return the interface of the management of (auto)styles
     IStyleAccess& GetIStyleAccess() { return *mpStyleAccess; }
 

sw/inc/frmfmt.hxx

@@ -23,8 +23,6 @@
 #include <cppuhelper/weakref.hxx>
 #include <tools/gen.hxx>
 #include <format.hxx>
-#include <map>
-#include <ndindex.hxx>
 #include "swdllapi.h"
 
 class SwFlyFrm;
@@ -304,27 +302,6 @@ public:
 
 SW_DLLPUBLIC bool IsFlyFrmFmtInHeader(const SwFrmFmt& rFmt);
 
-/**
- Fast mapping from node positions to SwFrmFmt objects anchored at them.
-
- SwFrmFmt::GetAnchor().GetCntntAnchor() provides the position where the object is anchored.
- This class provides the reverse mapping. It intentionally uses SwNodeIndex instead of SwPosition
- to allow simpler implementation, do SwIndex checking explicitly if needed.
-*/
-class SwFrmFmtAnchorMap
-{
-public:
-    SwFrmFmtAnchorMap( const SwDoc* doc );
-    void Add( SwFrmFmt* fmt, const SwNodeIndex& index );
-    void Remove( SwFrmFmt* fmt, const SwNodeIndex& index );
-    typedef std::multimap< SwNodeIndex, SwFrmFmt* >::const_iterator const_iterator;
-    typedef std::pair< const_iterator, const_iterator > const_iterator_pair;
-    const_iterator_pair equal_range( const SwNodeIndex& pos ) const;
-private:
-    std::multimap< SwNodeIndex, SwFrmFmt* > items;
-    const SwDoc* doc;
-};
-
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sw/inc/node.hxx

@@ -94,6 +94,11 @@ class SW_DLLPUBLIC SwNode
     long m_nSerial;
 #endif
 
+    /// all SwFrmFmt that are anchored at the node
+    /// invariant: SwFrmFmt is in the list iff
+    /// SwFrmFmt::GetAnchor().GetCntntAnchor() points to this node
+    std::unique_ptr<std::vector<SwFrmFmt*>> m_pAnchoredFlys;
+
 protected:
     SwStartNode* pStartOfSection;
 
@@ -277,6 +282,10 @@ public:
 
     sal_uInt8 HasPrevNextLayNode() const;
 
+    std::vector<SwFrmFmt *> const* GetAnchoredFlys() const { return m_pAnchoredFlys.get(); }
+    void AddAnchoredFly(SwFrmFmt *);
+    void RemoveAnchoredFly(SwFrmFmt *);
+
     /**
      * Dumps the node structure to the given destination (file nodes.xml in the current directory by default)
      * @since 3.5

sw/source/core/doc/doclay.cxx

@@ -1521,18 +1521,17 @@ bool SwDoc::IsInHeaderFooter( const SwNodeIndex& rIdx ) const
                 checkFmts.push_back( pFmt );
         }
 #endif
-        SwFrmFmtAnchorMap::const_iterator_pair range = GetFrmFmtAnchorMap()->equal_range( SwNodeIndex( *pFlyNd ));
-        SwFrmFmtAnchorMap::const_iterator it;
-        for( it = range.first;
-             it != range.second;
-             ++it )
+        std::vector<SwFrmFmt*> const*const pFlys(pFlyNd->GetAnchoredFlys());
+        bool bFound(false);
+        for (size_t i = 0; pFlys && i < pFlys->size(); ++i)
         {
-            const SwFrmFmt* pFmt = it->second;
+            const SwFrmFmt *const pFmt = (*pFlys)[i];
             const SwNodeIndex* pIdx = pFmt->GetCntnt().GetCntntIdx();
             if( pIdx && pFlyNd == &pIdx->GetNode() )
             {
 #if OSL_DEBUG_LEVEL > 0
-                std::list<const SwFrmFmt*>::iterator checkPos = std::find( checkFmts.begin(), checkFmts.end(), pFmt );
+                std::list<const SwFrmFmt*>::iterator checkPos = std::find(
+                        checkFmts.begin(), checkFmts.end(), pFmt );
                 assert( checkPos != checkFmts.end());
                 checkFmts.erase( checkPos );
 #endif
@@ -1545,12 +1544,13 @@ bool SwDoc::IsInHeaderFooter( const SwNodeIndex& rIdx ) const
 
                 pNd = &rAnchor.GetCntntAnchor()->nNode.GetNode();
                 pFlyNd = pNd->FindFlyStartNode();
+                bFound = true;
                 break;
             }
         }
-        if( it == range.second )
+        if (!bFound)
         {
-            OSL_ENSURE( mbInReading, "Found a FlySection but not a Format!" );
+            OSL_ENSURE(mbInReading, "Found a FlySection but not a Format!");
             return false;
         }
 #if OSL_DEBUG_LEVEL > 0

sw/source/core/doc/docnew.cxx

@@ -230,7 +230,6 @@ SwDoc::SwDoc()
     mpDfltTxtFmtColl( new SwTxtFmtColl( GetAttrPool(), sTxtCollStr ) ),
     mpDfltGrfFmtColl( new SwGrfFmtColl( GetAttrPool(), sGrfCollStr ) ),
     mpFrmFmtTbl( new SwFrmFmts() ),
-    mpFrmFmtAnchorMap( new SwFrmFmtAnchorMap( this ) ),
     mpCharFmtTbl( new SwCharFmts() ),
     mpSpzFrmFmtTbl( new SwFrmFmts() ),
     mpSectionFmtTbl( new SwSectionFmts() ),
@@ -577,7 +576,6 @@ SwDoc::~SwDoc()
     delete mpDfltCharFmt;
     delete mpDfltFrmFmt;
     delete mpLayoutCache;
-    delete mpFrmFmtAnchorMap;
 
     SfxItemPool::Free(mpAttrPool);
 }

sw/source/core/docnode/node.cxx

@@ -342,6 +342,7 @@ SwNode::SwNode( SwNodes& rNodes, sal_uLong nPos, const sal_uInt8 nNdType )
 
 SwNode::~SwNode()
 {
+    assert(!m_pAnchoredFlys || GetDoc()->IsInDtor()); // must all be deleted
 }
 
 /// Find the TableNode in which it is located.
@@ -2028,4 +2029,50 @@ bool SwNode::IsInRedlines() const
     return bResult;
 }
 
+void SwNode::AddAnchoredFly(SwFrmFmt *const pFlyFmt)
+{
+    assert(pFlyFmt);
+    assert(&pFlyFmt->GetAnchor(false).GetCntntAnchor()->nNode.GetNode() == this);
+    // check node type, cf. SwFmtAnchor::SetAnchor()
+    assert(IsTxtNode() || IsStartNode() || IsTableNode());
+    if (!m_pAnchoredFlys)
+    {
+        m_pAnchoredFlys.reset(new std::vector<SwFrmFmt*>);
+    }
+    m_pAnchoredFlys->push_back(pFlyFmt);
+}
+
+void SwNode::RemoveAnchoredFly(SwFrmFmt *const pFlyFmt)
+{
+    assert(pFlyFmt);
+    // cannot assert this in Remove because it is called when new anchor is already set
+//    assert(&pFlyFmt->GetAnchor(false).GetCntntAnchor()->nNode.GetNode() == this);
+    assert(IsTxtNode() || IsStartNode() || IsTableNode());
+    if (!m_pAnchoredFlys)
+    {
+        SwNodeIndex idx(GetNodes());
+        while (true)
+        {
+            SwNode & rNode(idx.GetNode());
+            if (rNode.m_pAnchoredFlys)
+            {
+                auto it(std::find(rNode.m_pAnchoredFlys->begin(), rNode.m_pAnchoredFlys->end(), pFlyFmt));
+                if (it != rNode.m_pAnchoredFlys->end())
+                {
+                    //XXX bug
+                }
+            }
+            ++idx;
+        }
+    }
+    assert(m_pAnchoredFlys);
+    auto it(std::find(m_pAnchoredFlys->begin(), m_pAnchoredFlys->end(), pFlyFmt));
+    assert(it != m_pAnchoredFlys->end());
+    m_pAnchoredFlys->erase(it);
+    if (m_pAnchoredFlys->empty())
+    {
+        m_pAnchoredFlys.reset();
+    }
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sw/source/core/layout/atrfrm.cxx

@@ -2532,9 +2532,11 @@ SwFrmFmt::~SwFrmFmt()
 {
     if( !GetDoc()->IsInDtor())
     {
-        const SwFmtAnchor& anchor = GetAnchor();
-        if( anchor.GetCntntAnchor() != NULL )
-            GetDoc()->GetFrmFmtAnchorMap()->Remove( this, anchor.GetCntntAnchor()->nNode );
+        const SwFmtAnchor& rAnchor = GetAnchor();
+        if (rAnchor.GetCntntAnchor() != nullptr)
+        {
+            rAnchor.GetCntntAnchor()->nNode.GetNode().RemoveAnchoredFly(this);
+        }
     }
 }
 
@@ -2627,9 +2629,13 @@ void SwFrmFmt::Modify( const SfxPoolItem* pOld, const SfxPoolItem* pNew )
     if( pOld && pOld->Which() == RES_ANCHOR )
         oldAnchorPosition = static_cast< const SwFmtAnchor* >( pOld )->GetCntntAnchor();
     if( oldAnchorPosition != NULL && ( newAnchorPosition == NULL || oldAnchorPosition->nNode.GetIndex() != newAnchorPosition->nNode.GetIndex()))
-        GetDoc()->GetFrmFmtAnchorMap()->Remove( this, oldAnchorPosition->nNode );
+    {
+        oldAnchorPosition->nNode.GetNode().RemoveAnchoredFly(this);
+    }
     if( newAnchorPosition != NULL && ( oldAnchorPosition == NULL || oldAnchorPosition->nNode.GetIndex() != newAnchorPosition->nNode.GetIndex()))
-        GetDoc()->GetFrmFmtAnchorMap()->Add( this, newAnchorPosition->nNode );
+    {
+        newAnchorPosition->nNode.GetNode().AddAnchoredFly(this);
+    }
 }
 
 void SwFrmFmt::RegisterToFormat( SwFmt& rFmt )
@@ -3481,39 +3487,4 @@ bool IsFlyFrmFmtInHeader(const SwFrmFmt& rFmt)
     return false;
 }
 
-
-SwFrmFmtAnchorMap::SwFrmFmtAnchorMap( const SwDoc* _doc )
-: doc( _doc )
-{
-}
-
-void SwFrmFmtAnchorMap::Add( SwFrmFmt* fmt, const SwNodeIndex& pos )
-{
-    (void) doc;
-    assert( pos.GetNode().GetDoc() == doc );
-    items.insert( std::make_pair( pos, fmt ));
-}
-
-void SwFrmFmtAnchorMap::Remove( SwFrmFmt* fmt, const SwNodeIndex& pos )
-{
-    (void) doc;
-    assert( pos.GetNode().GetDoc() == doc );
-    typedef std::multimap< SwNodeIndex, SwFrmFmt* >::iterator iterator;
-    std::pair< iterator, iterator > range = items.equal_range( pos );
-    for( iterator it = range.first; it != range.second; ++it )
-    {
-        if( it->second == fmt )
-        {
-            items.erase( it );
-            return;
-        }
-    }
-    assert( false );
-}
-
-SwFrmFmtAnchorMap::const_iterator_pair SwFrmFmtAnchorMap::equal_range( const SwNodeIndex& pos ) const
-{
-    return items.equal_range( pos );
-}
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sw/source/core/layout/frmtool.cxx

@@ -1019,12 +1019,11 @@ void AppendObjs( const SwFrmFmts *pTbl, sal_uLong nIndex,
 #else
     (void)pTbl;
 #endif
-    SwFrmFmtAnchorMap::const_iterator_pair range = doc->GetFrmFmtAnchorMap()->equal_range( SwNodeIndex( doc->GetNodes(), nIndex ));
-    for( std::multimap< SwNodeIndex, SwFrmFmt* >::const_iterator it = range.first;
-         it != range.second;
-         )
+    SwNode const& rNode(*doc->GetNodes()[nIndex]);
+    std::vector<SwFrmFmt*> const*const pFlys(rNode.GetAnchoredFlys());
+    for (size_t it = 0; pFlys && it != pFlys->size(); )
     {
-        SwFrmFmt *pFmt = it->second;
+        SwFrmFmt *const pFmt = (*pFlys)[it];
         const SwFmtAnchor &rAnch = pFmt->GetAnchor();
         if ( rAnch.GetCntntAnchor() &&
              (rAnch.GetCntntAnchor()->nNode.GetIndex() == nIndex) )

sw/source/core/txtnode/ndtxt.cxx

@@ -1133,12 +1133,10 @@ void SwTxtNode::Update(
             }
         }
 #endif
-        SwFrmFmtAnchorMap::const_iterator_pair range = GetDoc()->GetFrmFmtAnchorMap()->equal_range( SwNodeIndex( *this ));
-        for( SwFrmFmtAnchorMap::const_iterator it = range.first;
-             it != range.second;
-             ++it )
+        std::vector<SwFrmFmt*> const*const pFlys(GetAnchoredFlys());
+        for (size_t i = 0; pFlys && i != pFlys->size(); ++i)
         {
-            SwFrmFmt *pFmt = it->second;
+            SwFrmFmt const*const pFmt = (*pFlys)[i];
             const SwFmtAnchor& rAnchor = pFmt->GetAnchor();
             const SwPosition* pCntntAnchor = rAnchor.GetCntntAnchor();
             if (rAnchor.GetAnchorId() == FLY_AT_CHAR && pCntntAnchor)