tdf#161741 undo nits: NeedsClearRedo is private + prioritize TopLevel

moNeedsClearRedo is an implementation detail,
so there should not be public functions for it.

Plus, NeedsClearRedo can be called either against
the CurrentLevel stack or the TopLevel stack
(whatever that implies - there is no documentation...)
and since it is a delayed call that means that
multiple NeedsClearRedo attempts could be made,
theoretically against both stacks.

My initial implementation was "last one wins",
(which may very well prove to be the best one)
but whatever happens, it should be clearly intentional.
Based on my limited skill at code reading,
it sounds like CurrentLevel might be more of an implemention detail
or a temporary expansion of a ListUndoAction,
so I am guessing that any request for TopLevel clearing
should be given priority.

For Writer, it is always clearing the TopLevel stack
sw/source/core/undo/docundo.cxx: ClearRedo()
return SdrUndoManager::ImplClearRedo_NoLock(TopLevel);

Change-Id: I195aefb696599f018712135a2e015549d534791f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171984
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Reviewed-by: Justin Luth <jluth@mail.com>

2 files changed, 9 insertions, 19 deletions

diff --git a/include/svl/undo.hxx b/include/svl/undo.hxx
index 707a6d8b025b..2ca0f3b0f8a4 100644
--- a/include/svl/undo.hxx
+++ b/include/svl/undo.hxx
@@ -222,9 +222,6 @@ public:
     */
     virtual void            ClearRedo();
 
-    const std::optional<bool>& GetNeedsClearRedo() const;
-    void SetNeedsClearRedo(const std::optional<bool>& oSet);
-
     /** leaves any possible open list action (<member>IsInListAction</member>), and clears both the Undo and the
         Redo stack.
 
diff --git a/svl/source/undo/undo.cxx b/svl/source/undo/undo.cxx
index 51849ebc0744..ab33374507fc 100644
--- a/svl/source/undo/undo.cxx
+++ b/svl/source/undo/undo.cxx
@@ -185,7 +185,7 @@ struct SfxUndoManager_Data
     bool            mbDoing;
     bool            mbClearUntilTopLevel;
     bool            mbEmptyActions;
-    std::optional<bool> moNeedsClearRedo;
+    std::optional<bool> moNeedsClearRedo; // holds a requested ClearRedo until safe to clear stack
 
     UndoListeners   aListeners;
 
@@ -477,7 +477,10 @@ void SfxUndoManager::ImplClearRedo_NoLock( bool const i_currentLevel )
 {
     if (IsDoing())
     {
-        SetNeedsClearRedo(i_currentLevel);
+        // cannot clear redo while undo/redo is in process. Delay ClearRedo until safe to clear.
+        // (assuming if TopLevel requests a clear, it should have priority over CurrentLevel)
+        if (!m_xData->moNeedsClearRedo.has_value() || i_currentLevel == TopLevel)
+            m_xData->moNeedsClearRedo = i_currentLevel;
         return;
     }
     UndoManagerGuard aGuard( *m_xData );
@@ -492,16 +495,6 @@ void SfxUndoManager::ClearRedo()
     ImplClearRedo_NoLock( CurrentLevel );
 }
 
-const std::optional<bool>& SfxUndoManager::GetNeedsClearRedo() const
-{
-    return m_xData->moNeedsClearRedo;
-}
-
-void SfxUndoManager::SetNeedsClearRedo(const std::optional<bool>& oSet)
-{
-    m_xData->moNeedsClearRedo = oSet;
-}
-
 void SfxUndoManager::Reset()
 {
     UndoManagerGuard aGuard( *m_xData );
@@ -764,10 +757,10 @@ bool SfxUndoManager::ImplUndo( SfxUndoContext* i_contextOrNull )
     }
 
     m_xData->mbDoing = false;
-    if (GetNeedsClearRedo().has_value())
+    if (m_xData->moNeedsClearRedo.has_value())
     {
-        ImplClearRedo_NoLock(*GetNeedsClearRedo());
-        SetNeedsClearRedo(std::optional<bool>());
+        ImplClearRedo_NoLock(*m_xData->moNeedsClearRedo);
+        m_xData->moNeedsClearRedo.reset();
     }
 
     aGuard.scheduleNotification( &SfxUndoListener::actionUndone, sActionComment );
@@ -883,7 +876,7 @@ bool SfxUndoManager::ImplRedo( SfxUndoContext* i_contextOrNull )
     }
 
     m_xData->mbDoing = false;
-    assert(!GetNeedsClearRedo().has_value() && "Assuming I don't need to handle it here. What about if thrown?");
+    assert(!m_xData->moNeedsClearRedo.has_value() && "Assuming I don't need to handle it here. What about if thrown?");
     ImplCheckEmptyActions();
     aGuard.scheduleNotification( &SfxUndoListener::actionRedone, sActionComment );