Resolves: fdo#70703 guard against FlushImpl inside FlushImpl

where the inner one deletes Shells that the outer one is still
processing. Push the candidates onto a stack and let inner
FlushImpl modify them to inform outer FlushImpl's that an entry
has been deleted

Change-Id: I1db8546d53e24cc96c72f2cd5cbec57b6cecaff5
Reviewed-on: https://gerrit.libreoffice.org/6735
Reviewed-by: Michael Stahl <mstahl@redhat.com>
Tested-by: Michael Stahl <mstahl@redhat.com>

2 files changed, 68 insertions, 37 deletions

diff --git a/include/sfx2/dispatch.hxx b/include/sfx2/dispatch.hxx
index 2b02a5e620c6..4cf5fb176630 100644
--- a/include/sfx2/dispatch.hxx
+++ b/include/sfx2/dispatch.hxx
@@ -27,6 +27,7 @@
 
 #include <sfx2/bindings.hxx>
 #include <sfx2/viewfrm.hxx>
+#include <deque>
 #include <map>
 #include <vector>
 
@@ -76,10 +77,38 @@ public:
     }
 };
 
+struct SfxToDo_Impl
+{
+    SfxShell*  pCluster;
+    bool       bPush;
+    bool       bDelete;
+    bool       bDeleted;
+    bool       bUntil;
+
+    SfxToDo_Impl()
+        : pCluster(0)
+        , bPush(false)
+        , bDelete(false)
+        , bDeleted(false)
+        , bUntil(false)
+                {}
+    SfxToDo_Impl( bool bOpPush, bool bOpDelete, bool bOpUntil, SfxShell& rCluster )
+        : pCluster(&rCluster)
+        , bPush(bOpPush)
+        , bDelete(bOpDelete)
+        , bDeleted(false)
+        , bUntil(bOpUntil)
+                {}
+
+    bool operator==( const SfxToDo_Impl& rWith ) const
+    { return pCluster==rWith.pCluster && bPush==rWith.bPush; }
+};
+
 class SFX2_DLLPUBLIC SfxDispatcher
 {
-    SfxDispatcher_Impl*             pImp;
-    sal_Bool                            bFlushed;
+    SfxDispatcher_Impl*      pImp;
+    sal_Bool                 bFlushed;
+    std::deque< std::deque<SfxToDo_Impl> > aToDoCopyStack;
 
 private:
     // Search for temporary evaluated Todos
diff --git a/sfx2/source/control/dispatch.cxx b/sfx2/source/control/dispatch.cxx
index 9acf705ee3be..3e10fff6be1f 100644
--- a/sfx2/source/control/dispatch.cxx
+++ b/sfx2/source/control/dispatch.cxx
@@ -67,31 +67,6 @@ DBG_NAME(SfxDispatcherFillState)
 
 typedef std::vector<SfxRequest*> SfxRequestPtrArray;
 
-struct SfxToDo_Impl
-{
-    SfxShell*  pCluster;
-    bool       bPush;
-    bool       bDelete;
-    bool       bUntil;
-
-    SfxToDo_Impl()
-        : pCluster(0)
-        , bPush(false)
-        , bDelete(false)
-        , bUntil(false)
-                {}
-    SfxToDo_Impl( bool bOpPush, bool bOpDelete, bool bOpUntil, SfxShell& rCluster )
-        : pCluster(&rCluster)
-        , bPush(bOpPush)
-        , bDelete(bOpDelete)
-        , bUntil(bOpUntil)
-                {}
-    ~SfxToDo_Impl(){}
-
-    bool operator==( const SfxToDo_Impl& rWith ) const
-    { return pCluster==rWith.pCluster && bPush==rWith.bPush; }
-};
-
 struct SfxObjectBars_Impl
 {
     sal_uInt32     nResId;  // Resource - and ConfigId of the Toolbox
@@ -1625,22 +1600,49 @@ void SfxDispatcher::FlushImpl()
     bFlushed = sal_True;
     OSL_TRACE("Successfully flushed dispatcher!");
 
+    //fdo#70703 FlushImpl may call back into itself so use aToDoCopyStack to talk
+    //to outer levels of ourself. If DoActivate_Impl/DoDeactivate_Impl deletes
+    //an entry, then they will walk back up aToDoCopyStack and set outer
+    //levels's entries to bDeleted
+    aToDoCopyStack.push_back(aToDoCopy);
+    std::deque<SfxToDo_Impl>& rToDoCopy = aToDoCopyStack.back();
     // Activate the Shells and possible delete them in the 2nd round
-    for(std::deque<SfxToDo_Impl>::reverse_iterator i = aToDoCopy.rbegin(); i != aToDoCopy.rend(); ++i)
-    {
-        if(i->bPush)
-        {
-            if ( pImp->bActive )
-                i->pCluster->DoActivate_Impl(pImp->pFrame, sal_True);
-        }
-        else if ( pImp->bActive )
-                i->pCluster->DoDeactivate_Impl(pImp->pFrame, sal_True);
+    for(std::deque<SfxToDo_Impl>::reverse_iterator i = rToDoCopy.rbegin(); i != rToDoCopy.rend(); ++i)
+    {
+        if (i->bDeleted)
+            continue;
+        if (!pImp->bActive)
+            continue;
+        if (i->bPush)
+            i->pCluster->DoActivate_Impl(pImp->pFrame, sal_True);
+        else
+            i->pCluster->DoDeactivate_Impl(pImp->pFrame, sal_True);
     }
 
+    aToDoCopy = aToDoCopyStack.back();
+    aToDoCopyStack.pop_back();
+
     for(std::deque<SfxToDo_Impl>::reverse_iterator i = aToDoCopy.rbegin(); i != aToDoCopy.rend(); ++i)
     {
-        if(i->bDelete)
+        if (i->bDelete && !i->bDeleted)
+        {
+            if (!aToDoCopyStack.empty())
+            {
+                //fdo#70703 if there is an outer FlushImpl then inform it that
+                //we have deleted this cluster
+                for (std::deque< std::deque<SfxToDo_Impl> >::iterator aI = aToDoCopyStack.begin();
+                    aI != aToDoCopyStack.end(); ++aI)
+                {
+                    std::deque<SfxToDo_Impl> &v = *aI;
+                    for(std::deque<SfxToDo_Impl>::iterator aJ = v.begin(); aJ != v.end(); ++aJ)
+                    {
+                        if (aJ->pCluster == i->pCluster)
+                            aJ->bDeleted = true;
+                    }
+                }
+            }
             delete i->pCluster;
+        }
     }
     sal_Bool bAwakeBindings = !aToDoCopy.empty();
     if( bAwakeBindings )