From 9638e6207c7fc48712b1b238177462c00f5011e8 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 3 Dec 2014 22:01:57 +0100 Subject: ooo#93212 avoid slicing during construction of SdrPage Also hide copy ctor and assignment operator of all derived classes, to ensure that Clone() is the only method to make copies of them. Change-Id: Icb3b50c63b086abe8c9add32e3041fe19692d20b --- basctl/source/dlged/dlgedpage.cxx | 10 +++- basctl/source/inc/dlgedpage.hxx | 5 ++ include/svx/fmpage.hxx | 9 +++- include/svx/obj3d.hxx | 8 ++- include/svx/svdpage.hxx | 27 +++++++--- reportdesign/inc/RptPage.hxx | 2 + reportdesign/source/core/sdr/RptPage.cxx | 4 +- sc/inc/drawpage.hxx | 3 ++ sd/inc/sdpage.hxx | 6 ++- sd/source/core/sdpage2.cxx | 28 ++++++----- svx/source/engine3d/obj3d.cxx | 11 ++++- svx/source/form/fmpage.cxx | 11 ++++- svx/source/svdraw/svdpage.cxx | 84 +++++++++----------------------- sw/inc/dpage.hxx | 3 ++ 14 files changed, 122 insertions(+), 89 deletions(-) diff --git a/basctl/source/dlged/dlgedpage.cxx b/basctl/source/dlged/dlgedpage.cxx index 86a792f7ee69..15f7f1253869 100644 --- a/basctl/source/dlged/dlgedpage.cxx +++ b/basctl/source/dlged/dlgedpage.cxx @@ -33,6 +33,12 @@ DlgEdPage::DlgEdPage(DlgEdModel& rModel, bool bMasterPage) { } +DlgEdPage::DlgEdPage(const DlgEdPage& rSrcPage) + : SdrPage(rSrcPage) + , pDlgEdForm(0) +{ +} + DlgEdPage::~DlgEdPage() { Clear(); @@ -41,7 +47,9 @@ DlgEdPage::~DlgEdPage() SdrPage* DlgEdPage::Clone() const { - return new DlgEdPage( *this ); + DlgEdPage* const pNewPage = new DlgEdPage( *this ); + pNewPage->lateInit( *this ); + return pNewPage; } diff --git a/basctl/source/inc/dlgedpage.hxx b/basctl/source/inc/dlgedpage.hxx index b92760f68d7d..02752920769a 100644 --- a/basctl/source/inc/dlgedpage.hxx +++ b/basctl/source/inc/dlgedpage.hxx @@ -34,6 +34,8 @@ class DlgEdForm; class DlgEdPage : public SdrPage { + DlgEdPage& operator=(const DlgEdPage&) SAL_DELETED_FUNCTION; + private: DlgEdForm* pDlgEdForm; @@ -50,6 +52,9 @@ public: DlgEdForm* GetDlgEdForm() const { return pDlgEdForm; } virtual SdrObject* SetObjectOrdNum(size_t nOldObjNum, size_t nNewObjNum) SAL_OVERRIDE; + +protected: + DlgEdPage(const DlgEdPage& rSrcPage); }; } // namespace basctl diff --git a/include/svx/fmpage.hxx b/include/svx/fmpage.hxx index e381a64bada7..a48df9ce5fda 100644 --- a/include/svx/fmpage.hxx +++ b/include/svx/fmpage.hxx @@ -40,6 +40,8 @@ class HelpEvent; class SVX_DLLPUBLIC FmFormPage : public SdrPage { + FmFormPage& operator=(const FmFormPage&) SAL_DELETED_FUNCTION; + friend class FmFormObj; FmFormPageImpl* m_pImpl; OUString m_sPageName; @@ -48,12 +50,12 @@ public: TYPEINFO_OVERRIDE(); FmFormPage(FmFormModel& rModel, bool bMasterPage=false); - FmFormPage(const FmFormPage& rPage); virtual ~FmFormPage(); virtual void SetModel(SdrModel* pNewModel) SAL_OVERRIDE; virtual SdrPage* Clone() const SAL_OVERRIDE; + // TODO: Uh huh, how is this supposed to work? Creating a SdrPage from FmFormPage? using SdrPage::Clone; virtual void InsertObject(SdrObject* pObj, size_t nPos = SAL_MAX_SIZE, @@ -73,6 +75,11 @@ public: vcl::Window* pWin, SdrView* pView, const HelpEvent& rEvt ); + +protected: + FmFormPage(const FmFormPage& rPage); + + void lateInit(const FmFormPage& rPage); }; #endif // INCLUDED_SVX_FMPAGE_HXX diff --git a/include/svx/obj3d.hxx b/include/svx/obj3d.hxx index bdba2c34dae2..f370f20a5647 100644 --- a/include/svx/obj3d.hxx +++ b/include/svx/obj3d.hxx @@ -78,16 +78,22 @@ public: class E3dObjList : public SdrObjList { + E3dObjList &operator=(const E3dObjList& rSrcList) SAL_DELETED_FUNCTION; + public: TYPEINFO_OVERRIDE(); E3dObjList(SdrModel* pNewModel = 0, SdrPage* pNewPage = 0, E3dObjList* pNewUpList = 0); - SVX_DLLPUBLIC E3dObjList(const E3dObjList& rSrcList); SVX_DLLPUBLIC virtual ~E3dObjList(); + virtual E3dObjList* Clone() const; + virtual void NbcInsertObject(SdrObject* pObj, size_t nPos=SAL_MAX_SIZE, const SdrInsertReason* pReason=NULL) SAL_OVERRIDE; virtual void InsertObject(SdrObject* pObj, size_t nPos=SAL_MAX_SIZE, const SdrInsertReason* pReason=NULL) SAL_OVERRIDE; virtual SdrObject* NbcRemoveObject(size_t nObjNum) SAL_OVERRIDE; virtual SdrObject* RemoveObject(size_t nObjNum) SAL_OVERRIDE; + +protected: + SVX_DLLPUBLIC E3dObjList(const E3dObjList& rSrcList); }; /************************************************************************* diff --git a/include/svx/svdpage.hxx b/include/svx/svdpage.hxx index c73718618f71..87bf2f32e8bc 100644 --- a/include/svx/svdpage.hxx +++ b/include/svx/svdpage.hxx @@ -78,6 +78,9 @@ public: class SVX_DLLPUBLIC SdrObjList { + SdrObjList(const SdrObjList& rSrcList) SAL_DELETED_FUNCTION; + SdrObjList &operator=(const SdrObjList& rSrcList) SAL_DELETED_FUNCTION; + private: typedef ::std::vector SdrObjectContainerType; SdrObjectContainerType maList; @@ -97,20 +100,23 @@ friend class SdrEditView; protected: virtual void RecalcRects(); + SdrObjList(); + void lateInit(const SdrObjList& rSrcList); + private: /// simple ActionChildInserted forwarder to have it on a central place void impChildInserted(SdrObject& rChild) const; public: TYPEINFO(); SdrObjList(SdrModel* pNewModel, SdrPage* pNewPage, SdrObjList* pNewUpList=NULL); - SdrObjList(const SdrObjList& rSrcList); virtual ~SdrObjList(); + + virtual SdrObjList* Clone() const; + // !!! Diese Methode nur fuer Leute, die ganz genau wissen was sie tun !!! // #110094# This should not be needed (!) void SetObjOrdNumsDirty() { bObjOrdNumsDirty=true; } - // pModel, pPage, pUpList und pOwnerObj werden Zuweisungeoperator nicht veraendert! - void operator=(const SdrObjList& rSrcList); void CopyObjects(const SdrObjList& rSrcList); // alles Aufraeumen (ohne Undo) void Clear(); @@ -403,6 +409,7 @@ public: class SVX_DLLPUBLIC SdrPage : public SdrObjList, public tools::WeakBase< SdrPage > { + SdrPage& operator=(const SdrPage& rSrcPage) SAL_DELETED_FUNCTION; // start PageUser section private: @@ -473,15 +480,19 @@ protected: ::com::sun::star::drawing::XDrawPage> const&); virtual ::com::sun::star::uno::Reference< ::com::sun::star::uno::XInterface > createUnoPage(); + // Copying of pages is split into two parts: construction and copying of page objects, + // because the copying might need access to fully initialized page. Clone() is responsible + // to call lateInit() after copy-construction of a new object. Any initialization in derived + // classes that needs access to the page objects must be deferred to lateInit. And it must + // call lateInit() of its parent class. + SdrPage(const SdrPage& rSrcPage); + void lateInit(const SdrPage& rSrcPage); + public: TYPEINFO_OVERRIDE(); SdrPage(SdrModel& rNewModel, bool bMasterPage=false); - // Copy-Ctor und Zuweisungeoperator sind nicht getestet! - SdrPage(const SdrPage& rSrcPage); virtual ~SdrPage(); - // pModel, pPage, pUpList, pOwnerObj und mbInserted werden Zuweisungeoperator nicht veraendert! - SdrPage& operator=(const SdrPage& rSrcPage); - virtual SdrPage* Clone() const; + virtual SdrPage* Clone() const SAL_OVERRIDE; virtual SdrPage* Clone(SdrModel* pNewModel) const; bool IsMasterPage() const { return mbMaster; } void SetInserted(bool bNew = true); diff --git a/reportdesign/inc/RptPage.hxx b/reportdesign/inc/RptPage.hxx index b320a3eb0861..9ea966aea156 100644 --- a/reportdesign/inc/RptPage.hxx +++ b/reportdesign/inc/RptPage.hxx @@ -34,6 +34,8 @@ class OReportModel; class REPORTDESIGN_DLLPUBLIC OReportPage : public SdrPage { + OReportPage& operator=(const OReportPage&) SAL_DELETED_FUNCTION; + OReportModel& rModel; ::com::sun::star::uno::Reference< ::com::sun::star::report::XSection > m_xSection; bool m_bSpecialInsertMode; diff --git a/reportdesign/source/core/sdr/RptPage.cxx b/reportdesign/source/core/sdr/RptPage.cxx index 5009a662032b..c55a36082264 100644 --- a/reportdesign/source/core/sdr/RptPage.cxx +++ b/reportdesign/source/core/sdr/RptPage.cxx @@ -61,7 +61,9 @@ OReportPage::~OReportPage() SdrPage* OReportPage::Clone() const { - return new OReportPage( *this ); + OReportPage *const pNewPage = new OReportPage( *this ); + pNewPage->lateInit( *this ); + return pNewPage; } diff --git a/sc/inc/drawpage.hxx b/sc/inc/drawpage.hxx index c11d4ccf05dd..6f7b68ec7c66 100644 --- a/sc/inc/drawpage.hxx +++ b/sc/inc/drawpage.hxx @@ -26,6 +26,9 @@ class ScDrawLayer; class ScDrawPage: public FmFormPage { + ScDrawPage(const ScDrawPage&) SAL_DELETED_FUNCTION; + ScDrawPage& operator=(const ScDrawPage&) SAL_DELETED_FUNCTION; + public: ScDrawPage(ScDrawLayer& rNewModel, bool bMasterPage = false); virtual ~ScDrawPage(); diff --git a/sd/inc/sdpage.hxx b/sd/inc/sdpage.hxx index 767cf3907a67..13ec1c2c380b 100644 --- a/sd/inc/sdpage.hxx +++ b/sd/inc/sdpage.hxx @@ -98,6 +98,8 @@ namespace sd { class SD_DLLPUBLIC SdPage : public FmFormPage, public SdrObjUserCall { + SdPage& operator=(const SdPage&) SAL_DELETED_FUNCTION; + friend class SdGenericDrawPage; friend class SdDrawPage; friend class sd::UndoAnimation; @@ -153,11 +155,13 @@ protected: sal_Int32 mnTransitionFadeColor; double mfTransitionDuration; + SdPage(const SdPage& rSrcPage); + void lateInit(const SdPage& rSrcPage); + public: TYPEINFO_OVERRIDE(); SdPage(SdDrawDocument& rNewDoc, bool bMasterPage=false); - SdPage(const SdPage& rSrcPage); virtual ~SdPage(); virtual SdrPage* Clone() const SAL_OVERRIDE; virtual SdrPage* Clone(SdrModel* pNewModel) const SAL_OVERRIDE; diff --git a/sd/source/core/sdpage2.cxx b/sd/source/core/sdpage2.cxx index 450c646f5cbe..57f46990f44e 100644 --- a/sd/source/core/sdpage2.cxx +++ b/sd/source/core/sdpage2.cxx @@ -377,15 +377,6 @@ SdPage::SdPage(const SdPage& rSrcPage) mePageKind = rSrcPage.mePageKind; meAutoLayout = rSrcPage.meAutoLayout; - // use shape list directly to preserve constness of rSrcPage - const std::list< SdrObject* >& rShapeList = rSrcPage.maPresentationShapeList.getList(); - for( std::list< SdrObject* >::const_iterator aIter = rShapeList.begin(); - aIter != rShapeList.end(); ++aIter ) - { - SdrObject* pObj = *aIter; - InsertPresObj(GetObj(pObj->GetOrdNum()), rSrcPage.GetPresObjKind(pObj)); - } - mbSelected = false; mnTransitionType = rSrcPage.mnTransitionType; mnTransitionSubtype = rSrcPage.mnTransitionSubtype; @@ -410,10 +401,24 @@ SdPage::SdPage(const SdPage& rSrcPage) mnPaperBin = rSrcPage.mnPaperBin; meOrientation = rSrcPage.meOrientation; + mpPageLink = NULL; // is set when inserting via ConnectLink() +} + +void SdPage::lateInit(const SdPage& rSrcPage) +{ + FmFormPage::lateInit(rSrcPage); + + // use shape list directly to preserve constness of rSrcPage + const std::list< SdrObject* >& rShapeList = rSrcPage.maPresentationShapeList.getList(); + for( std::list< SdrObject* >::const_iterator aIter = rShapeList.begin(); + aIter != rShapeList.end(); ++aIter ) + { + SdrObject* pObj = *aIter; + InsertPresObj(GetObj(pObj->GetOrdNum()), rSrcPage.GetPresObjKind(pObj)); + } + // header footer setHeaderFooterSettings( rSrcPage.getHeaderFooterSettings() ); - - mpPageLink = NULL; // is set when inserting via ConnectLink() } /************************************************************************* @@ -433,6 +438,7 @@ SdrPage* SdPage::Clone(SdrModel* pNewModel) const (void)pNewModel; SdPage* pNewPage = new SdPage(*this); + pNewPage->lateInit( *this ); cloneAnimations( *pNewPage ); diff --git a/svx/source/engine3d/obj3d.cxx b/svx/source/engine3d/obj3d.cxx index eee59ad5bbec..ef50b3da671f 100644 --- a/svx/source/engine3d/obj3d.cxx +++ b/svx/source/engine3d/obj3d.cxx @@ -91,11 +91,18 @@ E3dObjList::E3dObjList(SdrModel* pNewModel, SdrPage* pNewPage, E3dObjList* pNewU { } -E3dObjList::E3dObjList(const E3dObjList& rSrcList) -: SdrObjList(rSrcList) +E3dObjList::E3dObjList(const E3dObjList&) +: SdrObjList() { } +E3dObjList* E3dObjList::Clone() const +{ + E3dObjList* const pObjList = new E3dObjList(*this); + pObjList->lateInit(*this); + return pObjList; +} + E3dObjList::~E3dObjList() { } diff --git a/svx/source/form/fmpage.cxx b/svx/source/form/fmpage.cxx index 365dfd38e325..96c3baaf7105 100644 --- a/svx/source/form/fmpage.cxx +++ b/svx/source/form/fmpage.cxx @@ -66,6 +66,12 @@ FmFormPage::FmFormPage(const FmFormPage& rPage) :SdrPage(rPage) ,m_pImpl(new FmFormPageImpl( *this ) ) { +} + +void FmFormPage::lateInit(const FmFormPage& rPage) +{ + SdrPage::lateInit( rPage ); + m_pImpl->initFrom( rPage.GetImpl() ); m_sPageName = rPage.m_sPageName; } @@ -113,8 +119,9 @@ void FmFormPage::SetModel(SdrModel* pNewModel) SdrPage* FmFormPage::Clone() const { - return new FmFormPage(*this); - // hier fehlt noch ein kopieren der Objekte + FmFormPage* const pNewPage = new FmFormPage(*this); + pNewPage->lateInit(*this); + return pNewPage; } diff --git a/svx/source/svdraw/svdpage.cxx b/svx/source/svdraw/svdpage.cxx index 94139448bcb3..da27d4a27f09 100644 --- a/svx/source/svdraw/svdpage.cxx +++ b/svx/source/svdraw/svdpage.cxx @@ -88,7 +88,7 @@ SdrObjList::SdrObjList(SdrModel* pNewModel, SdrPage* pNewPage, SdrObjList* pNewU eListKind=SDROBJLIST_UNKNOWN; } -SdrObjList::SdrObjList(const SdrObjList& rSrcList): +SdrObjList::SdrObjList(): maList(), mpNavigationOrder(), mbIsNavigationOrderDirty(false) @@ -101,7 +101,6 @@ SdrObjList::SdrObjList(const SdrObjList& rSrcList): bRectsDirty=false; pOwnerObj=NULL; eListKind=SDROBJLIST_UNKNOWN; - *this=rSrcList; } SdrObjList::~SdrObjList() @@ -115,9 +114,18 @@ SdrObjList::~SdrObjList() Clear(); // delete contents of container } -void SdrObjList::operator=(const SdrObjList& rSrcList) +SdrObjList* SdrObjList::Clone() const { - Clear(); + SdrObjList* const pObjList = new SdrObjList(); + pObjList->lateInit(*this); + return pObjList; +} + +void SdrObjList::lateInit(const SdrObjList& rSrcList) +{ + // this function is only supposed to be called once, right after construction + assert(maList.empty()); + eListKind=rSrcList.eListKind; CopyObjects(rSrcList); } @@ -1246,31 +1254,6 @@ SdrPage::SdrPage(const SdrPage& rSrcPage) mbPageBorderOnlyLeftRight(rSrcPage.mbPageBorderOnlyLeftRight) { aPrefVisiLayers.SetAll(); - eListKind = (mbMaster) ? SDROBJLIST_MASTERPAGE : SDROBJLIST_DRAWPAGE; - - // copy things from source - // Warning: this leads to slicing (see issue 93186) and has to be - // removed as soon as possible. - *this = rSrcPage; - OSL_ENSURE(mpSdrPageProperties, - "SdrPage::SdrPage: operator= did not create needed SdrPageProperties (!)"); - - // be careful and correct eListKind, a member of SdrObjList which - // will be changed by the SdrOIbjList::operator= before... - eListKind = (mbMaster) ? SDROBJLIST_MASTERPAGE : SDROBJLIST_DRAWPAGE; - - // The previous assignment to *this may have resulted in a call to - // createUnoPage at a partially initialized (sliced) SdrPage object. - // Due to the vtable being not yet fully set-up at this stage, - // createUnoPage() may have been called at the wrong class. - // To force a call to the right createUnoPage() at a later time when the - // new object is full constructed mxUnoPage is disposed now. - uno::Reference xComponent (mxUnoPage, uno::UNO_QUERY); - if (xComponent.is()) - { - mxUnoPage = NULL; - xComponent->dispose(); - } } SdrPage::~SdrPage() @@ -1319,20 +1302,10 @@ SdrPage::~SdrPage() } -SdrPage& SdrPage::operator=(const SdrPage& rSrcPage) +void SdrPage::lateInit(const SdrPage& rSrcPage) { - if( this == &rSrcPage ) - return *this; - if(mpViewContact) - { - delete mpViewContact; - mpViewContact = 0L; - } - - // Joe also sets some parameters for the class this one - // is derived from. SdrObjList does the same bad handling of - // copy constructor and operator=, so i better let it stand here. - pPage = this; + assert(!mpViewContact); + assert(!mpSdrPageProperties); // copy all the local parameters to make this instance // a valid copy of source page before copying and inserting @@ -1361,21 +1334,7 @@ SdrPage& SdrPage::operator=(const SdrPage& rSrcPage) mbObjectsNotPersistent = rSrcPage.mbObjectsNotPersistent; { - // #i111122# delete SdrPageProperties when model is different - if(mpSdrPageProperties && GetModel() != rSrcPage.GetModel()) - { - delete mpSdrPageProperties; - mpSdrPageProperties = 0; - } - - if(!mpSdrPageProperties) - { - mpSdrPageProperties = new SdrPageProperties(*this); - } - else - { - mpSdrPageProperties->ClearItem(0); - } + mpSdrPageProperties = new SdrPageProperties(*this); if(!IsMasterPage()) { @@ -1385,9 +1344,12 @@ SdrPage& SdrPage::operator=(const SdrPage& rSrcPage) mpSdrPageProperties->SetStyleSheet(rSrcPage.getSdrPageProperties().GetStyleSheet()); } - // Now copy the contained objects (by cloning them) - SdrObjList::operator=(rSrcPage); - return *this; + // Now copy the contained objects + SdrObjList::lateInit(rSrcPage); + + // be careful and correct eListKind, a member of SdrObjList which + // will be changed by the SdrObjList::lateInit before... + eListKind = (mbMaster) ? SDROBJLIST_MASTERPAGE : SDROBJLIST_DRAWPAGE; } SdrPage* SdrPage::Clone() const @@ -1399,7 +1361,7 @@ SdrPage* SdrPage::Clone(SdrModel* pNewModel) const { if (pNewModel==NULL) pNewModel=pModel; SdrPage* pPage2=new SdrPage(*pNewModel); - *pPage2=*this; + pPage2->lateInit(*this); return pPage2; } diff --git a/sw/inc/dpage.hxx b/sw/inc/dpage.hxx index 54aa789f18e0..8284f6e458e7 100644 --- a/sw/inc/dpage.hxx +++ b/sw/inc/dpage.hxx @@ -28,6 +28,9 @@ class SwDoc; class SwDPage : public FmFormPage, public SdrObjUserCall { + SwDPage(const SwDPage&) SAL_DELETED_FUNCTION; + SwDPage &operator=(const SwDPage&) SAL_DELETED_FUNCTION; + SdrPageGridFrameList* pGridLst; SwDoc& rDoc; -- cgit v1.2.3