[Libreoffice-commits] core.git: 5 commits - sc/inc sc/source

Eike Rathke erack at redhat.com
Mon Apr 10 21:32:24 UTC 2017


 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 <erack at redhat.com>
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 <erack at redhat.com>
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 <erack at redhat.com>
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 <erack at redhat.com>
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 <erack at redhat.com>
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


More information about the Libreoffice-commits mailing list