remove pimpl in SfxBroadcaster/SfxListener

and provide an optimised copy constructor, we can avoid a bunch of work.

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

2 files changed, 55 insertions, 82 deletions

diff --git a/svl/source/notify/SfxBroadcaster.cxx b/svl/source/notify/SfxBroadcaster.cxx
index 2e312a80ce85..6e1629bff9d5 100644
--- a/svl/source/notify/SfxBroadcaster.cxx
+++ b/svl/source/notify/SfxBroadcaster.cxx
@@ -28,23 +28,14 @@
 #include <vector>
 
 
-typedef std::vector<SfxListener*> SfxListenerArr_Impl;
-
-struct SfxBroadcaster::Impl
-{
-    /** Contains the positions of removed listeners. */
-    std::vector<size_t>     m_RemovedPositions;
-    SfxListenerArr_Impl     m_Listeners;
-};
-
 // broadcast immediately
 
 void SfxBroadcaster::Broadcast( const SfxHint &rHint )
 {
     // notify all registered listeners exactly once
-    for (size_t i = 0; i < mpImpl->m_Listeners.size(); ++i)
+    for (size_t i = 0; i < m_Listeners.size(); ++i)
     {
-        SfxListener *const pListener = mpImpl->m_Listeners[i];
+        SfxListener *const pListener = m_Listeners[i];
         if (pListener)
             pListener->Notify( *this, rHint );
     }
@@ -57,30 +48,22 @@ SfxBroadcaster::~SfxBroadcaster() COVERITY_NOEXCEPT_FALSE
     Broadcast( SfxHint(SfxHintId::Dying) );
 
     // remove all still registered listeners
-    for (size_t i = 0; i < mpImpl->m_Listeners.size(); ++i)
+    for (size_t i = 0; i < m_Listeners.size(); ++i)
     {
-        SfxListener *const pListener = mpImpl->m_Listeners[i];
+        SfxListener *const pListener = m_Listeners[i];
         if (pListener)
             pListener->RemoveBroadcaster_Impl(*this);
     }
 }
 
 
-// simple ctor of class SfxBroadcaster
-
-SfxBroadcaster::SfxBroadcaster() : mpImpl(new Impl)
-{
-}
-
-
 // copy ctor of class SfxBroadcaster
 
-
-SfxBroadcaster::SfxBroadcaster( const SfxBroadcaster &rBC ) : mpImpl(new Impl)
+SfxBroadcaster::SfxBroadcaster( const SfxBroadcaster &rOther )
 {
-    for (size_t i = 0; i < rBC.mpImpl->m_Listeners.size(); ++i)
+    for (size_t i = 0; i < rOther.m_Listeners.size(); ++i)
     {
-        SfxListener *const pListener = rBC.mpImpl->m_Listeners[i];
+        SfxListener *const pListener = rOther.m_Listeners[i];
         if (pListener)
             pListener->StartListening( *this );
     }
@@ -92,16 +75,16 @@ SfxBroadcaster::SfxBroadcaster( const SfxBroadcaster &rBC ) : mpImpl(new Impl)
 void SfxBroadcaster::AddListener( SfxListener& rListener )
 {
     DBG_TESTSOLARMUTEX();
-    if (mpImpl->m_RemovedPositions.empty())
+    if (m_RemovedPositions.empty())
     {
-        mpImpl->m_Listeners.push_back(&rListener);
+        m_Listeners.push_back(&rListener);
     }
     else
     {
-        size_t targetPosition = mpImpl->m_RemovedPositions.back();
-        mpImpl->m_RemovedPositions.pop_back();
-        assert(mpImpl->m_Listeners[targetPosition] == nullptr);
-        mpImpl->m_Listeners[targetPosition] = &rListener;
+        size_t targetPosition = m_RemovedPositions.back();
+        m_RemovedPositions.pop_back();
+        assert(m_Listeners[targetPosition] == nullptr);
+        m_Listeners[targetPosition] = &rListener;
     }
 }
 
@@ -110,9 +93,9 @@ void SfxBroadcaster::AddListener( SfxListener& rListener )
 
 void SfxBroadcaster::Forward(SfxBroadcaster& rBC, const SfxHint& rHint)
 {
-    for (size_t i = 0; i < mpImpl->m_Listeners.size(); ++i)
+    for (size_t i = 0; i < m_Listeners.size(); ++i)
     {
-        SfxListener *const pListener = mpImpl->m_Listeners[i];
+        SfxListener *const pListener = m_Listeners[i];
         if (pListener)
             pListener->Notify( rBC, rHint );
     }
@@ -128,14 +111,14 @@ void SfxBroadcaster::RemoveListener( SfxListener& rListener )
     // First, check the slots either side of the last removed slot, makes a significant
     // difference when the list is large.
     int positionOfRemovedElement = -1;
-    if (!mpImpl->m_RemovedPositions.empty())
+    if (!m_RemovedPositions.empty())
     {
-        auto i = mpImpl->m_RemovedPositions.back();
-        if (i < mpImpl->m_Listeners.size() - 2 && mpImpl->m_Listeners[i+1] == &rListener)
+        auto i = m_RemovedPositions.back();
+        if (i < m_Listeners.size() - 2 && m_Listeners[i+1] == &rListener)
         {
             positionOfRemovedElement = i + 1;
         }
-        else if (i > 0 && mpImpl->m_Listeners[i-1] == &rListener)
+        else if (i > 0 && m_Listeners[i-1] == &rListener)
         {
             positionOfRemovedElement = i-1;
         }
@@ -143,14 +126,13 @@ void SfxBroadcaster::RemoveListener( SfxListener& rListener )
     // then scan the whole list if we didn't find it
     if (positionOfRemovedElement == -1)
     {
-        SfxListenerArr_Impl::iterator aIter = std::find(
-                mpImpl->m_Listeners.begin(), mpImpl->m_Listeners.end(), &rListener);
-        positionOfRemovedElement = std::distance(mpImpl->m_Listeners.begin(), aIter);
+        auto aIter = std::find(m_Listeners.begin(), m_Listeners.end(), &rListener);
+        positionOfRemovedElement = std::distance(m_Listeners.begin(), aIter);
     }
     // DO NOT erase the listener, set the pointer to 0
     // because the current continuation may contain this->Broadcast
-    mpImpl->m_Listeners[positionOfRemovedElement] = nullptr;
-    mpImpl->m_RemovedPositions.push_back(positionOfRemovedElement);
+    m_Listeners[positionOfRemovedElement] = nullptr;
+    m_RemovedPositions.push_back(positionOfRemovedElement);
 }
 
 bool SfxBroadcaster::HasListeners() const
@@ -160,18 +142,18 @@ bool SfxBroadcaster::HasListeners() const
 
 size_t SfxBroadcaster::GetListenerCount() const
 {
-    assert(mpImpl->m_Listeners.size() >= mpImpl->m_RemovedPositions.size());
-    return mpImpl->m_Listeners.size() - mpImpl->m_RemovedPositions.size();
+    assert(m_Listeners.size() >= m_RemovedPositions.size());
+    return m_Listeners.size() - m_RemovedPositions.size();
 }
 
 size_t SfxBroadcaster::GetSizeOfVector() const
 {
-    return mpImpl->m_Listeners.size();
+    return m_Listeners.size();
 }
 
 SfxListener* SfxBroadcaster::GetListener( size_t nNo ) const
 {
-    return mpImpl->m_Listeners[nNo];
+    return m_Listeners[nNo];
 }
 
 
diff --git a/svl/source/notify/lstner.cxx b/svl/source/notify/lstner.cxx
index fe6f2b8af4e2..7a037a1b87db 100644
--- a/svl/source/notify/lstner.cxx
+++ b/svl/source/notify/lstner.cxx
@@ -29,27 +29,18 @@
 #include <memory>
 #include <map>
 
-struct SfxListener::Impl
-{
-    std::vector<SfxBroadcaster*> maBCs;
-#ifdef DBG_UTIL
-    std::map<SfxBroadcaster*, std::unique_ptr<sal::BacktraceState>>
-        maCallStacks;
-#endif
-};
-
-// simple ctor of class SfxListener
-
-SfxListener::SfxListener() : mpImpl(new Impl)
-{
-}
-
 // copy ctor of class SfxListener
 
-SfxListener::SfxListener( const SfxListener &rListener ) : mpImpl(new Impl)
+SfxListener::SfxListener( const SfxListener &rOther )
+    : maBCs( rOther.maBCs )
 {
-    for ( size_t n = 0; n < rListener.mpImpl->maBCs.size(); ++n )
-        StartListening( *rListener.mpImpl->maBCs[n] );
+    for ( size_t n = 0; n < maBCs.size(); ++n )
+    {
+        maBCs[n]->AddListener(*this);
+#ifdef DBG_UTIL
+        maCallStacks.emplace( maBCs[n], sal::backtrace_get(10) );
+#endif
+    }
 }
 
 // unregisters the SfxListener from its SfxBroadcasters
@@ -57,9 +48,9 @@ SfxListener::SfxListener( const SfxListener &rListener ) : mpImpl(new Impl)
 SfxListener::~SfxListener() COVERITY_NOEXCEPT_FALSE
 {
     // unregister at all remaining broadcasters
-    for ( size_t nPos = 0; nPos < mpImpl->maBCs.size(); ++nPos )
+    for ( size_t nPos = 0; nPos < maBCs.size(); ++nPos )
     {
-        SfxBroadcaster *pBC = mpImpl->maBCs[nPos];
+        SfxBroadcaster *pBC = maBCs[nPos];
         pBC->RemoveListener(*this);
     }
 }
@@ -69,11 +60,11 @@ SfxListener::~SfxListener() COVERITY_NOEXCEPT_FALSE
 
 void SfxListener::RemoveBroadcaster_Impl( SfxBroadcaster& rBroadcaster )
 {
-    auto it = std::find( mpImpl->maBCs.begin(), mpImpl->maBCs.end(), &rBroadcaster );
-    if (it != mpImpl->maBCs.end()) {
-        mpImpl->maBCs.erase( it );
+    auto it = std::find( maBCs.begin(), maBCs.end(), &rBroadcaster );
+    if (it != maBCs.end()) {
+        maBCs.erase( it );
 #ifdef DBG_UTIL
-        mpImpl->maCallStacks.erase( &rBroadcaster );
+        maCallStacks.erase( &rBroadcaster );
 #endif
     }
 }
@@ -93,7 +84,7 @@ void SfxListener::StartListening(SfxBroadcaster& rBroadcaster, DuplicateHandling
 #ifdef DBG_UTIL
     if (bListeningAlready && eDuplicateHanding == DuplicateHandling::Unexpected)
     {
-        auto f = mpImpl->maCallStacks.find( &rBroadcaster );
+        auto f = maCallStacks.find( &rBroadcaster );
         SAL_WARN("svl", "previous StartListening call came from: " << sal::backtrace_to_string(f->second.get()));
     }
 #endif
@@ -102,9 +93,9 @@ void SfxListener::StartListening(SfxBroadcaster& rBroadcaster, DuplicateHandling
     if (!bListeningAlready || eDuplicateHanding != DuplicateHandling::Prevent)
     {
         rBroadcaster.AddListener(*this);
-        mpImpl->maBCs.push_back( &rBroadcaster );
+        maBCs.push_back( &rBroadcaster );
 #ifdef DBG_UTIL
-        mpImpl->maCallStacks.emplace( &rBroadcaster, sal::backtrace_get(10) );
+        maCallStacks.emplace( &rBroadcaster, sal::backtrace_get(10) );
 #endif
         assert(IsListening(rBroadcaster) && "StartListening failed");
     }
@@ -114,18 +105,18 @@ void SfxListener::StartListening(SfxBroadcaster& rBroadcaster, DuplicateHandling
 
 void SfxListener::EndListening( SfxBroadcaster& rBroadcaster, bool bRemoveAllDuplicates )
 {
-    auto beginIt = mpImpl->maBCs.begin();
+    auto beginIt = maBCs.begin();
     do
     {
-        auto it = std::find( beginIt, mpImpl->maBCs.end(), &rBroadcaster );
-        if ( it == mpImpl->maBCs.end() )
+        auto it = std::find( beginIt, maBCs.end(), &rBroadcaster );
+        if ( it == maBCs.end() )
         {
             break;
         }
         rBroadcaster.RemoveListener(*this);
-        beginIt = mpImpl->maBCs.erase( it );
+        beginIt = maBCs.erase( it );
 #ifdef DBG_UTIL
-        mpImpl->maCallStacks.erase( &rBroadcaster );
+        maCallStacks.erase( &rBroadcaster );
 #endif
     }
     while ( bRemoveAllDuplicates );
@@ -137,28 +128,28 @@ void SfxListener::EndListening( SfxBroadcaster& rBroadcaster, bool bRemoveAllDup
 void SfxListener::EndListeningAll()
 {
     std::vector<SfxBroadcaster*> aBroadcasters;
-    std::swap(mpImpl->maBCs, aBroadcasters);
+    std::swap(maBCs, aBroadcasters);
     for (SfxBroadcaster *pBC : aBroadcasters)
         pBC->RemoveListener(*this);
 #ifdef DBG_UTIL
-    mpImpl->maCallStacks.clear();
+    maCallStacks.clear();
 #endif
 }
 
 
 bool SfxListener::IsListening( SfxBroadcaster& rBroadcaster ) const
 {
-    return mpImpl->maBCs.end() != std::find( mpImpl->maBCs.begin(), mpImpl->maBCs.end(), &rBroadcaster );
+    return maBCs.end() != std::find( maBCs.begin(), maBCs.end(), &rBroadcaster );
 }
 
 sal_uInt16 SfxListener::GetBroadcasterCount() const
 {
-    return mpImpl->maBCs.size();
+    return maBCs.size();
 }
 
 SfxBroadcaster* SfxListener::GetBroadcasterJOE( sal_uInt16 nNo ) const
 {
-    return mpImpl->maBCs[nNo];
+    return maBCs[nNo];
 }