summaryrefslogtreecommitdiff
path: root/sc
diff options
context:
space:
mode:
authorArmin Le Grand (allotropia) <armin.le.grand.extern@allotropia.de>2023-12-20 19:42:28 +0100
committerArmin Le Grand <Armin.Le.Grand@me.com>2023-12-21 21:13:55 +0100
commit789a737ac92c4f2b0eb9820b99c43cc8253c8b29 (patch)
treebc167ec04de29e63168844be973f59980fa36ac2 /sc
parent3430a2c639a9f714259f9d319515464a653d21ab (diff)
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>
Diffstat (limited to 'sc')
-rw-r--r--sc/source/ui/cctrl/cbnumberformat.cxx1
-rw-r--r--sc/source/ui/dbgui/PivotLayoutDialog.cxx6
-rw-r--r--sc/source/ui/navipi/scenwnd.cxx1
-rw-r--r--sc/source/ui/sidebar/AlignmentPropertyPanel.cxx1
-rw-r--r--sc/source/ui/sidebar/CellBorderStyleControl.cxx1
-rw-r--r--sc/source/ui/sidebar/CellLineStyleControl.cxx1
-rw-r--r--sc/source/ui/sidebar/NumberFormatPropertyPanel.cxx1
-rw-r--r--sc/source/ui/view/viewfun2.cxx12
-rw-r--r--sc/source/ui/view/viewfun4.cxx14
9 files changed, 23 insertions, 15 deletions
diff --git a/sc/source/ui/cctrl/cbnumberformat.cxx b/sc/source/ui/cctrl/cbnumberformat.cxx
index 29ab64e84e75..760d6a7f9ca1 100644
--- a/sc/source/ui/cctrl/cbnumberformat.cxx
+++ b/sc/source/ui/cctrl/cbnumberformat.cxx
@@ -24,6 +24,7 @@
#include <sfx2/viewfrm.hxx>
#include <sfx2/viewsh.hxx>
#include <svl/intitem.hxx>
+#include <svl/itemset.hxx>
#include <sc.hrc>
ScNumberFormat::ScNumberFormat(vcl::Window* pParent)
diff --git a/sc/source/ui/dbgui/PivotLayoutDialog.cxx b/sc/source/ui/dbgui/PivotLayoutDialog.cxx
index 17165eac6867..de1f6b3b6f8e 100644
--- a/sc/source/ui/dbgui/PivotLayoutDialog.cxx
+++ b/sc/source/ui/dbgui/PivotLayoutDialog.cxx
@@ -484,10 +484,10 @@ void ScPivotLayoutDialog::ApplyChanges()
SfxDispatcher* pDispatcher = GetBindings().GetDispatcher();
SfxCallMode const nCallMode = SfxCallMode::SLOT | SfxCallMode::RECORD;
- const SfxPoolItem* pResult = pDispatcher->ExecuteList(SID_PIVOT_TABLE,
- nCallMode, { &aPivotItem });
+ const SfxPoolItemHolder aResult(pDispatcher->ExecuteList(SID_PIVOT_TABLE,
+ nCallMode, { &aPivotItem }));
- if (pResult != nullptr)
+ if (nullptr != aResult.getItem())
{
// existing pivot table might have moved to a new range or a new sheet
if ( pOldDPObj != nullptr )
diff --git a/sc/source/ui/navipi/scenwnd.cxx b/sc/source/ui/navipi/scenwnd.cxx
index 5ca601d2e64c..22ee5148fc7b 100644
--- a/sc/source/ui/navipi/scenwnd.cxx
+++ b/sc/source/ui/navipi/scenwnd.cxx
@@ -22,6 +22,7 @@
#include <sfx2/viewfrm.hxx>
#include <svl/slstitm.hxx>
#include <svl/stritem.hxx>
+#include <svl/itemset.hxx>
#include <vcl/commandevent.hxx>
#include <vcl/event.hxx>
#include <vcl/svapp.hxx>
diff --git a/sc/source/ui/sidebar/AlignmentPropertyPanel.cxx b/sc/source/ui/sidebar/AlignmentPropertyPanel.cxx
index 370b791573ed..66f7e35871cb 100644
--- a/sc/source/ui/sidebar/AlignmentPropertyPanel.cxx
+++ b/sc/source/ui/sidebar/AlignmentPropertyPanel.cxx
@@ -25,6 +25,7 @@
#include <sfx2/bindings.hxx>
#include <sfx2/dispatch.hxx>
#include <svl/intitem.hxx>
+#include <svl/itemset.hxx>
#include <svx/rotmodit.hxx>
#include <svtools/unitconv.hxx>
#include <com/sun/star/lang/IllegalArgumentException.hpp>
diff --git a/sc/source/ui/sidebar/CellBorderStyleControl.cxx b/sc/source/ui/sidebar/CellBorderStyleControl.cxx
index 1c75a4a9ae34..e3b9e6a0232e 100644
--- a/sc/source/ui/sidebar/CellBorderStyleControl.cxx
+++ b/sc/source/ui/sidebar/CellBorderStyleControl.cxx
@@ -25,6 +25,7 @@
#include <svx/svxids.hrc>
#include <vcl/settings.hxx>
#include <editeng/lineitem.hxx>
+#include <svl/itemset.hxx>
#include <memory>
namespace sc::sidebar {
diff --git a/sc/source/ui/sidebar/CellLineStyleControl.cxx b/sc/source/ui/sidebar/CellLineStyleControl.cxx
index b6510ffa393d..e9ea1ad34e66 100644
--- a/sc/source/ui/sidebar/CellLineStyleControl.cxx
+++ b/sc/source/ui/sidebar/CellLineStyleControl.cxx
@@ -27,6 +27,7 @@
#include <sfx2/bindings.hxx>
#include <sfx2/dispatch.hxx>
#include <svx/svxids.hrc>
+#include <svl/itemset.hxx>
#include <scresid.hxx>
#include <strings.hrc>
diff --git a/sc/source/ui/sidebar/NumberFormatPropertyPanel.cxx b/sc/source/ui/sidebar/NumberFormatPropertyPanel.cxx
index 753e4ec5392b..c604d41b06fb 100644
--- a/sc/source/ui/sidebar/NumberFormatPropertyPanel.cxx
+++ b/sc/source/ui/sidebar/NumberFormatPropertyPanel.cxx
@@ -23,6 +23,7 @@
#include <sfx2/dispatch.hxx>
#include <svl/intitem.hxx>
#include <svl/stritem.hxx>
+#include <svl/itemset.hxx>
#include <svx/numfmtsh.hxx>
#include <o3tl/string_view.hxx>
#include <com/sun/star/lang/IllegalArgumentException.hpp>
diff --git a/sc/source/ui/view/viewfun2.cxx b/sc/source/ui/view/viewfun2.cxx
index 42c3ba2a62cd..224bb722e0dd 100644
--- a/sc/source/ui/view/viewfun2.cxx
+++ b/sc/source/ui/view/viewfun2.cxx
@@ -2882,14 +2882,14 @@ void ScViewFunc::MoveTable(
SfxStringItem aItem( SID_FILE_NAME, "private:factory/" + STRING_SCAPP );
SfxStringItem aTarget( SID_TARGETNAME, "_blank" );
- const SfxPoolItem* pRetItem = GetViewData().GetDispatcher().ExecuteList(
- SID_OPENDOC, SfxCallMode::API|SfxCallMode::SYNCHRON,
- { &aItem, &aTarget });
- if ( pRetItem )
+ const SfxPoolItemHolder aResult(GetViewData().GetDispatcher().ExecuteList(
+ SID_OPENDOC, SfxCallMode::API|SfxCallMode::SYNCHRON,
+ { &aItem, &aTarget }));
+ if (nullptr != aResult.getItem())
{
- if ( auto pObjectItem = dynamic_cast<const SfxObjectItem*>(pRetItem) )
+ if ( auto pObjectItem = dynamic_cast<const SfxObjectItem*>(aResult.getItem()) )
pDestShell = dynamic_cast<ScDocShell*>( pObjectItem->GetShell() );
- else if ( auto pViewFrameItem = dynamic_cast<const SfxViewFrameItem*>( pRetItem) )
+ else if ( auto pViewFrameItem = dynamic_cast<const SfxViewFrameItem*>(aResult.getItem()))
{
SfxViewFrame* pFrm = pViewFrameItem->GetFrame();
if (pFrm)
diff --git a/sc/source/ui/view/viewfun4.cxx b/sc/source/ui/view/viewfun4.cxx
index 8db1f1e2c85b..d75541b30234 100644
--- a/sc/source/ui/view/viewfun4.cxx
+++ b/sc/source/ui/view/viewfun4.cxx
@@ -587,9 +587,10 @@ bool ScViewFunc::PasteFile( const Point& rPos, const OUString& rFile, bool bLink
if( ::avmedia::MediaWindow::isMediaURL( aStrURL, ""/*TODO?*/ ) )
{
const SfxStringItem aMediaURLItem( SID_INSERT_AVMEDIA, aStrURL );
- return ( nullptr != GetViewData().GetDispatcher().ExecuteList(
- SID_INSERT_AVMEDIA, SfxCallMode::SYNCHRON,
- { &aMediaURLItem }) );
+ const SfxPoolItemHolder aResult(GetViewData().GetDispatcher().ExecuteList(
+ SID_INSERT_AVMEDIA, SfxCallMode::SYNCHRON,
+ { &aMediaURLItem }));
+ return (nullptr != aResult.getItem());
}
#endif
@@ -617,9 +618,10 @@ bool ScViewFunc::PasteFile( const Point& rPos, const OUString& rFile, bool bLink
// Open Asynchronously, because it can also happen from D&D
// and that is not so good for the MAC...
- return (nullptr != rDispatcher.ExecuteList(SID_OPENDOC,
- SfxCallMode::ASYNCHRON,
- { &aFileNameItem, &aFilterItem, &aTargetItem}));
+ const SfxPoolItemHolder aResult(rDispatcher.ExecuteList(SID_OPENDOC,
+ SfxCallMode::ASYNCHRON,
+ { &aFileNameItem, &aFilterItem, &aTargetItem}));
+ return (nullptr != aResult.getItem());
}
}