tdf#147731 sw: fix memory leak in SwDoc::CopyPageDesc()

Commit 963de9feb37105560fde14b44d992e47f341bb5b "sw: fix issue with
copying stashed frame format" fixed the actual bug here, but introduced
a new memory leak.

This causes an assert in CppunitTest_uiwriter3:
cppunittester: svl/source/items/itempool.cxx:779: void SfxItemPool::Remove(const SfxPoolItem&): Assertion `rItem.GetRefCount() && "RefCount == 0, Remove impossible"' failed.

The assert happens only when this is backported to the libreoffice-7-6
branch, because commit ab7c81f55621d7b0d1468c63305163016dd78837 "ITEM:
Get away from classic 'poolable' Item flag" removed the assert.

The problem is that a SwFormatFrameSize inside a footer SwFrameFormat is
leaked 4 times, because 4 SwFrameFormats are leaked; the leak is that
SwDoc::CopyPageDesc() creates a new pNewFormat, passed it to
StashFrameFormat(), which copies it but doesn't free it.

There is also a usage of std::shared_ptr here that is very questionable;
SwFrameFormat should never be shared between different SwPageDesc.

(regression from commit b802ab694a8a7357d4657f3e11b571144fa7c7bf)

Change-Id: I44133bc5e6789a51ce064f1aa5ea8b325224365b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163854
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
(cherry picked from commit 5c4ae1b19c51dcd62dad8e1d3e8beb87a0311352)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163846
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164012

3 files changed, 46 insertions, 46 deletions

diff --git a/sw/inc/pagedesc.hxx b/sw/inc/pagedesc.hxx
index 11bb347aa1fb..ddc7e659a5bb 100644
--- a/sw/inc/pagedesc.hxx
+++ b/sw/inc/pagedesc.hxx
@@ -151,9 +151,9 @@ class SW_DLLPUBLIC SwPageDesc final
 
     struct StashedPageDesc
     {
-        std::shared_ptr<SwFrameFormat> m_pStashedFirst;
-        std::shared_ptr<SwFrameFormat> m_pStashedLeft;
-        std::shared_ptr<SwFrameFormat> m_pStashedFirstLeft;
+        std::optional<SwFrameFormat> m_oStashedFirst;
+        std::optional<SwFrameFormat> m_oStashedLeft;
+        std::optional<SwFrameFormat> m_oStashedFirstLeft;
     };
 
     mutable StashedPageDesc m_aStashedHeader;
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index 17eaf5418e61..9145c67ee539 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -1556,21 +1556,21 @@ void SwDoc::CopyPageDesc( const SwPageDesc& rSrcDesc, SwPageDesc& rDstDesc,
                 {
                     if (pStashedFormatSrc->GetDoc() != this)
                     {
-                        SwFrameFormat* pNewFormat = new SwFrameFormat(GetAttrPool(), "CopyDesc", GetDfltFrameFormat());
+                        SwFrameFormat newFormat(GetAttrPool(), "CopyDesc", GetDfltFrameFormat());
 
                         SfxItemSet aAttrSet(pStashedFormatSrc->GetAttrSet());
                         aAttrSet.ClearItem(RES_HEADER);
                         aAttrSet.ClearItem(RES_FOOTER);
 
-                        pNewFormat->DelDiffs( aAttrSet );
-                        pNewFormat->SetFormatAttr( aAttrSet );
+                        newFormat.DelDiffs(aAttrSet);
+                        newFormat.SetFormatAttr(aAttrSet);
 
                         if (bHeader)
-                            CopyHeader(*pStashedFormatSrc, *pNewFormat);
+                            CopyHeader(*pStashedFormatSrc, newFormat);
                         else
-                            CopyFooter(*pStashedFormatSrc, *pNewFormat);
+                            CopyFooter(*pStashedFormatSrc, newFormat);
 
-                        rDstDesc.StashFrameFormat(*pNewFormat, bHeader, bLeft, bFirst);
+                        rDstDesc.StashFrameFormat(newFormat, bHeader, bLeft, bFirst);
                     }
                     else
                     {
diff --git a/sw/source/core/layout/pagedesc.cxx b/sw/source/core/layout/pagedesc.cxx
index 40a7b5865766..5bc08706b80a 100644
--- a/sw/source/core/layout/pagedesc.cxx
+++ b/sw/source/core/layout/pagedesc.cxx
@@ -83,13 +83,13 @@ SwPageDesc::SwPageDesc( const SwPageDesc &rCpy )
     , m_FootnoteInfo( rCpy.GetFootnoteInfo() )
     , m_pdList( nullptr )
 {
-    m_aStashedHeader.m_pStashedFirst = rCpy.m_aStashedHeader.m_pStashedFirst;
-    m_aStashedHeader.m_pStashedLeft = rCpy.m_aStashedHeader.m_pStashedLeft;
-    m_aStashedHeader.m_pStashedFirstLeft = rCpy.m_aStashedHeader.m_pStashedFirstLeft;
+    m_aStashedHeader.m_oStashedFirst = rCpy.m_aStashedHeader.m_oStashedFirst;
+    m_aStashedHeader.m_oStashedLeft = rCpy.m_aStashedHeader.m_oStashedLeft;
+    m_aStashedHeader.m_oStashedFirstLeft = rCpy.m_aStashedHeader.m_oStashedFirstLeft;
 
-    m_aStashedFooter.m_pStashedFirst = rCpy.m_aStashedFooter.m_pStashedFirst;
-    m_aStashedFooter.m_pStashedLeft = rCpy.m_aStashedFooter.m_pStashedLeft;
-    m_aStashedFooter.m_pStashedFirstLeft = rCpy.m_aStashedFooter.m_pStashedFirstLeft;
+    m_aStashedFooter.m_oStashedFirst = rCpy.m_aStashedFooter.m_oStashedFirst;
+    m_aStashedFooter.m_oStashedLeft = rCpy.m_aStashedFooter.m_oStashedLeft;
+    m_aStashedFooter.m_oStashedFirstLeft = rCpy.m_aStashedFooter.m_oStashedFirstLeft;
 
     if (rCpy.m_pTextFormatColl && rCpy.m_aDepends.IsListeningTo(rCpy.m_pTextFormatColl))
     {
@@ -110,13 +110,13 @@ SwPageDesc & SwPageDesc::operator = (const SwPageDesc & rSrc)
     m_FirstMaster = rSrc.m_FirstMaster;
     m_FirstLeft = rSrc.m_FirstLeft;
 
-    m_aStashedHeader.m_pStashedFirst = rSrc.m_aStashedHeader.m_pStashedFirst;
-    m_aStashedHeader.m_pStashedLeft = rSrc.m_aStashedHeader.m_pStashedLeft;
-    m_aStashedHeader.m_pStashedFirstLeft = rSrc.m_aStashedHeader.m_pStashedFirstLeft;
+    m_aStashedHeader.m_oStashedFirst = rSrc.m_aStashedHeader.m_oStashedFirst;
+    m_aStashedHeader.m_oStashedLeft = rSrc.m_aStashedHeader.m_oStashedLeft;
+    m_aStashedHeader.m_oStashedFirstLeft = rSrc.m_aStashedHeader.m_oStashedFirstLeft;
 
-    m_aStashedFooter.m_pStashedFirst = rSrc.m_aStashedFooter.m_pStashedFirst;
-    m_aStashedFooter.m_pStashedLeft = rSrc.m_aStashedFooter.m_pStashedLeft;
-    m_aStashedFooter.m_pStashedFirstLeft = rSrc.m_aStashedFooter.m_pStashedFirstLeft;
+    m_aStashedFooter.m_oStashedFirst = rSrc.m_aStashedFooter.m_oStashedFirst;
+    m_aStashedFooter.m_oStashedLeft = rSrc.m_aStashedFooter.m_oStashedLeft;
+    m_aStashedFooter.m_oStashedFirstLeft = rSrc.m_aStashedFooter.m_oStashedFirstLeft;
 
     m_aDepends.EndListeningAll();
     if (rSrc.m_pTextFormatColl && rSrc.m_aDepends.IsListeningTo(rSrc.m_pTextFormatColl))
@@ -409,30 +409,30 @@ void SwPageDesc::ChgFirstShare( bool bNew )
 void SwPageDesc::StashFrameFormat(const SwFrameFormat& rFormat, bool bHeader, bool bLeft, bool bFirst)
 {
     assert(rFormat.GetRegisteredIn());
-    std::shared_ptr<SwFrameFormat>* pFormat = nullptr;
+    std::optional<SwFrameFormat>* pFormat = nullptr;
 
     if (bHeader)
     {
         if (bLeft && !bFirst)
-            pFormat = &m_aStashedHeader.m_pStashedLeft;
+            pFormat = &m_aStashedHeader.m_oStashedLeft;
         else if (!bLeft && bFirst)
-            pFormat = &m_aStashedHeader.m_pStashedFirst;
+            pFormat = &m_aStashedHeader.m_oStashedFirst;
         else if (bLeft && bFirst)
-            pFormat = &m_aStashedHeader.m_pStashedFirstLeft;
+            pFormat = &m_aStashedHeader.m_oStashedFirstLeft;
     }
     else
     {
         if (bLeft && !bFirst)
-            pFormat = &m_aStashedFooter.m_pStashedLeft;
+            pFormat = &m_aStashedFooter.m_oStashedLeft;
         else if (!bLeft && bFirst)
-            pFormat = &m_aStashedFooter.m_pStashedFirst;
+            pFormat = &m_aStashedFooter.m_oStashedFirst;
         else if (bLeft && bFirst)
-            pFormat = &m_aStashedFooter.m_pStashedFirstLeft;
+            pFormat = &m_aStashedFooter.m_oStashedFirstLeft;
     }
 
     if (pFormat)
     {
-        *pFormat = std::make_shared<SwFrameFormat>(rFormat);
+        pFormat->emplace(rFormat);
     }
     else
     {
@@ -444,24 +444,24 @@ void SwPageDesc::StashFrameFormat(const SwFrameFormat& rFormat, bool bHeader, bo
 
 const SwFrameFormat* SwPageDesc::GetStashedFrameFormat(bool bHeader, bool bLeft, bool bFirst) const
 {
-    std::shared_ptr<SwFrameFormat>* pFormat = nullptr;
+    std::optional<SwFrameFormat>* pFormat = nullptr;
 
     if (bLeft && !bFirst)
     {
-        pFormat = bHeader ? &m_aStashedHeader.m_pStashedLeft : &m_aStashedFooter.m_pStashedLeft;
+        pFormat = bHeader ? &m_aStashedHeader.m_oStashedLeft : &m_aStashedFooter.m_oStashedLeft;
     }
     else if (!bLeft && bFirst)
     {
-        pFormat = bHeader ? &m_aStashedHeader.m_pStashedFirst : &m_aStashedFooter.m_pStashedFirst;
+        pFormat = bHeader ? &m_aStashedHeader.m_oStashedFirst : &m_aStashedFooter.m_oStashedFirst;
     }
     else if (bLeft && bFirst)
     {
-        pFormat = bHeader ? &m_aStashedHeader.m_pStashedFirstLeft : &m_aStashedFooter.m_pStashedFirstLeft;
+        pFormat = bHeader ? &m_aStashedHeader.m_oStashedFirstLeft : &m_aStashedFooter.m_oStashedFirstLeft;
     }
 
     if (pFormat)
     {
-        return pFormat->get();
+        return pFormat->has_value() ? &**pFormat : nullptr;
     }
     else
     {
@@ -476,15 +476,15 @@ bool SwPageDesc::HasStashedFormat(bool bHeader, bool bLeft, bool bFirst) const
     {
         if (bLeft && !bFirst)
         {
-            return m_aStashedHeader.m_pStashedLeft != nullptr;
+            return m_aStashedHeader.m_oStashedLeft.has_value();
         }
         else if (!bLeft && bFirst)
         {
-            return m_aStashedHeader.m_pStashedFirst != nullptr;
+            return m_aStashedHeader.m_oStashedFirst.has_value();
         }
         else if (bLeft && bFirst)
         {
-            return m_aStashedHeader.m_pStashedFirstLeft != nullptr;
+            return m_aStashedHeader.m_oStashedFirstLeft.has_value();
         }
         else
         {
@@ -496,15 +496,15 @@ bool SwPageDesc::HasStashedFormat(bool bHeader, bool bLeft, bool bFirst) const
     {
         if (bLeft && !bFirst)
         {
-            return m_aStashedFooter.m_pStashedLeft != nullptr;
+            return m_aStashedFooter.m_oStashedLeft.has_value();
         }
         else if (!bLeft && bFirst)
         {
-            return m_aStashedFooter.m_pStashedFirst != nullptr;
+            return m_aStashedFooter.m_oStashedFirst.has_value();
         }
         else if (bLeft && bFirst)
         {
-            return m_aStashedFooter.m_pStashedFirstLeft != nullptr;
+            return m_aStashedFooter.m_oStashedFirstLeft.has_value();
         }
         else
         {
@@ -520,15 +520,15 @@ void SwPageDesc::RemoveStashedFormat(bool bHeader, bool bLeft, bool bFirst)
     {
         if (bLeft && !bFirst)
         {
-            m_aStashedHeader.m_pStashedLeft.reset();
+            m_aStashedHeader.m_oStashedLeft.reset();
         }
         else if (!bLeft && bFirst)
         {
-            m_aStashedHeader.m_pStashedFirst.reset();
+            m_aStashedHeader.m_oStashedFirst.reset();
         }
         else if (bLeft && bFirst)
         {
-            m_aStashedHeader.m_pStashedFirstLeft.reset();
+            m_aStashedHeader.m_oStashedFirstLeft.reset();
         }
         else
         {
@@ -539,15 +539,15 @@ void SwPageDesc::RemoveStashedFormat(bool bHeader, bool bLeft, bool bFirst)
     {
         if (bLeft && !bFirst)
         {
-            m_aStashedFooter.m_pStashedLeft.reset();
+            m_aStashedFooter.m_oStashedLeft.reset();
         }
         else if (!bLeft && bFirst)
         {
-            m_aStashedFooter.m_pStashedFirst.reset();
+            m_aStashedFooter.m_oStashedFirst.reset();
         }
         else if (bLeft && bFirst)
         {
-            m_aStashedFooter.m_pStashedFirstLeft.reset();
+            m_aStashedFooter.m_oStashedFirstLeft.reset();
         }
         else
         {