sfx2/source/doc/objstor.cxx | 2 - sw/source/core/inc/frame.hxx | 17 +++++++----- sw/source/core/layout/calcmove.cxx | 6 ++-- sw/source/core/layout/flowfrm.cxx | 34 +----------------------- sw/source/core/layout/fly.cxx | 8 ++--- sw/source/core/layout/flycnt.cxx | 1 sw/source/core/layout/layact.cxx | 10 ++----- sw/source/core/layout/objectformattertxtfrm.cxx | 26 ++---------------- sw/source/core/layout/paintfrm.cxx | 2 - sw/source/core/layout/sectfrm.cxx | 3 -- sw/source/core/layout/ssfrm.cxx | 24 ++++++++++++---- sw/source/core/layout/tabfrm.cxx | 5 --- sw/source/core/text/frmform.cxx | 2 - 13 files changed, 46 insertions(+), 94 deletions(-)
New commits: commit 2f2057d0d182459c81e16f0636d2e3582fd13c0c Author: Jan-Marek Glogowski <jan-marek.glogow...@extern.cib.de> AuthorDate: Fri Dec 6 21:56:25 2019 +0100 Commit: Thorsten Behrens <thorsten.behr...@cib.de> CommitDate: Mon Jun 22 01:23:43 2020 +0200 WIP fix a preformance regression with table layout This eventually needs some test document? Revert "tdf#101821 sw: fix layout footnote use-after-free" This reverts commit 9dcb767c5bdccdf6606240afd6aa2c6bd3dcc9f4. Revert "tdf#124675 sw: fix crash when moving SwTextFrame in table to prev page" This reverts commit cc5916cd314a27b0cc99560ab887480026630a95. I tried to get rid of the IsDeleteForbidden() handling for footnotes, but that's its own can of worm, with regard to the SwFootnoteBossFrame::ResetFootnote handling. Using SwFrameDeleteGuard to protect this pointers... argh. Change-Id: Ia1c2fe179398094818c954a2742bcb3cef7c0b83 diff --git a/sfx2/source/doc/objstor.cxx b/sfx2/source/doc/objstor.cxx index 6d7b3f99ee69..dc670ebcb0e0 100644 --- a/sfx2/source/doc/objstor.cxx +++ b/sfx2/source/doc/objstor.cxx @@ -2432,7 +2432,7 @@ bool SfxObjectShell::ExportTo( SfxMedium& rMedium ) } return xFilter->filter( aArgs ); - }catch(...) + }catch(const uno::Exception&) {} } diff --git a/sw/source/core/inc/frame.hxx b/sw/source/core/inc/frame.hxx index b85ea2b7a36f..1ed2f2908586 100644 --- a/sw/source/core/inc/frame.hxx +++ b/sw/source/core/inc/frame.hxx @@ -903,7 +903,7 @@ public: void ValidateThisAndAllLowers( const sal_uInt16 nStage ); void ForbidDelete() { mbForbidDelete = true; } - void AllowDelete() { mbForbidDelete = false; } + void AllowDelete(bool bDestroy = true); drawinglayer::attribute::SdrAllFillAttributesHelperPtr getSdrAllFillAttributesHelper() const; bool supportsFullDrawingLayerFillAttributeSet() const; @@ -1230,18 +1230,21 @@ inline bool SwFrame::IsAccessibleFrame() const return bool(GetType() & FRM_ACCESSIBLE); } -//use this to protect a SwFrame for a given scope from getting deleted +// use this to protect a SwFrame for a given scope from getting deleted. +// normally it will destroy the protected frame in the constructor. +// but this doesn't make any sense for "this" frames, so if your frame +// is this, you very likely want to manage the proper lifecycle yourself. class SwFrameDeleteGuard { private: SwFrame *m_pForbidFrame; + bool m_bDestroy; + public: - //Flag pFrame for SwFrameDeleteGuard lifetime that we shouldn't delete - //it in e.g. SwSectionFrame::MergeNext etc because we will need it - //again after the SwFrameDeleteGuard dtor - explicit SwFrameDeleteGuard(SwFrame* pFrame) + explicit SwFrameDeleteGuard(SwFrame* pFrame, bool bDestroy = true) : m_pForbidFrame((pFrame && !pFrame->IsDeleteForbidden()) ? pFrame : nullptr) + , m_bDestroy(bDestroy) { if (m_pForbidFrame) m_pForbidFrame->ForbidDelete(); @@ -1252,7 +1255,7 @@ public: ~SwFrameDeleteGuard() { if (m_pForbidFrame) - m_pForbidFrame->AllowDelete(); + m_pForbidFrame->AllowDelete(m_bDestroy); } SwFrameDeleteGuard& operator=(const SwFrameDeleteGuard&) =delete; diff --git a/sw/source/core/layout/calcmove.cxx b/sw/source/core/layout/calcmove.cxx index caa81ecbd182..513932bda94f 100644 --- a/sw/source/core/layout/calcmove.cxx +++ b/sw/source/core/layout/calcmove.cxx @@ -243,7 +243,7 @@ void SwFrame::PrepareMake(vcl::RenderContext* pRenderContext) StackHack aHack; if ( GetUpper() ) { - SwFrameDeleteGuard aDeleteGuard(this); + SwFrameDeleteGuard aDeleteGuard(this, false); if ( lcl_IsCalcUpperAllowed( *this ) ) GetUpper()->Calc(pRenderContext); OSL_ENSURE( GetUpper(), ":-( Layout unstable (Upper gone)." ); @@ -377,7 +377,7 @@ void SwFrame::OptPrepareMake() !GetUpper()->IsFlyFrame() ) { { - SwFrameDeleteGuard aDeleteGuard(this); + SwFrameDeleteGuard aDeleteGuard(this, false); GetUpper()->Calc(getRootFrame()->GetCurrShell() ? getRootFrame()->GetCurrShell()->GetOut() : nullptr); } OSL_ENSURE( GetUpper(), ":-( Layout unstable (Upper gone)." ); @@ -1234,7 +1234,7 @@ void SwContentFrame::MakeAll(vcl::RenderContext* /*pRenderContext*/) return; } - auto xDeleteGuard = std::make_unique<SwFrameDeleteGuard>(this); + auto xDeleteGuard = std::make_unique<SwFrameDeleteGuard>(this, false); LockJoin(); long nFormatCount = 0; // - loop prevention diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx index 1b8997cd6c2b..3871b1f33091 100644 --- a/sw/source/core/layout/flowfrm.cxx +++ b/sw/source/core/layout/flowfrm.cxx @@ -1879,7 +1879,7 @@ bool SwFlowFrame::MoveFwd( bool bMakePage, bool bPageBreak, bool bMoveAlways ) } } - std::unique_ptr<SwFrameDeleteGuard> xDeleteGuard(bMakePage ? new SwFrameDeleteGuard(pOldBoss) : nullptr); + SwFrameDeleteGuard aDeleteGuard(bMakePage ? pOldBoss : nullptr); bool bSamePage = true; SwLayoutFrame *pNewUpper = @@ -1919,8 +1919,6 @@ bool SwFlowFrame::MoveFwd( bool bMakePage, bool bPageBreak, bool bMoveAlways ) pOldBoss = pOldBoss->FindFootnoteBossFrame( true ); SwPageFrame* pNewPage = pOldPage; - xDeleteGuard.reset(); - // First, we move the footnotes. bool bFootnoteMoved = false; @@ -2531,35 +2529,7 @@ bool SwFlowFrame::MoveBwd( bool &rbReformat ) bFollow = pSect->HasFollow(); } - { - auto const pOld = m_rThis.GetUpper(); -#if BOOST_VERSION < 105600 - std::list<SwFrameDeleteGuard> g; -#else - ::boost::optional<SwFrameDeleteGuard> g; -#endif - if (m_rThis.GetUpper()->IsCellFrame()) - { - // note: IsFollowFlowRow() is never set for new-style tables - SwTabFrame const*const pTabFrame(m_rThis.FindTabFrame()); - if ( pTabFrame->IsFollow() - && static_cast<SwTabFrame const*>(pTabFrame->GetPrecede())->HasFollowFlowLine() - && pTabFrame->GetFirstNonHeadlineRow() == m_rThis.GetUpper()->GetUpper()) - { - // lock follow-flow-row (similar to sections above) -#if BOOST_VERSION < 105600 - g.emplace_back(m_rThis.GetUpper()->GetUpper()); -#else - g.emplace(m_rThis.GetUpper()->GetUpper()); -#endif - assert(m_rThis.GetUpper()->GetUpper()->IsDeleteForbidden()); - } - } - pNewUpper->Calc(m_rThis.getRootFrame()->GetCurrShell()->GetOut()); - SAL_WARN_IF(pOld != m_rThis.GetUpper(), "sw.core", - "MoveBwd(): pNewUpper->Calc() moved this frame?"); - } - + pNewUpper->Calc(m_rThis.getRootFrame()->GetCurrShell()->GetOut()); m_rThis.Cut(); // optimization: format section, if its size is invalidated and if it's diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx index b5f364511f6e..9ef6fbffab53 100644 --- a/sw/source/core/layout/fly.cxx +++ b/sw/source/core/layout/fly.cxx @@ -1445,11 +1445,9 @@ void CalcContent( SwLayoutFrame *pLay, bool bNoColl ) } } - { - SwFrameDeleteGuard aDeletePageGuard(pSect ? pSect->FindPageFrame() : nullptr); - SwFrameDeleteGuard aDeleteGuard(pSect); - pFrame->Calc(pRenderContext); - } + SwFrameDeleteGuard aDeletePageGuard(pSect ? pSect->FindPageFrame() : nullptr); + SwFrameDeleteGuard aDeleteGuard(pSect); + pFrame->Calc(pRenderContext); // OD 14.03.2003 #i11760# - reset control flag for follow format. if ( pFrame->IsTextFrame() ) diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx index f3c442ac8cfa..dac2f7a4bfd5 100644 --- a/sw/source/core/layout/flycnt.cxx +++ b/sw/source/core/layout/flycnt.cxx @@ -403,6 +403,7 @@ void SwFlyAtContentFrame::MakeAll(vcl::RenderContext* pRenderContext) { SwTextFrame& rAnchPosAnchorFrame = dynamic_cast<SwTextFrame&>(*GetAnchorFrameContainingAnchPos()); + SwFrameDeleteGuard aDel(&rAnchPosAnchorFrame); // #i58182# - For the usage of new method // <SwObjectFormatterTextFrame::CheckMovedFwdCondition(..)> // to check move forward of anchor frame due to the object diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx index 04201e1c6f5e..c9bc309adb11 100644 --- a/sw/source/core/layout/layact.cxx +++ b/sw/source/core/layout/layact.cxx @@ -1200,10 +1200,8 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa aOldRect = static_cast<SwPageFrame*>(pLay)->GetBoundRect(pRenderContext); } - { - SwFrameDeleteGuard aDeleteGuard(pLay); - pLay->Calc(pRenderContext); - } + SwFrameDeleteGuard aDeleteGuard(pLay); + pLay->Calc(pRenderContext); if ( aOldFrame != pLay->getFrameArea() ) bChanged = true; @@ -1355,6 +1353,7 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa bool bTabChanged = false; while ( pLow && pLow->GetUpper() == pLay ) { + SwFrameDeleteGuard delG(pLow); if ( pLow->IsLayoutFrame() ) { if ( pLow->IsTabFrame() ) @@ -1579,11 +1578,10 @@ bool SwLayAction::FormatLayoutTab( SwTabFrame *pTab, bool bAddRect ) // format lowers, only if table frame is valid if ( pTab->isFrameAreaDefinitionValid() ) { - FlowFrameJoinLockGuard tabG(pTab); // tdf#124675 prevent Join() if pTab becomes empty SwLayoutFrame *pLow = static_cast<SwLayoutFrame*>(pTab->Lower()); while ( pLow ) { - SwFrameDeleteGuard rowG(pLow); // tdf#124675 prevent RemoveFollowFlowLine() + SwFrameDeleteGuard delG(pLow); bChanged |= FormatLayout( m_pImp->GetShell()->GetOut(), pLow, bAddRect ); if ( IsAgain() ) return false; diff --git a/sw/source/core/layout/objectformattertxtfrm.cxx b/sw/source/core/layout/objectformattertxtfrm.cxx index 1ba020a84901..d7aa3a29f41d 100644 --- a/sw/source/core/layout/objectformattertxtfrm.cxx +++ b/sw/source/core/layout/objectformattertxtfrm.cxx @@ -638,37 +638,17 @@ static void lcl_FormatContentOfLayoutFrame( SwLayoutFrame* pLayFrame, { break; } + + SwFrameDeleteGuard aCrudeHack(pLowerFrame); // ??? any issue setting this for non-footnote frames? if ( pLowerFrame->IsLayoutFrame() ) { - SwFrameDeleteGuard aCrudeHack(pLowerFrame); // ??? any issue setting this for non-footnote frames? lcl_FormatContentOfLayoutFrame( static_cast<SwLayoutFrame*>(pLowerFrame), pLastLowerFrame ); } else pLowerFrame->Calc(pLowerFrame->getRootFrame()->GetCurrShell()->GetOut()); - // Calc on a SwTextFrame in a footnote can move it to the next page - - // deletion of the SwFootnoteFrame was disabled with SwFrameDeleteGuard - // but now we have to clean up empty footnote frames to prevent crashes. - // Note: check it at this level, not lower: both container and footnote - // can be deleted at the same time! - SwFrame *const pNext = pLowerFrame->GetNext(); - if (pLowerFrame->IsFootnoteContFrame()) - { - for (SwFrame * pFootnote = pLowerFrame->GetLower(); pFootnote; ) - { - assert(pFootnote->IsFootnoteFrame()); - SwFrame *const pNextNote = pFootnote->GetNext(); - if (!pFootnote->IsDeleteForbidden() && !pFootnote->GetLower() && !pFootnote->IsColLocked() && - !static_cast<SwFootnoteFrame*>(pFootnote)->IsBackMoveLocked()) - { - pFootnote->Cut(); - SwFrame::DestroyFrame(pFootnote); - } - pFootnote = pNextNote; - } - } - pLowerFrame = pNext; + pLowerFrame = pLowerFrame->GetNext(); } } diff --git a/sw/source/core/layout/paintfrm.cxx b/sw/source/core/layout/paintfrm.cxx index 60b25866d073..ae4ba7bec41a 100644 --- a/sw/source/core/layout/paintfrm.cxx +++ b/sw/source/core/layout/paintfrm.cxx @@ -3337,7 +3337,7 @@ void SwLayoutFrame::PaintSwFrame(vcl::RenderContext& rRenderContext, SwRect cons if ( !pFrame ) return; - SwFrameDeleteGuard g(const_cast<SwLayoutFrame*>(this)); // lock because Calc() and recursion + SwFrameDeleteGuard g(const_cast<SwLayoutFrame*>(this), false); // lock because Calc() and recursion SwShortCut aShortCut( *pFrame, rRect ); bool bCnt = pFrame->IsContentFrame(); if ( bCnt ) diff --git a/sw/source/core/layout/sectfrm.cxx b/sw/source/core/layout/sectfrm.cxx index 524562585bd1..3f3f85bd94fc 100644 --- a/sw/source/core/layout/sectfrm.cxx +++ b/sw/source/core/layout/sectfrm.cxx @@ -462,9 +462,6 @@ bool SwSectionFrame::HasToBreak( const SwFrame* pFrame ) const |*/ void SwSectionFrame::MergeNext( SwSectionFrame* pNxt ) { - if (pNxt->IsDeleteForbidden()) - return; - if (!pNxt->IsJoinLocked() && GetSection() == pNxt->GetSection()) { PROTOCOL( this, PROT::Section, DbgAction::Merge, pNxt ) diff --git a/sw/source/core/layout/ssfrm.cxx b/sw/source/core/layout/ssfrm.cxx index 6905c1e06134..60aca6d10115 100644 --- a/sw/source/core/layout/ssfrm.cxx +++ b/sw/source/core/layout/ssfrm.cxx @@ -381,13 +381,23 @@ SwFrame::~SwFrame() void SwFrame::DestroyFrame(SwFrame *const pFrame) { - if (pFrame) - { - pFrame->m_isInDestroy = true; - pFrame->DestroyImpl(); - assert(pFrame->mbInDtor); // check that nobody forgot to call base class - delete pFrame; - } + if (!pFrame) + return; + + pFrame->m_isInDestroy = true; + if (pFrame->IsDeleteForbidden()) + return; + + pFrame->DestroyImpl(); + assert(pFrame->mbInDtor); // check that nobody forgot to call base class + delete pFrame; +} + +void SwFrame::AllowDelete(bool bDestroy) +{ + mbForbidDelete = false; + if (m_isInDestroy && bDestroy) + DestroyFrame(this); } const SwFrameFormat * SwLayoutFrame::GetFormat() const diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index 2406ff724cb3..70bc7af1b9d0 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -885,11 +885,6 @@ bool SwTabFrame::RemoveFollowFlowLine() // #140081# Make code robust. if ( !pFollowFlowLine || !pLastLine ) return true; - if (pFollowFlowLine->IsDeleteForbidden()) - { - SAL_WARN("sw.layout", "Cannot remove in-use Follow Flow Line"); - return false; - } // We have to reset the flag here, because lcl_MoveRowContent // calls a GrowFrame(), which has a different behavior if diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx index 69db90b6502d..5d94518b8839 100755 --- a/sw/source/core/text/frmform.cxx +++ b/sw/source/core/text/frmform.cxx @@ -580,7 +580,7 @@ void SwTextFrame::AdjustFollow_( SwTextFormatter &rLine, OSL_FAIL( "+SwTextFrame::JoinFrame: Follow is locked." ); return; } - if (GetFollow()->IsDeleteForbidden()) + if (GetFollow()->IsFootnoteFrame() && GetFollow()->IsDeleteForbidden()) return; JoinFrame(); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits