[Libreoffice-commits] core.git: sw/inc sw/source

[Michael Stahl]

 sw/inc/undobj.hxx                  |    2 ++
 sw/source/core/inc/UndoManager.hxx |    2 ++
 sw/source/core/undo/docundo.cxx    |   13 +++++++++++++
 sw/source/core/undo/undobj.cxx     |   12 +++++++++---
 4 files changed, 26 insertions(+), 3 deletions(-)

New commits:
commit 13d7778fc10638a145af7fb077688e032bd16f81
Author: Michael Stahl <mstahl at redhat.com>
Date:   Mon May 8 16:56:34 2017 +0200

    tdf#70960 sw::UndoManager: fix exponential Repeat proliferation
    
    In case of a multi-selection, the usual pattern is that the EditShell
    contains a loop around GetCursor()->GetRingContainer() that creates N
    "groups" of undo-actions, all of which are inside a SfxListUndoAction.
    
    If that is then repeated again with a multi-selection, the result will
    be N*N actions, etc.  This is usually not an issue because most
    repeatable actions are idempotent - except for the resource usage.
    
    Prevent the 2nd step of the proliferation by introducing a flag in
    SwUndo to do nothing in SwUndo::Repeat() that is set for all but the
    1st cursor of the multi-selection in sw::UndoManager::Repeat().
    
    (presumably regression from CWS undoapi)
    
    Change-Id: I095258f6d57af6bcaa8a39747632422e8311e694

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index 91185bbcded0..bfd7cf804051 100644
--- a/sw/inc/undobj.hxx
+++ b/sw/inc/undobj.hxx
@@ -53,6 +53,7 @@ class SwUndo
     SwUndoId const m_nId;
     RedlineFlags   nOrigRedlineFlags;
     ViewShellId    m_nViewShellId;
+    bool m_isRepeatIgnored; /// for multi-selection, only repeat 1st selection
 
 protected:
     bool bCacheComment;
@@ -124,6 +125,7 @@ public:
     static bool FillSaveDataForFormat( const SwPaM& , SwRedlineSaveDatas& );
     static void SetSaveData( SwDoc& rDoc, SwRedlineSaveDatas& rSData );
     static bool HasHiddenRedlines( const SwRedlineSaveDatas& rSData );
+    void IgnoreRepeat() { m_isRepeatIgnored = true; }
 };
 
 enum class DelContentType : sal_uInt16
diff --git a/sw/source/core/inc/UndoManager.hxx b/sw/source/core/inc/UndoManager.hxx
index ede9604059ad..6064757c4bc0 100644
--- a/sw/source/core/inc/UndoManager.hxx
+++ b/sw/source/core/inc/UndoManager.hxx
@@ -107,6 +107,8 @@ private:
     /// If true, then repair mode is enabled.
     bool m_bRepair;
     bool m_bLockUndoNoModifiedPosition : 1;
+    /// set the IgnoreRepeat flag on every added action
+    bool m_isAddWithIgnoreRepeat;
     /// position in Undo-Array at which Doc was saved (and is not modified)
     UndoStackMark m_UndoSaveMark;
     SwDocShell* m_pDocShell;
diff --git a/sw/source/core/undo/docundo.cxx b/sw/source/core/undo/docundo.cxx
index 833044fb3f49..42417c94af38 100644
--- a/sw/source/core/undo/docundo.cxx
+++ b/sw/source/core/undo/docundo.cxx
@@ -61,6 +61,7 @@ UndoManager::UndoManager(std::shared_ptr<SwNodes> const & xUndoNodes,
     ,   m_bDrawUndo(true)
     ,   m_bRepair(false)
     ,   m_bLockUndoNoModifiedPosition(false)
+    ,   m_isAddWithIgnoreRepeat(false)
     ,   m_UndoSaveMark(MARK_INVALID)
     ,   m_pDocShell(nullptr)
     ,   m_pView(nullptr)
@@ -520,6 +521,10 @@ void UndoManager::AddUndoAction(SfxUndoAction *pAction, bool bTryMerge)
         {
             pUndo->SetRedlineFlags( m_rRedlineAccess.GetRedlineFlags() );
         }
+        if (m_isAddWithIgnoreRepeat)
+        {
+            pUndo->IgnoreRepeat();
+        }
     }
     SdrUndoManager::AddUndoAction(pAction, bTryMerge);
     // if the undo nodes array is too large, delete some actions
@@ -685,10 +690,18 @@ bool UndoManager::Repeat(::sw::RepeatContext & rContext,
     for(SwPaM& rPaM : rContext.GetRepeatPaM().GetRingContainer())
     {    // iterate over ring
         rContext.m_pCurrentPaM = &rPaM;
+        if (DoesUndo() && & rPaM != pTmp)
+        {
+            m_isAddWithIgnoreRepeat = true;
+        }
         for (sal_uInt16 nRptCnt = nRepeatCount; nRptCnt > 0; --nRptCnt)
         {
             pRepeatAction->Repeat(rContext);
         }
+        if (DoesUndo() && & rPaM != pTmp)
+        {
+            m_isAddWithIgnoreRepeat = false;
+        }
         rContext.m_bDeleteRepeated = false; // reset for next PaM
     }
     rContext.m_pCurrentPaM = pTmp;
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index 96c8969dee6d..2ad6df159422 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -158,9 +158,10 @@ void SwUndo::RemoveIdxRel( sal_uLong nIdx, const SwPosition& rPos )
 }
 
 SwUndo::SwUndo(SwUndoId const nId, const SwDoc* pDoc)
-    : m_nId(nId), nOrigRedlineFlags(RedlineFlags::NONE),
-      m_nViewShellId(CreateViewShellId(pDoc)),
-      bCacheComment(true), pComment(nullptr)
+    : m_nId(nId), nOrigRedlineFlags(RedlineFlags::NONE)
+    , m_nViewShellId(CreateViewShellId(pDoc))
+    , m_isRepeatIgnored(false)
+    , bCacheComment(true), pComment(nullptr)
 {
 }
 
@@ -240,6 +241,10 @@ void SwUndo::RedoWithContext(SfxUndoContext & rContext)
 
 void SwUndo::Repeat(SfxRepeatTarget & rContext)
 {
+    if (m_isRepeatIgnored)
+    {
+        return; // ignore Repeat for multi-selections
+    }
     ::sw::RepeatContext *const pRepeatContext(
             dynamic_cast< ::sw::RepeatContext * >(& rContext));
     assert(pRepeatContext);
@@ -250,6 +255,7 @@ bool SwUndo::CanRepeat(SfxRepeatTarget & rContext) const
 {
     assert(dynamic_cast< ::sw::RepeatContext * >(& rContext));
     (void)rContext;
+    // a MultiSelection action that doesn't do anything must still return true
     return (SwUndoId::REPEAT_START <= GetId()) && (GetId() < SwUndoId::REPEAT_END);
 }