editeng/source/items/frmitems.cxx | 8 +-
editeng/source/items/paraitem.cxx | 4 -
editeng/source/items/textitem.cxx | 58 +++++++++------
include/editeng/udlnitem.hxx | 9 +-
include/svl/poolitem.hxx | 27 ++++++-
svl/source/items/cenumitm.cxx | 13 ++-
svl/source/items/itemset.cxx | 141 ++++++++++++++++++++++++++++----------
7 files changed, 188 insertions(+), 72 deletions(-)
New commits:
commit 37f148c58974210707e069f21da2cc2b9ae086dd
Author: Armin Le Grand (allotropia) <armin.le.grand.ext...@allotropia.de>
AuthorDate: Wed Jan 24 17:53:19 2024 +0100
Commit: Armin Le Grand <armin.le.gr...@me.com>
CommitDate: Thu Jan 25 01:58:36 2024 +0100
ITEM: Slight re-design of global Item-Reusage
Unfortunately I had overseen something with derived
classes, but it came now up on CI ASan/UBsan build
with a failing UnitTest - thanks to pointing me at
it.
The ItemInstanceManager at the SfxPoolItems are now
no longer static but constructed instances returned
on-demand.
Also added checks to really use an incarnated/
registered one *only* for that derivation and made
sure this is now correctly supported.
Had to change again, using createItemInstanceManager
to always create instances was less effective than
intended, back to getItemInstanceManager & static
instances in the Item implementations. Also added
some stuff to implCreateItemEntry/implCleanupItemEntry
to be more effective, e.g. direct handling of
slot stuff in latter one. Also some more asserts
and comments. Slot stuff is now handled without
RefCounting, takes some write accesses away...
Change-Id: I6cd69556b416510b5b23549dd042ff3ba155559d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162521
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <armin.le.gr...@me.com>
diff --git a/editeng/source/items/frmitems.cxx
b/editeng/source/items/frmitems.cxx
index c3a5836b082a..e84ae2140e33 100644
--- a/editeng/source/items/frmitems.cxx
+++ b/editeng/source/items/frmitems.cxx
@@ -3980,8 +3980,8 @@ void SvxLineItem::SetLine( const SvxBorderLine* pNew )
ItemInstanceManager* SvxBrushItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxBrushItem).hash_code());
+ return &aInstanceManager;
}
SvxBrushItem::SvxBrushItem(sal_uInt16 _nWhich)
@@ -4598,8 +4598,8 @@ void SvxBrushItem::dumpAsXml(xmlTextWriterPtr pWriter)
const
ItemInstanceManager* SvxFrameDirectionItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxFrameDirectionItem).hash_code());
+ return &aInstanceManager;
}
SvxFrameDirectionItem::SvxFrameDirectionItem( SvxFrameDirection nValue ,
diff --git a/editeng/source/items/paraitem.cxx
b/editeng/source/items/paraitem.cxx
index 2a9a514a6461..e080b517eb43 100644
--- a/editeng/source/items/paraitem.cxx
+++ b/editeng/source/items/paraitem.cxx
@@ -340,8 +340,8 @@ void SvxLineSpacingItem::SetEnumValue( sal_uInt16 nVal )
ItemInstanceManager* SvxAdjustItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxAdjustItem).hash_code());
+ return &aInstanceManager;
}
SvxAdjustItem::SvxAdjustItem(const SvxAdjust eAdjst, const sal_uInt16 nId )
diff --git a/editeng/source/items/textitem.cxx
b/editeng/source/items/textitem.cxx
index 0bed0a7d077d..77e4c6c9cda0 100644
--- a/editeng/source/items/textitem.cxx
+++ b/editeng/source/items/textitem.cxx
@@ -168,8 +168,17 @@ namespace
{
SvxFontItemMap maRegistered;
+ public:
+ SvxFontItemInstanceManager()
+ : ItemInstanceManager(typeid(SvxFontItem).hash_code())
+ {
+ }
+
+ private:
static size_t hashCode(const SfxPoolItem&);
+ // standard interface, accessed exclusively
+ // by implCreateItemEntry/implCleanupItemEntry
virtual const SfxPoolItem* find(const SfxPoolItem&) const override;
virtual void add(const SfxPoolItem&) override;
virtual void remove(const SfxPoolItem&) override;
@@ -209,8 +218,8 @@ namespace
ItemInstanceManager* SvxFontItem::getItemInstanceManager() const
{
- static SvxFontItemInstanceManager aManager;
- return &aManager;
+ static SvxFontItemInstanceManager aInstanceManager;
+ return &aInstanceManager;
}
SvxFontItem::SvxFontItem(
@@ -439,8 +448,8 @@ void SvxFontItem::dumpAsXml(xmlTextWriterPtr pWriter) const
ItemInstanceManager* SvxPostureItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxPostureItem).hash_code());
+ return &aInstanceManager;
}
SvxPostureItem::SvxPostureItem( const FontItalic ePosture, const sal_uInt16
nId ) :
@@ -561,8 +570,8 @@ void SvxPostureItem::dumpAsXml(xmlTextWriterPtr pWriter)
const
ItemInstanceManager* SvxWeightItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxWeightItem).hash_code());
+ return &aInstanceManager;
}
SvxWeightItem::SvxWeightItem( const FontWeight eWght, const sal_uInt16 nId ) :
@@ -691,8 +700,8 @@ void SvxWeightItem::dumpAsXml(xmlTextWriterPtr pWriter)
const
ItemInstanceManager* SvxFontHeightItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxFontHeightItem).hash_code());
+ return &aInstanceManager;
}
SvxFontHeightItem::SvxFontHeightItem( const sal_uInt32 nSz,
@@ -1038,12 +1047,6 @@ void SvxFontHeightItem::dumpAsXml(xmlTextWriterPtr
pWriter) const
// class SvxTextLineItem ------------------------------------------------
-ItemInstanceManager* SvxTextLineItem::getItemInstanceManager() const
-{
- static DefaultItemInstanceManager aManager;
- return &aManager;
-}
-
SvxTextLineItem::SvxTextLineItem( const FontLineStyle eSt, const sal_uInt16
nId )
: SfxEnumItem(nId, eSt)
, maColor(COL_TRANSPARENT)
@@ -1196,6 +1199,11 @@ bool SvxTextLineItem::operator==( const SfxPoolItem&
rItem ) const
// class SvxUnderlineItem ------------------------------------------------
+ItemInstanceManager* SvxUnderlineItem::getItemInstanceManager() const
+{
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxUnderlineItem).hash_code());
+ return &aInstanceManager;
+}
SvxUnderlineItem::SvxUnderlineItem( const FontLineStyle eSt, const sal_uInt16
nId )
: SvxTextLineItem( eSt, nId )
@@ -1238,6 +1246,12 @@ OUString SvxUnderlineItem::GetValueTextByPos( sal_uInt16
nPos ) const
// class SvxOverlineItem ------------------------------------------------
+ItemInstanceManager* SvxOverlineItem::getItemInstanceManager() const
+{
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxOverlineItem).hash_code());
+ return &aInstanceManager;
+}
+
SvxOverlineItem::SvxOverlineItem( const FontLineStyle eSt, const sal_uInt16
nId )
: SvxTextLineItem( eSt, nId )
{
@@ -1281,8 +1295,8 @@ OUString SvxOverlineItem::GetValueTextByPos( sal_uInt16
nPos ) const
ItemInstanceManager* SvxCrossedOutItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxCrossedOutItem).hash_code());
+ return &aInstanceManager;
}
SvxCrossedOutItem::SvxCrossedOutItem( const FontStrikeout eSt, const
sal_uInt16 nId )
@@ -2083,8 +2097,8 @@ bool SvxEscapementItem::PutValue( const uno::Any& rVal,
sal_uInt8 nMemberId )
ItemInstanceManager* SvxLanguageItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxLanguageItem).hash_code());
+ return &aInstanceManager;
}
SvxLanguageItem::SvxLanguageItem( const LanguageType eLang, const sal_uInt16
nId )
@@ -2231,8 +2245,8 @@ bool SvxBlinkItem::GetPresentation
ItemInstanceManager* SvxEmphasisMarkItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxEmphasisMarkItem).hash_code());
+ return &aInstanceManager;
}
SvxEmphasisMarkItem::SvxEmphasisMarkItem( const FontEmphasisMark nValue,
@@ -2671,8 +2685,8 @@ bool SvxCharScaleWidthItem::QueryValue( uno::Any& rVal,
sal_uInt8 /*nMemberId*/
ItemInstanceManager* SvxCharReliefItem::getItemInstanceManager() const
{
- static DefaultItemInstanceManager aManager;
- return &aManager;
+ static DefaultItemInstanceManager
aInstanceManager(typeid(SvxCharReliefItem).hash_code());
+ return &aInstanceManager;
}
SvxCharReliefItem::SvxCharReliefItem( FontRelief eValue,
diff --git a/include/editeng/udlnitem.hxx b/include/editeng/udlnitem.hxx
index 560833d4d088..b140c47b8bc5 100644
--- a/include/editeng/udlnitem.hxx
+++ b/include/editeng/udlnitem.hxx
@@ -33,9 +33,6 @@ class EDITENG_DLLPUBLIC SvxTextLineItem : public
SfxEnumItem<FontLineStyle>
Color maColor;
model::ComplexColor maComplexColor;
-protected:
- virtual ItemInstanceManager* getItemInstanceManager() const override;
-
public:
SvxTextLineItem( const FontLineStyle eSt,
const sal_uInt16 nId );
@@ -90,6 +87,9 @@ public:
class EDITENG_DLLPUBLIC SvxUnderlineItem final : public SvxTextLineItem
{
+protected:
+ virtual ItemInstanceManager* getItemInstanceManager() const override;
+
public:
static SfxPoolItem* CreateDefault();
@@ -106,6 +106,9 @@ public:
class EDITENG_DLLPUBLIC SvxOverlineItem final : public SvxTextLineItem
{
+protected:
+ virtual ItemInstanceManager* getItemInstanceManager() const override;
+
public:
static SfxPoolItem* CreateDefault();
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index 43d8c101dd05..22a5d5bcb9ff 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -176,7 +176,12 @@ protected:
void setNonShareable() { m_bShareable = false; }
// access ItemInstanceManager for this Item, default
- // is nullptr
+ // is nullptr. If you overload this it is expected that
+ // you return a ptr to a static, Item-local managed
+ // instance that exists the whole office lifetime. This
+ // usually means to have a static instance irectly in the
+ // implementation of the overloaded function (just grep
+ // for examples)
virtual ItemInstanceManager* getItemInstanceManager() const;
public:
@@ -297,6 +302,19 @@ class SVL_DLLPUBLIC ItemInstanceManager
friend SfxPoolItem const* implCreateItemEntry(SfxItemPool&, SfxPoolItem
const*, bool);
friend void implCleanupItemEntry(SfxPoolItem const*);
+ // Define for which class to register (usually typeid().hash_code()).
+ std::size_t m_aClassHash;
+
+public:
+ ItemInstanceManager(const std::size_t aClassHash)
+ : m_aClassHash(aClassHash)
+ {
+ }
+ virtual ~ItemInstanceManager() = default;
+
+ std::size_t getClassHash() const { return m_aClassHash; }
+
+private:
// standard interface, accessed exclusively
// by implCreateItemEntry/implCleanupItemEntry
virtual const SfxPoolItem* find(const SfxPoolItem&) const = 0;
@@ -317,7 +335,12 @@ class SVL_DLLPUBLIC DefaultItemInstanceManager : public
ItemInstanceManager
std::unordered_set<const SfxPoolItem*> maRegistered;
public:
- virtual ~DefaultItemInstanceManager() = default;
+ DefaultItemInstanceManager(const std::size_t aClassHash)
+ : ItemInstanceManager(aClassHash)
+ {
+ }
+
+private:
virtual const SfxPoolItem* find(const SfxPoolItem&) const override;
virtual void add(const SfxPoolItem&) override;
virtual void remove(const SfxPoolItem&) override;
diff --git a/svl/source/items/cenumitm.cxx b/svl/source/items/cenumitm.cxx
index 86e30b7e6bdc..4bb3dd8bcd70 100644
--- a/svl/source/items/cenumitm.cxx
+++ b/svl/source/items/cenumitm.cxx
@@ -91,6 +91,15 @@ namespace
{
SfxBoolItemMap maRegistered;
+ public:
+ SfxBoolItemInstanceManager()
+ : ItemInstanceManager(typeid(SfxBoolItem).hash_code())
+ {
+ }
+
+ private:
+ // standard interface, accessed exclusively
+ // by implCreateItemEntry/implCleanupItemEntry
virtual const SfxPoolItem* find(const SfxPoolItem&) const override;
virtual void add(const SfxPoolItem&) override;
virtual void remove(const SfxPoolItem&) override;
@@ -149,8 +158,8 @@ namespace
ItemInstanceManager* SfxBoolItem::getItemInstanceManager() const
{
- static SfxBoolItemInstanceManager aManager;
- return &aManager;
+ static SfxBoolItemInstanceManager aInstanceManager;
+ return &aInstanceManager;
}
void SfxBoolItem::SetValue(bool const bTheValue)
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 907daa23180f..d62f54766698 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -324,6 +324,42 @@ SfxItemSet::SfxItemSet(SfxItemPool& pool,
WhichRangesContainer wids)
assert(svl::detail::validRanges2(m_pWhichRanges));
}
+// Class that implements global Item sharing. It uses rtti to
+// associate every Item-derivation with a possible incarnation
+// of a DefaultItemInstanceManager. This is the default, it will
+// give direct implementatioons at the Items that overload
+// getItemInstanceManager() preference. These are expected to
+// return static instances of aderived implementation of a
+// ItemInstanceManager.
+// All in all there are now the following possibilities to support
+// this for individual Item derivations:
+// (1) Do nothing:
+// In that case, if the Item is shareable, the new mechanism
+// will kick in: It will start sharing the Item globally,
+// but not immediately: After a defined amount of allowed
+// non-shared ocurrences (look for NUMBER_OF_UNSHARED_INSTANCES)
+// an instance of the default ItemInstanceManager, a
+// DefaultItemInstanceManager, will be incarnated and used.
+// NOTE: Mixing shared/unshared instances is not a problem (we
+// might even implement a kind of 're-hash' when this kicks in,
+// but is not really needded).
+// (2) Overload getItemInstanceManager for SfxPoolItem in a class
+// derived from SfxPoolItem and...
+// (2a) Return a static incarnation of DefaultItemInstanceManager to
+// immediately start global sharing of that Item derivation.
+// (2b) Implement and return your own implementation and static
+// incarnation of ItemInstanceManager to do something better/
+// faster that the default implementation can do. Example:
+// SvxFontItem, uses hashing now.
+// There are two supported ENVVARs to use:
+// (a) SVL_DISABLE_ITEM_INSTANCE_MANAGER:
+// This disables the mechanism of global Item sharing completely.
+// This can be used to test/check speed/memory needs compared with
+// using it, but also may come in handy to check if evtl. errors/
+// regressions have to do with it.
+// (b) SVL_SHARE_ITEMS_GLOBALLY_INSTANTLY:
+// This internally forces the NUMBER_OF_UNSHARED_INSTANCES to be
+// ignored and start sharing ALL Item derivations instantly.
class InstanceManagerHelper
{
typedef std::unordered_map<std::size_t, std::pair<sal_uInt16,
DefaultItemInstanceManager*>> managerTypeMap;
@@ -348,24 +384,29 @@ public:
if (!rItem.isShareable())
return nullptr;
- // prefer getting an ItemInstanceManager directly from
+ // Prefer getting an ItemInstanceManager directly from
// the Item: These are the extra implemented (and thus
- // hopefully fast) incarnations
+ // hopefully fastest) incarnations
ItemInstanceManager* pManager(rItem.getItemInstanceManager());
- if (nullptr != pManager)
- return pManager;
- // get hash and check memory for existing entry
+ // Check for correct hash, there may be derivations of that class.
+ // Note that Managers from the Items are *not* added to local list,
+ // they are expected to be static instances at the Items
const std::size_t aHash(typeid(rItem).hash_code());
+ if (nullptr != pManager && pManager->getClassHash() == aHash)
+ return pManager;
+
+ // check local memory for existing entry
managerTypeMap::iterator aHit(maManagerPerType.find(aHash));
- // no instance yet, create one to start usage-counting
+ // no instance yet
if (aHit == maManagerPerType.end())
{
+ // create a default one to start usage-counting
if (g_bShareImmediately)
{
- // create and immediately start sharing
- DefaultItemInstanceManager* pNew(new
DefaultItemInstanceManager);
+ // create, insert locally and immediately start sharing
+ DefaultItemInstanceManager* pNew(new
DefaultItemInstanceManager(aHash));
maManagerPerType.insert({aHash, std::make_pair(0, pNew)});
return pNew;
}
@@ -375,21 +416,21 @@ public:
return nullptr;
}
- // if there is already a ItemInstanceManager incarnated, return it
+ // if there is already an ItemInstanceManager incarnated, return it
if (nullptr != aHit->second.second)
return aHit->second.second;
if (aHit->second.first > 0)
{
- // still not the neeed number of hits, countdown & return nullptr
+ // still not the needed number of hits, countdown & return nullptr
aHit->second.first--;
return nullptr;
}
- // here there is not yet a ItemInstanceManager incarnated. Do
- // so and return it
+ // here the countdown is zero and there is not yet a
ItemInstanceManager
+ // incarnated. Do so, regsiter and return it
assert(nullptr == aHit->second.second);
- DefaultItemInstanceManager* pNew(new DefaultItemInstanceManager);
+ DefaultItemInstanceManager* pNew(new
DefaultItemInstanceManager(aHash));
aHit->second.second = pNew;
return pNew;
@@ -405,15 +446,19 @@ public:
if (!rItem.isShareable())
return nullptr;
- // prefer getting an ItemInstanceManager directly from
+ // Prefer getting an ItemInstanceManager directly from
// the Item: These are the extra implemented (and thus
- // hopefully fast) incarnations
+ // hopefully fastest) incarnations
ItemInstanceManager* pManager(rItem.getItemInstanceManager());
- if (nullptr != pManager)
- return pManager;
- // get hash and check memory for existing entry
+ // Check for correct hash, there may be derivations of that class.
+ // Note that Managers from the Items are *not* added to local list,
+ // they are expected to be static instances at the Items
const std::size_t aHash(typeid(rItem).hash_code());
+ if (nullptr != pManager && pManager->getClassHash() == aHash)
+ return pManager;
+
+ // check local memory for existing entry
managerTypeMap::iterator aHit(maManagerPerType.find(aHash));
if (aHit == maManagerPerType.end())
@@ -424,7 +469,7 @@ public:
if (nullptr != aHit->second.second)
return aHit->second.second;
- // count-up neeed number of hits again if item is released
+ // count-up needed number of hits again if item is released
if (aHit->second.first < NUMBER_OF_UNSHARED_INSTANCES)
aHit->second.first++;
@@ -432,6 +477,7 @@ public:
}
};
+// the single static instance that takes over that global Item sharing
static InstanceManagerHelper aInstanceManagerHelper;
SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const*
pSource, bool bPassingOwnership)
@@ -442,12 +488,12 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool&
rPool, SfxPoolItem const* pS
return nullptr;
if (IsInvalidItem(pSource))
- // SfxItemState::DONTCARE aka invalid item
+ // SfxItemState::DONTCARE aka INVALID_POOL_ITEM
// just use pSource which equals INVALID_POOL_ITEM
return pSource;
if (IsDisabledItem(pSource))
- // SfxItemState::DISABLED aka invalid item
+ // SfxItemState::DISABLED aka DISABLED_POOL_ITEM
// just use pSource which equals DISABLED_POOL_ITEM
return pSource;
@@ -472,28 +518,27 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool&
rPool, SfxPoolItem const* pS
return pSource->Clone();
}
- // get correct target WhichID
- sal_uInt16 nWhich(pSource->Which());
+#ifdef DBG_UTIL
+ // remember WhichID due to being able to assert Clone() error(s)
+ const sal_uInt16 nWhich(pSource->Which());
+#endif
- if (SfxItemPool::IsSlot(nWhich))
+ if (SfxItemPool::IsSlot(pSource->Which()))
{
// SlotItems were always cloned in original (even when
bPassingOwnership),
// so do that here, too (but without bPassingOwnership).
// They do not need to be registered at pool (actually impossible,
pools
// do not have entries for SlotItems) so handle here early
if (!bPassingOwnership)
- pSource = pSource->Clone(rPool.GetMasterPool());
-
- // ARGH! Found out that *some* ::Clone implementations fail to also
clone the
- // WhichID set at the original Item, e.g. SfxFrameItem. Corrected
there, but
- // there may be more cases, so add a SAL_WARN here and correct this
- if (pSource->Which() != nWhich)
{
- SAL_WARN("svl.items", "ITEM: Clone of Item with class " <<
typeid(*pSource).name() << " did NOT copy/set WhichID (!)");
- const_cast<SfxPoolItem*>(pSource)->SetWhich(nWhich);
+ pSource = pSource->Clone(rPool.GetMasterPool());
+ // ARGH! Found out that *some* ::Clone implementations fail to
also clone the
+ // WhichID set at the original Item, e.g. SfxFrameItem. Assert,
this is an error
+#ifdef DBG_UTIL
+ assert(pSource->Which() == nWhich && "ITEM: Clone of Item did NOT
copy/set WhichID (!)");
+#endif
}
- pSource->AddRef();
return pSource;
}
@@ -515,7 +560,7 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool,
SfxPoolItem const* pS
// if (IsStaticDefaultItem(pSource))
// {
// assert(!bPassingOwnership && "ITEM: PassingOwnership not possible
combined with StaticDefault (!)");
- // const SfxPoolItem* pStatic(pTargetPool->GetItem2Default(nWhich));
+ // const SfxPoolItem*
pStatic(pTargetPool->GetItem2Default(pSource->Which()));
// if (nullptr != pStatic)
// {
// if (SfxPoolItem::areSame(pSource, pStatic))
@@ -530,7 +575,7 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool,
SfxPoolItem const* pS
// else if (IsDefaultItem(pSource))
// {
// assert(!bPassingOwnership && "ITEM: PassingOwnership not possible
combined with DynamicDefault (!)");
- // const SfxPoolItem*
pDynamic(pTargetPool->GetPoolDefaultItem(nWhich));
+ // const SfxPoolItem*
pDynamic(pTargetPool->GetPoolDefaultItem(pSource->Which()));
// if (nullptr != pDynamic)
// {
// if (SfxPoolItem::areSame(pSource, pDynamic))
@@ -572,6 +617,12 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool,
SfxPoolItem const* pS
// no already globally shared one found, done
break;
+ // Here we do *not* need to check if it is an SfxSetItem
+ // and cannot be shared if they are in/use another pool:
+ // The SfxItemSet::operator== will check for SfxItemPools
+ // being equal, thus when found in global share the Pool
+ // cannot be equal
+
// need to delete evtl. handed over ownership change Item
if (bPassingOwnership)
delete pSource;
@@ -591,6 +642,9 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool,
SfxPoolItem const* pS
{
const SfxPoolItem* pOld(pSource);
pSource = pSource->Clone(pMasterPool);
+#ifdef DBG_UTIL
+ assert(pSource->Which() == nWhich && "ITEM: Clone of Item did NOT
copy/set WhichID (!)");
+#endif
delete pOld;
}
@@ -602,7 +656,12 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool,
SfxPoolItem const* pS
// when we reach this line we know that we have to add/create a new item.
If
// bPassingOwnership is given just use the item, else clone it
if (!bPassingOwnership)
+ {
pSource = pSource->Clone(pMasterPool);
+#ifdef DBG_UTIL
+ assert(pSource->Which() == nWhich && "ITEM: Clone of Item did NOT
copy/set WhichID (!)");
+#endif
+ }
// increase RefCnt 0->1
pSource->AddRef();
@@ -622,12 +681,13 @@ void implCleanupItemEntry(SfxPoolItem const* pSource)
return;
if (IsInvalidItem(pSource))
+ // SfxItemState::DONTCARE aka INVALID_POOL_ITEM
// nothing to do for invalid item entries
return;
if (IsDisabledItem(pSource))
- // SfxItemState::DISABLED aka invalid item
- // just use pSource which equals DISABLED_POOL_ITEM
+ // SfxItemState::DONTCARE aka DISABLED_POOL_ITEM
+ // nothing to do for disabled item entries
return;
if (0 == pSource->Which())
@@ -639,6 +699,13 @@ void implCleanupItemEntry(SfxPoolItem const* pSource)
return;
}
+ if (SfxItemPool::IsSlot(pSource->Which()))
+ {
+ // SlotItems are cloned, so delete
+ delete pSource;
+ return;
+ }
+
if (1 < pSource->GetRefCount())
{
// Still multiple references present, so just alter the RefCount
