Remove DeleteItemOnIdlex

There are some CrashReports in 7.6 which have
DeleteItemOnIdle on the stack, but there is nothing
reproducable. So I took a look...

I first thought it's a MCGR regression, due to classes
on the stack. But the Item involved is just random, can
happen with any Item.

Then I thought it may have to do with ITEM refactorings,
but it happens with DeleteItemOnIdle involved, so also
not the case. I already saw DeleteItemOnIdle when doing
these and qualified as 'hack' in the way. already

It is only on Windows and DeleteItemOnIdle is involved.
This again (took a deeper look now) is an old hack to
keep an SfxPoolItem 'alive' for some 'time'. For that,
it triggers an async reschedule which then deletes the
Item when being called. If the Item will be used after
that is pure coincidence - seems to work in most cases.

It seems as if for Windows the timing slightly changed
for some scenarios, so a reschedule is too early. This
can happen with this hack anytime.

DeleteItemOnIdle is used in scenarios where SfxPoolItem*
is e.g. returned, but is *not* anchored, so e.g. not
member of an SfxItemSet. Or in short: Lifetime is not
safe.

DeleteItemOnIdle exists since 1st import, but was
changed to AsyncEvent ca. 4 months ago (see
57145acf9ec47c23e307b7a5c0029d21d937cc35), so that may
have caused it. It is possible that these errors happen
on Windows since then. Before something more complicated
was used to delete it late, but surely also not really
safe.

Due to ITEM refactor I have the knowledge/tooling to
solve this. It will not be a 1-5 lines fix, but it is
a hack and in the way for further ITEM refactor anyways.
What we have nowadays is a SfxPoolItemHolder -> it's
like an SfxItemSet for a single Item. It safely holds/
controls the lifetime of an SfxPoolItem. It is already
used in quite some places. It helps to solve many hacks,
also the ones putting Items directly to the Pool - due
to there never was an alternative for that. In principle
the ItemPool/ItemSet/Item paradigm was never complete
without SfxPoolItemHolder.

Thus I started to fix that (and remove that hack for
good, sooo many changes over the years, sigh), but as
said is not straightforward. Will have to change
retvals of involved stuff to SfxPoolItemHolder - it's
just two pointers and designed to be copied (one is a
Pool, needed to cleanup when destructing).
CopyConstruct/destroy just counts the RefCnt up/down,
so cheap.

1st version compiling, let's check on gerrit...
Corrected one error in QueryState for securitypage, also
added some security features/asserts.

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

18 files changed, 40 insertions, 22 deletions

diff --git a/svx/source/dialog/_bmpmask.cxx b/svx/source/dialog/_bmpmask.cxx
index 3e6a56a79f91..1385aea0c974 100644
--- a/svx/source/dialog/_bmpmask.cxx
+++ b/svx/source/dialog/_bmpmask.cxx
@@ -22,6 +22,7 @@
 #include <vcl/virdev.hxx>
 #include <svtools/valueset.hxx>
 #include <svl/eitem.hxx>
+#include <svl/itemset.hxx>
 #include <sfx2/dispatch.hxx>
 #include <svtools/colrdlg.hxx>
 
diff --git a/svx/source/dialog/fontwork.cxx b/svx/source/dialog/fontwork.cxx
index 15740a27037f..5f9b681b026b 100644
--- a/svx/source/dialog/fontwork.cxx
+++ b/svx/source/dialog/fontwork.cxx
@@ -37,6 +37,7 @@
 #include <svx/svxids.hrc>
 #include <bitmaps.hlst>
 #include <svx/fontwork.hxx>
+#include <svl/itemset.hxx>
 
 #define WIDTH_CHARS 10
 
diff --git a/svx/source/dialog/rubydialog.cxx b/svx/source/dialog/rubydialog.cxx
index 0802c302b75a..83cfe3052c05 100644
--- a/svx/source/dialog/rubydialog.cxx
+++ b/svx/source/dialog/rubydialog.cxx
@@ -43,6 +43,7 @@
 #include <vcl/event.hxx>
 #include <vcl/settings.hxx>
 #include <vcl/svapp.hxx>
+#include <svl/itemset.hxx>
 
 using namespace css::uno;
 using namespace css::frame;
diff --git a/svx/source/dialog/srchdlg.cxx b/svx/source/dialog/srchdlg.cxx
index b1c87a6db7c8..f06822ceaf0b 100644
--- a/svx/source/dialog/srchdlg.cxx
+++ b/svx/source/dialog/srchdlg.cxx
@@ -424,14 +424,14 @@ void SvxSearchDialog::Construct_Impl()
 
     // Get attribute sets only once in constructor()
     const SfxPoolItem* ppArgs[] = { pSearchItem.get(), nullptr };
-    const SvxSetItem* pSrchSetItem =
-        static_cast<const SvxSetItem*>( rBindings.GetDispatcher()->Execute( FID_SEARCH_SEARCHSET, SfxCallMode::SLOT, ppArgs ) );
+    SfxPoolItemHolder aResult(rBindings.GetDispatcher()->Execute(FID_SEARCH_SEARCHSET, SfxCallMode::SLOT, ppArgs));
+    const SvxSetItem* pSrchSetItem(static_cast<const SvxSetItem*>(aResult.getItem()));
 
     if ( pSrchSetItem )
         InitAttrList_Impl( &pSrchSetItem->GetItemSet(), nullptr );
 
-    const SvxSetItem* pReplSetItem =
-        static_cast<const SvxSetItem*>( rBindings.GetDispatcher()->Execute( FID_SEARCH_REPLACESET, SfxCallMode::SLOT, ppArgs ) );
+    aResult = rBindings.GetDispatcher()->Execute(FID_SEARCH_REPLACESET, SfxCallMode::SLOT, ppArgs);
+    const SvxSetItem* pReplSetItem(static_cast<const SvxSetItem*>(aResult.getItem()));
 
     if ( pReplSetItem )
         InitAttrList_Impl( nullptr, &pReplSetItem->GetItemSet() );
@@ -900,14 +900,14 @@ void SvxSearchDialog::Init_Impl( bool bSearchPattern )
             {
                 // Get attribute sets, if it not has been done already
                 const SfxPoolItem* ppArgs[] = { pSearchItem.get(), nullptr };
-                const SvxSetItem* pSrchSetItem =
-                    static_cast<const SvxSetItem*>(rBindings.GetDispatcher()->Execute( FID_SEARCH_SEARCHSET, SfxCallMode::SLOT, ppArgs ));
+                SfxPoolItemHolder aResult(rBindings.GetDispatcher()->Execute(FID_SEARCH_SEARCHSET, SfxCallMode::SLOT, ppArgs));
+                const SvxSetItem* pSrchSetItem(static_cast<const SvxSetItem*>(aResult.getItem()));
 
                 if ( pSrchSetItem )
                     InitAttrList_Impl( &pSrchSetItem->GetItemSet(), nullptr );
 
-                const SvxSetItem* pReplSetItem =
-                    static_cast<const SvxSetItem*>( rBindings.GetDispatcher()->Execute( FID_SEARCH_REPLACESET, SfxCallMode::SLOT, ppArgs ) );
+                aResult = rBindings.GetDispatcher()->Execute(FID_SEARCH_REPLACESET, SfxCallMode::SLOT, ppArgs);
+                const SvxSetItem* pReplSetItem(static_cast<const SvxSetItem*>(aResult.getItem()));
 
                 if ( pReplSetItem )
                     InitAttrList_Impl( nullptr, &pReplSetItem->GetItemSet() );
diff --git a/svx/source/dialog/svxruler.cxx b/svx/source/dialog/svxruler.cxx
index ffc34cd5bfb5..6323583fd271 100644
--- a/svx/source/dialog/svxruler.cxx
+++ b/svx/source/dialog/svxruler.cxx
@@ -43,6 +43,7 @@
 #include <osl/diagnose.h>
 #include <rtl/math.hxx>
 #include <o3tl/string_view.hxx>
+#include <svl/itemset.hxx>
 
 #include "rlrcitem.hxx"
 #include <memory>
diff --git a/svx/source/gallery2/galctrl.cxx b/svx/source/gallery2/galctrl.cxx
index 716a198ec87f..1c3ffbf0326b 100644
--- a/svx/source/gallery2/galctrl.cxx
+++ b/svx/source/gallery2/galctrl.cxx
@@ -34,6 +34,7 @@
 #include <vcl/commandevent.hxx>
 #include <vcl/graphicfilter.hxx>
 #include <bitmaps.hlst>
+#include <svl/itemset.hxx>
 
 GalleryPreview::GalleryPreview(GalleryBrowser2* pParent, std::unique_ptr<weld::ScrolledWindow> xScrolledWindow)
     : mxScrolledWindow(std::move(xScrolledWindow))
diff --git a/svx/source/sidebar/area/AreaPropertyPanel.cxx b/svx/source/sidebar/area/AreaPropertyPanel.cxx
index fa634ee4499b..951028d61f32 100644
--- a/svx/source/sidebar/area/AreaPropertyPanel.cxx
+++ b/svx/source/sidebar/area/AreaPropertyPanel.cxx
@@ -23,6 +23,7 @@
 #include <svx/xflftrit.hxx>
 #include <sfx2/dispatch.hxx>
 #include <sfx2/bindings.hxx>
+#include <svl/itemset.hxx>
 
 
 using namespace css;
@@ -90,9 +91,9 @@ void AreaPropertyPanel::setFillTransparence(const XFillTransparenceItem& rItem)
 void AreaPropertyPanel::setFillUseBackground(const XFillStyleItem* pStyleItem,
                                              const XFillUseSlideBackgroundItem& rItem)
 {
-    const SfxPoolItem* pItem = nullptr;
+    SfxPoolItemHolder aResult;
     auto pDispatcher = GetBindings()->GetDispatcher();
-    auto state = pDispatcher->QueryState(SID_ATTR_FILL_USE_SLIDE_BACKGROUND, pItem);
+    auto state = pDispatcher->QueryState(SID_ATTR_FILL_USE_SLIDE_BACKGROUND, aResult);
     // FillUseSlideBackground is only available in Impress
     if (state == SfxItemState::DISABLED)
     {
diff --git a/svx/source/sidebar/effect/EffectPropertyPanel.cxx b/svx/source/sidebar/effect/EffectPropertyPanel.cxx
index 400dda997962..f6be6ddaff49 100644
--- a/svx/source/sidebar/effect/EffectPropertyPanel.cxx
+++ b/svx/source/sidebar/effect/EffectPropertyPanel.cxx
@@ -18,6 +18,7 @@
 #include <svx/svddef.hxx>
 #include <svx/svxids.hrc>
 #include <svx/xcolit.hxx>
+#include <svl/itemset.hxx>
 
 namespace svx::sidebar
 {
diff --git a/svx/source/sidebar/graphic/GraphicPropertyPanel.cxx b/svx/source/sidebar/graphic/GraphicPropertyPanel.cxx
index 5503649551e6..11a3130d466b 100644
--- a/svx/source/sidebar/graphic/GraphicPropertyPanel.cxx
+++ b/svx/source/sidebar/graphic/GraphicPropertyPanel.cxx
@@ -25,6 +25,7 @@
 #include <sfx2/bindings.hxx>
 #include <sfx2/dispatch.hxx>
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
+#include <svl/itemset.hxx>
 
 using namespace css;
 using namespace css::uno;
diff --git a/svx/source/sidebar/line/LinePropertyPanel.cxx b/svx/source/sidebar/line/LinePropertyPanel.cxx
index 02dd597bd056..83a33b922888 100644
--- a/svx/source/sidebar/line/LinePropertyPanel.cxx
+++ b/svx/source/sidebar/line/LinePropertyPanel.cxx
@@ -26,6 +26,7 @@
 #include <svx/xlncapit.hxx>
 #include <svx/xlinjoit.hxx>
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
+#include <svl/itemset.hxx>
 
 using namespace css;
 using namespace css::uno;
diff --git a/svx/source/sidebar/media/MediaPlaybackPanel.cxx b/svx/source/sidebar/media/MediaPlaybackPanel.cxx
index 4027e16ec730..53c9190b832e 100644
--- a/svx/source/sidebar/media/MediaPlaybackPanel.cxx
+++ b/svx/source/sidebar/media/MediaPlaybackPanel.cxx
@@ -21,6 +21,7 @@
 #include <sfx2/sfxsids.hrc>
 #include <sfx2/dispatch.hxx>
 #include <avmedia/MediaControlBase.hxx>
+#include <svl/itemset.hxx>
 
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
 
diff --git a/svx/source/sidebar/paragraph/ParaLineSpacingControl.cxx b/svx/source/sidebar/paragraph/ParaLineSpacingControl.cxx
index 4c756a5b1224..bda7eb62e632 100644
--- a/svx/source/sidebar/paragraph/ParaLineSpacingControl.cxx
+++ b/svx/source/sidebar/paragraph/ParaLineSpacingControl.cxx
@@ -29,6 +29,7 @@
 
 #include <svl/intitem.hxx>
 #include <svl/itempool.hxx>
+#include <svl/itemset.hxx>
 
 #include <ParaLineSpacingPopup.hxx>
 
@@ -84,10 +85,10 @@ ParaLineSpacingControl::ParaLineSpacingControl(SvxLineSpacingToolBoxControl* pCo
     mxLineDistAtMetricBox->connect_value_changed( aLink2 );
 
     FieldUnit eUnit = FieldUnit::INCH;
-    const SfxUInt16Item* pItem = nullptr;
-    SfxViewFrame* pCurrent = SfxViewFrame::Current();
-    if (pCurrent && pCurrent->GetBindings().GetDispatcher()->QueryState(SID_ATTR_METRIC, pItem) >= SfxItemState::DEFAULT)
-        eUnit = static_cast<FieldUnit>(pItem->GetValue());
+    SfxPoolItemHolder aResult;
+    SfxViewFrame* pCurrent(SfxViewFrame::Current());
+    if (pCurrent && pCurrent->GetBindings().GetDispatcher()->QueryState(SID_ATTR_METRIC, aResult) >= SfxItemState::DEFAULT)
+        eUnit = static_cast<FieldUnit>(static_cast<const SfxUInt16Item*>(aResult.getItem())->GetValue());
     else
         eUnit = SfxModule::GetCurrentFieldUnit();
 
@@ -124,18 +125,18 @@ ParaLineSpacingControl::~ParaLineSpacingControl()
 
 void ParaLineSpacingControl::Initialize()
 {
-    const SvxLineSpacingItem* pItem(nullptr);
+    SfxPoolItemHolder aResult;
     SfxViewFrame* pCurrent = SfxViewFrame::Current();
     const bool bItemStateSet(nullptr != pCurrent);
     const SfxItemState eState(bItemStateSet
-        ? pCurrent->GetBindings().GetDispatcher()->QueryState(SID_ATTR_PARA_LINESPACE, pItem)
+        ? pCurrent->GetBindings().GetDispatcher()->QueryState(SID_ATTR_PARA_LINESPACE, aResult)
         : SfxItemState::DEFAULT);
 
     mxLineDist->set_sensitive(true);
 
     if( bItemStateSet && (eState == SfxItemState::DEFAULT || eState == SfxItemState::SET) )
     {
-        const SvxLineSpacingItem* currSPItem = pItem;
+        const SvxLineSpacingItem* currSPItem(static_cast<const SvxLineSpacingItem*>(aResult.getItem()));
         // It seems draw/impress and writer require different MapUnit values for fixed line spacing
         // metric values to be correctly calculated.
         MapUnit eUnit = MapUnit::Map100thMM; // works for draw/impress
diff --git a/svx/source/sidebar/paragraph/ParaSpacingWindow.cxx b/svx/source/sidebar/paragraph/ParaSpacingWindow.cxx
index f5968510e2ea..ac8749b3b332 100644
--- a/svx/source/sidebar/paragraph/ParaSpacingWindow.cxx
+++ b/svx/source/sidebar/paragraph/ParaSpacingWindow.cxx
@@ -24,6 +24,7 @@
 #include <sfx2/app.hxx>
 #include <sfx2/viewfrm.hxx>
 #include <svl/itempool.hxx>
+#include <svl/itemset.hxx>
 
 using namespace svx;
 
diff --git a/svx/source/sidebar/shadow/ShadowPropertyPanel.cxx b/svx/source/sidebar/shadow/ShadowPropertyPanel.cxx
index 914d13de695d..36b0d780d9a5 100644
--- a/svx/source/sidebar/shadow/ShadowPropertyPanel.cxx
+++ b/svx/source/sidebar/shadow/ShadowPropertyPanel.cxx
@@ -22,6 +22,7 @@
 #include <svx/sdsxyitm.hxx>
 #include <svx/sdshcitm.hxx>
 #include <comphelper/lok.hxx>
+#include <svl/itemset.hxx>
 
 using namespace css;
 using namespace css::uno;
diff --git a/svx/source/sidebar/text/TextCharacterSpacingControl.cxx b/svx/source/sidebar/text/TextCharacterSpacingControl.cxx
index 765a307b9361..28eb699d2d25 100644
--- a/svx/source/sidebar/text/TextCharacterSpacingControl.cxx
+++ b/svx/source/sidebar/text/TextCharacterSpacingControl.cxx
@@ -26,6 +26,7 @@
 #include <sfx2/viewfrm.hxx>
 #include <TextCharacterSpacingPopup.hxx>
 #include <svl/itempool.hxx>
+#include <svl/itemset.hxx>
 #include <helpids.h>
 
 #include <com/sun/star/beans/NamedValue.hpp>
@@ -106,10 +107,10 @@ TextCharacterSpacingControl::~TextCharacterSpacingControl()
 
 void TextCharacterSpacingControl::Initialize()
 {
-    const SvxKerningItem* pKerningItem(nullptr);
+    SfxPoolItemHolder aResult;
     SfxViewFrame* pViewFrm = SfxViewFrame::Current();
-    SfxItemState eState = pViewFrm ? pViewFrm->GetBindings().GetDispatcher()->QueryState(SID_ATTR_CHAR_KERNING, pKerningItem) : SfxItemState::UNKNOWN;
-
+    const SfxItemState eState(pViewFrm ? pViewFrm->GetBindings().GetDispatcher()->QueryState(SID_ATTR_CHAR_KERNING, aResult) : SfxItemState::UNKNOWN);
+    const SvxKerningItem* pKerningItem(static_cast<const SvxKerningItem*>(aResult.getItem()));
     tools::Long nKerning = 0;
 
     if (pKerningItem)
diff --git a/svx/source/sidebar/text/TextUnderlineControl.cxx b/svx/source/sidebar/text/TextUnderlineControl.cxx
index ac65b827198d..34c22b94286b 100644
--- a/svx/source/sidebar/text/TextUnderlineControl.cxx
+++ b/svx/source/sidebar/text/TextUnderlineControl.cxx
@@ -23,6 +23,7 @@
 #include <TextUnderlinePopup.hxx>
 #include <editeng/editids.hrc>
 #include <editeng/udlnitem.hxx>
+#include <svl/itemset.hxx>
 #include <helpids.h>
 
 namespace svx {
@@ -101,8 +102,9 @@ Color GetUnderlineColor()
 {
     if (SfxViewFrame* pViewFrm = SfxViewFrame::Current())
     {
-        const SvxUnderlineItem* pUnderlineItem;
-        pViewFrm->GetBindings().GetDispatcher()->QueryState(SID_ATTR_CHAR_UNDERLINE, pUnderlineItem);
+        SfxPoolItemHolder aResult;
+        pViewFrm->GetBindings().GetDispatcher()->QueryState(SID_ATTR_CHAR_UNDERLINE, aResult);
+        const SvxUnderlineItem* pUnderlineItem(static_cast<const SvxUnderlineItem*>(aResult.getItem()));
 
         if (pUnderlineItem)
             return pUnderlineItem->GetColor();
diff --git a/svx/source/sidebar/textcolumns/TextColumnsPropertyPanel.cxx b/svx/source/sidebar/textcolumns/TextColumnsPropertyPanel.cxx
index 2a06c57059ea..0d9916b5a9d4 100644
--- a/svx/source/sidebar/textcolumns/TextColumnsPropertyPanel.cxx
+++ b/svx/source/sidebar/textcolumns/TextColumnsPropertyPanel.cxx
@@ -19,6 +19,7 @@
 #include <svx/sdmetitm.hxx>
 #include <svx/svddef.hxx>
 #include <svx/svxids.hrc>
+#include <svl/itemset.hxx>
 
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
 
diff --git a/svx/source/tbxctrls/fillctrl.cxx b/svx/source/tbxctrls/fillctrl.cxx
index 085d02ebd50b..5e860b3de851 100644
--- a/svx/source/tbxctrls/fillctrl.cxx
+++ b/svx/source/tbxctrls/fillctrl.cxx
@@ -25,6 +25,7 @@
 #include <vcl/settings.hxx>
 #include <vcl/toolbox.hxx>
 #include <vcl/virdev.hxx>
+#include <svl/itemset.hxx>
 #include <svx/svxids.hrc>
 #include <tools/json_writer.hxx>