reduce number of SvxUnoShape listeners on SdrModel

when we have a lot of shapes, the number of listeners attached to
SdrModel becomes a serious bottleneck because of the number of
broadcast/notify function calls.

So
(1) make SvxUnoShape listen to the SdrObject
which achieves the same end, and reduces the bottleneck on the SdrModel.
(2) repurpose the maAllIncarnatedObjects set, which tracks all
objects linked to the SdrModel, so we can shut down the associated
UNO shapes during ClearModel. There is no other good way of doing
this, and if we don't do it, we get various use-after-free
issues during shutdown.

This takes the load time of a large DOCX with lots of shapes,
from 35s to 17s on my machine.

Change-Id: I34a6ac35c90de8b7103a7373aafe6e9607cc01c4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169612
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins

3 files changed, 29 insertions, 35 deletions

diff --git a/svx/source/svdraw/svdmodel.cxx b/svx/source/svdraw/svdmodel.cxx
index 835689a00726..3c43cf2c81bc 100644
--- a/svx/source/svdraw/svdmodel.cxx
+++ b/svx/source/svdraw/svdmodel.cxx
@@ -44,6 +44,7 @@
 #include <svx/svdpool.hxx>
 #include <svx/svdobj.hxx>
 #include <svx/svdotext.hxx>
+#include <svx/unoshape.hxx>
 #include <textchain.hxx>
 #include <svx/svdetc.hxx>
 #include <svx/svdoutl.hxx>
@@ -222,7 +223,6 @@ SdrModel::~SdrModel()
     implDtorClearModel();
 
 #ifdef DBG_UTIL
-    // SdrObjectLifetimeWatchDog:
     if(!maAllIncarnatedObjects.empty())
     {
         SAL_WARN("svx",
@@ -580,6 +580,25 @@ void SdrModel::ClearModel(bool bCalledFromDestructor)
         mbInDestruction = true;
     }
 
+    // Disconnect all SvxShape's from their SdrObjects to prevent the SdrObjects
+    // from hanging around and causing use-after-free.
+    // Make a copy because it might modified during InvalidateSdrObject calls.
+    std::vector<rtl::Reference<SdrObject>> allObjs(maAllIncarnatedObjects.begin(), maAllIncarnatedObjects.end());
+    for (const auto & pSdrObj : allObjs)
+    {
+        uno::Reference<uno::XInterface> xShape = pSdrObj->getWeakUnoShape().get();
+        rtl::Reference<SvxShape> pSvxShape = dynamic_cast<SvxShape*>(xShape.get());
+        // calling getWeakUnoShape so we don't accidentally create new UNO shapes
+        if (pSvxShape)
+            pSvxShape->InvalidateSdrObject();
+        else
+        {
+            // because some things like SwXShape don't subclass SvxShape
+            uno::Reference<lang::XComponent> xComp(xShape, uno::UNO_QUERY);
+            if (xComp)
+                xComp->dispose();
+        }
+    }
     sal_Int32 i;
     // delete all drawing pages
     sal_Int32 nCount=GetPageCount();
diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx
index 6c8330185f0a..1b768aee12d9 100644
--- a/svx/source/svdraw/svdobj.cxx
+++ b/svx/source/svdraw/svdobj.cxx
@@ -321,8 +321,6 @@ void SdrObject::SetBoundRectDirty()
     resetOutRectangle();
 }
 
-#ifdef DBG_UTIL
-// SdrObjectLifetimeWatchDog:
 void impAddIncarnatedSdrObjectToSdrModel(SdrObject& rSdrObject, SdrModel& rSdrModel)
 {
     rSdrModel.maAllIncarnatedObjects.insert(&rSdrObject);
@@ -334,7 +332,6 @@ void impRemoveIncarnatedSdrObjectToSdrModel(SdrObject& rSdrObject, SdrModel& rSd
         assert(false && "SdrObject::~SdrObject: Destructed incarnation of SdrObject not member of this SdrModel (!)");
     }
 }
-#endif
 
 SdrObject::SdrObject(SdrModel& rSdrModel)
     : mpFillGeometryDefiningShape(nullptr)
@@ -369,10 +366,7 @@ SdrObject::SdrObject(SdrModel& rSdrModel)
     m_bIs3DObj=false;
     m_bMarkProt=false;
     m_bIsUnoObj=false;
-#ifdef DBG_UTIL
-    // SdrObjectLifetimeWatchDog:
     impAddIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
-#endif
 }
 
 SdrObject::SdrObject(SdrModel& rSdrModel, SdrObject const & rSource)
@@ -440,10 +434,7 @@ SdrObject::SdrObject(SdrModel& rSdrModel, SdrObject const & rSource)
     m_pGrabBagItem.reset();
     if (rSource.m_pGrabBagItem!=nullptr)
         m_pGrabBagItem.reset(rSource.m_pGrabBagItem->Clone());
-#ifdef DBG_UTIL
-    // SdrObjectLifetimeWatchDog:
     impAddIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
-#endif
 }
 
 SdrObject::~SdrObject()
@@ -470,10 +461,7 @@ SdrObject::~SdrObject()
     m_pGrabBagItem.reset();
     mpProperties.reset();
     mpViewContact.reset();
-#ifdef DBG_UTIL
-    // SdrObjectLifetimeWatchDog:
     impRemoveIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
-#endif
 }
 
 void SdrObject::acquire() noexcept
diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx
index 5c3807817a6f..b547db4b436a 100644
--- a/svx/source/unodraw/unoshape.cxx
+++ b/svx/source/unodraw/unoshape.cxx
@@ -192,7 +192,7 @@ SvxShape::~SvxShape() noexcept
 
     if ( mxSdrObject )
     {
-        EndListening(mxSdrObject->getSdrModelFromSdrObject());
+        mxSdrObject->RemoveListener(*this);
         mxSdrObject->setUnoShape(nullptr);
         mxSdrObject.clear();
     }
@@ -205,7 +205,7 @@ void SvxShape::InvalidateSdrObject()
 {
     if(mxSdrObject)
     {
-        EndListening(mxSdrObject->getSdrModelFromSdrObject());
+        mxSdrObject->RemoveListener(*this);
         mxSdrObject.clear();
     }
 };
@@ -285,7 +285,7 @@ void SvxShape::impl_construct()
 {
     if ( HasSdrObject() )
     {
-        StartListening(GetSdrObject()->getSdrModelFromSdrObject());
+        GetSdrObject()->AddListener(*this);
         impl_initFromSdrObject();
     }
 }
@@ -355,14 +355,14 @@ void SvxShape::Create( SdrObject* pNewObj, SvxDrawPage* /*pNewPage*/ )
 
     if( HasSdrObject() )
     {
-        EndListening( GetSdrObject()->getSdrModelFromSdrObject() );
+        GetSdrObject()->RemoveListener( *this );
     }
 
     mxSdrObject = pNewObj;
 
     if( HasSdrObject() )
     {
-        StartListening( GetSdrObject()->getSdrModelFromSdrObject() );
+        GetSdrObject()->AddListener( *this );
     }
 
     OSL_ENSURE( !mbIsMultiPropertyCall, "SvxShape::Create: hmm?" );
@@ -929,8 +929,7 @@ void SvxShape::Notify( SfxBroadcaster&, const SfxHint& rHint ) noexcept
         return;
     const SdrHint* pSdrHint = static_cast<const SdrHint*>(&rHint);
     // #i55919# SdrHintKind::ObjectChange is only interesting if it's for this object
-    if ((pSdrHint->GetKind() != SdrHintKind::ModelCleared) &&
-         (pSdrHint->GetKind() != SdrHintKind::ObjectChange || pSdrHint->GetObject() != mxSdrObject.get() ))
+    if (pSdrHint->GetKind() != SdrHintKind::ObjectChange || pSdrHint->GetObject() != mxSdrObject.get())
         return;
 
     // prevent object being deleted from under us
@@ -938,24 +937,12 @@ void SvxShape::Notify( SfxBroadcaster&, const SfxHint& rHint ) noexcept
     uno::Reference< uno::XInterface > xSelf( mxSdrObject->getWeakUnoShape() );
     if( !xSelf.is() )
     {
-        EndListening(mxSdrObject->getSdrModelFromSdrObject());
+        mxSdrObject->RemoveListener(*this);
         mxSdrObject.clear();
         return;
     }
 
-    if (pSdrHint->GetKind() == SdrHintKind::ObjectChange)
-    {
-        updateShapeKind();
-    }
-    else // (pSdrHint->GetKind() == SdrHintKind::ModelCleared)
-    {
-        EndListening(mxSdrObject->getSdrModelFromSdrObject());
-        mxSdrObject->setUnoShape(nullptr);
-        mxSdrObject.clear();
-
-        if(!mpImpl->mbDisposing)
-            dispose();
-    }
+    updateShapeKind();
 }
 
 // XShape
@@ -1190,7 +1177,7 @@ void SAL_CALL SvxShape::dispose()
     if (!pObject)
         return;
 
-    EndListening( pObject->getSdrModelFromSdrObject() );
+    pObject->RemoveListener( *this );
 
     if ( pObject->IsInserted() && pObject->getSdrPageFromSdrObject() )
     {