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

Eike Rathke erack at redhat.com
Wed Apr 12 22:49:11 UTC 2017


 sc/source/core/data/postit.cxx |   32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

New commits:
commit b9573789b5f47d67e4cb881aaa0fbdb3cb25fb31
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Apr 13 00:46:33 2017 +0200

    finally switch the workaround off
    
    Change-Id: I284292a2749c2b38ef874315d5b526e403d578e8

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 225b46edcf76..c0a47df3c99c 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -697,8 +697,9 @@ void ScCaptionPtr::decRefAndDestroy()
         assert(!mpNext);                    // this must be one and only one
         assert(mpCaption);
 
-#if 1
-        // FIXME: there are still cases where the caption pointer is dangling
+#if 0
+        // Quick workaround for when there are still cases where the caption
+        // pointer is dangling
         mpCaption = nullptr;
         mbNotOwner = false;
 #else
commit 6c0e633e79e3a8af075ddc17e98ff390e66cd496
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Apr 13 00:34:34 2017 +0200

    turn assert into SAL_INFO
    
    The old assert conditions don't hold anymore since removeFromDrawPageAndFree()
    only deletes the SdrCaptionObj on the last refcount, but info can be useful.
    
    Change-Id: I456149b8799a0509dcd7a2da09d627fb0de1a912

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index ee8f82ee0a2c..225b46edcf76 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1135,20 +1135,16 @@ void ScPostIt::RemoveCaption()
     if (pDrawLayer == maNoteData.mxCaption->GetModel())
         maNoteData.mxCaption.removeFromDrawPageAndFree();
 
-#if 0
-    // 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
-    // just destroyed which leaves us with one reference.
-    // Let's detect other use cases..
-    assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 ||
-            (!mrDoc.IsUndo() && !mrDoc.IsClipboard()) || (mrDoc.IsUndo() && mrDoc.IsInDtorClear()));
-#endif
+    SAL_INFO("sc.core","ScPostIt::RemoveCaption - refs: " << maNoteData.mxCaption.getRefs() <<
+            "  IsUndo: " << mrDoc.IsUndo() << "  IsClip: " << mrDoc.IsClipboard() <<
+            "  Dtor: " << mrDoc.IsInDtorClear());
 
-    // Forget the caption object if removeFromDrawPageAndFree() above did not
-    // free it.
+    // Forget the caption object if removeFromDrawPageAndFree() did not free it.
     if (maNoteData.mxCaption)
+    {
+        SAL_INFO("sc.core","ScPostIt::RemoveCaption - forgetting one ref");
         maNoteData.mxCaption.forget();
+    }
 }
 
 ScCaptionPtr ScNoteUtil::CreateTempCaption(
commit 06e427c24d75311c0ab79d2da586b961881db766
Author: Eike Rathke <erack at redhat.com>
Date:   Wed Apr 12 23:24:34 2017 +0200

    control deletion of SdrCaptionObj within ScCaptionPtr by refcount
    
    I guess this is about the first time ever that repeated Undo and Redo of
    Cut&Paste of a cell comment does not crash..
    
    Change-Id: I493a0a5439efde133a07d73ddcbcdf5bda4bc276

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 20f2f485bfe4..ee8f82ee0a2c 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -761,7 +761,9 @@ void ScCaptionPtr::removeFromDrawPageAndFree( bool bIgnoreUndo )
         }
         // remove the object from the drawing page, delete if undo is disabled
         removeFromDrawPage( *pDrawPage );
-        if (!bRecording)
+        // If called from outside mnRefs must be 1 to delete. If called from
+        // decRefAndDestroy() mnRefs is already 0.
+        if (!bRecording && getRefs() <= 1)
         {
             SdrObject* pObj = release();
             SdrObject::Free( pObj );
@@ -1133,6 +1135,7 @@ void ScPostIt::RemoveCaption()
     if (pDrawLayer == maNoteData.mxCaption->GetModel())
         maNoteData.mxCaption.removeFromDrawPageAndFree();
 
+#if 0
     // 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
@@ -1140,6 +1143,7 @@ void ScPostIt::RemoveCaption()
     // Let's detect other use cases..
     assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 ||
             (!mrDoc.IsUndo() && !mrDoc.IsClipboard()) || (mrDoc.IsUndo() && mrDoc.IsInDtorClear()));
+#endif
 
     // Forget the caption object if removeFromDrawPageAndFree() above did not
     // free it.
commit 6b4b2752fa876c219ee37a799b64211529f950e2
Author: Eike Rathke <erack at redhat.com>
Date:   Tue Apr 11 21:23:27 2017 +0200

    bail out early if there is no caption to remove
    
    Change-Id: Id08d82751560092fd6225131970f607dbb2e4801

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 7d3effc671fd..20f2f485bfe4 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1123,12 +1123,14 @@ void ScPostIt::CreateCaption( const ScAddress& rPos, const SdrCaptionObj* pCapti
 
 void ScPostIt::RemoveCaption()
 {
+    if (!maNoteData.mxCaption)
+        return;
 
     /*  Remove caption object only, if this note is its owner (e.g. notes in
         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()))
+    if (pDrawLayer == maNoteData.mxCaption->GetModel())
         maNoteData.mxCaption.removeFromDrawPageAndFree();
 
     // Either the caption object is gone or, because of Undo or clipboard is
commit 2780f44627e487761c1f37f6475239bca6a262e0
Author: Eike Rathke <erack at redhat.com>
Date:   Tue Apr 11 20:58:34 2017 +0200

    in RemoveCaption() forget() instead of reset(nullptr)
    
    Change-Id: Id97d4d97c1d46ac6de6198515756a0786a54626e

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index e8afbb2479dc..7d3effc671fd 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1130,6 +1130,7 @@ void ScPostIt::RemoveCaption()
     ScDrawLayer* pDrawLayer = mrDoc.GetDrawLayer();
     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
@@ -1137,7 +1138,11 @@ void ScPostIt::RemoveCaption()
     // Let's detect other use cases..
     assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 ||
             (!mrDoc.IsUndo() && !mrDoc.IsClipboard()) || (mrDoc.IsUndo() && mrDoc.IsInDtorClear()));
-    maNoteData.mxCaption.reset(nullptr);
+
+    // Forget the caption object if removeFromDrawPageAndFree() above did not
+    // free it.
+    if (maNoteData.mxCaption)
+        maNoteData.mxCaption.forget();
 }
 
 ScCaptionPtr ScNoteUtil::CreateTempCaption(


More information about the Libreoffice-commits mailing list