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

13 files changed, 46 insertions, 94 deletions

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();
         }