[Libreoffice-commits] core.git: Branch 'feature/cib_contract4236' - 3 commits - external/breakpad sfx2/source sw/qa sw/source
Jan-Marek Glogowski (via logerrit)
logerrit at kemper.freedesktop.org
Mon Dec 16 10:36:36 UTC 2019
Rebased ref, commits from common ancestor:
commit a16f22ef9a714541227952de2db33b89ba3dd1e8
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 Dec 16 11:35:09 2019 +0100
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 b3092cd1cce8..4f390a017781 100644
--- a/sfx2/source/doc/objstor.cxx
+++ b/sfx2/source/doc/objstor.cxx
@@ -2448,7 +2448,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 52a68b3b995d..eae8e15d8a8b 100644
--- a/sw/source/core/inc/frame.hxx
+++ b/sw/source/core/inc/frame.hxx
@@ -901,7 +901,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;
@@ -1228,18 +1228,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();
@@ -1250,7 +1253,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 cb956d8f916c..4ac16fe97ffb 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 fb87c6025061..43b80dd2ee37 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -1878,7 +1878,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 =
@@ -1918,8 +1918,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;
@@ -2520,35 +2518,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 1f566f373125..86f43fcd5909 100644
--- a/sw/source/core/layout/fly.cxx
+++ b/sw/source/core/layout/fly.cxx
@@ -1447,11 +1447,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 174e15699e46..ad29af3ab114 100644
--- a/sw/source/core/layout/layact.cxx
+++ b/sw/source/core/layout/layact.cxx
@@ -1204,10 +1204,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;
@@ -1359,6 +1357,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() )
@@ -1583,11 +1582,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 bb547a588f3d..9be57b50ce98 100644
--- a/sw/source/core/layout/paintfrm.cxx
+++ b/sw/source/core/layout/paintfrm.cxx
@@ -3332,7 +3332,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 58d0c7d4d5ad..fc420a91897f 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 fa0a4e525258..f0b6f835c4d6 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 0fb98f161648..5b1ce63b0de0 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 492a738941c3..f890cf11289d 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -581,7 +581,7 @@ void SwTextFrame::AdjustFollow_( SwTextFormatter &rLine,
OSL_FAIL( "+SwTextFrame::JoinFrame: Follow is locked." );
return;
}
- if (GetFollow()->IsDeleteForbidden())
+ if (GetFollow()->IsFootnoteFrame() && GetFollow()->IsDeleteForbidden())
return;
JoinFrame();
}
commit 7993f86fcfa0de396790c1df9440f26855bef6d2
Author: Caolán McNamara <caolanm at redhat.com>
AuthorDate: Fri Jul 19 16:56:12 2019 +0100
Commit: Thorsten Behrens <Thorsten.Behrens at CIB.de>
CommitDate: Mon Dec 16 11:35:04 2019 +0100
crashtesting: failures on swfootnoteframe_colunlock_heap_use_after_free.sample
undo forcepoint80 hackery and try a different solution
Change-Id: I52b5f9b41074e122bd32e70967e198ce9f86aec7
Reviewed-on: https://gerrit.libreoffice.org/76072
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
Tested-by: Caolán McNamara <caolanm at redhat.com>
diff --git a/sw/qa/extras/layout/data/forcepoint-swfootnoteframe-1.rtf b/sw/qa/extras/layout/data/forcepoint-swfootnoteframe-1.rtf
new file mode 100644
index 000000000000..319f6f571826
--- /dev/null
+++ b/sw/qa/extras/layout/data/forcepoint-swfootnoteframe-1.rtf
@@ -0,0 +1 @@
... etc. - the rest is truncated
More information about the Libreoffice-commits
mailing list