[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-5.3' - 38 commits - sc/inc sc/qa sc/source
Eike Rathke
erack at redhat.com
Mon May 15 20:11:53 UTC 2017
sc/inc/postit.hxx | 121 +++++++-
sc/qa/unit/ucalc.cxx | 89 ++++-
sc/qa/unit/ucalc.hxx | 5
sc/qa/unit/ucalc_sort.cxx | 3
sc/source/core/data/postit.cxx | 568 +++++++++++++++++++++++++++++++-------
sc/source/filter/xml/xmlcelli.cxx | 2
sc/source/ui/docshell/docfunc.cxx | 2
sc/source/ui/inc/notemark.hxx | 4
sc/source/ui/undo/undocell.cxx | 25 +
sc/source/ui/view/drawview.cxx | 2
sc/source/ui/view/notemark.cxx | 22 -
sc/source/ui/view/viewdata.cxx | 6
12 files changed, 697 insertions(+), 152 deletions(-)
New commits:
commit 601898857272de205e8dfcc0be0bae590babae75
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 2ac98a57cc0b..044393426ce1 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 23d06332abb81f4eef2a5dcc63fab1ce9d42aaaa
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 310bddfa606f..2ac98a57cc0b 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1138,20 +1138,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 5ef67aa7c9a4f370b091fdf4b280596d5668553a
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 b48bdcf8d206..310bddfa606f 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 );
@@ -1136,6 +1138,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
@@ -1143,6 +1146,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 b7280da0b67cc53bfb8d384389832cfdef4a9b48
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 a197f53a46dc..b48bdcf8d206 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1126,12 +1126,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 9c28bab7a9404ab36fd7cee3ef47b1ccafa3638b
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 f4e5b3bbc723..a197f53a46dc 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1133,6 +1133,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
@@ -1140,7 +1141,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(
commit f57c78d9164b3e3677d650322fa9cb4b24ec8d88
Author: Eike Rathke <erack at redhat.com>
Date: Tue Apr 11 19:05:46 2017 +0200
deleting pModel also deletes the SdrCaptionObj
Change-Id: Icf3aed35ede1c211d6238dc66d86cb2866b247cd
diff --git a/sc/source/ui/view/notemark.cxx b/sc/source/ui/view/notemark.cxx
index 564d98d83329..55f8812048ff 100644
--- a/sc/source/ui/view/notemark.cxx
+++ b/sc/source/ui/view/notemark.cxx
@@ -65,6 +65,9 @@ ScNoteMarker::ScNoteMarker( vcl::Window* pWin, vcl::Window* pRight, vcl::Window*
ScNoteMarker::~ScNoteMarker()
{
+ if (pModel)
+ mxObject.release(); // deleting pModel also deletes the SdrCaptionObj
+
InvalidateWin();
delete pModel;
commit 033bd8233e328b4881864c3ff1868951ec8f8ab6
Author: Eike Rathke <erack at redhat.com>
Date: Tue Apr 11 18:53:36 2017 +0200
set mbNotOwner at various places
Change-Id: I1ff14c573d556cad15513dfe3f0fecbf9107fa41
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index c6a91d25bec0..f4e5b3bbc723 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -481,6 +481,7 @@ ScCaptionPtr::ScCaptionPtr( ScCaptionPtr&& r ) :
{
r.replaceInList( this );
r.mpCaption = nullptr;
+ r.mbNotOwner = false;
}
ScCaptionPtr& ScCaptionPtr::operator=( ScCaptionPtr&& r )
@@ -490,9 +491,11 @@ ScCaptionPtr& ScCaptionPtr::operator=( ScCaptionPtr&& r )
mpHead = r.mpHead;
mpNext = r.mpNext;
mpCaption = r.mpCaption;
+ mbNotOwner = r.mbNotOwner;
r.replaceInList( this );
r.mpCaption = nullptr;
+ r.mbNotOwner = false;
return *this;
}
@@ -525,6 +528,7 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
removeFromList();
mpCaption = r.mpCaption;
+ mbNotOwner = r.mbNotOwner;
// That head is this' master.
mpHead = r.mpHead;
// Insert into list.
@@ -656,6 +660,7 @@ void ScCaptionPtr::reset( SdrCaptionObj* p )
decRefAndDestroy();
removeFromList();
mpCaption = p;
+ mbNotOwner = false;
if (p)
{
newHead();
@@ -776,6 +781,7 @@ bool ScCaptionPtr::forget()
bool bRet = decRef();
removeFromList();
mpCaption = nullptr;
+ mbNotOwner = false;
return bRet;
}
@@ -800,6 +806,7 @@ void ScCaptionPtr::clear()
mpHead = nullptr;
mpNext = nullptr;
mpCaption = nullptr;
+ mbNotOwner = false;
}
commit 5c1566503cbf3310bca6cf4c121cc421ec8d5808
Author: Eike Rathke <erack at redhat.com>
Date: Tue Apr 11 18:05:09 2017 +0200
reset variables when not owner
Change-Id: Ieab4bf36b89abac2d2ff377fc2b6f31ce0e1d3aa
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 87b03142fb18..c6a91d25bec0 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -695,9 +695,16 @@ void ScCaptionPtr::decRefAndDestroy()
#if 1
// FIXME: there are still cases where the caption pointer is dangling
mpCaption = nullptr;
+ mbNotOwner = false;
#else
- // Destroying Draw Undo deletes its SdrObject, don't attempt that twice.
- if (!mbNotOwner)
+ // Destroying Draw Undo and some other delete the SdrObject, don't
+ // attempt that twice.
+ if (mbNotOwner)
+ {
+ mpCaption = nullptr;
+ mbNotOwner = false;
+ }
+ else
{
removeFromDrawPageAndFree( true ); // ignoring Undo
if (mpCaption)
commit 9f18a9dd14561eb54fdbab098bea9d79b6307baa
Author: Eike Rathke <erack at redhat.com>
Date: Tue Apr 11 17:16:32 2017 +0200
rename ScCaptionPtr (mb|set)InUndo to (mb|set)NotOwner
... which better suits the general purpose we'll need
Change-Id: I32805c91d17180d5f18225a02c8a436826242e19
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 1d81db68b32e..8d2b5f7823c5 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -86,7 +86,7 @@ public:
bool forget();
/** Flag that this instance is in Undo, so drawing layer owns it. */
- void setInUndo();
+ void setNotOwner();
oslInterlockedCount getRefs() const;
@@ -104,7 +104,7 @@ 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
+ bool mbNotOwner; ///< whether this caption object is owned by something else, e.g. held in Undo
/* TODO: can that be moved to Head?
* It's unclear when to reset, so
* each instance has its own flag.
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index cd76cebcc65f..87b03142fb18 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), mbInUndo(false)
+ mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr), mbNotOwner(false)
{
}
ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) :
- mpHead(nullptr), mpNext(nullptr), mpCaption(p), mbInUndo(false)
+ mpHead(nullptr), mpNext(nullptr), mpCaption(p), mbNotOwner(false)
{
if (p)
{
@@ -459,7 +459,7 @@ ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) :
}
ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) :
- mpHead(r.mpHead), mpCaption(r.mpCaption), mbInUndo(false)
+ mpHead(r.mpHead), mpCaption(r.mpCaption), mbNotOwner(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), mbInUndo(false)
+ mpHead(r.mpHead), mpNext(r.mpNext), mpCaption(r.mpCaption), mbNotOwner(false)
{
r.replaceInList( this );
r.mpCaption = nullptr;
@@ -534,9 +534,9 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
return *this;
}
-void ScCaptionPtr::setInUndo()
+void ScCaptionPtr::setNotOwner()
{
- mbInUndo = true;
+ mbNotOwner = true;
}
ScCaptionPtr::Head::Head( ScCaptionPtr* p ) :
@@ -697,7 +697,7 @@ void ScCaptionPtr::decRefAndDestroy()
mpCaption = nullptr;
#else
// Destroying Draw Undo deletes its SdrObject, don't attempt that twice.
- if (!mbInUndo)
+ if (!mbNotOwner)
{
removeFromDrawPageAndFree( true ); // ignoring Undo
if (mpCaption)
diff --git a/sc/source/ui/undo/undocell.cxx b/sc/source/ui/undo/undocell.cxx
index 1e39da276459..44a56627b214 100644
--- a/sc/source/ui/undo/undocell.cxx
+++ b/sc/source/ui/undo/undocell.cxx
@@ -712,12 +712,12 @@ ScUndoReplaceNote::ScUndoReplaceNote( ScDocShell& rDocShell, const ScAddress& rP
if (bInsert)
{
maNewData = rNoteData;
- maNewData.mxCaption.setInUndo();
+ maNewData.mxCaption.setNotOwner();
}
else
{
maOldData = rNoteData;
- maOldData.mxCaption.setInUndo();
+ maOldData.mxCaption.setNotOwner();
}
}
@@ -731,8 +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();
+ maOldData.mxCaption.setNotOwner();
+ maNewData.mxCaption.setNotOwner();
}
ScUndoReplaceNote::~ScUndoReplaceNote()
commit a4418108d13a9b19a6934f542c3b64499ecf18d2
Author: Eike Rathke <erack at redhat.com>
Date: Tue Apr 11 13:41:44 2017 +0200
there are still cases where the caption pointer is dangling
Change-Id: I8c186fa32d7fc3f26d7952268cb1e614025ecf37
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index c8f244fb3e5b..cd76cebcc65f 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -692,6 +692,10 @@ 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
+ mpCaption = nullptr;
+#else
// Destroying Draw Undo deletes its SdrObject, don't attempt that twice.
if (!mbInUndo)
{
@@ -705,6 +709,7 @@ void ScCaptionPtr::decRefAndDestroy()
SdrObject::Free( pObj );
}
}
+#endif
delete mpHead;
mpHead = nullptr;
}
commit d091cc41bf44ce3f69151e2765d1148969d25170
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 8f0f00200d76..c8f244fb3e5b 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 dc1833e012c581c8b534652c28ebf635108ba279
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 ee8ec67237c2..1e39da276459 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 26337c1307a1cdee72b2d457cd0d210227662e29
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 b5a99206aab8..1d81db68b32e 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 60225e639f74..8f0f00200d76 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 21049854751bcc2b1f42947181eed3ee2a39404b
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 bff01bf95e86..b5a99206aab8 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 be2896447f52..60225e639f74 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 acfd6f78c75812031bc310ab26bf60e9a703cd85
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 d741cdf86bbc..bff01bf95e86 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 1156ba2e9258..be2896447f52 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;
@@ -1077,27 +1101,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
commit 774d59059777ef0d08b5fd6ff9f04e0cc323adff
Author: Eike Rathke <erack at redhat.com>
Date: Mon Apr 10 18:54:17 2017 +0200
narrow the assert condition further down
Change-Id: Ia9b1db652b2f15b66b89b51038d16fb0da6ffb6d
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 520f0553df0a..1156ba2e9258 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1100,10 +1100,11 @@ void ScPostIt::RemoveCaption()
}
// 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 was deleted, or the Undo document is just destroyed
- // which leaves us with one reference.
+ // 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.IsInDtorClear());
+ assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 ||
+ (!mrDoc.IsUndo() && !mrDoc.IsClipboard()) || (mrDoc.IsUndo() && mrDoc.IsInDtorClear()));
maNoteData.mxCaption.reset(nullptr);
}
commit a01559a579537956def399bbbbe99b76b7039b23
Author: Eike Rathke <erack at redhat.com>
Date: Mon Apr 10 16:25:45 2017 +0200
can't keep track of drawlayer insertion
Draw Undo independently can reinsert a caption to the drawlayer, which
is beyond our knowledge. To track that cumbersome manual tracking (or
callbacks or whatever) would be needed, which actually this tries to get
rid of instead of increasing..
Change-Id: I373843ad61d0b6e19b9d3f98fd8f9e01a448296d
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index e3a91ab2d73a..d741cdf86bbc 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -89,7 +89,6 @@ private:
{
ScCaptionPtr* mpFirst; ///< first in list
oslInterlockedCount mnRefs; ///< use count
- bool mbInDrawPage; ///< caption object is owned by draw page
Head() = delete;
explicit Head( ScCaptionPtr* );
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 2077de6c84ee..520f0553df0a 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -535,7 +535,7 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
}
ScCaptionPtr::Head::Head( ScCaptionPtr* p ) :
- mpFirst(p), mnRefs(1), mbInDrawPage(false)
+ mpFirst(p), mnRefs(1)
{
}
@@ -686,7 +686,8 @@ 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 (mpHead->mbInDrawPage)
+#if 0
+ if (what?)
{
/* FIXME: this should eventually remove the caption from drawing layer
* foo and call SdrObject::Free(), likely with mpCaption, see
@@ -695,6 +696,7 @@ void ScCaptionPtr::decRefAndDestroy()
}
/* FIXME: once we got ownership right */
//SdrObject::Free( mpCaption );
+#endif
mpCaption = nullptr;
delete mpHead;
mpHead = nullptr;
@@ -704,24 +706,15 @@ void ScCaptionPtr::decRefAndDestroy()
void ScCaptionPtr::insertToDrawPage( SdrPage& rDrawPage )
{
assert(mpHead && mpCaption);
- assert(!mpHead->mbInDrawPage); // multiple assignments not possible
rDrawPage.InsertObject( mpCaption );
- mpHead->mbInDrawPage = true;
}
void ScCaptionPtr::removeFromDrawPage( SdrPage& rDrawPage )
{
assert(mpHead && mpCaption);
- SAL_WARN_IF(!mpHead->mbInDrawPage,"sc.core","ScCaptionPtr::removeFromDrawPage - not in draw page");
- /* FIXME: that should assert, but currently fails in
- * Test::testCopyToDocument() probably due to CopyStaticToDocument()
- * lacking something. */
- //assert(mpHead->mbInDrawPage); // did we lose track anywhere?
-
SdrObject* pObj = rDrawPage.RemoveObject( mpCaption->GetOrdNum() );
assert(pObj == mpCaption); (void)pObj;
- mpHead->mbInDrawPage = false;
}
SdrCaptionObj* ScCaptionPtr::release()
commit 656142a690ee341f2409cbd5be21a04959e25b6f
Author: Eike Rathke <erack at redhat.com>
Date: Fri Apr 7 16:58:13 2017 +0200
yet another mxCaption refs==1 case to exclude from assert
Change-Id: Iffa8f2bc7d0bb77d5145a569da2c03aefbb9de4a
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index dd451734be28..2077de6c84ee 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1106,9 +1106,11 @@ void ScPostIt::RemoveCaption()
}
}
// Either the caption object is gone or, because of Undo or clipboard is
- // held in at least two instances, or the Undo document is just destroyed
+ // held in at least two instances, or only one instance in Undo because the
+ // original sheet was deleted, or the Undo document is just destroyed
// which leaves us with one reference.
- assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 || (mrDoc.IsUndo() && mrDoc.IsInDtorClear()));
+ // Let's detect other use cases..
+ assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 || !mrDoc.IsUndo() || mrDoc.IsInDtorClear());
maNoteData.mxCaption.reset(nullptr);
}
commit e9d1c62d8f7cfbc22174d34ee603139b9872c0f7
Author: Eike Rathke <erack at redhat.com>
Date: Tue Mar 14 19:57:04 2017 +0100
no assert for one reference while destroying the Undo document
Change-Id: Idf9e0b2600d503ff50cd6269e8d528c0fad12a3e
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 078a30fa37b3..dd451734be28 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1106,8 +1106,9 @@ void ScPostIt::RemoveCaption()
}
}
// Either the caption object is gone or, because of Undo or clipboard is
- // held in at least two instances.
- assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2);
+ // held in at least two instances, or the Undo document is just destroyed
+ // which leaves us with one reference.
+ assert(!maNoteData.mxCaption || maNoteData.mxCaption.getRefs() >= 2 || (mrDoc.IsUndo() && mrDoc.IsInDtorClear()));
maNoteData.mxCaption.reset(nullptr);
}
commit 83cca3c8d7f93f792a2b2415bd02378c348c1e65
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 23:15:42 2017 +0100
finally turn this into a hard assert
Change-Id: Iba6abafeaa2542fc94b76a642ddb0eb5b70b572d
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 66e91517338e..078a30fa37b3 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -959,7 +959,9 @@ void ScPostIt::UpdateCaptionPos( const ScAddress& rPos )
void ScPostIt::CreateCaptionFromInitData( const ScAddress& rPos ) const
{
- OSL_ENSURE( maNoteData.mxCaption || maNoteData.mxInitData.get(), "ScPostIt::CreateCaptionFromInitData - need caption object or initial caption data" );
+ // Captions are not created in Undo documents and only rarely in Clipboard,
+ // but otherwise we need caption or initial data.
+ assert((maNoteData.mxCaption || maNoteData.mxInitData.get()) || mrDoc.IsUndo() || mrDoc.IsClipboard());
if( maNoteData.mxInitData.get() )
{
/* This function is called from ScPostIt::Clone() when copying cells
commit 3deb09f16d9c68914be4f89c5fc4bed005cb7c0a
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 22:38:57 2017 +0100
it's raining drawing layers
Change-Id: Ieee5cb5792535185ef09c3775072ed739fb0e4b0
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index d64dbf7efde0..9e0f9496b9d4 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -724,6 +724,9 @@ void Test::testCopyToDocument()
{
CPPUNIT_ASSERT_MESSAGE ("failed to insert sheet", m_pDoc->InsertTab (0, "src"));
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
m_pDoc->SetString(0, 0, 0, "Header");
m_pDoc->SetString(0, 1, 0, "1");
m_pDoc->SetString(0, 2, 0, "2");
@@ -1934,6 +1937,9 @@ void Test::testSheetCopy()
CPPUNIT_ASSERT_EQUAL_MESSAGE("document should have one sheet to begin with.",
static_cast<SCTAB>(1), m_pDoc->GetTableCount());
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
// Insert text in A1.
m_pDoc->SetString(ScAddress(0,0,0), "copy me");
@@ -5062,6 +5068,9 @@ void Test::testNoteDeleteCol()
ScDocument& rDoc = getDocShell().GetDocument();
rDoc.InsertTab(0, "Sheet1");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
ScAddress rAddr(1, 1, 0);
ScPostIt* pNote = m_pDoc->GetOrCreateNote(rAddr);
pNote->SetText(rAddr, "Hello");
@@ -5227,6 +5236,9 @@ void Test::testAreasWithNotes()
ScDocument& rDoc = getDocShell().GetDocument();
rDoc.InsertTab(0, "Sheet1");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
ScAddress rAddr(1, 5, 0);
ScPostIt* pNote = m_pDoc->GetOrCreateNote(rAddr);
pNote->SetText(rAddr, "Hello");
@@ -6026,6 +6038,9 @@ void Test::testSetStringAndNote()
{
m_pDoc->InsertTab(0, "Test");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
//note on A1
ScAddress aAdrA1 (0, 0, 0);
ScPostIt* pNote = m_pDoc->GetOrCreateNote(aAdrA1);
commit d1903be02477369990e7f1eff53224f820dc2fe8
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 22:09:55 2017 +0100
add/use ScCaptionPtr::removeFromDrawPage()
Change-Id: Ibe073f071b120b61738b7e813a14824248f1fcfc
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 7266109200c0..e3a91ab2d73a 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -63,6 +63,11 @@ public:
*/
void insertToDrawPage( SdrPage& rDrawPage );
+ /** Remove from draw page. The caption object is not owned anymore by the
+ draw page then.
+ */
+ void removeFromDrawPage( SdrPage& rDrawPage );
+
/** 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 ee9b89e78ef7..66e91517338e 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -710,6 +710,20 @@ void ScCaptionPtr::insertToDrawPage( SdrPage& rDrawPage )
mpHead->mbInDrawPage = true;
}
+void ScCaptionPtr::removeFromDrawPage( SdrPage& rDrawPage )
+{
+ assert(mpHead && mpCaption);
+ SAL_WARN_IF(!mpHead->mbInDrawPage,"sc.core","ScCaptionPtr::removeFromDrawPage - not in draw page");
+ /* FIXME: that should assert, but currently fails in
+ * Test::testCopyToDocument() probably due to CopyStaticToDocument()
+ * lacking something. */
+ //assert(mpHead->mbInDrawPage); // did we lose track anywhere?
+
+ SdrObject* pObj = rDrawPage.RemoveObject( mpCaption->GetOrdNum() );
+ assert(pObj == mpCaption); (void)pObj;
+ mpHead->mbInDrawPage = false;
+}
+
SdrCaptionObj* ScCaptionPtr::release()
{
SdrCaptionObj* pTmp = mpCaption;
@@ -1081,10 +1095,10 @@ void ScPostIt::RemoveCaption()
if( bRecording )
pDrawLayer->AddCalcUndo( new SdrUndoDelObj( *maNoteData.mxCaption.get() ) );
// remove the object from the drawing page, delete if undo is disabled
- SdrObject* pObj = pDrawPage->RemoveObject( maNoteData.mxCaption->GetOrdNum() );
+ maNoteData.mxCaption.removeFromDrawPage( *pDrawPage );
if( !bRecording )
{
- maNoteData.mxCaption.release();
+ SdrObject* pObj = maNoteData.mxCaption.release();
SdrObject::Free( pObj );
}
}
commit d3fa5013570cfe3c0785ebb3304e8d98f1ee6004
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 22:08:38 2017 +0100
sprinkle some drawing layers over test cases
... so things actually work like intended and creation of caption objects
doesn't silently fail. Well, it does SAL_WARN or OSL_ENSURE but that's never
displayed unless a test fails.
Change-Id: Ibf4cc075cc3d6dadbe8f6208b2949310124b5749
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index 68c84c24627d..d64dbf7efde0 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -738,22 +738,29 @@ void Test::testCopyToDocument()
// Copy statically to another document.
- ScDocument aDestDoc(SCDOCMODE_DOCUMENT);
- aDestDoc.InsertTab(0, "src");
- m_pDoc->CopyStaticToDocument(ScRange(0,1,0,0,3,0), 0, &aDestDoc); // Copy A2:A4
- m_pDoc->CopyStaticToDocument(ScAddress(0,0,0), 0, &aDestDoc); // Copy A1
- m_pDoc->CopyStaticToDocument(ScRange(0,4,0,0,7,0), 0, &aDestDoc); // Copy A5:A8
-
- CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,0,0), aDestDoc.GetString(0,0,0));
- CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,1,0), aDestDoc.GetString(0,1,0));
- CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,2,0), aDestDoc.GetString(0,2,0));
- CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,3,0), aDestDoc.GetString(0,3,0));
- CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,4,0), aDestDoc.GetString(0,4,0));
+ ScDocShellRef xDocSh2;
+ getNewDocShell(xDocSh2);
+ ScDocument* pDestDoc = &xDocSh2->GetDocument();
+ pDestDoc->InsertTab(0, "src");
+ pDestDoc->InitDrawLayer(xDocSh2.get()); // for note caption objects
+
+ m_pDoc->CopyStaticToDocument(ScRange(0,1,0,0,3,0), 0, pDestDoc); // Copy A2:A4
+ m_pDoc->CopyStaticToDocument(ScAddress(0,0,0), 0, pDestDoc); // Copy A1
+ m_pDoc->CopyStaticToDocument(ScRange(0,4,0,0,7,0), 0, pDestDoc); // Copy A5:A8
+
+ CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,0,0), pDestDoc->GetString(0,0,0));
+ CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,1,0), pDestDoc->GetString(0,1,0));
+ CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,2,0), pDestDoc->GetString(0,2,0));
+ CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,3,0), pDestDoc->GetString(0,3,0));
+ CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,4,0), pDestDoc->GetString(0,4,0));
// verify note
- CPPUNIT_ASSERT_MESSAGE("There should be a note in A1 destDocument", aDestDoc.HasNote(ScAddress(0, 0, 0)));
+ CPPUNIT_ASSERT_MESSAGE("There should be a note in A1 destDocument", pDestDoc->HasNote(ScAddress(0, 0, 0)));
CPPUNIT_ASSERT_EQUAL_MESSAGE("The notes content should be the same on both documents",
- m_pDoc->GetNote(ScAddress(0, 0, 0))->GetText(), aDestDoc.GetNote(ScAddress(0, 0, 0))->GetText());
+ m_pDoc->GetNote(ScAddress(0, 0, 0))->GetText(), pDestDoc->GetNote(ScAddress(0, 0, 0))->GetText());
+
+ pDestDoc->DeleteTab(0);
+ closeDocShell(xDocSh2);
m_pDoc->DeleteTab(0);
}
@@ -3041,6 +3048,10 @@ void Test::testCopyPaste()
{
m_pDoc->InsertTab(0, "Sheet1");
m_pDoc->InsertTab(1, "Sheet2");
+
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
//test copy&paste + ScUndoPaste
//copy local and global range names in formulas
//string cells and value cells
@@ -3256,6 +3267,9 @@ void Test::testCopyPasteTranspose()
m_pDoc->InsertTab(0, "Sheet1");
m_pDoc->InsertTab(1, "Sheet2");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
m_pDoc->SetValue(0, 0, 0, 1);
m_pDoc->SetString(1, 0, 0, "=A1+1");
m_pDoc->SetString(2, 0, 0, "test");
@@ -3844,6 +3858,9 @@ void Test::testMoveBlock()
{
m_pDoc->InsertTab(0, "SheetNotes");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
m_pDoc->SetValue(0, 0, 0, 1);
m_pDoc->SetString(1, 0, 0, "=A1+1");
m_pDoc->SetString(2, 0, 0, "test");
@@ -4835,6 +4852,9 @@ void Test::testShiftCells()
{
m_pDoc->InsertTab(0, "foo");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
OUString aTestVal("Some Text");
// Text into cell E5.
@@ -4872,6 +4892,9 @@ void Test::testNoteBasic()
{
m_pDoc->InsertTab(0, "PostIts");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
CPPUNIT_ASSERT(!m_pDoc->HasNotes());
// Check for note's presence in all tables before inserting any notes.
diff --git a/sc/qa/unit/ucalc_sort.cxx b/sc/qa/unit/ucalc_sort.cxx
index 7ec786d7a5ad..d99132f7dda5 100644
--- a/sc/qa/unit/ucalc_sort.cxx
+++ b/sc/qa/unit/ucalc_sort.cxx
@@ -31,6 +31,9 @@ void Test::testSort()
{
m_pDoc->InsertTab(0, "test1");
+ // We need a drawing layer in order to create caption objects.
+ m_pDoc->InitDrawLayer(&getDocShell());
+
ScRange aDataRange;
ScAddress aPos(0,0,0);
{
commit 108edf7c758c6001554a24ef99f9bba55a1d8cbc
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 19:50:31 2017 +0100
probably should work like sketched
Change-Id: I5ad52c718cf53c2f3fb14a7970917e0012d01875
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 618bbe462b9f..ee9b89e78ef7 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -683,13 +683,19 @@ void ScCaptionPtr::decRefAndDestroy()
{
if (decRef())
{
- /* FIXME: this should eventually remove the caption from drawing layer
- * foo and call SdrObject::Free(), likely with mpHead->mpCaption, see
- * ScPostIt::RemoveCaption(). Further work needed to be able to do so.
- * */
assert(mpHead->mpFirst == this); // this must be one and only one
- mpCaption = nullptr;
assert(!mpNext); // this must be one and only one
+ assert(mpCaption);
+ if (mpHead->mbInDrawPage)
+ {
+ /* 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.
+ * */
+ }
+ /* FIXME: once we got ownership right */
+ //SdrObject::Free( mpCaption );
+ mpCaption = nullptr;
delete mpHead;
mpHead = nullptr;
}
commit 326cfd39d5ddafc938cc6281452f9062d5b118e1
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 16:05:12 2017 +0100
use ScCaptionPtr::insertToDrawPage()
Change-Id: I98dafdf8e571e4745e05df6cbcbf00fd9ecd8ec6
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 638dfa643204..618bbe462b9f 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -419,7 +419,7 @@ ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& r
// store note position in user data of caption object
ScCaptionUtil::SetCaptionUserData( *rNoteData.mxCaption, rPos );
// insert object into draw page
- pDrawPage->InsertObject( rNoteData.mxCaption.get() );
+ rNoteData.mxCaption.insertToDrawPage( *pDrawPage );
}
}
}
@@ -1117,10 +1117,11 @@ ScCaptionPtr ScNoteUtil::CreateTempCaption(
// create the caption object
ScCaptionCreator aCreator( rDoc, rPos, bTailFront );
- SdrCaptionObj* pCaption = aCreator.GetCaption().get(); // just for ease of use
// insert caption into page (needed to set caption text)
- rDrawPage.InsertObject( pCaption );
+ aCreator.GetCaption().insertToDrawPage( rDrawPage );
+
+ SdrCaptionObj* pCaption = aCreator.GetCaption().get(); // just for ease of use
// clone the edit text object, unless user text is present, then set this text
if( pNoteCaption && rUserText.isEmpty() )
commit 58f894c86505c95f3924fa5e6d0c9523e062430e
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 15:22:57 2017 +0100
add ScCaptionPtr::insertToDrawPage()
Change-Id: I1266b55c2558d306b20b0f2d9fba07b0bc46544e
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index e198870479f6..7266109200c0 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -31,6 +31,7 @@ class EditTextObject;
class OutlinerParaObject;
class SdrCaptionObj;
class SdrPage;
+
class SfxItemSet;
class ScDocument;
class Rectangle;
@@ -58,6 +59,10 @@ public:
// Does not default to nullptr to make it visually obvious where such is used.
void reset( SdrCaptionObj* p );
+ /** Insert to draw page. The caption object is owned by the draw page then.
+ */
+ void insertToDrawPage( SdrPage& rDrawPage );
+
/** Release all management of the SdrCaptionObj* in all instances of this
list and dissolve. The SdrCaptionObj pointer returned is ready to be
managed elsewhere.
@@ -77,8 +82,12 @@ private:
struct Head
{
- ScCaptionPtr* mpFirst; ///< first in list
- oslInterlockedCount mnRefs; ///< use count
+ ScCaptionPtr* mpFirst; ///< first in list
+ oslInterlockedCount mnRefs; ///< use count
+ bool mbInDrawPage; ///< caption object is owned by draw page
+
+ Head() = delete;
+ explicit Head( ScCaptionPtr* );
};
Head* mpHead; ///< points to the "master" entry
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 2ae4a75d7750..638dfa643204 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -534,12 +534,15 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
return *this;
}
+ScCaptionPtr::Head::Head( ScCaptionPtr* p ) :
+ mpFirst(p), mnRefs(1), mbInDrawPage(false)
+{
+}
+
void ScCaptionPtr::newHead()
{
assert(!mpHead);
- mpHead = new Head;
- mpHead->mpFirst = this;
- mpHead->mnRefs = 1;
+ mpHead = new Head(this);
}
void ScCaptionPtr::replaceInList( ScCaptionPtr* pNew )
@@ -692,6 +695,15 @@ void ScCaptionPtr::decRefAndDestroy()
}
}
+void ScCaptionPtr::insertToDrawPage( SdrPage& rDrawPage )
+{
+ assert(mpHead && mpCaption);
+ assert(!mpHead->mbInDrawPage); // multiple assignments not possible
+
+ rDrawPage.InsertObject( mpCaption );
+ mpHead->mbInDrawPage = true;
+}
+
SdrCaptionObj* ScCaptionPtr::release()
{
SdrCaptionObj* pTmp = mpCaption;
commit 4c004a4bb09afbb69a1345d218ae79b6fa011034
Author: Eike Rathke <erack at redhat.com>
Date: Thu Mar 9 14:29:40 2017 +0100
move assignment onto self should not happen
Change-Id: Ic44f4362762cb1c1fe027b69a78baf768c0a53da
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 2111b8ebb4e4..2ae4a75d7750 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -485,8 +485,7 @@ ScCaptionPtr::ScCaptionPtr( ScCaptionPtr&& r ) :
ScCaptionPtr& ScCaptionPtr::operator=( ScCaptionPtr&& r )
{
- if (this == &r)
- return *this;
+ assert(this != &r);
mpHead = r.mpHead;
mpNext = r.mpNext;
commit 961d286ea355feda57b3138cb8339d6e2200d98a
Author: Eike Rathke <erack at redhat.com>
Date: Wed Mar 8 18:14:29 2017 +0100
let ScNoteUtil::CreateTempCaption() return ScCaptionPtr
Change-Id: Idcb7fc24a13d650d88bec9ba359d7c78006096ec
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 0801b2493abc..e198870479f6 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -247,7 +247,7 @@ class SC_DLLPUBLIC ScNoteUtil
public:
/** Creates and returns a caption object for a temporary caption. */
- static SdrCaptionObj* CreateTempCaption( ScDocument& rDoc, const ScAddress& rPos,
+ static ScCaptionPtr CreateTempCaption( ScDocument& rDoc, const ScAddress& rPos,
SdrPage& rDrawPage, const OUString& rUserText,
const Rectangle& rVisRect, bool bTailFront );
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 4071bb45993b..2111b8ebb4e4 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1078,7 +1078,7 @@ void ScPostIt::RemoveCaption()
maNoteData.mxCaption.reset(nullptr);
}
-SdrCaptionObj* ScNoteUtil::CreateTempCaption(
+ScCaptionPtr ScNoteUtil::CreateTempCaption(
ScDocument& rDoc, const ScAddress& rPos, SdrPage& rDrawPage,
const OUString& rUserText, const Rectangle& rVisRect, bool bTailFront )
{
@@ -1095,7 +1095,7 @@ SdrCaptionObj* ScNoteUtil::CreateTempCaption(
// create a caption if any text exists
if( !pNoteCaption && aBuffer.isEmpty() )
- return nullptr;
+ return ScCaptionPtr();
// prepare visible rectangle (add default distance to all borders)
Rectangle aVisRect(
@@ -1138,9 +1138,8 @@ SdrCaptionObj* ScNoteUtil::CreateTempCaption(
// move caption into visible area
aCreator.AutoPlaceCaption( &aVisRect );
- // The caption object returned is completely unmanaged and stored elsewhere.
// XXX Note it is already inserted to the draw page.
- return aCreator.GetCaption().release();
+ return aCreator.GetCaption();
}
ScPostIt* ScNoteUtil::CreateNoteFromCaption(
diff --git a/sc/source/ui/inc/notemark.hxx b/sc/source/ui/inc/notemark.hxx
index 2d83b8fe500e..b02bf812f623 100644
--- a/sc/source/ui/inc/notemark.hxx
+++ b/sc/source/ui/inc/notemark.hxx
@@ -24,9 +24,9 @@
#include <vcl/timer.hxx>
#include "global.hxx"
#include "address.hxx"
+#include "postit.hxx"
class SdrModel;
-class SdrObject;
class ScNoteMarker
{
@@ -46,7 +46,7 @@ private:
Rectangle aRect;
SdrModel* pModel;
- SdrObject* pObject;
+ ScCaptionPtr mxObject;
bool bVisible;
Point aGridOff;
DECL_LINK( TimeHdl, Timer*, void );
diff --git a/sc/source/ui/view/notemark.cxx b/sc/source/ui/view/notemark.cxx
index 9d5282e58a77..564d98d83329 100644
--- a/sc/source/ui/view/notemark.cxx
+++ b/sc/source/ui/view/notemark.cxx
@@ -48,7 +48,6 @@ ScNoteMarker::ScNoteMarker( vcl::Window* pWin, vcl::Window* pRight, vcl::Window*
bLeft( bLeftEdge ),
bByKeyboard( bKeyboard ),
pModel( nullptr ),
- pObject( nullptr ),
bVisible( false )
{
Size aSizePixel = pWindow->GetOutputSizePixel();
@@ -94,11 +93,11 @@ IMPL_LINK_NOARG(ScNoteMarker, TimeHdl, Timer *, void)
if( SdrPage* pPage = pModel->AllocPage( false ) )
{
- pObject = ScNoteUtil::CreateTempCaption( *pDoc, aDocPos, *pPage, aUserText, aVisRect, bLeft );
- if( pObject )
+ mxObject = ScNoteUtil::CreateTempCaption( *pDoc, aDocPos, *pPage, aUserText, aVisRect, bLeft );
+ if( mxObject )
{
- pObject->SetGridOffset( aGridOff );
- aRect = pObject->GetCurrentBoundRect();
+ mxObject->SetGridOffset( aGridOff );
+ aRect = mxObject->GetCurrentBoundRect();
}
// Insert page so that the model recognise it and also deleted
@@ -141,21 +140,21 @@ static MapMode lcl_MoveMapMode( const MapMode& rMap, const Size& rMove )
void ScNoteMarker::Draw()
{
- if ( pObject && bVisible )
+ if ( mxObject && bVisible )
{
- lcl_DrawWin( pObject, pWindow, aMapMode );
+ lcl_DrawWin( mxObject.get(), pWindow, aMapMode );
if ( pRightWin || pBottomWin )
{
Size aWinSize = pWindow->PixelToLogic( pWindow->GetOutputSizePixel(), aMapMode );
if ( pRightWin )
- lcl_DrawWin( pObject, pRightWin,
+ lcl_DrawWin( mxObject.get(), pRightWin,
lcl_MoveMapMode( aMapMode, Size( aWinSize.Width(), 0 ) ) );
if ( pBottomWin )
- lcl_DrawWin( pObject, pBottomWin,
+ lcl_DrawWin( mxObject.get(), pBottomWin,
lcl_MoveMapMode( aMapMode, Size( 0, aWinSize.Height() ) ) );
if ( pDiagWin )
- lcl_DrawWin( pObject, pDiagWin, lcl_MoveMapMode( aMapMode, aWinSize ) );
+ lcl_DrawWin( mxObject.get(), pDiagWin, lcl_MoveMapMode( aMapMode, aWinSize ) );
}
}
}
commit 2401cac9ff2d219520079c93736a947b9e99bacf
Author: Eike Rathke <erack at redhat.com>
Date: Tue Mar 7 21:26:56 2017 +0100
coverity#1401471 implement move assignment and move ctor at ScCaptionPtr
Change-Id: Ic429f5e177bb1a35857f00c6e13e5cbb34d46578
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 129989b94fa5..0801b2493abc 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -45,9 +45,11 @@ public:
ScCaptionPtr();
explicit ScCaptionPtr( SdrCaptionObj* p );
ScCaptionPtr( const ScCaptionPtr& r );
+ ScCaptionPtr( ScCaptionPtr&& r );
~ScCaptionPtr();
ScCaptionPtr& operator=( const ScCaptionPtr& r );
+ ScCaptionPtr& operator=( ScCaptionPtr&& r );
inline explicit operator bool() const { return mpCaption != nullptr; }
inline SdrCaptionObj* get() const { return mpCaption; }
inline SdrCaptionObj* operator->() const { return mpCaption; }
@@ -96,6 +98,12 @@ private:
*/
void removeFromList();
+ /** Replace this instance with pNew in a list, if any.
+
+ Used by move-ctor and move assignment operator.
+ */
+ void replaceInList( ScCaptionPtr* pNew );
+
/** Dissolve list when the caption object is released or gone. */
void dissolve();
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 24b3aad8b88f..4071bb45993b 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -476,6 +476,28 @@ ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) :
}
}
+ScCaptionPtr::ScCaptionPtr( ScCaptionPtr&& r ) :
+ mpHead(r.mpHead), mpNext(r.mpNext), mpCaption(r.mpCaption)
+{
+ r.replaceInList( this );
+ r.mpCaption = nullptr;
+}
+
+ScCaptionPtr& ScCaptionPtr::operator=( ScCaptionPtr&& r )
+{
+ if (this == &r)
+ return *this;
+
+ mpHead = r.mpHead;
+ mpNext = r.mpNext;
+ mpCaption = r.mpCaption;
+
+ r.replaceInList( this );
+ r.mpCaption = nullptr;
+
+ return *this;
+}
+
ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
{
if (this == &r)
@@ -521,6 +543,32 @@ void ScCaptionPtr::newHead()
mpHead->mnRefs = 1;
}
+void ScCaptionPtr::replaceInList( ScCaptionPtr* pNew )
+{
+ if (!mpHead && !mpNext)
+ return;
+
+ assert(mpHead);
+ assert(mpCaption == pNew->mpCaption);
+
+ ScCaptionPtr* pThat = mpHead->mpFirst;
+ while (pThat && pThat != this && pThat->mpNext != this)
+ {
+ pThat = pThat->mpNext;
+ }
+ if (pThat && pThat != this)
+ {
+ assert(pThat->mpNext == this);
+ pThat->mpNext = pNew;
+ }
+ pNew->mpNext = mpNext;
+ if (mpHead->mpFirst == this)
+ mpHead->mpFirst = pNew;
+
+ mpHead = nullptr;
+ mpNext = nullptr;
+}
+
void ScCaptionPtr::removeFromList()
{
if (!mpHead && !mpNext && !mpCaption)
commit bd18465a70ec734ba266df421bf660dbb1ab4642
Author: Eike Rathke <erack at redhat.com>
Date: Tue Mar 7 20:35:45 2017 +0100
Resolves: tdf#106385 don't release ScCaptionPtr too early
Change-Id: Ibf0afa1d6c582251a5a2e00f2d6d9c1f267bf746
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 1e30882e70b7..24b3aad8b88f 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -1058,9 +1058,7 @@ SdrCaptionObj* ScNoteUtil::CreateTempCaption(
// create the caption object
ScCaptionCreator aCreator( rDoc, rPos, bTailFront );
- // The caption object returned is completely unmanaged and stored
- // elsewhere.
- SdrCaptionObj* pCaption = aCreator.GetCaption().release();
+ SdrCaptionObj* pCaption = aCreator.GetCaption().get(); // just for ease of use
// insert caption into page (needed to set caption text)
rDrawPage.InsertObject( pCaption );
@@ -1091,7 +1089,10 @@ SdrCaptionObj* ScNoteUtil::CreateTempCaption(
// move caption into visible area
aCreator.AutoPlaceCaption( &aVisRect );
- return pCaption;
+
+ // The caption object returned is completely unmanaged and stored elsewhere.
+ // XXX Note it is already inserted to the draw page.
+ return aCreator.GetCaption().release();
}
ScPostIt* ScNoteUtil::CreateNoteFromCaption(
commit 6c151cf87819568ab26cfb5bb4f500e44becae46
Author: Eike Rathke <erack at redhat.com>
Date: Tue Mar 7 10:46:12 2017 +0100
a size is a size
Change-Id: I98b54ac353bc0c078f31b447aec2eff7e80b4308
diff --git a/sc/source/ui/view/viewdata.cxx b/sc/source/ui/view/viewdata.cxx
index 87a77ad4692d..309528e3619d 100644
--- a/sc/source/ui/view/viewdata.cxx
+++ b/sc/source/ui/view/viewdata.cxx
@@ -1803,9 +1803,9 @@ void ScViewData::CreateSelectedTabData()
void ScViewData::EnsureTabDataSize(size_t nSize)
{
- if (nSize >= maTabData.size())
+ if (nSize > maTabData.size())
{
- size_t n = nSize - maTabData.size() + 1;
+ size_t n = nSize - maTabData.size();
maTabData.insert(maTabData.end(), n, nullptr);
}
}
@@ -3135,7 +3135,7 @@ void ScViewData::ReadUserDataSequence(const uno::Sequence <beans::PropertyValue>
sal_Int16 nTemp16(0);
bool bPageMode(false);
- EnsureTabDataSize(GetDocument()->GetTableCount()-1);
+ EnsureTabDataSize(GetDocument()->GetTableCount());
for (sal_Int32 i = 0; i < nCount; i++)
{
commit 6a3f36e5e6edaddf80132a071901b4e6ed1057b6
Author: Eike Rathke <erack at redhat.com>
Date: Tue Feb 28 18:28:53 2017 +0100
in decRefAndDestroy() the remaining element must be one and only one
So head can be destroyed already there and removeFromList() take a short cut.
Change-Id: I8f53d252c4e0ad867674ee410ecfaa300ac0c731
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 9f25cfe6ab9b..1e30882e70b7 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -637,14 +637,11 @@ void ScCaptionPtr::decRefAndDestroy()
* foo and call SdrObject::Free(), likely with mpHead->mpCaption, see
* ScPostIt::RemoveCaption(). Further work needed to be able to do so.
* */
- ScCaptionPtr* pThat = (mpHead ? mpHead->mpFirst : this);
- do
- {
- pThat->mpCaption = nullptr;
- pThat = pThat->mpNext;
- }
- while (pThat);
- assert(!mpCaption); // this ought to be in list and nulled
+ assert(mpHead->mpFirst == this); // this must be one and only one
+ mpCaption = nullptr;
+ assert(!mpNext); // this must be one and only one
+ delete mpHead;
+ mpHead = nullptr;
}
}
commit 85ba0caf47f43c1542762c0b13793d6958f80ad0
Author: Eike Rathke <erack at redhat.com>
Date: Tue Feb 28 17:16:33 2017 +0100
dissolve() needs to delete head now that it's not a list element anymore
Change-Id: I9949a1006e6d1b4b50dd5350106ad69b643e833c
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 0f456611eebb..9f25cfe6ab9b 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -665,15 +665,18 @@ bool ScCaptionPtr::forget()
void ScCaptionPtr::dissolve()
{
+ ScCaptionPtr::Head* pHead = mpHead;
ScCaptionPtr* pThat = (mpHead ? mpHead->mpFirst : this);
while (pThat)
{
- assert(!pThat->mpNext || mpHead); // next without head is bad
+ assert(!pThat->mpNext || pThat->mpHead); // next without head is bad
+ assert(pThat->mpHead == pHead); // same head required within one list
ScCaptionPtr* p = pThat->mpNext;
pThat->clear();
pThat = p;
}
- clear();
+ assert(!mpHead && !mpNext && !mpCaption); // should had been cleared during list walk
+ delete pHead;
}
void ScCaptionPtr::clear()
commit 318ded2d7b9a03c8f76e2adda50997d5352e5607
Author: Eike Rathke <erack at redhat.com>
Date: Tue Feb 28 13:47:54 2017 +0100
assert that nullptr captions are not in a list
Change-Id: I0c286891454d290ec4373dbc37e31d65c22c746d
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 8c636693b3b3..0f456611eebb 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -494,6 +494,8 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
// Let's find some weird usage.
// Assigning without head doesn't make sense unless it is a nullptr caption.
assert(r.mpHead || !r.mpCaption);
+ // A nullptr caption must not be in a list and thus not have a head.
+ assert(!r.mpHead || r.mpCaption);
// Same captions were caught above, so here different heads must be present.
assert(r.mpHead != mpHead);
@@ -533,7 +535,7 @@ void ScCaptionPtr::removeFromList()
// Use the walk to check consistency on the fly.
assert(pThat->mpHead == mpHead); // all belong to the same
assert(pThat->mpHead || !pThat->mpNext); // next without head is bad
- assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
+ assert(pThat->mpCaption == mpCaption);
pThat = pThat->mpNext;
#if OSL_DEBUG_LEVEL > 0
++nCount;
@@ -556,7 +558,7 @@ void ScCaptionPtr::removeFromList()
{
assert(pThat->mpHead == mpHead); // all belong to the same
assert(pThat->mpHead || !pThat->mpNext); // next without head is bad
- assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
+ assert(pThat->mpCaption == mpCaption);
++nCount;
}
while ((pThat = pThat->mpNext) != nullptr);
commit 344dd43b3f790d44abfb4c0addc4378b827a7fe1
Author: Eike Rathke <erack at redhat.com>
Date: Mon Feb 27 22:24:14 2017 +0100
move assign() into operator=() and add the relevant bits to copy-ctor
Change-Id: Iac606a8e5e4fc9beb019d95d2a05f0ab9d9dad34
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index a6bfe232dd4b..129989b94fa5 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -88,9 +88,6 @@ private:
bool decRef() const; //< @returns <TRUE/> if the last reference was decremented.
void decRefAndDestroy(); //< Destroys caption object if the last reference was decremented.
- /** Operations common to ctor and assignment operator, maintaining the lists. */
- void assign( const ScCaptionPtr& r, bool bAssignment );
-
/** Remove from current list and close gap.
Usually there are only very few instances, so maintaining a doubly
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index f79a70e8256c..8c636693b3b3 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -459,21 +459,28 @@ ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) :
}
ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) :
- mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr)
+ mpHead(r.mpHead), mpCaption(r.mpCaption)
{
- assign( r, false);
+ if (r.mpCaption)
+ {
+ assert(r.mpHead);
+ r.incRef();
+ // Insert into list.
+ mpNext = r.mpNext;
+ r.mpNext = this;
+ }
+ else
+ {
+ assert(!r.mpHead);
+ mpNext = nullptr;
+ }
}
-void ScCaptionPtr::newHead()
+ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
{
- assert(!mpHead);
- mpHead = new Head;
- mpHead->mpFirst = this;
- mpHead->mnRefs = 1;
-}
+ if (this == &r)
+ return *this;
-void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
-{
if (mpCaption == r.mpCaption)
{
// Two lists for the same caption is bad.
@@ -481,7 +488,7 @@ void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
assert(!mpCaption); // assigning same caption pointer within same list is weird
// Nullptr captions are not inserted to the list, so nothing to do here
// if both are.
- return;
+ return *this;
}
// Let's find some weird usage.
@@ -491,17 +498,25 @@ void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
assert(r.mpHead != mpHead);
r.incRef();
- if (bAssignment)
- {
- decRefAndDestroy();
- removeFromList();
- }
+ decRefAndDestroy();
+ removeFromList();
+
mpCaption = r.mpCaption;
// That head is this' master.
mpHead = r.mpHead;
// Insert into list.
mpNext = r.mpNext;
r.mpNext = this;
+
+ return *this;
+}
+
+void ScCaptionPtr::newHead()
+{
+ assert(!mpHead);
+ mpHead = new Head;
+ mpHead->mpFirst = this;
+ mpHead->mnRefs = 1;
}
void ScCaptionPtr::removeFromList()
@@ -666,15 +681,6 @@ void ScCaptionPtr::clear()
mpCaption = nullptr;
}
-ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
-{
- if (this == &r)
- return *this;
-
- assign( r, true);
- return *this;
-}
-
struct ScCaptionInitData
{
commit 36ce8cb4ed9d811a59159cfd4018d56cfa660ec9
Author: Eike Rathke <erack at redhat.com>
Date: Mon Feb 27 19:50:07 2017 +0100
rework ScCaptionPtr to have a distinct head element
Not only saves a pointer per list element, but future versions could
hold a document pointer and/or drawing layer's draw page as well.
Change-Id: I85e05981239223bec88c47f2ebe4c22e50cd9a0d
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index e8f212208289..a6bfe232dd4b 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -72,11 +72,18 @@ public:
oslInterlockedCount getRefs() const;
private:
- ScCaptionPtr* mpHead; ///< points to the "master" entry
- mutable ScCaptionPtr* mpNext; ///< next in list
- SdrCaptionObj* mpCaption; ///< the caption object, managed by head master
- mutable oslInterlockedCount mnRefs; ///< use count, managed by head master
+ struct Head
+ {
+ ScCaptionPtr* mpFirst; ///< first in list
+ oslInterlockedCount mnRefs; ///< use count
+ };
+
+ Head* mpHead; ///< points to the "master" entry
+ mutable ScCaptionPtr* mpNext; ///< next in list
+ SdrCaptionObj* mpCaption; ///< the caption object, managed by head master
+
+ void newHead(); //< Allocate a new Head and init.
void incRef() const;
bool decRef() const; //< @returns <TRUE/> if the last reference was decremented.
void decRefAndDestroy(); //< Destroys caption object if the last reference was decremented.
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index baeca99d25b2..f79a70e8256c 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -445,34 +445,50 @@ ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& r
ScCaptionPtr::ScCaptionPtr() :
- mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr), mnRefs(0)
+ mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr)
{
}
ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) :
- mpHead( p ? this : nullptr ), mpNext(nullptr), mpCaption(p), mnRefs( p ? 1 : 0 )
+ mpHead(nullptr), mpNext(nullptr), mpCaption(p)
{
+ if (p)
+ {
+ newHead();
+ }
}
ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) :
- mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr), mnRefs(0)
+ mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr)
{
assign( r, false);
}
+void ScCaptionPtr::newHead()
+{
+ assert(!mpHead);
+ mpHead = new Head;
+ mpHead->mpFirst = this;
+ mpHead->mnRefs = 1;
+}
+
void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
{
if (mpCaption == r.mpCaption)
+ {
+ // Two lists for the same caption is bad.
+ assert(!mpCaption || mpHead == r.mpHead);
+ assert(!mpCaption); // assigning same caption pointer within same list is weird
+ // Nullptr captions are not inserted to the list, so nothing to do here
+ // if both are.
return;
+ }
// Let's find some weird usage.
- // Though assigning some other of the same list to head may syntactically
- // be valid, why would we do that?
- assert(r.mpHead != this);
- // Same for assigning head to some other.
- assert(mpHead != &r);
- // And any other of the same list.
- assert(mpHead != r.mpHead);
+ // Assigning without head doesn't make sense unless it is a nullptr caption.
+ assert(r.mpHead || !r.mpCaption);
+ // Same captions were caught above, so here different heads must be present.
+ assert(r.mpHead != mpHead);
r.incRef();
if (bAssignment)
@@ -483,7 +499,6 @@ void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
mpCaption = r.mpCaption;
// That head is this' master.
mpHead = r.mpHead;
- mnRefs = 0;
// Insert into list.
mpNext = r.mpNext;
r.mpNext = this;
@@ -491,54 +506,62 @@ void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
void ScCaptionPtr::removeFromList()
{
- if (!mpHead && !mpNext && !mpCaption && !mnRefs)
+ if (!mpHead && !mpNext && !mpCaption)
return;
- ScCaptionPtr* pNewHead = (mpHead == this ? mpNext : nullptr);
- ScCaptionPtr* pThat = (mpHead == this ? mpNext : mpHead); // do not replace on self if head
- while (pThat && pThat->mpNext != this)
+#if OSL_DEBUG_LEVEL > 0
+ oslInterlockedCount nCount = 0;
+#endif
+ ScCaptionPtr* pThat = (mpHead ? mpHead->mpFirst : nullptr);
+ while (pThat && pThat != this && pThat->mpNext != this)
{
// Use the walk to check consistency on the fly.
+ assert(pThat->mpHead == mpHead); // all belong to the same
+ assert(pThat->mpHead || !pThat->mpNext); // next without head is bad
assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
- assert(pThat->mnRefs == 0 || pThat == mpHead);
- if (pNewHead)
- {
- assert(pThat->mpHead == mpHead);
- pThat->mpHead = pNewHead;
- }
pThat = pThat->mpNext;
+#if OSL_DEBUG_LEVEL > 0
+ ++nCount;
+#endif
}
- assert(pThat || mpHead == this); // not found only if this was head
+ assert(pThat || !mpHead); // not found only if this was standalone
if (pThat)
{
- pThat->mpNext = mpNext;
- if (pNewHead)
+ if (pThat != this)
{
- do
- {
- assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
- assert(pThat->mnRefs == 0 || pThat == mpHead);
- assert(pThat->mpHead == mpHead);
- pThat->mpHead = pNewHead;
- }
- while ((pThat = pThat->mpNext) != nullptr);
+#if OSL_DEBUG_LEVEL > 0
+ // The while loop above was not executed, and for this
+ // (pThat->mpNext) the loop below won't either.
+ ++nCount;
+#endif
+ pThat->mpNext = mpNext;
}
- else
+#if OSL_DEBUG_LEVEL > 0
+ do
{
+ assert(pThat->mpHead == mpHead); // all belong to the same
+ assert(pThat->mpHead || !pThat->mpNext); // next without head is bad
+ assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
+ ++nCount;
+ }
+ while ((pThat = pThat->mpNext) != nullptr);
+#endif
+ }
#if OSL_DEBUG_LEVEL > 0
- // XXX exactly as for the if() branch but without replacing head.
- do
- {
- assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
- assert(pThat->mnRefs == 0 || pThat == mpHead);
- assert(pThat->mpHead == mpHead);
- }
- while ((pThat = pThat->mpNext) != nullptr);
+ // If part of a list then refs were already decremented.
+ assert(nCount == (mpHead ? mpHead->mnRefs + 1 : 0));
#endif
+ if (mpHead && mpHead->mpFirst == this)
+ {
+ if (mpNext)
+ mpHead->mpFirst = mpNext;
+ else
+ {
+ // The only one destroys also head.
+ assert(mpHead->mnRefs == 0); // cough
+ delete mpHead; // DEAD now
}
}
- if (pNewHead)
- pNewHead->mnRefs = mnRefs;
mpHead = nullptr;
mpNext = nullptr;
}
@@ -550,7 +573,7 @@ void ScCaptionPtr::reset( SdrCaptionObj* p )
if (p)
{
// Check if we end up with a duplicated management in this list.
- ScCaptionPtr* pThat = mpHead;
+ ScCaptionPtr* pThat = (mpHead ? mpHead->mpFirst : nullptr);
while (pThat)
{
assert(pThat->mpCaption != p);
@@ -563,13 +586,7 @@ void ScCaptionPtr::reset( SdrCaptionObj* p )
mpCaption = p;
if (p)
{
- mpHead = this;
- mnRefs = 1;
- }
- else
- {
- mpHead = nullptr;
- mnRefs = 0;
+ newHead();
}
}
@@ -581,8 +598,7 @@ ScCaptionPtr::~ScCaptionPtr()
oslInterlockedCount ScCaptionPtr::getRefs() const
{
- assert(!mnRefs || mpHead == this);
- return mpHead ? mpHead->mnRefs : mnRefs;
+ return mpHead ? mpHead->mnRefs : 0;
}
void ScCaptionPtr::incRef() const
@@ -604,16 +620,14 @@ void ScCaptionPtr::decRefAndDestroy()
* foo and call SdrObject::Free(), likely with mpHead->mpCaption, see
* ScPostIt::RemoveCaption(). Further work needed to be able to do so.
* */
- ScCaptionPtr* pThat = mpHead;
+ ScCaptionPtr* pThat = (mpHead ? mpHead->mpFirst : this);
do
{
pThat->mpCaption = nullptr;
- assert(pThat->mnRefs == 0);
- pThat->mnRefs = 0;
pThat = pThat->mpNext;
}
while (pThat);
- assert(!mpCaption); // this ought to be in list and nulled
+ assert(!mpCaption); // this ought to be in list and nulled
}
}
@@ -629,15 +643,15 @@ bool ScCaptionPtr::forget()
bool bRet = decRef();
removeFromList();
mpCaption = nullptr;
- mnRefs = 0;
return bRet;
}
void ScCaptionPtr::dissolve()
{
- ScCaptionPtr* pThat = mpHead;
+ ScCaptionPtr* pThat = (mpHead ? mpHead->mpFirst : this);
while (pThat)
{
+ assert(!pThat->mpNext || mpHead); // next without head is bad
ScCaptionPtr* p = pThat->mpNext;
pThat->clear();
pThat = p;
@@ -650,7 +664,6 @@ void ScCaptionPtr::clear()
mpHead = nullptr;
mpNext = nullptr;
mpCaption = nullptr;
- mnRefs = 0;
}
ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
commit f4fe5251ee225a383d6a9b3b439793990a694a75
Author: Eike Rathke <erack at redhat.com>
Date: Fri Feb 24 15:30:24 2017 +0100
a first stab against the note caption ownership mess
This should not change any existing behavior, but may help tracking down
what happens where and when. The final goal is to let ScCaptionPtr also
handle deletion of caption objects once they're unreferenced and guard
against dangling pointers and double delete, and/or to manage transfer
of ownership to the drawing layer. Further improvement to the structure
could involve a head data element so that the duplicated (and unused
except in head) mnRefs field could be eliminated and the walk simplified
when removing an element from the list.
Change-Id: Ifbb2fb1d9dc4d2594a1eae2a8489270dd1fe0d0c
Reviewed-on: https://gerrit.libreoffice.org/34616
Reviewed-by: Eike Rathke <erack at redhat.com>
Tested-by: Jenkins <ci at libreoffice.org>
diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 87553f3f5c3e..e8f212208289 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -36,6 +36,69 @@ class ScDocument;
class Rectangle;
struct ScCaptionInitData;
+/** Some desperate attempt to fight against the caption object ownership mess,
+ to which none of shared/weak/plain pointer is a cure.
+ */
+class ScCaptionPtr
+{
+public:
+ ScCaptionPtr();
+ explicit ScCaptionPtr( SdrCaptionObj* p );
+ ScCaptionPtr( const ScCaptionPtr& r );
+ ~ScCaptionPtr();
+
+ ScCaptionPtr& operator=( const ScCaptionPtr& r );
+ inline explicit operator bool() const { return mpCaption != nullptr; }
+ inline SdrCaptionObj* get() const { return mpCaption; }
+ inline SdrCaptionObj* operator->() const { return mpCaption; }
+ inline SdrCaptionObj& operator*() const { return *mpCaption; }
+
+ // Does not default to nullptr to make it visually obvious where such is used.
+ void reset( SdrCaptionObj* p );
+
+ /** Release all management of the SdrCaptionObj* in all instances of this
+ list and dissolve. The SdrCaptionObj pointer returned is ready to be
+ managed elsewhere.
+ */
+ SdrCaptionObj* release();
+
+ /** Forget the SdrCaptionObj pointer in this one instance.
+ Decrements a use count but does not destroy the object, it's up to the
+ caller to manage this mess..
+ @returns <TRUE/> if the last reference was decremented.
+ */
+ bool forget();
+
+ oslInterlockedCount getRefs() const;
+
+private:
+ ScCaptionPtr* mpHead; ///< points to the "master" entry
+ mutable ScCaptionPtr* mpNext; ///< next in list
+ SdrCaptionObj* mpCaption; ///< the caption object, managed by head master
+ mutable oslInterlockedCount mnRefs; ///< use count, managed by head master
+
+ void incRef() const;
+ bool decRef() const; //< @returns <TRUE/> if the last reference was decremented.
+ void decRefAndDestroy(); //< Destroys caption object if the last reference was decremented.
+
+ /** Operations common to ctor and assignment operator, maintaining the lists. */
+ void assign( const ScCaptionPtr& r, bool bAssignment );
+
+ /** Remove from current list and close gap.
+
+ Usually there are only very few instances, so maintaining a doubly
+ linked list isn't worth memory/performance wise and a simple walk does
+ it.
+ */
+ void removeFromList();
+
+ /** Dissolve list when the caption object is released or gone. */
+ void dissolve();
+
+ /** Just clear everything, while dissolving the list. */
+ void clear();
+};
+
/** Internal data for a cell annotation. */
struct SC_DLLPUBLIC ScNoteData
{
@@ -44,7 +107,7 @@ struct SC_DLLPUBLIC ScNoteData
OUString maDate; /// Creation date of the note.
OUString maAuthor; /// Author of the note.
ScCaptionInitDataRef mxInitData; /// Initial data for invisible notes without SdrObject.
- SdrCaptionObj* mpCaption; /// Drawing object representing the cell note.
+ ScCaptionPtr mxCaption; /// Drawing object representing the cell note.
bool mbShown; /// True = note is visible.
explicit ScNoteData( bool bShown = false );
@@ -124,10 +187,12 @@ public:
void SetText( const ScAddress& rPos, const OUString& rText );
/** Returns an existing note caption object. returns null, if the note
- contains initial caption data needed to construct a caption object. */
- SdrCaptionObj* GetCaption() const { return maNoteData.mpCaption;}
+ contains initial caption data needed to construct a caption object.
+ The SdrCaptionObj* returned is unmanaged and must not be stored elsewhere. */
+ SdrCaptionObj* GetCaption() const { return maNoteData.mxCaption.get();}
/** Returns the caption object of this note. Creates the caption object, if
- the note contains initial caption data instead of the caption. */
+ the note contains initial caption data instead of the caption.
+ The SdrCaptionObj* returned is unmanaged and must not be stored elsewhere. */
SdrCaptionObj* GetOrCreateCaption( const ScAddress& rPos ) const;
/** Forgets the pointer to the note caption object.
@@ -179,8 +244,10 @@ public:
This function is used in import filters to reuse the imported drawing
object as note caption object.
- @param rCaption The drawing object for the cell note. This object MUST
+ @param pCaption The drawing object for the cell note. This object MUST
be inserted into the document at the correct drawing page already.
+ The underlying ScPostIt::ScNoteData::ScCaptionPtr takes managing
+ ownership of the pointer.
@return Pointer to the new cell note object if insertion was
successful (i.e. the passed cell position was valid), null
@@ -190,7 +257,7 @@ public:
*/
static ScPostIt* CreateNoteFromCaption(
ScDocument& rDoc, const ScAddress& rPos,
- SdrCaptionObj& rCaption, bool bShown );
+ SdrCaptionObj* pCaption, bool bShown );
/** Creates a cell note based on the passed caption object data.
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 83c6111823fa..baeca99d25b2 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -161,12 +161,12 @@ public:
/** Create a new caption. The caption will not be inserted into the document. */
explicit ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, bool bTailFront );
/** Manipulate an existing caption. */
- explicit ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, SdrCaptionObj& rCaption );
+ explicit ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, ScCaptionPtr& xCaption );
/** Returns the drawing layer page of the sheet contained in maPos. */
SdrPage* GetDrawPage();
/** Returns the caption drawing object. */
- inline SdrCaptionObj* GetCaption() { return mpCaption; }
+ inline ScCaptionPtr GetCaption() { return mxCaption; }
/** Moves the caption inside the passed rectangle. Uses page area if 0 is passed. */
void FitCaptionToRect( const Rectangle* pVisRect = nullptr );
@@ -193,7 +193,7 @@ private:
private:
ScDocument& mrDoc;
ScAddress maPos;
- SdrCaptionObj* mpCaption;
+ ScCaptionPtr mxCaption;
Rectangle maPageRect;
Rectangle maCellRect;
bool mbNegPage;
@@ -201,25 +201,23 @@ private:
ScCaptionCreator::ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, bool bTailFront ) :
mrDoc( rDoc ),
- maPos( rPos ),
- mpCaption( nullptr )
+ maPos( rPos )
{
Initialize();
CreateCaption( true/*bShown*/, bTailFront );
}
-ScCaptionCreator::ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, SdrCaptionObj& rCaption ) :
+ScCaptionCreator::ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, ScCaptionPtr& xCaption ) :
mrDoc( rDoc ),
maPos( rPos ),
- mpCaption( &rCaption )
+ mxCaption( xCaption )
{
Initialize();
}
ScCaptionCreator::ScCaptionCreator( ScDocument& rDoc, const ScAddress& rPos ) :
mrDoc( rDoc ),
- maPos( rPos ),
- mpCaption( nullptr )
+ maPos( rPos )
{
Initialize();
}
@@ -235,13 +233,13 @@ void ScCaptionCreator::FitCaptionToRect( const Rectangle* pVisRect )
const Rectangle& rVisRect = GetVisRect( pVisRect );
// tail position
- Point aTailPos = mpCaption->GetTailPos();
+ Point aTailPos = mxCaption->GetTailPos();
aTailPos.X() = ::std::max( ::std::min( aTailPos.X(), rVisRect.Right() ), rVisRect.Left() );
aTailPos.Y() = ::std::max( ::std::min( aTailPos.Y(), rVisRect.Bottom() ), rVisRect.Top() );
- mpCaption->SetTailPos( aTailPos );
+ mxCaption->SetTailPos( aTailPos );
// caption rectangle
- Rectangle aCaptRect = mpCaption->GetLogicRect();
+ Rectangle aCaptRect = mxCaption->GetLogicRect();
Point aCaptPos = aCaptRect.TopLeft();
// move textbox inside right border of visible area
aCaptPos.X() = ::std::min< long >( aCaptPos.X(), rVisRect.Right() - aCaptRect.GetWidth() );
@@ -253,7 +251,7 @@ void ScCaptionCreator::FitCaptionToRect( const Rectangle* pVisRect )
aCaptPos.Y() = ::std::max< long >( aCaptPos.Y(), rVisRect.Top() );
// update caption
aCaptRect.SetPos( aCaptPos );
- mpCaption->SetLogicRect( aCaptRect );
+ mxCaption->SetLogicRect( aCaptRect );
}
void ScCaptionCreator::AutoPlaceCaption( const Rectangle* pVisRect )
@@ -261,7 +259,7 @@ void ScCaptionCreator::AutoPlaceCaption( const Rectangle* pVisRect )
const Rectangle& rVisRect = GetVisRect( pVisRect );
// caption rectangle
- Rectangle aCaptRect = mpCaption->GetLogicRect();
+ Rectangle aCaptRect = mxCaption->GetLogicRect();
long nWidth = aCaptRect.GetWidth();
long nHeight = aCaptRect.GetHeight();
@@ -319,7 +317,7 @@ void ScCaptionCreator::AutoPlaceCaption( const Rectangle* pVisRect )
// update textbox position in note caption object
aCaptRect.SetPos( aCaptPos );
- mpCaption->SetLogicRect( aCaptRect );
+ mxCaption->SetLogicRect( aCaptRect );
FitCaptionToRect( pVisRect );
}
@@ -328,33 +326,33 @@ void ScCaptionCreator::UpdateCaptionPos()
ScDrawLayer* pDrawLayer = mrDoc.GetDrawLayer();
// update caption position
- const Point& rOldTailPos = mpCaption->GetTailPos();
+ const Point& rOldTailPos = mxCaption->GetTailPos();
Point aTailPos = CalcTailPos( false );
if( rOldTailPos != aTailPos )
{
// create drawing undo action
if( pDrawLayer && pDrawLayer->IsRecording() )
- pDrawLayer->AddCalcUndo( new SdrUndoGeoObj( *mpCaption ) );
+ pDrawLayer->AddCalcUndo( new SdrUndoGeoObj( *mxCaption ) );
// calculate new caption rectangle (#i98141# handle LTR<->RTL switch correctly)
- Rectangle aCaptRect = mpCaption->GetLogicRect();
+ Rectangle aCaptRect = mxCaption->GetLogicRect();
long nDiffX = (rOldTailPos.X() >= 0) ? (aCaptRect.Left() - rOldTailPos.X()) : (rOldTailPos.X() - aCaptRect.Right());
if( mbNegPage ) nDiffX = -nDiffX - aCaptRect.GetWidth();
long nDiffY = aCaptRect.Top() - rOldTailPos.Y();
aCaptRect.SetPos( aTailPos + Point( nDiffX, nDiffY ) );
// set new tail position and caption rectangle
- mpCaption->SetTailPos( aTailPos );
- mpCaption->SetLogicRect( aCaptRect );
+ mxCaption->SetTailPos( aTailPos );
+ mxCaption->SetLogicRect( aCaptRect );
// fit caption into draw page
FitCaptionToRect();
}
// update cell position in caption user data
- ScDrawObjData* pCaptData = ScDrawLayer::GetNoteCaptionData( mpCaption, maPos.Tab() );
+ ScDrawObjData* pCaptData = ScDrawLayer::GetNoteCaptionData( mxCaption.get(), maPos.Tab() );
if( pCaptData && (maPos != pCaptData->maStart) )
{
// create drawing undo action
if( pDrawLayer && pDrawLayer->IsRecording() )
- pDrawLayer->AddCalcUndo( new ScUndoObjData( mpCaption, pCaptData->maStart, pCaptData->maEnd, maPos, pCaptData->maEnd ) );
+ pDrawLayer->AddCalcUndo( new ScUndoObjData( mxCaption.get(), pCaptData->maStart, pCaptData->maEnd, maPos, pCaptData->maEnd ) );
// set new position
pCaptData->maStart = maPos;
}
@@ -376,9 +374,9 @@ void ScCaptionCreator::CreateCaption( bool bShown, bool bTailFront )
// create the caption drawing object
Rectangle aTextRect( Point( 0 , 0 ), Size( SC_NOTECAPTION_WIDTH, SC_NOTECAPTION_HEIGHT ) );
Point aTailPos = CalcTailPos( bTailFront );
- mpCaption = new SdrCaptionObj( aTextRect, aTailPos );
+ mxCaption.reset( new SdrCaptionObj( aTextRect, aTailPos ));
// basic caption settings
- ScCaptionUtil::SetBasicCaptionSettings( *mpCaption, bShown );
+ ScCaptionUtil::SetBasicCaptionSettings( *mxCaption, bShown );
}
void ScCaptionCreator::Initialize()
@@ -402,7 +400,7 @@ public:
/** Create a new caption object and inserts it into the document. */
explicit ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, ScNoteData& rNoteData );
/** Manipulate an existing caption. */
- explicit ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, SdrCaptionObj& rCaption, bool bShown );
+ explicit ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, ScCaptionPtr& xCaption, bool bShown );
};
ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, ScNoteData& rNoteData ) :
@@ -414,37 +412,257 @@ ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& r
{
// create the caption drawing object
CreateCaption( rNoteData.mbShown, false );
- rNoteData.mpCaption = GetCaption();
- OSL_ENSURE( rNoteData.mpCaption, "ScNoteCaptionCreator::ScNoteCaptionCreator - missing caption object" );
- if( rNoteData.mpCaption )
+ rNoteData.mxCaption = GetCaption();
+ OSL_ENSURE( rNoteData.mxCaption, "ScNoteCaptionCreator::ScNoteCaptionCreator - missing caption object" );
+ if( rNoteData.mxCaption )
{
// store note position in user data of caption object
- ScCaptionUtil::SetCaptionUserData( *rNoteData.mpCaption, rPos );
+ ScCaptionUtil::SetCaptionUserData( *rNoteData.mxCaption, rPos );
// insert object into draw page
- pDrawPage->InsertObject( rNoteData.mpCaption );
+ pDrawPage->InsertObject( rNoteData.mxCaption.get() );
}
}
}
-ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, SdrCaptionObj& rCaption, bool bShown ) :
- ScCaptionCreator( rDoc, rPos, rCaption )
+ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& rPos, ScCaptionPtr& xCaption, bool bShown ) :
+ ScCaptionCreator( rDoc, rPos, xCaption )
{
SdrPage* pDrawPage = GetDrawPage();
OSL_ENSURE( pDrawPage, "ScNoteCaptionCreator::ScNoteCaptionCreator - no drawing page" );
- OSL_ENSURE( rCaption.GetPage() == pDrawPage, "ScNoteCaptionCreator::ScNoteCaptionCreator - wrong drawing page in caption" );
- if( pDrawPage && (rCaption.GetPage() == pDrawPage) )
+ OSL_ENSURE( xCaption->GetPage() == pDrawPage, "ScNoteCaptionCreator::ScNoteCaptionCreator - wrong drawing page in caption" );
+ if( pDrawPage && (xCaption->GetPage() == pDrawPage) )
{
// store note position in user data of caption object
- ScCaptionUtil::SetCaptionUserData( rCaption, rPos );
+ ScCaptionUtil::SetCaptionUserData( *xCaption, rPos );
// basic caption settings
- ScCaptionUtil::SetBasicCaptionSettings( rCaption, bShown );
+ ScCaptionUtil::SetBasicCaptionSettings( *xCaption, bShown );
// set correct tail position
- rCaption.SetTailPos( CalcTailPos( false ) );
+ xCaption->SetTailPos( CalcTailPos( false ) );
}
}
} // namespace
+
+ScCaptionPtr::ScCaptionPtr() :
+ mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr), mnRefs(0)
+{
+}
+
+ScCaptionPtr::ScCaptionPtr( SdrCaptionObj* p ) :
+ mpHead( p ? this : nullptr ), mpNext(nullptr), mpCaption(p), mnRefs( p ? 1 : 0 )
+{
+}
+
+ScCaptionPtr::ScCaptionPtr( const ScCaptionPtr& r ) :
+ mpHead(nullptr), mpNext(nullptr), mpCaption(nullptr), mnRefs(0)
+{
+ assign( r, false);
+}
+
+void ScCaptionPtr::assign( const ScCaptionPtr& r, bool bAssignment )
+{
+ if (mpCaption == r.mpCaption)
+ return;
+
+ // Let's find some weird usage.
+ // Though assigning some other of the same list to head may syntactically
+ // be valid, why would we do that?
+ assert(r.mpHead != this);
+ // Same for assigning head to some other.
+ assert(mpHead != &r);
+ // And any other of the same list.
+ assert(mpHead != r.mpHead);
+
+ r.incRef();
+ if (bAssignment)
+ {
+ decRefAndDestroy();
+ removeFromList();
+ }
+ mpCaption = r.mpCaption;
+ // That head is this' master.
+ mpHead = r.mpHead;
+ mnRefs = 0;
+ // Insert into list.
+ mpNext = r.mpNext;
+ r.mpNext = this;
+}
+
+void ScCaptionPtr::removeFromList()
+{
+ if (!mpHead && !mpNext && !mpCaption && !mnRefs)
+ return;
+
+ ScCaptionPtr* pNewHead = (mpHead == this ? mpNext : nullptr);
+ ScCaptionPtr* pThat = (mpHead == this ? mpNext : mpHead); // do not replace on self if head
+ while (pThat && pThat->mpNext != this)
+ {
+ // Use the walk to check consistency on the fly.
+ assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
+ assert(pThat->mnRefs == 0 || pThat == mpHead);
+ if (pNewHead)
+ {
+ assert(pThat->mpHead == mpHead);
+ pThat->mpHead = pNewHead;
+ }
+ pThat = pThat->mpNext;
+ }
+ assert(pThat || mpHead == this); // not found only if this was head
+ if (pThat)
+ {
+ pThat->mpNext = mpNext;
+ if (pNewHead)
+ {
+ do
+ {
+ assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
+ assert(pThat->mnRefs == 0 || pThat == mpHead);
+ assert(pThat->mpHead == mpHead);
+ pThat->mpHead = pNewHead;
+ }
+ while ((pThat = pThat->mpNext) != nullptr);
+ }
+ else
+ {
+#if OSL_DEBUG_LEVEL > 0
+ // XXX exactly as for the if() branch but without replacing head.
+ do
+ {
+ assert(pThat->mpCaption == nullptr || pThat->mpCaption == mpCaption);
+ assert(pThat->mnRefs == 0 || pThat == mpHead);
+ assert(pThat->mpHead == mpHead);
+ }
+ while ((pThat = pThat->mpNext) != nullptr);
+#endif
+ }
+ }
+ if (pNewHead)
... etc. - the rest is truncated
More information about the Libreoffice-commits
mailing list