sc/inc/postit.hxx | 15 +++++++ sc/source/core/data/postit.cxx | 86 ++++++++++++++++++++++++----------------- sc/source/ui/undo/undocell.cxx | 13 +++++- 3 files changed, 78 insertions(+), 36 deletions(-)
New commits: commit 13b70bc6f18f8dd910e373694de5a6a0cd3eb559 Author: Eike Rathke <[email protected]> Date: Mon Apr 10 23:31:14 2017 +0200 finally free the SdrObject in ScCaptionPtr::decRefAndDestroy() There may be cases left still to be discovered where a setInUndo() is necessary in some Undo situations, but this is a start. Change-Id: Ic62267e3c3d24e4587343ff42da0292fbb166929 diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx index 67efdf202648..02502ed93c84 100644 --- a/sc/source/core/data/postit.cxx +++ b/sc/source/core/data/postit.cxx @@ -691,18 +691,20 @@ void ScCaptionPtr::decRefAndDestroy() assert(mpHead->mpFirst == this); // this must be one and only one assert(!mpNext); // this must be one and only one assert(mpCaption); -#if 0 - if (what?) + + // Destroying Draw Undo deletes its SdrObject, don't attempt that twice. + if (!mbInUndo) { - /* FIXME: this should eventually remove the caption from drawing layer - * foo and call SdrObject::Free(), likely with mpCaption, see - * ScPostIt::RemoveCaption(). Further work needed to be able to do so. - * */ + removeFromDrawPageAndFree( true ); // ignoring Undo + if (mpCaption) + { + // There's no draw page associated so removeFromDrawPageAndFree() + // didn't do anything, but still we want to delete the caption + // object. release()/dissolve() also resets mpCaption. + SdrObject* pObj = release(); + SdrObject::Free( pObj ); + } } - /* FIXME: once we got ownership right */ - //SdrObject::Free( mpCaption ); -#endif - mpCaption = nullptr; delete mpHead; mpHead = nullptr; } commit 8984ca204dd4753246782a4f5b8f6058bb232d33 Author: Eike Rathke <[email protected]> Date: Mon Apr 10 23:25:34 2017 +0200 flag ScCaptionPtr::setInUndo() in ScUndoReplaceNote Change-Id: I174be1262074e1fed784806d2f052b36749dff0d diff --git a/sc/source/ui/undo/undocell.cxx b/sc/source/ui/undo/undocell.cxx index ddd0ebdb66a9..202acdd26f45 100644 --- a/sc/source/ui/undo/undocell.cxx +++ b/sc/source/ui/undo/undocell.cxx @@ -709,7 +709,16 @@ ScUndoReplaceNote::ScUndoReplaceNote( ScDocShell& rDocShell, const ScAddress& rP mpDrawUndo( pDrawUndo ) { OSL_ENSURE( rNoteData.mxCaption, "ScUndoReplaceNote::ScUndoReplaceNote - missing note caption" ); - (bInsert ? maNewData : maOldData) = rNoteData; + if (bInsert) + { + maNewData = rNoteData; + maNewData.mxCaption.setInUndo(); + } + else + { + maOldData = rNoteData; + maOldData.mxCaption.setInUndo(); + } } ScUndoReplaceNote::ScUndoReplaceNote( ScDocShell& rDocShell, const ScAddress& rPos, @@ -722,6 +731,8 @@ ScUndoReplaceNote::ScUndoReplaceNote( ScDocShell& rDocShell, const ScAddress& rP { OSL_ENSURE( maOldData.mxCaption || maNewData.mxCaption, "ScUndoReplaceNote::ScUndoReplaceNote - missing note captions" ); OSL_ENSURE( !maOldData.mxInitData.get() && !maNewData.mxInitData.get(), "ScUndoReplaceNote::ScUndoReplaceNote - unexpected unitialized note" ); + maOldData.mxCaption.setInUndo(); + maNewData.mxCaption.setInUndo(); } ScUndoReplaceNote::~ScUndoReplaceNote() commit 98940fc97fe134af332bd0f9d41ec76e21bc521c Author: Eike Rathke <[email protected]> Date: Mon Apr 10 23:24:00 2017 +0200 introduce ScCaptionPtr InUndo Change-Id: Iccc2671b61f524244107233b77b56aaa45f5c72a diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx index 993de2af6565..a630aef271db 100644 --- a/sc/inc/postit.hxx +++ b/sc/inc/postit.hxx @@ -85,6 +85,9 @@ public: */ bool forget(); + /** Flag that this instance is in Undo, so drawing layer owns it. */ + void setInUndo(); + oslInterlockedCount getRefs() const; private: @@ -101,6 +104,14 @@ private: Head* mpHead; ///< points to the "master" entry mutable ScCaptionPtr* mpNext; ///< next in list SdrCaptionObj* mpCaption; ///< the caption object, managed by head master + bool mbInUndo; ///< whether this caption object is held in Undo + /* TODO: can that be moved to Head? + * It's unclear when to reset, so + * each instance has its own flag. + * The last reference count + * decrement automatically has the + * then current state available. + * */ void newHead(); //< Allocate a new Head and init. void incRef() const; diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx index 524328ea0a81..67efdf202648 100644 --- a/sc/source/core/data/postit.cxx +++ b/sc/source/core/data/postit.cxx @@ -445,12 +445,12 @@ ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& r ScCaptionPtr::ScCaptionPtr() : - mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr) + mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr), mbInUndo(false) { } ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) : - mpHead(nullptr), mpNext(nullptr), mpCaption(p) + mpHead(nullptr), mpNext(nullptr), mpCaption(p), mbInUndo(false) { if (p) { @@ -459,7 +459,7 @@ ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) : } ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) : - mpHead(r.mpHead), mpCaption(r.mpCaption) + mpHead(r.mpHead), mpCaption(r.mpCaption), mbInUndo(false) { if (r.mpCaption) { @@ -477,7 +477,7 @@ ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) : } ScCaptionPtr::ScCaptionPtr( ScCaptionPtr&& r ) : - mpHead(r.mpHead), mpNext(r.mpNext), mpCaption(r.mpCaption) + mpHead(r.mpHead), mpNext(r.mpNext), mpCaption(r.mpCaption), mbInUndo(false) { r.replaceInList( this ); r.mpCaption = nullptr; @@ -534,6 +534,11 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r ) return *this; } +void ScCaptionPtr::setInUndo() +{ + mbInUndo = true; +} + ScCaptionPtr::Head::Head( ScCaptionPtr* p ) : mpFirst(p), mnRefs(1) { commit 71f942731b5cec0abb7eecfe5e8dad9b516cd213 Author: Eike Rathke <[email protected]> Date: Mon Apr 10 21:26:35 2017 +0200 Add bool bIgnoreUndo parameter to removeFromDrawPageAndFree() In preparation of using that in the dtor. Change-Id: I9a8713390c548e774c1e23cef201effe00a29be9 diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx index 8791a8b26b1d..993de2af6565 100644 --- a/sc/inc/postit.hxx +++ b/sc/inc/postit.hxx @@ -70,7 +70,7 @@ public: /** Remove from draw page and free caption object if no Undo recording. */ - void removeFromDrawPageAndFree(); + void removeFromDrawPageAndFree( bool bIgnoreUndo = false ); /** Release all management of the SdrCaptionObj* in all instances of this list and dissolve. The SdrCaptionObj pointer returned is ready to be diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx index 36d1f82f47fb..524328ea0a81 100644 --- a/sc/source/core/data/postit.cxx +++ b/sc/source/core/data/postit.cxx @@ -717,7 +717,7 @@ void ScCaptionPtr::removeFromDrawPage( SdrPage& rDrawPage ) assert(pObj == mpCaption); (void)pObj; } -void ScCaptionPtr::removeFromDrawPageAndFree() +void ScCaptionPtr::removeFromDrawPageAndFree( bool bIgnoreUndo ) { assert(mpHead && mpCaption); SdrPage* pDrawPage = mpCaption->GetPage(); @@ -725,12 +725,16 @@ void ScCaptionPtr::removeFromDrawPageAndFree() if (pDrawPage) { pDrawPage->RecalcObjOrdNums(); - ScDrawLayer* pDrawLayer = dynamic_cast<ScDrawLayer*>(mpCaption->GetModel()); - SAL_WARN_IF( !pDrawLayer, "sc.core", "ScCaptionPtr::removeFromDrawPageAndFree - object without drawing layer"); - // create drawing undo action (before removing the object to have valid draw page in undo action) - const bool bRecording = (pDrawLayer && pDrawLayer->IsRecording()); - if (bRecording) - pDrawLayer->AddCalcUndo( new SdrUndoDelObj( *mpCaption )); + bool bRecording = false; + if (!bIgnoreUndo) + { + ScDrawLayer* pDrawLayer = dynamic_cast<ScDrawLayer*>(mpCaption->GetModel()); + SAL_WARN_IF( !pDrawLayer, "sc.core", "ScCaptionPtr::removeFromDrawPageAndFree - object without drawing layer"); + // create drawing undo action (before removing the object to have valid draw page in undo action) + bRecording = (pDrawLayer && pDrawLayer->IsRecording()); + if (bRecording) + pDrawLayer->AddCalcUndo( new SdrUndoDelObj( *mpCaption )); + } // remove the object from the drawing page, delete if undo is disabled removeFromDrawPage( *pDrawPage ); if (!bRecording) commit adcf24117d939f0f2bb0a00d89da3105db134ebf Author: Eike Rathke <[email protected]> Date: Mon Apr 10 20:00:52 2017 +0200 move implementation from RemoveCaption() to removeFromDrawPageAndFree() Change-Id: I4f98112c13dfcd5c6c2fdb5b682cca494d63a954 diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx index 4c0c10b9b36c..8791a8b26b1d 100644 --- a/sc/inc/postit.hxx +++ b/sc/inc/postit.hxx @@ -68,6 +68,10 @@ public: */ void removeFromDrawPage( SdrPage& rDrawPage ); + /** Remove from draw page and free caption object if no Undo recording. + */ + void removeFromDrawPageAndFree(); + /** Release all management of the SdrCaptionObj* in all instances of this list and dissolve. The SdrCaptionObj pointer returned is ready to be managed elsewhere. diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx index 2acc07a09939..36d1f82f47fb 100644 --- a/sc/source/core/data/postit.cxx +++ b/sc/source/core/data/postit.cxx @@ -717,6 +717,30 @@ void ScCaptionPtr::removeFromDrawPage( SdrPage& rDrawPage ) assert(pObj == mpCaption); (void)pObj; } +void ScCaptionPtr::removeFromDrawPageAndFree() +{ + assert(mpHead && mpCaption); + SdrPage* pDrawPage = mpCaption->GetPage(); + SAL_WARN_IF( !pDrawPage, "sc.core", "ScCaptionPtr::removeFromDrawPageAndFree - object without drawing page"); + if (pDrawPage) + { + pDrawPage->RecalcObjOrdNums(); + ScDrawLayer* pDrawLayer = dynamic_cast<ScDrawLayer*>(mpCaption->GetModel()); + SAL_WARN_IF( !pDrawLayer, "sc.core", "ScCaptionPtr::removeFromDrawPageAndFree - object without drawing layer"); + // create drawing undo action (before removing the object to have valid draw page in undo action) + const bool bRecording = (pDrawLayer && pDrawLayer->IsRecording()); + if (bRecording) + pDrawLayer->AddCalcUndo( new SdrUndoDelObj( *mpCaption )); + // remove the object from the drawing page, delete if undo is disabled + removeFromDrawPage( *pDrawPage ); + if (!bRecording) + { + SdrObject* pObj = release(); + SdrObject::Free( pObj ); + } + } +} + SdrCaptionObj* ScCaptionPtr::release() { SdrCaptionObj* pTmp = mpCaption; @@ -1074,27 +1098,8 @@ void ScPostIt::RemoveCaption() undo documents refer to captions in original document, do not remove them from drawing layer here). */ ScDrawLayer* pDrawLayer = mrDoc.GetDrawLayer(); - if( maNoteData.mxCaption && (pDrawLayer == maNoteData.mxCaption->GetModel()) ) - { - OSL_ENSURE( pDrawLayer, "ScPostIt::RemoveCaption - object without drawing layer" ); - SdrPage* pDrawPage = maNoteData.mxCaption->GetPage(); - OSL_ENSURE( pDrawPage, "ScPostIt::RemoveCaption - object without drawing page" ); - if( pDrawPage ) - { - pDrawPage->RecalcObjOrdNums(); - // create drawing undo action (before removing the object to have valid draw page in undo action) - const bool bRecording = (pDrawLayer && pDrawLayer->IsRecording()); - if( bRecording ) - pDrawLayer->AddCalcUndo( new SdrUndoDelObj( *maNoteData.mxCaption.get() ) ); - // remove the object from the drawing page, delete if undo is disabled - maNoteData.mxCaption.removeFromDrawPage( *pDrawPage ); - if( !bRecording ) - { - SdrObject* pObj = maNoteData.mxCaption.release(); - SdrObject::Free( pObj ); - } - } - } + if (maNoteData.mxCaption && (pDrawLayer == maNoteData.mxCaption->GetModel())) + maNoteData.mxCaption.removeFromDrawPageAndFree(); // Either the caption object is gone or, because of Undo or clipboard is // held in at least two instances, or only one instance in Undo because the // original sheet in this document is just deleted, or the Undo document is _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
