tdf#116977 secured ::Clone methods

Renamed SdrPage::Clone -> SdrPage::CloneSdrPage
Renamed SdrObject::Clone -> SdrObject::CloneSdrObject

Giving SdrModel is no longer an option, but a must (as
reference). This makes future changes more safe by force
usage to think about it. Also equals the constructors
which already require a target SdrModel.
Done the same for ::CloneSdrPage.

Change-Id: I06f0129e15140bd8693db27a445037d7e2f7f652
Reviewed-on: https://gerrit.libreoffice.org/53933
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Armin Le Grand <Armin.Le.Grand@cib.de>

11 files changed, 51 insertions, 27 deletions

diff --git a/sw/inc/dcontact.hxx b/sw/inc/dcontact.hxx
index 7898c68154ab..544e55973526 100644
--- a/sw/inc/dcontact.hxx
+++ b/sw/inc/dcontact.hxx
@@ -186,7 +186,9 @@ public:
 class SW_DLLPUBLIC SwFlyDrawContact final : public SwContact
 {
 private:
-    std::unique_ptr<SwFlyDrawObj> mpMasterObj;
+    typedef std::unique_ptr< SwFlyDrawObj, SdrObjectFreeOp > SwFlyDrawObjPtr;
+
+    SwFlyDrawObjPtr mpMasterObj;
     void SwClientNotify(const SwModify&, const SfxHint& rHint) override;
     sal_uInt32 GetOrdNumForNewRef(const SwFlyFrame* pFly);
 
@@ -236,18 +238,19 @@ class SwDrawVirtObj : public SdrVirtObj
          of original SnapRect) */
         virtual sdr::contact::ViewContact* CreateObjectSpecificViewContact() override;
 
+        // protected destructor
+        virtual ~SwDrawVirtObj() override;
+
     public:
         SwDrawVirtObj(
             SdrModel& rSdrModel,
             SdrObject& _rNewObj,
             SwDrawContact& _rDrawContact);
 
-        virtual ~SwDrawVirtObj() override;
-
         /// access to offset
         virtual const Point GetOffset() const override;
 
-        virtual SwDrawVirtObj* Clone(SdrModel* pTargetModel = nullptr) const override;
+        virtual SwDrawVirtObj* CloneSdrObject(SdrModel& rTargetModel) const override;
         SwDrawVirtObj& operator= (const SwDrawVirtObj& rObj);
 
         /// connection to writer layout
@@ -309,6 +312,8 @@ bool CheckControlLayer( const SdrObject *pObj );
 /** ContactObject for connection of formats as representatives of draw objects
  in SwClient and the objects themselves in Drawing (SDrObjUserCall). */
 
+typedef std::unique_ptr< SwDrawVirtObj, SdrObjectFreeOp > SwDrawVirtObjPtr;
+
 class SwDrawContact final : public SwContact
 {
     private:
@@ -317,7 +322,7 @@ class SwDrawContact final : public SwContact
         SwAnchoredDrawObject maAnchoredDrawObj;
 
         /** container for 'virtual' drawing object supporting drawing objects in headers/footers. */
-        std::vector<std::unique_ptr<SwDrawVirtObj>> maDrawVirtObjs;
+        std::vector< SwDrawVirtObjPtr > maDrawVirtObjs;
 
         /** boolean indicating set 'master' drawing
          object has been cleared. */
diff --git a/sw/inc/dpage.hxx b/sw/inc/dpage.hxx
index 7b3b50e4295d..a599a462d2bb 100644
--- a/sw/inc/dpage.hxx
+++ b/sw/inc/dpage.hxx
@@ -39,7 +39,7 @@ public:
     explicit SwDPage(SwDrawModel& rNewModel, bool bMasterPage);
     virtual ~SwDPage() override;
 
-    virtual SwDPage* Clone(SdrModel* pNewModel = nullptr) const override;
+    virtual SwDPage* CloneSdrPage(SdrModel& rTargetModel) const override;
 
     // #i3694#
     // This GetOffset() method is not needed anymore, it even leads to errors.
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index 58e44dc5346d..d50c9f232f94 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -126,7 +126,7 @@ SdrObject* SwDoc::CloneSdrObj( const SdrObject& rObj, bool bMoveWithinDoc,
     }
 
     // TTTT Clone directly to target SdrModel
-    SdrObject *pObj = rObj.Clone(getIDocumentDrawModelAccess().GetDrawModel());
+    SdrObject *pObj(rObj.CloneSdrObject(*getIDocumentDrawModelAccess().GetDrawModel()));
 
     if( bMoveWithinDoc && SdrInventor::FmForm == pObj->GetObjInventor() )
     {
diff --git a/sw/source/core/draw/dcontact.cxx b/sw/source/core/draw/dcontact.cxx
index c690b2514156..de8d91615722 100644
--- a/sw/source/core/draw/dcontact.cxx
+++ b/sw/source/core/draw/dcontact.cxx
@@ -80,6 +80,7 @@ namespace
     struct VirtObjAnchoredAtFramePred
     {
         const SwFrame* m_pAnchorFrame;
+
         // #i26791# - compare with master frame
         static const SwFrame* FindFrame(const SwFrame* pFrame)
         {
@@ -89,10 +90,16 @@ namespace
             while(pContentFrame->IsFollow())
                 pContentFrame = pContentFrame->FindMaster();
             return pContentFrame;
-        };
-        VirtObjAnchoredAtFramePred(const SwFrame* pAnchorFrame) : m_pAnchorFrame(FindFrame(pAnchorFrame)) {};
-        bool operator()(const std::unique_ptr<SwDrawVirtObj>& rpDrawVirtObj)
-            { return FindFrame(rpDrawVirtObj->GetAnchorFrame()) == m_pAnchorFrame; };
+        }
+
+        VirtObjAnchoredAtFramePred(const SwFrame* pAnchorFrame)
+        :   m_pAnchorFrame(FindFrame(pAnchorFrame))
+        {}
+
+        bool operator()(const SwDrawVirtObjPtr& rpDrawVirtObj)
+        {
+            return FindFrame(rpDrawVirtObj->GetAnchorFrame()) == m_pAnchorFrame;
+        }
     };
 }
 
@@ -813,7 +820,7 @@ SwFrame* SwDrawContact::GetAnchorFrame(SdrObject const *const pDrawObj)
 SwDrawVirtObj* SwDrawContact::AddVirtObj()
 {
     maDrawVirtObjs.push_back(
-        std::unique_ptr<SwDrawVirtObj>(
+        SwDrawVirtObjPtr(
             new SwDrawVirtObj(
                 GetMaster()->getSdrModelFromSdrObject(),
                 *GetMaster(),
@@ -1676,7 +1683,8 @@ void SwDrawContact::DisconnectObjFromLayout( SdrObject* _pDrawObj )
     else
     {
         const auto ppVirtDrawObj(std::find_if(maDrawVirtObjs.begin(), maDrawVirtObjs.end(),
-                [] (const std::unique_ptr<SwDrawVirtObj>& pObj) { return pObj->IsConnected(); }));
+                [] (const SwDrawVirtObjPtr& pObj) { return pObj->IsConnected(); }));
+
         if(ppVirtDrawObj != maDrawVirtObjs.end())
         {
             // replace found 'virtual' drawing object by 'master' drawing
@@ -2180,10 +2188,10 @@ SwDrawVirtObj& SwDrawVirtObj::operator=( const SwDrawVirtObj& rObj )
     return *this;
 }
 
-SwDrawVirtObj* SwDrawVirtObj::Clone(SdrModel* pTargetModel) const
+SwDrawVirtObj* SwDrawVirtObj::CloneSdrObject(SdrModel& rTargetModel) const
 {
     SwDrawVirtObj* pObj = new SwDrawVirtObj(
-        nullptr == pTargetModel ? getSdrModelFromSdrObject() : *pTargetModel,
+        rTargetModel,
         rRefObj,
         mrDrawContact);
 
diff --git a/sw/source/core/draw/dpage.cxx b/sw/source/core/draw/dpage.cxx
index d50a7f5780cf..cfedb393d854 100644
--- a/sw/source/core/draw/dpage.cxx
+++ b/sw/source/core/draw/dpage.cxx
@@ -72,9 +72,9 @@ void SwDPage::lateInit(const SwDPage& rSrcPage)
     }
 }
 
-SwDPage* SwDPage::Clone(SdrModel* const pNewModel) const
+SwDPage* SwDPage::CloneSdrPage(SdrModel& rTargetModel) const
 {
-    SwDrawModel& rSwDrawModel(static_cast< SwDrawModel& >(nullptr == pNewModel ? getSdrModelFromSdrPage() : *pNewModel));
+    SwDrawModel& rSwDrawModel(static_cast< SwDrawModel& >(rTargetModel));
     SwDPage* pClonedSwDPage(
         new SwDPage(
             rSwDrawModel,
diff --git a/sw/source/core/frmedt/fecopy.cxx b/sw/source/core/frmedt/fecopy.cxx
index 849787aedc01..478bb781b116 100644
--- a/sw/source/core/frmedt/fecopy.cxx
+++ b/sw/source/core/frmedt/fecopy.cxx
@@ -1362,7 +1362,7 @@ void SwFEShell::Paste( SvStream& rStrm, SwPasteSdr nAction, const Point* pPt )
                     }
                 }
 
-                SdrObject* pNewObj = pClpObj->Clone();
+                SdrObject* pNewObj(pClpObj->CloneSdrObject(pOldObj->getSdrModelFromSdrObject()));
                 tools::Rectangle aOldObjRect( pOldObj->GetCurrentBoundRect() );
                 Size aOldObjSize( aOldObjRect.GetSize() );
                 tools::Rectangle aNewRect( pNewObj->GetCurrentBoundRect() );
@@ -1542,7 +1542,7 @@ bool SwFEShell::Paste(const Graphic &rGrf, const OUString& rURL)
 
         if(dynamic_cast< SdrGrafObj* >(pObj))
         {
-            SdrGrafObj* pNewGrafObj = static_cast<SdrGrafObj*>(pObj->Clone());
+            SdrGrafObj* pNewGrafObj(static_cast<SdrGrafObj*>(pObj->CloneSdrObject(pObj->getSdrModelFromSdrObject())));
 
             pNewGrafObj->SetGraphic(rGrf);
 
diff --git a/sw/source/core/inc/dflyobj.hxx b/sw/source/core/inc/dflyobj.hxx
index 6e157b1ed3db..c22ffae16251 100644
--- a/sw/source/core/inc/dflyobj.hxx
+++ b/sw/source/core/inc/dflyobj.hxx
@@ -40,10 +40,11 @@ protected:
     // is called when RecalcBoundRect() is used
     virtual sdr::contact::ViewContact* CreateObjectSpecificViewContact() override;
 
-public:
+    // protected destructor
+    virtual ~SwFlyDrawObj() override;
 
+public:
     SwFlyDrawObj(SdrModel& rSdrModel);
-    virtual ~SwFlyDrawObj() override;
 
     // for instantiation of this class while loading (via factory)
     virtual SdrInventor GetObjInventor()     const override;
@@ -68,6 +69,9 @@ protected:
     // of original SnapRect)
     virtual sdr::contact::ViewContact* CreateObjectSpecificViewContact() override;
 
+    // protected destructor
+    virtual ~SwVirtFlyDrawObj() override;
+
 public:
     // for paints triggered form ExecutePrimitive
     void wrap_DoPaintObject(
@@ -84,7 +88,6 @@ public:
         SdrModel& rSdrModel,
         SdrObject& rNew,
         SwFlyFrame* pFly);
-    virtual ~SwVirtFlyDrawObj() override;
 
     // override method of base class SdrVirtObj
     virtual void     TakeObjInfo( SdrObjTransformInfoRec& rInfo ) const override;
diff --git a/sw/source/core/layout/atrfrm.cxx b/sw/source/core/layout/atrfrm.cxx
index d7b84212e52a..bcc42a8b06f8 100644
--- a/sw/source/core/layout/atrfrm.cxx
+++ b/sw/source/core/layout/atrfrm.cxx
@@ -3389,7 +3389,7 @@ OUString SwDrawFrameFormat::GetDescription() const
     {
         if (pSdrObj != m_pSdrObjectCached)
         {
-            SdrObject * pSdrObjCopy = pSdrObj->Clone();
+            SdrObject * pSdrObjCopy(pSdrObj->CloneSdrObject(pSdrObj->getSdrModelFromSdrObject()));
             SdrUndoNewObj * pSdrUndo = new SdrUndoNewObj(*pSdrObjCopy);
             m_sSdrObjectCachedComment = pSdrUndo->GetComment();
 
diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx
index 7e6876432a67..e87b5a06dae7 100644
--- a/sw/source/core/layout/fly.cxx
+++ b/sw/source/core/layout/fly.cxx
@@ -376,8 +376,14 @@ void SwFlyFrame::FinitDrawObj()
             }
         }
     }
-    GetVirtDrawObj()->SetUserCall(nullptr); // Else calls delete of the ContactObj
-    delete GetVirtDrawObj();            // Deregisters itself at the Master
+
+    // Else calls delete of the ContactObj
+    GetVirtDrawObj()->SetUserCall(nullptr);
+
+    // Deregisters itself at the Master
+    // always use SdrObject::Free(...) for SdrObjects (!)
+    SdrObject* pTemp(GetVirtDrawObj());
+    SdrObject::Free(pTemp);
 }
 
 void SwFlyFrame::ChainFrames( SwFlyFrame *pMaster, SwFlyFrame *pFollow )
diff --git a/sw/source/filter/ww8/wrtww8gr.cxx b/sw/source/filter/ww8/wrtww8gr.cxx
index ee2f92901b1e..6497403e5421 100644
--- a/sw/source/filter/ww8/wrtww8gr.cxx
+++ b/sw/source/filter/ww8/wrtww8gr.cxx
@@ -196,7 +196,9 @@ bool WW8Export::TestOleNeedsGraphic(const SwAttrSet& rSet,
                 delete pGraphicStream;
             }
 
-            delete pRet;
+            // always use SdrObject::Free(...) for SdrObjects (!)
+            SdrObject* pTemp(pRet);
+            SdrObject::Free(pTemp);
         }
     }
     else
diff --git a/sw/source/uibase/shells/drawsh.cxx b/sw/source/uibase/shells/drawsh.cxx
index 2f84dc2fed6c..f9e461d8b8ba 100644
--- a/sw/source/uibase/shells/drawsh.cxx
+++ b/sw/source/uibase/shells/drawsh.cxx
@@ -138,7 +138,7 @@ void SwDrawShell::InsertPictureFromFile(SdrObject& rObject)
 
                 if (SdrGrafObj* pSdrGrafObj = dynamic_cast<SdrGrafObj*>(&rObject))
                 {
-                    SdrGrafObj* pNewGrafObj = pSdrGrafObj->Clone();
+                    SdrGrafObj* pNewGrafObj(pSdrGrafObj->CloneSdrObject(pSdrGrafObj->getSdrModelFromSdrObject()));
 
                     pNewGrafObj->SetGraphic(aGraphic);