[Libreoffice-commits] core.git: Branch 'feature/cib_contract4236b' - sfx2/source sw/source
Jan-Marek Glogowski (via logerrit)
logerrit at kemper.freedesktop.org
Sun Jun 21 23:28:00 UTC 2020
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.glogowski at extern.cib.de>
AuthorDate: Fri Dec 6 21:56:25 2019 +0100
Commit: Thorsten Behrens <Thorsten.Behrens at 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();
}
More information about the Libreoffice-commits
mailing list