From dfe435f81c5c28a20e41b9127027b1be4f207055 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Fri, 8 May 2015 23:27:49 +0200 Subject: sw: fix assert with frames anchored in redlines on rhbz490395-1.odt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Tested-by: Caolán McNamara --- 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 + 7 files changed, 75 insertions(+), 61 deletions(-) (limited to 'sw/source/core') diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx index 6e35170c25d3..57248bd9b84d 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 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::iterator checkPos = std::find( checkFmts.begin(), checkFmts.end(), pFmt ); + std::list::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 95a3ded11f8d..eac9cc5f7552 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 d57b18f93026..ad1354f3904a 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); + } + 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 3720cd3933fe..7138f1752ac9 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 5ea9e503784f..92d64c8fbe38 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 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 281c483608f8..0e9e61be0321 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 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 e5a194a529bf..2492090dea88 100644 --- a/sw/source/core/undo/undoflystrattr.cxx +++ b/sw/source/core/undo/undoflystrattr.cxx @@ -19,6 +19,7 @@ #include #include +#include SwUndoFlyStrAttr::SwUndoFlyStrAttr( SwFlyFrmFmt& rFlyFrmFmt, const SwUndoId eUndoId, -- cgit v1.2.3