include/svl/itempool.hxx | 38 ++++-
sc/source/core/data/documen9.cxx | 37 +++--
sd/source/core/drawdoc2.cxx | 173 ++++++++++++++-------------
svl/source/items/itempool.cxx | 151 +++++++++++++++++------
svl/source/items/itemset.cxx | 17 ++
svx/source/unodraw/UnoNameItemTable.cxx | 42 +++---
svx/source/unodraw/unomtabl.cxx | 30 ++--
sw/source/core/doc/DocumentFieldsManager.cxx | 2
sw/source/core/doc/docfld.cxx | 9 -
sw/source/core/doc/docfmt.cxx | 4
sw/source/core/table/swtable.cxx | 1
11 files changed, 320 insertions(+), 184 deletions(-)
New commits:
commit 26aa2e799a22b2a95cf865ac0fd57df1262d4a0a
Author: Armin Le Grand (allotropia) <armin.le.grand.ext...@allotropia.de>
AuthorDate: Thu Jan 18 15:29:46 2024 +0100
Commit: Armin Le Grand <armin.le.gr...@me.com>
CommitDate: Fri Jan 19 18:05:30 2024 +0100
ITEM: Needed reworks on ItemSurrogate mechanism
ItemSurrogates are a possibility to iterate over
all SfxPoolItems associated with a WhichID at a
ItemPool to collect or change data.
It is in general not a good idea: the correct action
would be to iterate over the model data and change/
adapt/collect data on the Items of the type
in question. This is because the *assumtion* that you
iteate over all the Items associated with a document
model is *not* correct:
The ItemPool of the model is used for various ItemSets,
e.g. Dialogs/Sidebar and others, so you might get Items
not only from the DocumentModel but from elsewhere.
You might even get Items from *other* models, so
changing these might have unpredictable effects (!)
It is clear to me that this mechanism is more convenient
that iterating over the document models, and it might
have been invented due to this and then used in more
and maore cases. There should be a lambda/callback-based
mechanism in every document model to do this. Until then
we have to live with this 'compromize'. There are over
100 usages currrently, so no way to easily replace this.
For those reasons I changed this to be more safe: There
are two methods to do this now:
1: iterateItemSurrogates to allow read/write access. I
identified six places where that mechanism was used
to change SfxPoolItems, with the use of const_cast.
This now offers a lambda/callback mechanism and the
needed data in a helper (SurrogateData). Internally
it iterates over ItemSets and ItmHolders registered
and thus associated with the Pool. Changing an Item
means to set a changed Item at the Pool/replace the
holder.
2: GetItemSurrogates/FindItemSurrogate to allow
read-only iteration (2nd one with filter). This
collects the Items in a vector that you provide over
which you can then iterate. Do *not* use const_cast
and change the Item when using this (!)
Note that mechanism (2) pe-filters the Items so that
you get each only once: Of couse one Item can be set
in more than one ItemSet/Holder (but also in more than
one model). This filtering is not possible for (1), you
have to evtl. do the same replace action for the same
item, but this is the only way to not change Items that
are associated wth another model.
Note that (2) could also be changed to a lambda/callback
mechanism similar to (1), but there are too many places
that would beed to be adapted. That would have the
advantage to not need to pre-collect the candidates in a
first run.
Also removed/replaced FindItemSurrogate with using
GetItemSurrogates and locally filtering with that needle.
That also made me remove/cleanup CollectSurrogates, it's
only used in one place now.
Change-Id: I0fe2f51f4fca45e1e5aea032cb96bb77b4567f4d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162254
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <armin.le.gr...@me.com>
diff --git a/include/svl/itempool.hxx b/include/svl/itempool.hxx
index 30a638d6609d..83b22d068bd4 100644
--- a/include/svl/itempool.hxx
+++ b/include/svl/itempool.hxx
@@ -56,9 +56,9 @@ SVL_DLLPUBLIC size_t getAllDirectlyPooledSfxPoolItemCount();
SVL_DLLPUBLIC size_t getRemainingDirectlyPooledSfxPoolItemCount();
#endif
-typedef std::unordered_set<const SfxItemSet*> registeredSfxItemSets;
+typedef std::unordered_set<SfxItemSet*> registeredSfxItemSets;
class SfxPoolItemHolder;
-typedef std::unordered_set<const SfxPoolItemHolder*>
registeredSfxPoolItemHolders;
+typedef std::unordered_set<SfxPoolItemHolder*> registeredSfxPoolItemHolders;
typedef std::unordered_set<SfxPoolItemHolder*> directPutSfxPoolItemHolders;
typedef std::vector<const SfxPoolItem*> ItemSurrogates;
@@ -114,8 +114,6 @@ private:
void registerPoolItemHolder(SfxPoolItemHolder& rHolder);
void unregisterPoolItemHolder(SfxPoolItemHolder& rHolder);
- void CollectSurrogates(std::unordered_set<const SfxPoolItem*>& rTarget,
sal_uInt16 nWhich) const;
-
public:
// for default SfxItemSet::CTOR, set default WhichRanges
void FillItemIdRanges_Impl(
WhichRangesContainer& pWhichRanges ) const;
@@ -199,13 +197,33 @@ public:
{ return static_cast<const T*>(GetItem2Default(sal_uInt16(nWhich))); }
public:
+ // SurrogateData callback helper for iterateItemSurrogates
+ class SurrogateData
+ {
+ public:
+ virtual ~SurrogateData() = default;
+ SurrogateData(const SurrogateData&) = default;
+ SurrogateData() = default;
+
+ // read-access to Item
+ virtual const SfxPoolItem& getItem() const = 0;
+
+ // write-access when Item needs to be modified
+ virtual const SfxPoolItem* setItem(std::unique_ptr<SfxPoolItem>) = 0;
+ };
+
+ // Iterate using a lambda/callback with read/write access to registered
SfxPoolItems.
+ // If you use this, look for current usages, inside the callback you may
+ // return true; // to continue callback (like 'continue')
+ // return false; // to end callbacks (like 'break')
+ void iterateItemSurrogates(
+ sal_uInt16 nWhich,
+ const std::function<bool(SfxItemPool::SurrogateData& rData)>&
rItemCallback) const;
+
+ // read-only access to registered SfxPoolItems
+ // NOTE: In no case use static_cast and change those Items (!)
+ // Read commit text for more information
void GetItemSurrogates(ItemSurrogates& rTarget, sal_uInt16 nWhich) const;
- /*
- This is only valid for SfxPoolItem that override IsSortable and
operator<.
- Returns a range of items defined by using operator<.
- @param rNeedle must be the same type or a supertype of the pool items
for nWhich.
- */
- void FindItemSurrogate(ItemSurrogates& rTarget, sal_uInt16 nWhich,
SfxPoolItem const & rNeedle) const;
sal_uInt16 GetFirstWhich() const;
sal_uInt16 GetLastWhich() const;
diff --git a/sc/source/core/data/documen9.cxx b/sc/source/core/data/documen9.cxx
index c4c569557ffb..b48e01234f3a 100644
--- a/sc/source/core/data/documen9.cxx
+++ b/sc/source/core/data/documen9.cxx
@@ -546,27 +546,32 @@ void ScDocument::UpdateFontCharSet()
return;
ScDocumentPool* pPool = mxPoolHelper->GetDocPool();
- ItemSurrogates aSurrogates;
- pPool->GetItemSurrogates(aSurrogates, ATTR_FONT);
- for (const SfxPoolItem* pItem : aSurrogates)
+
+ pPool->iterateItemSurrogates(ATTR_FONT, [&](SfxItemPool::SurrogateData&
rData)
{
- auto pFontItem = const_cast<SvxFontItem*>(dynamic_cast<const
SvxFontItem*>(pItem));
- if ( pFontItem && ( pFontItem->GetCharSet() == eSrcSet ||
- ( bUpdateOld && pFontItem->GetCharSet() !=
RTL_TEXTENCODING_SYMBOL ) ) )
- pFontItem->SetCharSet(eSysSet);
- }
+ const SvxFontItem& rSvxFontItem(static_cast<const
SvxFontItem&>(rData.getItem()));
+ if (eSrcSet == rSvxFontItem.GetCharSet() || (bUpdateOld &&
RTL_TEXTENCODING_SYMBOL != rSvxFontItem.GetCharSet()))
+ {
+ SvxFontItem* pNew(rSvxFontItem.Clone(pPool));
+ pNew->SetCharSet(eSysSet);
+ rData.setItem(std::unique_ptr<SfxPoolItem>(pNew));
+ }
+ return true; // continue callbacks
+ });
if ( mpDrawLayer )
{
- SfxItemPool& rDrawPool = mpDrawLayer->GetItemPool();
- rDrawPool.GetItemSurrogates(aSurrogates, EE_CHAR_FONTINFO);
- for (const SfxPoolItem* pItem : aSurrogates)
+ pPool->iterateItemSurrogates(EE_CHAR_FONTINFO,
[&](SfxItemPool::SurrogateData& rData)
{
- SvxFontItem* pFontItem =
const_cast<SvxFontItem*>(dynamic_cast<const SvxFontItem*>(pItem));
- if ( pFontItem && ( pFontItem->GetCharSet() == eSrcSet ||
- ( bUpdateOld && pFontItem->GetCharSet() !=
RTL_TEXTENCODING_SYMBOL ) ) )
- pFontItem->SetCharSet( eSysSet );
- }
+ const SvxFontItem& rSvxFontItem(static_cast<const
SvxFontItem&>(rData.getItem()));
+ if (eSrcSet == rSvxFontItem.GetCharSet() || (bUpdateOld &&
RTL_TEXTENCODING_SYMBOL != rSvxFontItem.GetCharSet()))
+ {
+ SvxFontItem* pNew(rSvxFontItem.Clone(pPool));
+ pNew->SetCharSet(eSysSet);
+ rData.setItem(std::unique_ptr<SfxPoolItem>(pNew));
+ }
+ return true; // continue callbacks
+ });
}
}
diff --git a/sd/source/core/drawdoc2.cxx b/sd/source/core/drawdoc2.cxx
index 328952c577c5..7804c98cb7b5 100644
--- a/sd/source/core/drawdoc2.cxx
+++ b/sd/source/core/drawdoc2.cxx
@@ -268,43 +268,53 @@ void
SdDrawDocument::UpdatePageRelativeURLs(std::u16string_view aOldName, std::u
return;
SfxItemPool& rPool(GetPool());
- ItemSurrogates aSurrogates;
- rPool.GetItemSurrogates(aSurrogates, EE_FEATURE_FIELD);
- for (const SfxPoolItem* pItem : aSurrogates)
+
+ rPool.iterateItemSurrogates(EE_FEATURE_FIELD,
[&](SfxItemPool::SurrogateData& rData)
{
- const SvxFieldItem* pFldItem = dynamic_cast< const SvxFieldItem * >
(pItem);
+ const SvxFieldItem* pFieldItem(dynamic_cast<const
SvxFieldItem*>(&rData.getItem()));
- if(pFldItem)
- {
- SvxURLField* pURLField = const_cast< SvxURLField* >(
dynamic_cast<const SvxURLField*>( pFldItem->GetField() ) );
+ if (nullptr == pFieldItem)
+ return true; // continue callbacks
- if(pURLField)
- {
- OUString aURL = pURLField->GetURL();
+ const SvxURLField* pURLField(dynamic_cast<const
SvxURLField*>(pFieldItem->GetField()));
- if (!aURL.isEmpty() && (aURL[0] == 35) &&
(aURL.indexOf(aOldName, 1) == 1))
- {
- if (aURL.getLength() == sal_Int32(aOldName.size() + 1)) //
standard page name
- {
- aURL = aURL.replaceAt(1, aURL.getLength() - 1, u"") +
- aNewName;
- pURLField->SetURL(aURL);
- }
- else
- {
- const OUString sNotes(SdResId(STR_NOTES));
- if (aURL.getLength() == sal_Int32(aOldName.size()) + 2
+ sNotes.getLength()
- && aURL.indexOf(sNotes, aOldName.size() + 2) ==
sal_Int32(aOldName.size() + 2))
- {
- aURL = aURL.replaceAt(1, aURL.getLength() - 1,
u"") +
- aNewName + " " + sNotes;
- pURLField->SetURL(aURL);
- }
- }
- }
+ if (nullptr == pURLField)
+ return true; // continue callbacks
+
+ OUString aURL(pURLField->GetURL());
+
+ if (aURL.isEmpty() || (aURL[0] != 35) || (aURL.indexOf(aOldName, 1) !=
1))
+ return true; // continue callbacks
+
+ bool bURLChange(false);
+
+ if (aURL.getLength() == sal_Int32(aOldName.size() + 1)) // standard
page name
+ {
+ aURL = aURL.replaceAt(1, aURL.getLength() - 1, u"") +
+ aNewName;
+ bURLChange = true;
+ }
+ else
+ {
+ const OUString sNotes(SdResId(STR_NOTES));
+ if (aURL.getLength() == sal_Int32(aOldName.size()) + 2 +
sNotes.getLength()
+ && aURL.indexOf(sNotes, aOldName.size() + 2) ==
sal_Int32(aOldName.size() + 2))
+ {
+ aURL = aURL.replaceAt(1, aURL.getLength() - 1, u"") +
+ aNewName + " " + sNotes;
+ bURLChange = true;
}
}
- }
+
+ if(bURLChange)
+ {
+ SvxFieldItem* pNewFieldItem(pFieldItem->Clone(&rPool));
+ const_cast<SvxURLField*>(static_cast<const
SvxURLField*>(pNewFieldItem->GetField()))->SetURL(aURL);
+ rData.setItem(std::unique_ptr<SfxPoolItem>(pNewFieldItem));
+ }
+
+ return true; // continue callbacks
+ });
}
void SdDrawDocument::UpdatePageRelativeURLs(SdPage const * pPage, sal_uInt16
nPos, sal_Int32 nIncrement)
@@ -312,60 +322,63 @@ void SdDrawDocument::UpdatePageRelativeURLs(SdPage const
* pPage, sal_uInt16 nPo
bool bNotes = (pPage->GetPageKind() == PageKind::Notes);
SfxItemPool& rPool(GetPool());
- ItemSurrogates aSurrogates;
- rPool.GetItemSurrogates(aSurrogates, EE_FEATURE_FIELD);
- for (const SfxPoolItem* pItem : aSurrogates)
+ rPool.iterateItemSurrogates(EE_FEATURE_FIELD,
[&](SfxItemPool::SurrogateData& rData)
{
- const SvxFieldItem* pFldItem;
+ const SvxFieldItem* pFieldItem(static_cast<const
SvxFieldItem*>(&rData.getItem()));
- if ((pFldItem = dynamic_cast< const SvxFieldItem * > (pItem)) !=
nullptr)
- {
- SvxURLField* pURLField = const_cast< SvxURLField* >(
dynamic_cast<const SvxURLField*>( pFldItem->GetField() ) );
+ if (nullptr == pFieldItem)
+ return true; // continue callbacks
- if(pURLField)
- {
- OUString aURL = pURLField->GetURL();
+ const SvxURLField* pURLField(dynamic_cast<const
SvxURLField*>(pFieldItem->GetField()));
- if (!aURL.isEmpty() && (aURL[0] == 35))
- {
- OUString aHashSlide = "#" + SdResId(STR_PAGE);
-
- if (aURL.startsWith(aHashSlide))
- {
- OUString aURLCopy = aURL;
- const OUString sNotes(SdResId(STR_NOTES));
-
- aURLCopy = aURLCopy.replaceAt(0,
aHashSlide.getLength(), u"");
-
- bool bNotesLink = ( aURLCopy.getLength() >=
sNotes.getLength() + 3
- && aURLCopy.endsWith(sNotes) );
-
- if (bNotesLink != bNotes)
- continue; // no compatible link and page
-
- if (bNotes)
- aURLCopy = aURLCopy.replaceAt(aURLCopy.getLength()
- sNotes.getLength(), sNotes.getLength(), u"");
-
- sal_Int32 number = aURLCopy.toInt32();
- sal_uInt16 realPageNumber = (nPos + 1)/ 2;
-
- if ( number >= realPageNumber )
- {
- // update link page number
- number += nIncrement;
- aURL = aURL.replaceAt(aHashSlide.getLength() + 1,
aURL.getLength() - aHashSlide.getLength() - 1, u"") +
- OUString::number(number);
- if (bNotes)
- {
- aURL += " " + sNotes;
- }
- pURLField->SetURL(aURL);
- }
- }
- }
- }
+ if (nullptr == pURLField)
+ return true; // continue callbacks
+
+ OUString aURL(pURLField->GetURL());
+
+ if (aURL.isEmpty() || (aURL[0] != 35))
+ return true; // continue callbacks
+
+ OUString aHashSlide = "#" + SdResId(STR_PAGE);
+
+ if (!aURL.startsWith(aHashSlide))
+ return true; // continue callbacks
+
+ OUString aURLCopy = aURL;
+ const OUString sNotes(SdResId(STR_NOTES));
+
+ aURLCopy = aURLCopy.replaceAt(0, aHashSlide.getLength(), u"");
+
+ bool bNotesLink = ( aURLCopy.getLength() >= sNotes.getLength() + 3
+ && aURLCopy.endsWith(sNotes) );
+
+ if (bNotesLink != bNotes)
+ return true; // no compatible link and page, continue callbacks
+
+ if (bNotes)
+ aURLCopy = aURLCopy.replaceAt(aURLCopy.getLength() -
sNotes.getLength(), sNotes.getLength(), u"");
+
+ sal_Int32 number = aURLCopy.toInt32();
+ sal_uInt16 realPageNumber = (nPos + 1)/ 2;
+
+ if ( number < realPageNumber )
+ return true; // continue callbacks
+
+ // update link page number
+ number += nIncrement;
+ aURL = aURL.replaceAt(aHashSlide.getLength() + 1, aURL.getLength() -
aHashSlide.getLength() - 1, u"") +
+ OUString::number(number);
+ if (bNotes)
+ {
+ aURL += " " + sNotes;
}
- }
+
+ SvxFieldItem* pNewFieldItem(pFieldItem->Clone(&rPool));
+ const_cast<SvxURLField*>(static_cast<const
SvxURLField*>(pNewFieldItem->GetField()))->SetURL(aURL);
+ rData.setItem(std::unique_ptr<SfxPoolItem>(pNewFieldItem));
+
+ return true; // continue callbacks
+ });
}
// Move page
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index 341cbd3672ff..32b1c0499cf4 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -875,64 +875,139 @@ const SfxPoolItem
*SfxItemPool::GetItem2Default(sal_uInt16 nWhich) const
return (*mpStaticDefaults)[ GetIndex_Impl(nWhich) ];
}
-void SfxItemPool::CollectSurrogates(std::unordered_set<const SfxPoolItem*>&
rTarget, sal_uInt16 nWhich) const
+namespace
+{
+ class SurrogateData_ItemSet : public SfxItemPool::SurrogateData
+ {
+ const SfxPoolItem* mpItem;
+ SfxItemSet* mpSet;
+
+ public:
+ SurrogateData_ItemSet(const SfxPoolItem& rItem, SfxItemSet& rSet)
+ : SfxItemPool::SurrogateData()
+ , mpItem(&rItem)
+ , mpSet(&rSet)
+ {
+ }
+
+ SurrogateData_ItemSet(const SurrogateData_ItemSet&) = default;
+
+ virtual const SfxPoolItem& getItem() const override
+ {
+ return *mpItem;
+ }
+
+ virtual const SfxPoolItem* setItem(std::unique_ptr<SfxPoolItem> aNew)
override
+ {
+ return mpSet->Put(std::unique_ptr<SfxPoolItem>(aNew.release()));
+ }
+ };
+
+ class SurrogateData_ItemHolder : public SfxItemPool::SurrogateData
+ {
+ SfxPoolItemHolder* mpHolder;
+
+ public:
+ SurrogateData_ItemHolder(SfxPoolItemHolder& rHolder)
+ : SfxItemPool::SurrogateData()
+ , mpHolder(&rHolder)
+ {
+ }
+
+ SurrogateData_ItemHolder(const SurrogateData_ItemHolder&) = default;
+
+ virtual const SfxPoolItem& getItem() const override
+ {
+ return *mpHolder->getItem();
+ }
+
+ virtual const SfxPoolItem* setItem(std::unique_ptr<SfxPoolItem> aNew)
override
+ {
+ *mpHolder = SfxPoolItemHolder(mpHolder->getPool(), aNew.release(),
true);
+ return mpHolder->getItem();
+ }
+ };
+}
+
+void SfxItemPool::iterateItemSurrogates(
+ sal_uInt16 nWhich,
+ const std::function<bool(SurrogateData& rCand)>& rItemCallback) const
+{
+ // 1st source for surrogates
+ const registeredSfxItemSets&
rSets(GetMasterPool()->maRegisteredSfxItemSets);
+
+ if(!rSets.empty())
+ {
+ const SfxPoolItem* pItem(nullptr);
+ std::vector<SurrogateData_ItemSet> aEntries;
+
+ // NOTE: this collects the callback data in a preparing run. This
+ // is by purpose, else any write change may change the iterators
+ // used at registeredSfxItemSets. I tied with direct feed and
+ // that worked most of the time, but failed for ItemHolders due
+ // to these being changed and being re-registered. I have avoided
+ // this in SfxPoolItemHolder::operator=, but it's just a question
+ // that in some scenario someone replaces an Item even with a
+ // different type/WhichID that this will then break/crash
+ for (const auto& rCand : rSets)
+ if (SfxItemState::SET == rCand->GetItemState(nWhich, false,
&pItem))
+ aEntries.emplace_back(*pItem, *rCand);
+
+ if (!aEntries.empty())
+ for (auto& rCand : aEntries)
+ if (!rItemCallback(rCand))
+ return;
+ }
+
+ // 2nd source for surrogates
+ const registeredSfxPoolItemHolders&
rHolders(GetMasterPool()->maRegisteredSfxPoolItemHolders);
+
+ if (!rHolders.empty())
+ {
+ std::vector<SurrogateData_ItemHolder> aEntries;
+
+ // NOTE: same as above, look there
+ for (auto& rCand : rHolders)
+ if (rCand->Which() == nWhich && nullptr != rCand->getItem())
+ aEntries.emplace_back(*rCand);
+
+ if (!aEntries.empty())
+ for (auto& rCand : aEntries)
+ if (!rItemCallback(rCand))
+ return;
+ }
+}
+
+void SfxItemPool::GetItemSurrogates(ItemSurrogates& rTarget, sal_uInt16
nWhich) const
{
rTarget.clear();
if (0 == nWhich)
return;
+ // NOTE: This is pre-collected, in this case mainly to
+ // remove all double listings of SfxPoolItems which can
+ // of course be referenced multiple times in multiple
+ // ItemSets/ItemHolders. It comes handy that
+ // std::unordered_set does this by definition
+ std::unordered_set<const SfxPoolItem*> aNewSurrogates;
+
// 1st source for surrogates
const registeredSfxItemSets&
rSets(GetMasterPool()->maRegisteredSfxItemSets);
const SfxPoolItem* pItem(nullptr);
for (const auto& rCand : rSets)
if (SfxItemState::SET == rCand->GetItemState(nWhich, false, &pItem))
- rTarget.insert(pItem);
+ aNewSurrogates.insert(pItem);
// 2nd source for surrogates
const registeredSfxPoolItemHolders&
rHolders(GetMasterPool()->maRegisteredSfxPoolItemHolders);
for (const auto& rCand : rHolders)
if (rCand->Which() == nWhich && nullptr != rCand->getItem())
- rTarget.insert(rCand->getItem());
-
- // the 3rd source for surrogates is the list of direct put items
- // but since these use SfxPoolItemHolder now they are automatically
- // registered at 2nd source - IF NeedsSurrogateSupport is set. So
- // as long as we have this DirectPutItem stuff, iterate here and
- // warn if an Item was added
- const directPutSfxPoolItemHolders&
rDirects(GetMasterPool()->maDirectPutItems);
-#ifdef DBG_UTIL
- const size_t aBefore(rTarget.size());
-#endif
- for (const auto& rCand : rDirects)
- if (rCand->Which() == nWhich && nullptr != rCand->getItem())
- rTarget.insert(rCand->getItem());
-#ifdef DBG_UTIL
- const size_t aAfter(rTarget.size());
- if (aBefore != aAfter)
- {
- SAL_WARN("svl.items", "SfxItemPool: Found non-automatically registered
Item for Surrogates in DirectPutItems (!)");
- }
-#endif
-}
+ aNewSurrogates.insert(rCand->getItem());
-void SfxItemPool::GetItemSurrogates(ItemSurrogates& rTarget, sal_uInt16
nWhich) const
-{
- std::unordered_set<const SfxPoolItem*> aNewSurrogates;
- CollectSurrogates(aNewSurrogates, nWhich);
rTarget = ItemSurrogates(aNewSurrogates.begin(), aNewSurrogates.end());
}
-void SfxItemPool::FindItemSurrogate(ItemSurrogates& rTarget, sal_uInt16
nWhich, SfxPoolItem const & rSample) const
-{
- std::unordered_set<const SfxPoolItem*> aNewSurrogates;
- CollectSurrogates(aNewSurrogates, nWhich);
- rTarget.clear();
- for (const auto& rCand : aNewSurrogates)
- if (rSample == *rCand)
- rTarget.push_back(rCand);
-}
-
sal_uInt16 SfxItemPool::GetWhich( sal_uInt16 nSlotId, bool bDeep ) const
{
if ( !IsSlot(nSlotId) )
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 3b0ba11aef36..6ac8a60d782b 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -127,8 +127,10 @@ const SfxPoolItemHolder&
SfxPoolItemHolder::operator=(const SfxPoolItemHolder& r
if (this == &rHolder || *this == rHolder)
return *this;
- if (nullptr != m_pItem &&
getPool().NeedsSurrogateSupport(m_pItem->Which()))
- getPool().unregisterPoolItemHolder(*this);
+ // avoid unneccessary unregister/register actions
+ const bool bWasRegistered(nullptr != m_pItem &&
getPool().NeedsSurrogateSupport(m_pItem->Which()));
+ const bool bWillBeRegistered(nullptr != rHolder.m_pItem &&
rHolder.getPool().NeedsSurrogateSupport(rHolder.m_pItem->Which()));
+
if (nullptr != m_pItem)
implCleanupItemEntry(m_pItem);
@@ -137,8 +139,15 @@ const SfxPoolItemHolder&
SfxPoolItemHolder::operator=(const SfxPoolItemHolder& r
if (nullptr != m_pItem)
m_pItem = implCreateItemEntry(getPool(), m_pItem, false);
- if (nullptr != m_pItem &&
getPool().NeedsSurrogateSupport(m_pItem->Which()))
- getPool().registerPoolItemHolder(*this);
+
+ if (bWasRegistered != bWillBeRegistered)
+ {
+ // adapt registration if needed
+ if (bWillBeRegistered)
+ getPool().registerPoolItemHolder(*this);
+ else
+ getPool().unregisterPoolItemHolder(*this);
+ }
return *this;
}
diff --git a/svx/source/unodraw/UnoNameItemTable.cxx
b/svx/source/unodraw/UnoNameItemTable.cxx
index ef7f193c40d5..2612bbf9b67d 100644
--- a/svx/source/unodraw/UnoNameItemTable.cxx
+++ b/svx/source/unodraw/UnoNameItemTable.cxx
@@ -39,8 +39,8 @@ using namespace ::cppu;
namespace
{
// We need to override operator== here and specifically bypass the assert
- // in SfxPoolItem::operator== in order to make the FindItemSurrogate call
- // in SvxUnoNameItemTable::hasByName safe.
+ // in SfxPoolItem::operator== in order to make the GetItemSurrogates call
+ // and comparing it's results in SvxUnoNameItemTable::hasByName safe.
class SampleItem : public NameOrIndex
{
public:
@@ -183,13 +183,15 @@ void SAL_CALL SvxUnoNameItemTable::replaceByName( const
OUString& aApiName, cons
{
SampleItem aSample(mnWhich, aName);
ItemSurrogates aSurrogates;
- mpModelPool->FindItemSurrogate(aSurrogates, mnWhich, aSample);
+ mpModelPool->GetItemSurrogates(aSurrogates, mnWhich);
+
for (const SfxPoolItem* pNameOrIndex : aSurrogates)
- if (isValid(static_cast<const NameOrIndex*>(pNameOrIndex)))
- {
- const_cast<SfxPoolItem*>(pNameOrIndex)->PutValue( aElement,
mnMemberId );
- bFound = true;
- }
+ if (aSample == *pNameOrIndex)
+ if (isValid(static_cast<const NameOrIndex*>(pNameOrIndex)))
+ {
+ const_cast<SfxPoolItem*>(pNameOrIndex)->PutValue(
aElement, mnMemberId );
+ bFound = true;
+ }
}
if( !bFound )
@@ -213,14 +215,16 @@ uno::Any SAL_CALL SvxUnoNameItemTable::getByName( const
OUString& aApiName )
{
SampleItem aSample(mnWhich, aName);
ItemSurrogates aSurrogates;
- mpModelPool->FindItemSurrogate(aSurrogates, mnWhich, aSample);
+ mpModelPool->GetItemSurrogates(aSurrogates, mnWhich);
+
for (const SfxPoolItem* pFindItem : aSurrogates)
- if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
- {
- uno::Any aAny;
- pFindItem->QueryValue( aAny, mnMemberId );
- return aAny;
- }
+ if (aSample == *pFindItem)
+ if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
+ {
+ uno::Any aAny;
+ pFindItem->QueryValue( aAny, mnMemberId );
+ return aAny;
+ }
}
throw container::NoSuchElementException();
@@ -266,10 +270,12 @@ sal_Bool SAL_CALL SvxUnoNameItemTable::hasByName( const
OUString& aApiName )
SampleItem aSample(mnWhich, aName);
ItemSurrogates aSurrogates;
- mpModelPool->FindItemSurrogate(aSurrogates, mnWhich, aSample);
+ mpModelPool->GetItemSurrogates(aSurrogates, mnWhich);
+
for (const SfxPoolItem* pFindItem : aSurrogates)
- if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
- return true;
+ if (aSample == *pFindItem)
+ if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
+ return true;
return false;
}
diff --git a/svx/source/unodraw/unomtabl.cxx b/svx/source/unodraw/unomtabl.cxx
index 27699ea651f7..5a6872c0b777 100644
--- a/svx/source/unodraw/unomtabl.cxx
+++ b/svx/source/unodraw/unomtabl.cxx
@@ -247,34 +247,36 @@ void SAL_CALL SvxUnoMarkerTable::replaceByName( const
OUString& aApiName, const
if (mpModelPool)
{
- ItemSurrogates aSurrogates;
- mpModelPool->GetItemSurrogates(aSurrogates, XATTR_LINESTART);
- for (const SfxPoolItem* p : aSurrogates)
+ mpModelPool->iterateItemSurrogates(XATTR_LINESTART,
[&](SfxItemPool::SurrogateData& rData)
{
- NameOrIndex *pItem = const_cast<NameOrIndex*>(static_cast<const
NameOrIndex*>(p));
+ const NameOrIndex* pItem(static_cast<const
NameOrIndex*>(&rData.getItem()));
if( pItem && pItem->GetName() == aName )
{
- pItem->PutValue( aElement, 0 );
+ NameOrIndex* pNew(pItem->Clone(mpModelPool));
+ pNew->PutValue(aElement, 0);
+ rData.setItem(std::unique_ptr<SfxPoolItem>(pNew));
bFound = true;
- break;
+ return false; // interrupt callbacks
}
- }
+ return true; // continue callbacks
+ });
}
if (mpModelPool)
{
- ItemSurrogates aSurrogates;
- mpModelPool->GetItemSurrogates(aSurrogates, XATTR_LINEEND);
- for (const SfxPoolItem* p : aSurrogates)
+ mpModelPool->iterateItemSurrogates(XATTR_LINEEND,
[&](SfxItemPool::SurrogateData& rData)
{
- NameOrIndex *pItem = const_cast<NameOrIndex*>(static_cast<const
NameOrIndex*>(p));
+ const NameOrIndex* pItem(static_cast<const
NameOrIndex*>(&rData.getItem()));
if( pItem && pItem->GetName() == aName )
{
- pItem->PutValue( aElement, 0 );
+ NameOrIndex* pNew(pItem->Clone(mpModelPool));
+ pNew->PutValue(aElement, 0);
+ rData.setItem(std::unique_ptr<SfxPoolItem>(pNew));
bFound = true;
- break;
+ return false; // interrupt callbacks
}
- }
+ return true; // continue callbacks
+ });
}
if( !bFound )
diff --git a/sw/source/core/doc/DocumentFieldsManager.cxx
b/sw/source/core/doc/DocumentFieldsManager.cxx
index f2faaa4bac73..aaf26ce9eb46 100644
--- a/sw/source/core/doc/DocumentFieldsManager.cxx
+++ b/sw/source/core/doc/DocumentFieldsManager.cxx
@@ -618,6 +618,7 @@ void DocumentFieldsManager::UpdateTableFields(const
SwTable* pTable)
m_rDoc.GetAttrPool().GetItemSurrogates(aSurrogates, RES_BOXATR_FORMULA);
for (const SfxPoolItem* pItem : aSurrogates)
{
+ // SwTableBoxFormula is non-shareable, so const_cast is somewhat OK
auto pBoxFormula =
const_cast<SwTableBoxFormula*>(pItem->DynamicWhichCast(RES_BOXATR_FORMULA));
if(pBoxFormula && pBoxFormula->GetDefinedIn())
pBoxFormula->ChangeState();
@@ -718,6 +719,7 @@ void DocumentFieldsManager::UpdateTableFields(const
SwTable* pTable)
m_rDoc.GetAttrPool().GetItemSurrogates(aSurrogates, RES_BOXATR_FORMULA);
for (const SfxPoolItem* pItem : aSurrogates)
{
+ // SwTableBoxFormula is non-shareable, so const_cast is somewhat OK
auto pFormula =
const_cast<SwTableBoxFormula*>(pItem->DynamicWhichCast(RES_BOXATR_FORMULA));
if(!pFormula || !pFormula->GetDefinedIn() || pFormula->IsValid())
continue;
diff --git a/sw/source/core/doc/docfld.cxx b/sw/source/core/doc/docfld.cxx
index 317063be07f2..be30bb34bc30 100644
--- a/sw/source/core/doc/docfld.cxx
+++ b/sw/source/core/doc/docfld.cxx
@@ -631,12 +631,12 @@ void SwDoc::ChangeDBFields( const std::vector<OUString>&
rOldNames,
GetAttrPool().GetItemSurrogates(aSurrogates, nWhichHint);
for (const SfxPoolItem* pItem : aSurrogates)
{
- SwFormatField* pFormatField =
const_cast<SwFormatField*>(static_cast<const SwFormatField*>(pItem));
- SwTextField* pTextField = pFormatField->GetTextField();
+ const SwFormatField* pFormatField = static_cast<const
SwFormatField*>(pItem);
+ const SwTextField* pTextField = pFormatField->GetTextField();
if (!pTextField ||
!pTextField->GetTextNode().GetNodes().IsDocNodes())
continue;
- SwField* pField = pFormatField->GetField();
+ SwField*
pField(const_cast<SwFormatField*>(pFormatField)->GetField());
bool bExpand = false;
switch( pField->GetTyp()->Which() )
@@ -650,7 +650,8 @@ void SwDoc::ChangeDBFields( const std::vector<OUString>&
rOldNames,
SwDBFieldType* pTyp =
static_cast<SwDBFieldType*>(getIDocumentFieldsAccess().InsertFieldType(
SwDBFieldType(this, pOldTyp->GetColumnName(),
aNewDBData)));
- pFormatField->RegisterToFieldType( *pTyp );
+ // SwFormatField is non-shareable, so const_cast is
somewhat OK
+
const_cast<SwFormatField*>(pFormatField)->RegisterToFieldType( *pTyp );
pField->ChgTyp(pTyp);
static_cast<SwDBField*>(pField)->ClearInitialized();
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index 139f1fd205b6..d2543da6f057 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -641,6 +641,10 @@ void SwDoc::SetDefault( const SfxItemSet& rSet )
GetAttrPool().GetItemSurrogates(aSurrogates, RES_PARATR_TABSTOP);
for (const SfxPoolItem* pItem2 : aSurrogates)
{
+ // pItem2 and thus pTabStopItem is a evtl. shared & RefCounted
+ // Item and *should* not be changed that way.
lcl_SetNewDefTabStops
+ // seems to change pTabStopItem (!). This may need to be
changed
+ // to use iterateItemSurrogates and a defined write cycle.
if(auto pTabStopItem =
pItem2->DynamicWhichCast(RES_PARATR_TABSTOP))
bChg |= lcl_SetNewDefTabStops( nOldWidth, nNewWidth,
*const_cast<SvxTabStopItem*>(pTabStopItem) );
diff --git a/sw/source/core/table/swtable.cxx b/sw/source/core/table/swtable.cxx
index 9654c6dc5b61..6646737aeb03 100644
--- a/sw/source/core/table/swtable.cxx
+++ b/sw/source/core/table/swtable.cxx
@@ -1726,6 +1726,7 @@ void SwTable::UpdateFields(TableFormulaUpdateFlags eFlags)
pDoc->GetAttrPool().GetItemSurrogates(aSurrogates, RES_BOXATR_FORMULA);
for(const SfxPoolItem* pItem : aSurrogates)
{
+ // SwTableBoxFormula is non-shareable, so const_cast is somewhat OK
auto pBoxFormula =
const_cast<SwTableBoxFormula*>(pItem->DynamicWhichCast(RES_BOXATR_FORMULA));
if(pBoxFormula && pBoxFormula->GetDefinedIn())
{
