sw/inc/doc.hxx | 5 --- sw/inc/frmfmt.hxx | 23 -------------- sw/inc/node.hxx | 9 +++++ sw/source/core/doc/doclay.cxx | 18 +++++------ sw/source/core/doc/docnew.cxx | 2 - sw/source/core/docnode/node.cxx | 47 ++++++++++++++++++++++++++++++ sw/source/core/layout/atrfrm.cxx | 51 +++++++-------------------------- sw/source/core/layout/frmtool.cxx | 9 ++--- sw/source/core/txtnode/ndtxt.cxx | 8 +---- sw/source/core/undo/undoflystrattr.cxx | 1 10 files changed, 84 insertions(+), 89 deletions(-)
New commits: commit dfe435f81c5c28a20e41b9127027b1be4f207055 Author: Michael Stahl <mst...@redhat.com> Date: Fri May 8 23:27:49 2015 +0200 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 (cherry picked from commit e07feb9457f2ffb373ae69b73dda290140e4005f) Reviewed-on: https://gerrit.libreoffice.org/15783 Reviewed-by: Caolán McNamara <caol...@redhat.com> Tested-by: Caolán McNamara <caol...@redhat.com> diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx index 2abd17c..b962f6e 100644 --- a/sw/inc/doc.hxx +++ b/sw/inc/doc.hxx @@ -102,7 +102,6 @@ class SwFmt; class SwFmtINetFmt; class SwFmtRefMark; class SwFrmFmt; -class SwFrmFmtAnchorMap; class SwFrmFmts; class SwFtnIdxs; class SwFtnInfo; @@ -306,7 +305,6 @@ class SW_DLLPUBLIC SwDoc : SwGrfFmtColl *mpDfltGrfFmtColl; SwFrmFmts *mpFrmFmtTbl; //< Format table - SwFrmFmtAnchorMap *mpFrmFmtAnchorMap; SwCharFmts *mpCharFmtTbl; SwFrmFmts *mpSpzFrmFmtTbl; SwSectionFmts *mpSectionFmtTbl; @@ -816,9 +814,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 e73f6bd..d3ad96a 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; @@ -303,27 +301,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 a8c7f29..591f193 100644 --- a/sw/inc/node.hxx +++ b/sw/inc/node.hxx @@ -98,6 +98,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; @@ -281,6 +286,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 6e35170..57248bd 100644 --- a/sw/source/core/doc/doclay.cxx +++ b/sw/source/core/doc/doclay.cxx @@ -1533,18 +1533,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 @@ -1557,12 +1556,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; } } diff --git a/sw/source/core/doc/docnew.cxx b/sw/source/core/doc/docnew.cxx index 95a3ded..eac9cc5 100644 --- a/sw/source/core/doc/docnew.cxx +++ b/sw/source/core/doc/docnew.cxx @@ -231,7 +231,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() ), @@ -610,7 +609,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 d57b18f..ad1354f 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. @@ -1942,4 +1943,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 3720cd3..7138f17 100644 --- a/sw/source/core/layout/atrfrm.cxx +++ b/sw/source/core/layout/atrfrm.cxx @@ -2430,9 +2430,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); + } } } @@ -2525,9 +2527,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 ) @@ -3337,39 +3343,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 5ea9e50..92d64c8 100644 --- a/sw/source/core/layout/frmtool.cxx +++ b/sw/source/core/layout/frmtool.cxx @@ -1015,12 +1015,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 281c483..0e9e61b 100644 --- a/sw/source/core/txtnode/ndtxt.cxx +++ b/sw/source/core/txtnode/ndtxt.cxx @@ -1142,12 +1142,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) diff --git a/sw/source/core/undo/undoflystrattr.cxx b/sw/source/core/undo/undoflystrattr.cxx index e5a194a..2492090 100644 --- a/sw/source/core/undo/undoflystrattr.cxx +++ b/sw/source/core/undo/undoflystrattr.cxx @@ -19,6 +19,7 @@ #include <undoflystrattr.hxx> #include <frmfmt.hxx> +#include <osl/diagnose.h> SwUndoFlyStrAttr::SwUndoFlyStrAttr( SwFlyFrmFmt& rFlyFrmFmt, const SwUndoId eUndoId,
_______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits