i#114206 sd: unshare shape properties for the same type before insertion

Regression from commit 9bd99c08e8662becdd5ac8b47ef6f953c14e65fc
(CWS-TOOLING: integrate CWS os128, 2009-06-03), the problem was that the
SvxItemPropertySet was both used to store a property map (information
about UNO properties) and also as a descriptor for a not yet inserted
shape.

The above commit improved performance by sharing a single instance of an
SvxItemPropertySet for the same shape types: this works for the property
map, but doing the same for property values is problematic.

In practice, this eliminates the need for a workaround in oox/, the
user-visible problem was that loading a document with smartart, then
loading a document with group shapes (but without smartart) and saving
it would copy information from the first, closed document (!) to the
second document.

Just removing the oox/ workaround would make

make -C oox -sr CppunitTest_oox_drawingml CPPUNIT_TEST_NAME="testGroupShapeSmartArt testTdf131082"

fail, unsharing the descriptor piece makes it pass again.

Change-Id: Icdff2108ad0da18ce0ade081b1938dd74bc0ae90
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120996
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

1 files changed, 38 insertions, 43 deletions

diff --git a/editeng/source/uno/unoipset.cxx b/editeng/source/uno/unoipset.cxx
index 3f524805f4ed..da068755ce8d 100644
--- a/editeng/source/uno/unoipset.cxx
+++ b/editeng/source/uno/unoipset.cxx
@@ -31,14 +31,6 @@
 using namespace ::com::sun::star;
 
 
-struct SvxIDPropertyCombine
-{
-    sal_uInt16  nWID;
-    sal_uInt8   memberId;
-    uno::Any    aAny;
-};
-
-
 SvxItemPropertySet::SvxItemPropertySet( const SfxItemPropertyMapEntry* pMap, SfxItemPool& rItemPool )
 :   m_aPropertyMap( pMap ),
     mrItemPool( rItemPool )
@@ -48,35 +40,6 @@ SvxItemPropertySet::SvxItemPropertySet( const SfxItemPropertyMapEntry* pMap, Sfx
 
 SvxItemPropertySet::~SvxItemPropertySet()
 {
-    ClearAllUsrAny();
-}
-
-
-uno::Any* SvxItemPropertySet::GetUsrAnyForID(SfxItemPropertyMapEntry const & entry) const
-{
-    for (auto const & rActual : aCombineList)
-    {
-        if( rActual.nWID == entry.nWID && rActual.memberId == entry.nMemberId )
-            return const_cast<uno::Any*>(&rActual.aAny);
-    }
-    return nullptr;
-}
-
-
-void SvxItemPropertySet::AddUsrAnyForID(
-    const uno::Any& rAny, SfxItemPropertyMapEntry const & entry)
-{
-    SvxIDPropertyCombine aNew;
-    aNew.nWID = entry.nWID;
-    aNew.memberId = entry.nMemberId;
-    aNew.aAny = rAny;
-    aCombineList.push_back( std::move(aNew) );
-}
-
-
-void SvxItemPropertySet::ClearAllUsrAny()
-{
-    aCombineList.clear();
 }
 
 
@@ -184,10 +147,10 @@ void SvxItemPropertySet::setPropertyValue( const SfxItemPropertyMapEntry* pMap,
 }
 
 
-uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pMap ) const
+uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pMap, SvxItemPropertySetUsrAnys& rAnys ) const
 {
     // Already entered a value? Then finish quickly
-    uno::Any* pUsrAny = GetUsrAnyForID(*pMap);
+    uno::Any* pUsrAny = rAnys.GetUsrAnyForID(*pMap);
     if(pUsrAny)
         return *pUsrAny;
 
@@ -213,7 +176,7 @@ uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pM
         if(eState >= SfxItemState::DEFAULT && pItem)
         {
             pItem->QueryValue( aVal, nMemberId );
-            const_cast<SvxItemPropertySet*>(this)->AddUsrAnyForID(aVal, *pMap);
+            rAnys.AddUsrAnyForID(aVal, *pMap);
         }
     }
 
@@ -236,11 +199,11 @@ uno::Any SvxItemPropertySet::getPropertyValue( const SfxItemPropertyMapEntry* pM
 }
 
 
-void SvxItemPropertySet::setPropertyValue( const SfxItemPropertyMapEntry* pMap, const uno::Any& rVal ) const
+void SvxItemPropertySet::setPropertyValue( const SfxItemPropertyMapEntry* pMap, const uno::Any& rVal, SvxItemPropertySetUsrAnys& rAnys )
 {
-    uno::Any* pUsrAny = GetUsrAnyForID(*pMap);
+    uno::Any* pUsrAny = rAnys.GetUsrAnyForID(*pMap);
     if(!pUsrAny)
-        const_cast<SvxItemPropertySet*>(this)->AddUsrAnyForID(rVal, *pMap);
+        rAnys.AddUsrAnyForID(rVal, *pMap);
     else
         *pUsrAny = rVal;
 }
@@ -335,4 +298,36 @@ void SvxUnoConvertFromMM( const MapUnit eDestinationMapUnit, uno::Any & rMetric
     }
 }
 
+SvxItemPropertySetUsrAnys::SvxItemPropertySetUsrAnys() = default;
+
+SvxItemPropertySetUsrAnys::~SvxItemPropertySetUsrAnys()
+{
+    ClearAllUsrAny();
+}
+
+uno::Any* SvxItemPropertySetUsrAnys::GetUsrAnyForID(SfxItemPropertyMapEntry const & entry) const
+{
+    for (auto const & rActual : aCombineList)
+    {
+        if( rActual.nWID == entry.nWID && rActual.memberId == entry.nMemberId )
+            return const_cast<uno::Any*>(&rActual.aAny);
+    }
+    return nullptr;
+}
+
+void SvxItemPropertySetUsrAnys::AddUsrAnyForID(
+    const uno::Any& rAny, SfxItemPropertyMapEntry const & entry)
+{
+    SvxIDPropertyCombine aNew;
+    aNew.nWID = entry.nWID;
+    aNew.memberId = entry.nMemberId;
+    aNew.aAny = rAny;
+    aCombineList.push_back( std::move(aNew) );
+}
+
+void SvxItemPropertySetUsrAnys::ClearAllUsrAny()
+{
+    aCombineList.clear();
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */