avoid copying when placing items into SfxItemSet

Change-Id: I05c627f590e7794c1ba11b66021dc30aa3285eb0
Reviewed-on: https://gerrit.libreoffice.org/71941
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 28 insertions, 11 deletions

diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index c18e0405667f..bed2259d40ea 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -443,10 +443,13 @@ bool SfxItemSet::HasItem(sal_uInt16 nWhich, const SfxPoolItem** ppItem) const
     return bRet;
 }
 
-const SfxPoolItem* SfxItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich )
+const SfxPoolItem* SfxItemSet::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWhich, bool bPassingOwnership )
 {
     if ( !nWhich )
+    {
+        assert(!bPassingOwnership);
         return nullptr; //FIXME: Only because of Outliner bug
+    }
 
     SfxPoolItem const** ppFnd = m_pItems.get();
     const sal_uInt16* pPtr = m_pWhichRanges;
@@ -460,13 +463,16 @@ const SfxPoolItem* SfxItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich
             {
                 // Same Item already present?
                 if ( *ppFnd == &rItem )
+                {
+                    assert(!bPassingOwnership);
                     return nullptr;
+                }
 
                 // Will 'dontcare' or 'disabled' be overwritten with some real value?
                 if ( rItem.Which() && ( IsInvalidItem(*ppFnd) || !(*ppFnd)->Which() ) )
                 {
                     auto const old = *ppFnd;
-                    *ppFnd = &m_pPool->Put( rItem, nWhich );
+                    *ppFnd = &m_pPool->PutImpl( rItem, nWhich, bPassingOwnership );
                     if (!IsInvalidItem(old)) {
                         assert(old->Which() == 0);
                         delete old;
@@ -480,16 +486,22 @@ const SfxPoolItem* SfxItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich
                     if (IsInvalidItem(*ppFnd) || (*ppFnd)->Which() != 0) {
                         *ppFnd = rItem.Clone(m_pPool);
                     }
+                    if (bPassingOwnership)
+                        delete &rItem;
                     return nullptr;
                 }
                 else
                 {
                     // Same value already present?
                     if ( rItem == **ppFnd )
+                    {
+                        if (bPassingOwnership)
+                            delete &rItem;
                         return nullptr;
+                    }
 
                     // Add the new one, remove the old one
-                    const SfxPoolItem& rNew = m_pPool->Put( rItem, nWhich );
+                    const SfxPoolItem& rNew = m_pPool->PutImpl( rItem, nWhich, bPassingOwnership );
                     const SfxPoolItem* pOld = *ppFnd;
                     *ppFnd = &rNew;
                     if (SfxItemPool::IsWhich(nWhich))
@@ -501,9 +513,14 @@ const SfxPoolItem* SfxItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich
             {
                 ++m_nCount;
                 if( !rItem.Which() )
+                {
                     *ppFnd = rItem.Clone(m_pPool);
-                else {
-                    const SfxPoolItem& rNew = m_pPool->Put( rItem, nWhich );
+                    if (bPassingOwnership)
+                        delete &rItem;
+                }
+                else
+                {
+                    const SfxPoolItem& rNew = m_pPool->PutImpl( rItem, nWhich, bPassingOwnership );
                     *ppFnd = &rNew;
                     if (SfxItemPool::IsWhich(nWhich))
                     {
@@ -514,7 +531,7 @@ const SfxPoolItem* SfxItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich
                     }
                 }
             }
-            SAL_WARN_IF(m_pPool->IsItemPoolable(nWhich) &&
+            SAL_WARN_IF(!bPassingOwnership && m_pPool->IsItemPoolable(nWhich) &&
                         dynamic_cast<const SfxSetItem*>( &rItem ) == nullptr &&
                         **ppFnd != rItem,
                         "svl.items", "putted Item unequal, with ID/pos " << nWhich );
@@ -523,6 +540,8 @@ const SfxPoolItem* SfxItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich
         ppFnd += *(pPtr+1) - *pPtr + 1;
         pPtr += 2;
     }
+    if (bPassingOwnership)
+        delete &rItem;
     return nullptr;
 }
 
@@ -1582,7 +1601,7 @@ static void AddItem_Impl(std::unique_ptr<SfxPoolItem const*[]> & rpItems, sal_uI
 /**
  * Putting with automatic extension of the WhichId with the ID of the Item.
  */
-const SfxPoolItem* SfxAllItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich )
+const SfxPoolItem* SfxAllItemSet::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWhich, bool bPassingOwnership )
 {
     sal_uInt16 nPos = 0; // Position for 'rItem' in 'm_pItems'
     const sal_uInt16 nItemCount = TotalCount();
@@ -1668,7 +1687,7 @@ const SfxPoolItem* SfxAllItemSet::Put( const SfxPoolItem& rItem, sal_uInt16 nWhi
     }
 
     // Add new Item to Pool
-    const SfxPoolItem& rNew = m_pPool->Put( rItem, nWhich );
+    const SfxPoolItem& rNew = m_pPool->PutImpl( rItem, nWhich, bPassingOwnership );
 
     // Remember old Item
     bool bIncrementCount = false;
diff --git a/svl/source/items/poolcach.cxx b/svl/source/items/poolcach.cxx
index 01f238caa3fb..05401669216c 100644
--- a/svl/source/items/poolcach.cxx
+++ b/svl/source/items/poolcach.cxx
@@ -88,9 +88,7 @@ const SfxSetItem& SfxItemPoolCache::ApplyTo( const SfxSetItem &rOrigItem )
     }
     else
         pNewItem->GetItemSet().Put( *pSetToPut );
-    const SfxSetItem* pNewPoolItem = &pPool->Put( *pNewItem );
-    DBG_ASSERT( pNewPoolItem != pNewItem.get(), "Pool: same in and out?" );
-    pNewItem.reset();
+    const SfxSetItem* pNewPoolItem = &pPool->Put( std::move(pNewItem) );
 
     // Adapt refcount; one each for the cache
     pNewPoolItem->AddRef( pNewPoolItem != &rOrigItem ? 2 : 1 );