ref-count SfxItemPool

so we can remove SfxItemPoolUser, which is a right
performance hog when we have large calc spreadsheets

Change-Id: I344002f536f6eead5cf98c6647dd1667fd9c8874
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115247
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

7 files changed, 28 insertions, 61 deletions

diff --git a/svl/CppunitTest_svl_itempool.mk b/svl/CppunitTest_svl_itempool.mk
index 63745dd4312a..892107e906ca 100644
--- a/svl/CppunitTest_svl_itempool.mk
+++ b/svl/CppunitTest_svl_itempool.mk
@@ -21,6 +21,7 @@ $(eval $(call gb_CppunitTest_use_libraries,svl_itempool, \
 	svl \
 	comphelper \
 	sal \
+	salhelper \
 	cppu \
 	cppuhelper \
 ))
diff --git a/svl/CppunitTest_svl_items.mk b/svl/CppunitTest_svl_items.mk
index 28e8772d71ad..60bccd8a5852 100644
--- a/svl/CppunitTest_svl_items.mk
+++ b/svl/CppunitTest_svl_items.mk
@@ -22,6 +22,7 @@ $(eval $(call gb_CppunitTest_use_libraries,svl_items, \
 	svl \
 	comphelper \
 	sal \
+	salhelper \
 	cppu \
 	cppuhelper \
 ))
diff --git a/svl/Library_svl.mk b/svl/Library_svl.mk
index 19b9d9f8b940..a4e41b6f9984 100644
--- a/svl/Library_svl.mk
+++ b/svl/Library_svl.mk
@@ -73,6 +73,7 @@ $(eval $(call gb_Library_use_libraries,svl,\
     $(if $(ENABLE_JAVA), \
         jvmfwk) \
     sal \
+    salhelper \
     sot \
     tl \
     ucbhelper \
diff --git a/svl/qa/unit/items/stylepool.cxx b/svl/qa/unit/items/stylepool.cxx
index c334360f5e3f..5fb94f29f9cd 100644
--- a/svl/qa/unit/items/stylepool.cxx
+++ b/svl/qa/unit/items/stylepool.cxx
@@ -28,7 +28,7 @@ CPPUNIT_TEST_FIXTURE(StylePoolTest, testIterationOrder)
     std::vector<SfxPoolItem*> aDefaults{ &aDefault1 };
     SfxItemInfo const aItems[] = { { 1, false } };
 
-    SfxItemPool* pPool = new SfxItemPool("test", 1, 1, aItems);
+    rtl::Reference<SfxItemPool> pPool = new SfxItemPool("test", 1, 1, aItems);
     pPool->SetDefaults(&aDefaults);
     {
         // Set up parents in mixed order to make sure we do not sort by pointer address.
@@ -76,7 +76,6 @@ CPPUNIT_TEST_FIXTURE(StylePoolTest, testIterationOrder)
         CPPUNIT_ASSERT_EQUAL(OUString("Item3"), pItem3->GetValue());
         CPPUNIT_ASSERT(!pIter->getNext());
     }
-    SfxItemPool::Free(pPool);
 }
 }
 
diff --git a/svl/qa/unit/items/test_itempool.cxx b/svl/qa/unit/items/test_itempool.cxx
index fc19eec64752..4d15edaffd86 100644
--- a/svl/qa/unit/items/test_itempool.cxx
+++ b/svl/qa/unit/items/test_itempool.cxx
@@ -40,8 +40,8 @@ void PoolItemTest::testPool()
           { 4, false /* not poolable */}
         };
 
-    SfxItemPool *pPool = new SfxItemPool("testpool", 1, 4, aItems);
-    SfxItemPool_Impl *pImpl = SfxItemPool_Impl::GetImpl(pPool);
+    rtl::Reference<SfxItemPool> pPool = new SfxItemPool("testpool", 1, 4, aItems);
+    SfxItemPool_Impl *pImpl = SfxItemPool_Impl::GetImpl(pPool.get());
     CPPUNIT_ASSERT(pImpl != nullptr);
     CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), pImpl->maPoolItemArrays.size());
 
@@ -91,7 +91,6 @@ void PoolItemTest::testPool()
     pPool->Put(aNotherFour);
     CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), pImpl->maPoolItemArrays[3].size());
 
-    SfxItemPool::Free(pPool);
 }
 
 
diff --git a/svl/source/inc/poolio.hxx b/svl/source/inc/poolio.hxx
index d5da51bc2d99..3ec1be562a01 100644
--- a/svl/source/inc/poolio.hxx
+++ b/svl/source/inc/poolio.hxx
@@ -20,6 +20,7 @@
 #ifndef INCLUDED_SVL_SOURCE_INC_POOLIO_HXX
 #define INCLUDED_SVL_SOURCE_INC_POOLIO_HXX
 
+#include <rtl/ref.hxx>
 #include <svl/itempool.hxx>
 #include <svl/SfxBroadcaster.hxx>
 #include <tools/debug.hxx>
@@ -151,12 +152,11 @@ struct SfxItemPool_Impl
 {
     SfxBroadcaster                  aBC;
     std::vector<SfxPoolItemArray_Impl> maPoolItemArrays;
-    std::vector<SfxItemPoolUser*>   maSfxItemPoolUsers; /// ObjectUser section
     OUString                        aName;
     std::vector<SfxPoolItem*>       maPoolDefaults;
     std::vector<SfxPoolItem*>*      mpStaticDefaults;
     SfxItemPool*                    mpMaster;
-    SfxItemPool*                    mpSecondary;
+    rtl::Reference<SfxItemPool>     mpSecondary;
     std::unique_ptr<sal_uInt16[]>   mpPoolRanges;
     sal_uInt16                      mnStart;
     sal_uInt16                      mnEnd;
@@ -168,7 +168,6 @@ struct SfxItemPool_Impl
         , maPoolDefaults(nEnd - nStart + 1)
         , mpStaticDefaults(nullptr)
         , mpMaster(pMaster)
-        , mpSecondary(nullptr)
         , mnStart(nStart)
         , mnEnd(nEnd)
         , eDefMetric(MapUnit::MapCM)
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index 24fda80d1bfe..20d0dcaca68e 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -73,7 +73,7 @@ lcl_CheckSlots2(std::map<sal_uInt16, sal_uInt16> & rSlotMap,
 #define CHECK_SLOTS() \
 do { \
     std::map<sal_uInt16, sal_uInt16> slotmap; \
-    for (SfxItemPool * p = pImpl->mpMaster; p; p = p->pImpl->mpSecondary) \
+    for (SfxItemPool * p = pImpl->mpMaster; p; p = p->pImpl->mpSecondary.get()) \
     { \
         lcl_CheckSlots2(slotmap, *p, p->pItemInfos); \
     } \
@@ -84,24 +84,6 @@ do { \
 #endif
 
 
-void SfxItemPool::AddSfxItemPoolUser(SfxItemPoolUser& rNewUser)
-{
-    // maintain sorted to reduce cost of remove
-    const auto insertIt = ::std::lower_bound(
-        pImpl->maSfxItemPoolUsers.begin(), pImpl->maSfxItemPoolUsers.end(), &rNewUser);
-    pImpl->maSfxItemPoolUsers.insert(insertIt, &rNewUser);
-}
-
-void SfxItemPool::RemoveSfxItemPoolUser(SfxItemPoolUser& rOldUser)
-{
-    const auto aFindResult = ::std::lower_bound(
-        pImpl->maSfxItemPoolUsers.begin(), pImpl->maSfxItemPoolUsers.end(), &rOldUser);
-    if(aFindResult != pImpl->maSfxItemPoolUsers.end() && *aFindResult == &rOldUser)
-    {
-        pImpl->maSfxItemPoolUsers.erase(aFindResult);
-    }
-}
-
 const SfxPoolItem* SfxItemPool::GetPoolDefaultItem( sal_uInt16 nWhich ) const
 {
     const SfxPoolItem* pRet;
@@ -126,7 +108,7 @@ bool SfxItemPool::IsItemPoolable_Impl( sal_uInt16 nPos ) const
 
 bool SfxItemPool::IsItemPoolable( sal_uInt16 nWhich ) const
 {
-    for ( const SfxItemPool *pPool = this; pPool; pPool = pPool->pImpl->mpSecondary )
+    for ( const SfxItemPool *pPool = this; pPool; pPool = pPool->pImpl->mpSecondary.get() )
     {
         if ( pPool->IsInRange(nWhich) )
             return pPool->IsItemPoolable_Impl( pPool->GetIndex_Impl(nWhich));
@@ -199,6 +181,7 @@ SfxItemPool::SfxItemPool
                                                     false
                                                     Take over static Defaults */
 ) :
+    salhelper::SimpleReferenceObject(),
     pItemInfos(rPool.pItemInfos),
     pImpl( new SfxItemPool_Impl( this, rPool.pImpl->aName, rPool.pImpl->mnStart, rPool.pImpl->mnEnd ) )
 {
@@ -229,7 +212,7 @@ SfxItemPool::SfxItemPool
 
     // Repair linkage
     if ( rPool.pImpl->mpSecondary )
-        SetSecondaryPool( rPool.pImpl->mpSecondary->Clone() );
+        SetSecondaryPool( rPool.pImpl->mpSecondary->Clone().get() );
 }
 
 void SfxItemPool::SetDefaults( std::vector<SfxPoolItem*>* pDefaults )
@@ -345,28 +328,6 @@ SfxItemPool::~SfxItemPool()
     }
 }
 
-void SfxItemPool::Free(SfxItemPool* pPool)
-{
-    if(!pPool)
-        return;
-
-    // tell all the registered SfxItemPoolUsers that the pool is in destruction
-    std::vector<SfxItemPoolUser*> aListCopy(pPool->pImpl->maSfxItemPoolUsers);
-    for(SfxItemPoolUser* pSfxItemPoolUser : aListCopy)
-    {
-        DBG_ASSERT(pSfxItemPoolUser, "corrupt SfxItemPoolUser list (!)");
-        pSfxItemPoolUser->ObjectInDestruction(*pPool);
-    }
-
-    // Clear the vector. This means that user do not need to call RemoveSfxItemPoolUser()
-    // when they get called from ObjectInDestruction().
-    pPool->pImpl->maSfxItemPoolUsers.clear();
-
-    // delete pool
-    delete pPool;
-}
-
-
 void SfxItemPool::SetSecondaryPool( SfxItemPool *pPool )
 {
     // Reset Master in attached Pools
@@ -398,15 +359,15 @@ void SfxItemPool::SetSecondaryPool( SfxItemPool *pPool )
         }
 #endif
 
-        pImpl->mpSecondary->pImpl->mpMaster = pImpl->mpSecondary;
-        for ( SfxItemPool *p = pImpl->mpSecondary->pImpl->mpSecondary; p; p = p->pImpl->mpSecondary )
-            p->pImpl->mpMaster = pImpl->mpSecondary;
+        pImpl->mpSecondary->pImpl->mpMaster = pImpl->mpSecondary.get();
+        for ( SfxItemPool *p = pImpl->mpSecondary->pImpl->mpSecondary.get(); p; p = p->pImpl->mpSecondary.get() )
+            p->pImpl->mpMaster = pImpl->mpSecondary.get();
     }
 
     // Set Master of new Secondary Pools
     DBG_ASSERT( !pPool || pPool->pImpl->mpMaster == pPool, "Secondary is present in two Pools" );
     SfxItemPool *pNewMaster = GetMasterPool() ? pImpl->mpMaster : this;
-    for ( SfxItemPool *p = pPool; p; p = p->pImpl->mpSecondary )
+    for ( SfxItemPool *p = pPool; p; p = p->pImpl->mpSecondary.get() )
         p->pImpl->mpMaster = pNewMaster;
 
     // Remember new Secondary Pool
@@ -430,9 +391,15 @@ MapUnit SfxItemPool::GetMetric( sal_uInt16 ) const
 
 void SfxItemPool::SetDefaultMetric( MapUnit eNewMetric )
 {
+//    assert((pImpl->eDefMetric == eNewMetric || !pImpl->mpPoolRanges) && "pool already frozen, cannot change metric");
     pImpl->eDefMetric = eNewMetric;
 }
 
+MapUnit SfxItemPool::GetDefaultMetric() const
+{
+    return pImpl->eDefMetric;
+}
+
 const OUString& SfxItemPool::GetName() const
 {
     return pImpl->aName;
@@ -452,10 +419,9 @@ bool SfxItemPool::GetPresentation
 }
 
 
-SfxItemPool* SfxItemPool::Clone() const
+rtl::Reference<SfxItemPool> SfxItemPool::Clone() const
 {
-    SfxItemPool *pPool = new SfxItemPool( *this );
-    return pPool;
+    return new SfxItemPool( *this );
 }
 
 
@@ -770,7 +736,7 @@ const SfxPoolItem& SfxItemPool::GetDefaultItem( sal_uInt16 nWhich ) const
 
 SfxItemPool* SfxItemPool::GetSecondaryPool() const
 {
-    return pImpl->mpSecondary;
+    return pImpl->mpSecondary.get();
 }
 
 /* get the last pool by following the GetSecondaryPool chain */
@@ -796,6 +762,7 @@ SfxItemPool* SfxItemPool::GetMasterPool() const
  */
 void SfxItemPool::FreezeIdRanges()
 {
+    assert(!pImpl->mpPoolRanges && "pool already frozen, cannot freeze twice");
     FillItemIdRanges_Impl( pImpl->mpPoolRanges );
 }
 
@@ -806,13 +773,13 @@ void SfxItemPool::FillItemIdRanges_Impl( std::unique_ptr<sal_uInt16[]>& pWhichRa
 
     const SfxItemPool *pPool;
     sal_uInt16 nLevel = 0;
-    for( pPool = this; pPool; pPool = pPool->pImpl->mpSecondary )
+    for( pPool = this; pPool; pPool = pPool->pImpl->mpSecondary.get() )
         ++nLevel;
 
     pWhichRanges.reset(new sal_uInt16[ 2*nLevel + 1 ]);
 
     nLevel = 0;
-    for( pPool = this; pPool; pPool = pPool->pImpl->mpSecondary )
+    for( pPool = this; pPool; pPool = pPool->pImpl->mpSecondary.get() )
     {
         pWhichRanges[nLevel++] = pPool->pImpl->mnStart;
         pWhichRanges[nLevel++] = pPool->pImpl->mnEnd;