diff options
author | Michael Stahl <mstahl@redhat.com> | 2015-05-08 23:27:49 +0200 |
---|---|---|
committer | Michael Stahl <mstahl@redhat.com> | 2015-05-09 11:02:26 +0200 |
commit | e07feb9457f2ffb373ae69b73dda290140e4005f (patch) | |
tree | 0d0bccf291f44c36d3cb174a02bc1a0000ae3b34 | |
parent | fb73274fcce1edd3d898b7f79700fea96d0040f9 (diff) |
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 738fb2ad77e5a1a4d6e2dc540886a17f4527e4db)
Change-Id: I396d92b9d0b2045e98bad6d0b374303cd4e62b59
-rw-r--r-- | sw/inc/doc.hxx | 5 | ||||
-rw-r--r-- | sw/inc/frmfmt.hxx | 23 | ||||
-rw-r--r-- | sw/inc/node.hxx | 9 | ||||
-rw-r--r-- | sw/source/core/doc/doclay.cxx | 18 | ||||
-rw-r--r-- | sw/source/core/doc/docnew.cxx | 2 | ||||
-rw-r--r-- | sw/source/core/docnode/node.cxx | 47 | ||||
-rw-r--r-- | sw/source/core/layout/atrfrm.cxx | 51 | ||||
-rw-r--r-- | sw/source/core/layout/frmtool.cxx | 9 | ||||
-rw-r--r-- | sw/source/core/txtnode/ndtxt.cxx | 8 |
9 files changed, 83 insertions, 89 deletions
diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx index 7969c6976c38..186865035d7c 100644 --- a/sw/inc/doc.hxx +++ b/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; } diff --git a/sw/inc/frmfmt.hxx b/sw/inc/frmfmt.hxx index 53f3694b55ee..33a16b2019c8 100644 --- a/sw/inc/frmfmt.hxx +++ b/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: */ diff --git a/sw/inc/node.hxx b/sw/inc/node.hxx index d7621be69b26..f3990c6a0578 100644 --- a/sw/inc/node.hxx +++ b/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 diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx index c1a16273b933..454caac494c1 100644 --- a/sw/source/core/doc/doclay.cxx +++ b/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 diff --git a/sw/source/core/doc/docnew.cxx b/sw/source/core/doc/docnew.cxx index 0e945b36eed7..899c03ad7214 100644 --- a/sw/source/core/doc/docnew.cxx +++ b/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); } diff --git a/sw/source/core/docnode/node.cxx b/sw/source/core/docnode/node.cxx index 7ad39abd8184..e733d108c3e1 100644 --- a/sw/source/core/docnode/node.cxx +++ b/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: */ diff --git a/sw/source/core/layout/atrfrm.cxx b/sw/source/core/layout/atrfrm.cxx index f92ec8bbfeb2..335c85e96092 100644 --- a/sw/source/core/layout/atrfrm.cxx +++ b/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: */ diff --git a/sw/source/core/layout/frmtool.cxx b/sw/source/core/layout/frmtool.cxx index 234f2b97838f..39fdfae1416b 100644 --- a/sw/source/core/layout/frmtool.cxx +++ b/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) ) diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx index c291f14879ab..f1a0a7bcb632 100644 --- a/sw/source/core/txtnode/ndtxt.cxx +++ b/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) |