SvtBroadcaster unify the normal and PrepareForDestruction paths

since this approach is better in that it avoids O(n^2) behaviour
of lots of listeners deleting from a std::vector, lets just
always use this approach.

Change-Id: I9204996ee8c9379ac71dfc168a6c1fc653e63a8e
Reviewed-on: https://gerrit.libreoffice.org/61204
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

1 files changed, 40 insertions, 32 deletions

diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 626a48a332e9..7e67de0bcdf1 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -40,9 +40,8 @@ void SvtBroadcaster::Normalize() const
 
 void SvtBroadcaster::Add( SvtListener* p )
 {
-    assert(!mbDisposing && "called inside my own destructor?");
-    assert(!mbAboutToDie && "called after PrepareForDestruction()?");
-    if (mbDisposing || mbAboutToDie)
+    assert(!mbAboutToDie && "called inside my own destructor / after PrepareForDestruction()?");
+    if (mbAboutToDie)
         return;
     // only reset mbNormalized if we are going to become unsorted
     if (!maListeners.empty() && maListeners.back() > p)
@@ -52,13 +51,12 @@ void SvtBroadcaster::Add( SvtListener* p )
 
 void SvtBroadcaster::Remove( SvtListener* p )
 {
-    if (mbDisposing)
-        return;
-
     if (mbAboutToDie)
     {
+        // only reset mbDestNormalized if we are going to become unsorted
+        if (!maDestructedListeners.empty() && maDestructedListeners.back() > p)
+            mbDestNormalized = false;
         maDestructedListeners.push_back(p);
-        mbDestNormalized = false;
         return;
     }
 
@@ -75,14 +73,13 @@ void SvtBroadcaster::Remove( SvtListener* p )
         ListenersGone();
 }
 
-SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbDisposing(false), mbNormalized(true), mbDestNormalized(true) {}
+SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbNormalized(true), mbDestNormalized(true) {}
 
 SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
-    mbAboutToDie(false), mbDisposing(false),
+    mbAboutToDie(false),
     mbNormalized(true), mbDestNormalized(true)
 {
-    assert(!rBC.mbAboutToDie && "copying an object marked with PrepareForDestruction()?");
-    assert(!rBC.mbDisposing && "copying an object that is in it's destructor?");
+    assert(!rBC.mbAboutToDie && "copying an object marked with PrepareForDestruction() / that is in it's destructor?");
 
     rBC.Normalize(); // so that insert into ourself is in-order, and therefore we do not need to Normalize()
     maListeners.reserve(rBC.maListeners.size());
@@ -92,41 +89,52 @@ SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
 
 SvtBroadcaster::~SvtBroadcaster()
 {
-    mbDisposing = true;
-    Broadcast( SfxHint(SfxHintId::Dying) );
+    PrepareForDestruction();
+
+    Normalize();
+
+    {
+        SfxHint aHint(SfxHintId::Dying);
+        ListenersType::const_iterator dest(maDestructedListeners.begin());
+        for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
+        {
+            // skip the destructed ones
+            while (dest != maDestructedListeners.end() && (*dest < *it))
+                ++dest;
+
+            if (dest == maDestructedListeners.end() || *dest != *it)
+                (*it)->Notify(aHint);
+        }
+    }
 
     Normalize();
 
     // now when both lists are sorted, we can linearly unregister all
     // listeners, with the exception of those that already asked to be removed
     // during their own destruction
-    ListenersType::const_iterator dest(maDestructedListeners.begin());
-    for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
     {
-        // skip the destructed ones
-        while (dest != maDestructedListeners.end() && (*dest < *it))
-            ++dest;
-
-        if (dest == maDestructedListeners.end() || *dest != *it)
-            (*it)->BroadcasterDying(*this);
+        ListenersType::const_iterator dest(maDestructedListeners.begin());
+        for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
+        {
+            // skip the destructed ones
+            while (dest != maDestructedListeners.end() && (*dest < *it))
+                ++dest;
+
+            if (dest == maDestructedListeners.end() || *dest != *it)
+                (*it)->BroadcasterDying(*this);
+        }
     }
 }
 
 void SvtBroadcaster::Broadcast( const SfxHint &rHint )
 {
-    Normalize();
+    assert(!mbAboutToDie && "broadcasting after PrepareForDestruction() / from inside my own destructor?");
+    if (mbAboutToDie)
+        return;
 
-    ListenersType::const_iterator dest(maDestructedListeners.begin());
     ListenersType aListeners(maListeners); // this copy is important to avoid erasing entries while iterating
-    for (ListenersType::iterator it(aListeners.begin()); it != aListeners.end(); ++it)
-    {
-        // skip the destructed ones
-        while (dest != maDestructedListeners.end() && (*dest < *it))
-            ++dest;
-
-        if (dest == maDestructedListeners.end() || *dest != *it)
-            (*it)->Notify(rHint);
-    }
+    for (SvtListener * p : aListeners)
+        p->Notify(rHint);
 }
 
 void SvtBroadcaster::ListenersGone() {}