[Libreoffice-commits] core.git: include/svl svl/source

Noel Grandin (via logerrit) logerrit at kemper.freedesktop.org
Tue Nov 3 15:50:42 UTC 2020


 include/svl/SfxBroadcaster.hxx       |    8 ++-
 include/svl/lstner.hxx               |   14 +++++-
 svl/source/notify/SfxBroadcaster.cxx |   72 +++++++++++++----------------------
 svl/source/notify/lstner.cxx         |   65 +++++++++++++------------------
 4 files changed, 71 insertions(+), 88 deletions(-)

New commits:
commit 41f1b2bd1deaf67a08de240eb806189e122d9852
Author:     Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Mon Nov 2 22:13:50 2020 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue Nov 3 16:49:49 2020 +0100

    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 at collabora.co.uk>
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/include/svl/SfxBroadcaster.hxx b/include/svl/SfxBroadcaster.hxx
index bacef8d2d3ce..847643b8aceb 100644
--- a/include/svl/SfxBroadcaster.hxx
+++ b/include/svl/SfxBroadcaster.hxx
@@ -21,6 +21,7 @@
 
 #include <svl/svldllapi.h>
 #include <memory>
+#include <vector>
 
 class SfxListener;
 class SfxHint;
@@ -28,8 +29,9 @@ class SfxBroadcasterTest;
 
 class SVL_DLLPUBLIC SfxBroadcaster
 {
-    struct Impl;
-    std::unique_ptr<Impl> mpImpl;
+    /** Contains the positions of removed listeners. */
+    std::vector<size_t>     m_RemovedPositions;
+    std::vector<SfxListener*> m_Listeners;
 
 private:
     void                    AddListener( SfxListener& rListener );
@@ -41,7 +43,7 @@ protected:
 
 public:
 
-                            SfxBroadcaster();
+                            SfxBroadcaster() {}
                             SfxBroadcaster( const SfxBroadcaster &rBC );
     virtual                 ~SfxBroadcaster() COVERITY_NOEXCEPT_FALSE;
 
diff --git a/include/svl/lstner.hxx b/include/svl/lstner.hxx
index 1e3aa6becbdd..1b0e8806376b 100644
--- a/include/svl/lstner.hxx
+++ b/include/svl/lstner.hxx
@@ -20,7 +20,12 @@
 #define INCLUDED_SVL_LSTNER_HXX
 
 #include <svl/svldllapi.h>
+#include <map>
 #include <memory>
+#include <vector>
+#ifdef DBG_UTIL
+#include <sal/backtrace.hxx>
+#endif
 
 class SfxBroadcaster;
 class SfxHint;
@@ -37,15 +42,18 @@ enum class DuplicateHandling { Unexpected, Prevent, Allow };
 
 class SVL_DLLPUBLIC SfxListener
 {
-    struct Impl;
-    std::unique_ptr<Impl> mpImpl;
+    std::vector<SfxBroadcaster*> maBCs;
+#ifdef DBG_UTIL
+    std::map<SfxBroadcaster*, std::unique_ptr<sal::BacktraceState>>
+        maCallStacks;
+#endif
 
 private:
     const SfxListener&  operator=(const SfxListener &) = delete;
 
 public:
 
-                        SfxListener();
+                        SfxListener() {}
                         SfxListener( const SfxListener &rCopy );
     virtual             ~SfxListener() COVERITY_NOEXCEPT_FALSE;
 
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];
 }
 
 


More information about the Libreoffice-commits mailing list