[Libreoffice] [PATCH] removing mpNotes field from ScBaseCell

[Noel Grandin]

Hi

So I've been working on this patch to move mpNotes from ScBaseCell to
ScTable, and I got it mostly working.
But I was struggling with leaks and getting the lifecycle of the notes
to exactly match the lifecycle of the ScBaseCell object.

So then I had a weird idea - how about if I tied the lifecycles
together implicitly?
So I came up with this patch.

It compiles, and passes a make check.

I'll do a memcheck run on Monday when I can get access to my other
machine with tons of memory.

Regards, Noel Grandin
-------------- next part --------------
diff --git a/sc/inc/cell.hxx b/sc/inc/cell.hxx
old mode 100644
new mode 100755
index fa1b719..1755591
--- a/sc/inc/cell.hxx
+++ b/sc/inc/cell.hxx
@@ -122,11 +122,11 @@ public:
     inline void     SetScriptType( sal_uInt8 nNew ) { nScriptType = nNew; }
 
     /** Returns true, if the cell contains a note. */
-    inline bool     HasNote() const { return mpNote != 0; }
+    bool     		HasNote() const;
     /** Returns the pointer to a cell note object (read-only). */
-    inline const ScPostIt* GetNote() const { return mpNote; }
+    const ScPostIt* GetNote() const;
     /** Returns the pointer to a cell note object. */
-    inline ScPostIt* GetNote() { return mpNote; }
+    ScPostIt* 		GetNote();
     /** Takes ownership of the passed cell note object. */
     void            TakeNote( ScPostIt* pNote );
     /** Returns and forgets the own cell note object. Caller takes ownership! */
@@ -169,7 +169,6 @@ private:
     ScBaseCell&     operator=( const ScBaseCell& );
 
 private:
-    ScPostIt*       mpNote;         /// The cell note. Cell takes ownership!
     SvtBroadcaster* mpBroadcaster;  /// Broadcaster for changed values. Cell takes ownership!
 
 protected:
diff --git a/sc/source/core/data/cell.cxx b/sc/source/core/data/cell.cxx
old mode 100644
new mode 100755
index 6cc8209..04aae8c
--- a/sc/source/core/data/cell.cxx
+++ b/sc/source/core/data/cell.cxx
@@ -82,10 +82,16 @@ IMPL_FIXEDMEMPOOL_NEWDEL( ScStringCell,  nMemPoolStringCell, nMemPoolStringCell
 IMPL_FIXEDMEMPOOL_NEWDEL( ScNoteCell,    nMemPoolNoteCell, nMemPoolNoteCell )
 #endif
 
+/**
+ * The notes map. Because only a handful of notes typically exist in an application, but there can be millions
+ * of cells, we don't want to store this on the cell itself
+ */
+typedef ::std::map<const ScBaseCell*, ScPostIt*> ScNotesMap;
+static ScNotesMap notesMap;
+
 // ============================================================================
 
 ScBaseCell::ScBaseCell( CellType eNewType ) :
-    mpNote( 0 ),
     mpBroadcaster( 0 ),
     nTextWidth( TEXTWIDTH_DIRTY ),
     eCellType( sal::static_int_cast<sal_uInt8>(eNewType) ),
@@ -94,7 +100,6 @@ ScBaseCell::ScBaseCell( CellType eNewType ) :
 }
 
 ScBaseCell::ScBaseCell( const ScBaseCell& rCell ) :
-    mpNote( 0 ),
     mpBroadcaster( 0 ),
     nTextWidth( rCell.nTextWidth ),
     eCellType( rCell.eCellType ),
@@ -104,7 +109,12 @@ ScBaseCell::ScBaseCell( const ScBaseCell& rCell ) :
 
 ScBaseCell::~ScBaseCell()
 {
-    delete mpNote;
+	ScNotesMap::iterator it = notesMap.find(this);
+	if (it != notesMap.end()) {
+		ScPostIt *pNote = it->second;
+		notesMap.erase(this);
+		delete pNote;
+	}
     delete mpBroadcaster;
     OSL_ENSURE( eCellType == CELLTYPE_DESTROYED, "BaseCell Destructor" );
 }
@@ -244,12 +254,14 @@ ScBaseCell* ScBaseCell::CloneWithoutNote( ScDocument& rDestDoc, const ScAddress&
 ScBaseCell* ScBaseCell::CloneWithNote( const ScAddress& rOwnPos, ScDocument& rDestDoc, const ScAddress& rDestPos, int nCloneFlags ) const
 {
     ScBaseCell* pNewCell = lclCloneCell( *this, rDestDoc, rDestPos, nCloneFlags );
-    if( mpNote )
+	ScNotesMap::iterator it = notesMap.find(this);
+	if (it != notesMap.end())
     {
         if( !pNewCell )
             pNewCell = new ScNoteCell;
         bool bCloneCaption = (nCloneFlags & SC_CLONECELL_NOCAPTION) == 0;
-        pNewCell->TakeNote( mpNote->Clone( rOwnPos, rDestDoc, rDestPos, bCloneCaption ) );
+        ScPostIt* pNote = it->second;
+        pNewCell->TakeNote( pNote->Clone( rOwnPos, rDestDoc, rDestPos, bCloneCaption ) );
     }
     return pNewCell;
 }
@@ -282,27 +294,63 @@ void ScBaseCell::Delete()
 
 bool ScBaseCell::IsBlank( bool bIgnoreNotes ) const
 {
-    return (eCellType == CELLTYPE_NOTE) && (bIgnoreNotes || !mpNote);
+    return (eCellType == CELLTYPE_NOTE) && (bIgnoreNotes || notesMap.find(this) == notesMap.end());
+}
+
+/** Returns true, if the cell contains a note. */
+bool ScBaseCell::HasNote() const
+{ 
+	return notesMap.find(this) != notesMap.end();
+}
+
+/** Returns the pointer to a cell note object. */
+ScPostIt* ScBaseCell::GetNote()
+{ 
+	ScNotesMap::iterator it = notesMap.find(this);
+	return it == notesMap.end() ? 0 : it->second;
+}
+    
+/** Returns the pointer to a cell note object (read-only). */
+const ScPostIt* ScBaseCell::GetNote() const
+{
+	ScNotesMap::iterator it = notesMap.find(this);
+	return it == notesMap.end() ? 0 : it->second;
 }
 
 void ScBaseCell::TakeNote( ScPostIt* pNote )
 {
-    delete mpNote;
-    mpNote = pNote;
+	ScNotesMap::iterator it = notesMap.find(this);
+	if (it != notesMap.end())
+    {
+		delete it->second;
+	}
+	notesMap[this] = pNote;
 }
 
 ScPostIt* ScBaseCell::ReleaseNote()
 {
-    ScPostIt* pNote = mpNote;
-    mpNote = 0;
-    return pNote;
+	ScNotesMap::iterator it = notesMap.find(this);
+	if (it != notesMap.end())
+    {
+		ScPostIt* pNote = it->second;
+		notesMap.erase(it);
+		return pNote;
+	}
+	else
+		return 0;
 }
 
 void ScBaseCell::DeleteNote()
 {
-    DELETEZ( mpNote );
+	ScNotesMap::iterator it = notesMap.find(this);
+	if (it != notesMap.end())
+    {
+		ScPostIt* pNote = it->second;
+		notesMap.erase(it);
+		delete pNote;
+	}
 }
-
+		
 void ScBaseCell::TakeBroadcaster( SvtBroadcaster* pBroadcaster )
 {
     delete mpBroadcaster;