ITEM: Use SfxPoolItemHolder in SvxSearchDialog

I replaced that SfxPoolItem* in SearchAttrInfo with
a much safer SfxPoolItemHolder and adapted and
simplified used code - it does not need to take
care of Item lifetime/Cloning itself. That works
well.
I did not find out why that 'invalid' state is used
in the SrchAttrInfoList, but seems to be needed.
Thus I keep this for now as it is (it can be all
expressed/used using SfxPoolItemHolder, too).

Change-Id: I4b769f43128cb2e25153919f7652b2f032393123
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165167
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>

3 files changed, 33 insertions, 56 deletions

diff --git a/cui/source/dialogs/srchxtra.cxx b/cui/source/dialogs/srchxtra.cxx
index 4c672b9839f2..f8080f537775 100644
--- a/cui/source/dialogs/srchxtra.cxx
+++ b/cui/source/dialogs/srchxtra.cxx
@@ -135,7 +135,7 @@ SvxSearchAttributeDialog::SvxSearchAttributeDialog(weld::Window* pParent,
                     if ( nSlot == rList[i].nSlot )
                     {
                         bFound = true;
-                        if ( IsInvalidItem( rList[i].pItemPtr ) )
+                        if ( IsInvalidItem( rList[i].aItemPtr.getItem() ) )
                             bChecked = true;
                     }
                 }
@@ -167,13 +167,13 @@ SvxSearchAttributeDialog::~SvxSearchAttributeDialog()
 
 IMPL_LINK_NOARG(SvxSearchAttributeDialog, OKHdl, weld::Button&, void)
 {
-    SearchAttrInfo aInvalidItem;
-    aInvalidItem.pItemPtr = INVALID_POOL_ITEM;
+    DBG_ASSERT( SfxObjectShell::Current(), "No DocShell" );
+    SfxItemPool& rPool(SfxObjectShell::Current()->GetPool());
 
     for (int i = 0, nCount = m_xAttrLB->n_children(); i < nCount; ++i)
     {
-        sal_uInt16 nSlot = m_xAttrLB->get_id(i).toUInt32();
-        bool bChecked = m_xAttrLB->get_toggle(i) == TRISTATE_TRUE;
+        const sal_uInt16 nSlot(m_xAttrLB->get_id(i).toUInt32());
+        const bool bChecked(TRISTATE_TRUE == m_xAttrLB->get_toggle(i));
 
         sal_uInt16 j;
         for ( j = rList.Count(); j; )
@@ -182,13 +182,9 @@ IMPL_LINK_NOARG(SvxSearchAttributeDialog, OKHdl, weld::Button&, void)
             if( rItem.nSlot == nSlot )
             {
                 if( bChecked )
-                {
-                    if( !IsInvalidItem( rItem.pItemPtr ) )
-                        delete rItem.pItemPtr;
-                    rItem.pItemPtr = INVALID_POOL_ITEM;
-                }
-                else if( IsInvalidItem( rItem.pItemPtr ) )
-                    rItem.pItemPtr = nullptr;
+                    rItem.aItemPtr = SfxPoolItemHolder(rPool, INVALID_POOL_ITEM);
+                else if( IsInvalidItem( rItem.aItemPtr.getItem() ) )
+                    rItem.aItemPtr = SfxPoolItemHolder();
                 j = 1;
                 break;
             }
@@ -196,14 +192,13 @@ IMPL_LINK_NOARG(SvxSearchAttributeDialog, OKHdl, weld::Button&, void)
 
         if ( !j && bChecked )
         {
-            aInvalidItem.nSlot = nSlot;
-            rList.Insert( aInvalidItem );
+            rList.Insert( { nSlot, SfxPoolItemHolder(rPool, INVALID_POOL_ITEM) });
         }
     }
 
     // remove invalid items (pItem == NULL)
     for ( sal_uInt16 n = rList.Count(); n; )
-        if ( !rList[ --n ].pItemPtr )
+        if ( !rList[ --n ].aItemPtr.getItem() )
             rList.Remove( n );
 
     m_xDialog->response(RET_OK);
diff --git a/include/svx/srchdlg.hxx b/include/svx/srchdlg.hxx
index 7223a51cd324..1b9a0f067f93 100644
--- a/include/svx/srchdlg.hxx
+++ b/include/svx/srchdlg.hxx
@@ -24,6 +24,7 @@
 #include <svl/poolitem.hxx>
 #include <svl/srchdefs.hxx>
 #include <svl/srchitem.hxx>
+#include <svl/itemset.hxx>
 #include <svx/svxdllapi.h>
 #include <vcl/timer.hxx>
 #include <memory>
@@ -40,8 +41,16 @@ enum class TransliterationFlags;
 
 struct SearchAttrInfo
 {
+    SearchAttrInfo()
+    : nSlot(0)
+    , aItemPtr() {}
+
+    SearchAttrInfo(sal_uInt16 Slot, const SfxPoolItemHolder& ItemPtr)
+    : nSlot(Slot)
+    , aItemPtr(ItemPtr) {}
+
     sal_uInt16          nSlot;
-    const SfxPoolItem*  pItemPtr;
+    SfxPoolItemHolder   aItemPtr;
 };
 
 typedef std::vector<SearchAttrInfo> SrchAttrInfoList;
@@ -51,7 +60,7 @@ class SVX_DLLPUBLIC SearchAttrItemList : private SrchAttrInfoList
 public:
     SearchAttrItemList() {}
     SearchAttrItemList( const SearchAttrItemList& rList );
-    SearchAttrItemList( SearchAttrItemList&& rList );
+    SearchAttrItemList( SearchAttrItemList&& rList ) noexcept;
     ~SearchAttrItemList();
 
     void            Put( const SfxItemSet& rSet );
diff --git a/svx/source/dialog/srchdlg.cxx b/svx/source/dialog/srchdlg.cxx
index dbd55651fbc3..93d593d8729f 100644
--- a/svx/source/dialog/srchdlg.cxx
+++ b/svx/source/dialog/srchdlg.cxx
@@ -173,20 +173,14 @@ static void StrArrToList_Impl( TypedWhichId<SfxStringListItem> nId, const std::v
     SfxGetpApp()->PutItem( SfxStringListItem( nId, &rStrLst ) );
 }
 
-SearchAttrItemList::SearchAttrItemList( SearchAttrItemList&& rList ) :
+SearchAttrItemList::SearchAttrItemList( SearchAttrItemList&& rList ) noexcept :
     SrchAttrInfoList(std::move(rList))
 {
-    for ( size_t i = 0; i < size(); ++i )
-        if ( !IsInvalidItem( (*this)[i].pItemPtr ) )
-            (*this)[i].pItemPtr = (*this)[i].pItemPtr->Clone();
 }
 
 SearchAttrItemList::SearchAttrItemList( const SearchAttrItemList& rList ) :
     SrchAttrInfoList(rList)
 {
-    for ( size_t i = 0; i < size(); ++i )
-        if ( !IsInvalidItem( (*this)[i].pItemPtr ) )
-            (*this)[i].pItemPtr = (*this)[i].pItemPtr->Clone();
 }
 
 SearchAttrItemList::~SearchAttrItemList()
@@ -201,27 +195,14 @@ void SearchAttrItemList::Put( const SfxItemSet& rSet )
 
     SfxItemPool* pPool = rSet.GetPool();
     SfxItemIter aIter( rSet );
-    SearchAttrInfo aItem;
     const SfxPoolItem* pItem = aIter.GetCurItem();
-    sal_uInt16 nWhich;
 
     do
     {
-        // only test that it is available?
-        if( IsInvalidItem( pItem ) )
-        {
-            nWhich = rSet.GetWhichByOffset( aIter.GetCurPos() );
-            aItem.pItemPtr = pItem;
-        }
-        else
-        {
-            nWhich = pItem->Which();
-            aItem.pItemPtr = pItem->Clone();
-        }
-
-        aItem.nSlot = pPool->GetSlotId( nWhich );
-        Insert( aItem );
+        const sal_uInt16 nWhich(rSet.GetWhichByOffset(aIter.GetCurPos()));
+        const sal_uInt16 nSlot(pPool->GetSlotId(nWhich));
 
+        emplace_back(nSlot, SfxPoolItemHolder(*pPool, pItem));
         pItem = aIter.NextItem();
     } while (pItem);
 }
@@ -232,19 +213,16 @@ SfxItemSet& SearchAttrItemList::Get( SfxItemSet& rSet )
     SfxItemPool* pPool = rSet.GetPool();
 
     for ( size_t i = 0; i < size(); ++i )
-        if ( IsInvalidItem( (*this)[i].pItemPtr ) )
+        if ( IsInvalidItem( (*this)[i].aItemPtr.getItem() ) )
             rSet.InvalidateItem( pPool->GetWhichIDFromSlotID( (*this)[i].nSlot ) );
         else
-            rSet.Put( *(*this)[i].pItemPtr );
+            rSet.Put( *(*this)[i].aItemPtr.getItem() );
     return rSet;
 }
 
 
 void SearchAttrItemList::Clear()
 {
-    for ( size_t i = 0; i < size(); ++i )
-        if ( !IsInvalidItem( (*this)[i].pItemPtr ) )
-            delete (*this)[i].pItemPtr;
     SrchAttrInfoList::clear();
 }
 
@@ -256,10 +234,6 @@ void SearchAttrItemList::Remove(size_t nPos)
     if ( nPos + nLen > size() )
         nLen = size() - nPos;
 
-    for ( size_t i = nPos; i < nPos + nLen; ++i )
-        if ( !IsInvalidItem( (*this)[i].pItemPtr ) )
-            delete (*this)[i].pItemPtr;
-
     SrchAttrInfoList::erase( begin() + nPos, begin() + nPos + nLen );
 }
 
@@ -2019,13 +1993,12 @@ IMPL_LINK_NOARG(SvxSearchDialog, FormatHdl_Impl, weld::Button&, void)
     for( sal_uInt16 n = 0; n < pList->Count(); ++n )
     {
         SearchAttrInfo* pAItem = &pList->GetObject(n);
-        if( !IsInvalidItem( pAItem->pItemPtr ) &&
+        if( !IsInvalidItem( pAItem->aItemPtr.getItem() ) &&
             SfxItemState::SET == aOutSet.GetItemState(
-                pAItem->pItemPtr->Which(), false, &pItem ) )
+                pAItem->aItemPtr.Which(), false, &pItem ) )
         {
-            delete pAItem->pItemPtr;
-            pAItem->pItemPtr = pItem->Clone();
-            aOutSet.ClearItem( pAItem->pItemPtr->Which() );
+            pAItem->aItemPtr = SfxPoolItemHolder(*aOutSet.GetPool(), pItem);
+            aOutSet.ClearItem( pAItem->aItemPtr.Which() );
         }
     }
 
@@ -2162,10 +2135,10 @@ OUString& SvxSearchDialog::BuildAttrText_Impl( OUString& rStr,
         if ( !rStr.isEmpty() )
             rStr += ", ";
 
-        if ( !IsInvalidItem( rItem.pItemPtr ) )
+        if ( !IsInvalidItem( rItem.aItemPtr.getItem() ) )
         {
             OUString aStr;
-            rPool.GetPresentation(*rItem.pItemPtr, eMapUnit, aStr, aIntlWrapper);
+            rPool.GetPresentation(*rItem.aItemPtr.getItem(), eMapUnit, aStr, aIntlWrapper);
             if (aStr.isEmpty())
             {
                 if (rStr.endsWith(", "))