From 53b022c615fa81a81abe99debc0791d5f3e6e4ef Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 28 Jan 2010 21:34:13 +0000 Subject: [PATCH] Add safety check to UndoStack to prevent corruption when the document is changed by UpdateStarted/UpdateFinished events within Undo() or Redo() calls. git-svn-id: svn://svn.sharpdevelop.net/sharpdevelop/trunk@5454 1ccf3a8d-04fe-1044-b7c0-cef0b8235c61 --- .../Document/DocumentChangeOperation.cs | 19 +++- .../Document/IUndoableOperation.cs | 6 ++ .../Document/UndoOperationGroup.cs | 16 +++- .../Document/UndoStack.cs | 93 ++++++++++++++----- 4 files changed, 107 insertions(+), 27 deletions(-) diff --git a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/DocumentChangeOperation.cs b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/DocumentChangeOperation.cs index 3ac9dceacc..d9e0d8296d 100644 --- a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/DocumentChangeOperation.cs +++ b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/DocumentChangeOperation.cs @@ -6,13 +6,14 @@ // using System; +using System.Diagnostics; namespace ICSharpCode.AvalonEdit.Document { /// /// Describes a change to a TextDocument. /// - sealed class DocumentChangeOperation : IUndoableOperation + sealed class DocumentChangeOperation : IUndoableOperationWithContext { TextDocument document; DocumentChangeEventArgs change; @@ -23,6 +24,22 @@ namespace ICSharpCode.AvalonEdit.Document this.change = change; } + public void Undo(UndoStack stack) + { + Debug.Assert(stack.state == UndoStack.StatePlayback); + stack.state = UndoStack.StatePlaybackModifyDocument; + this.Undo(); + stack.state = UndoStack.StatePlayback; + } + + public void Redo(UndoStack stack) + { + Debug.Assert(stack.state == UndoStack.StatePlayback); + stack.state = UndoStack.StatePlaybackModifyDocument; + this.Redo(); + stack.state = UndoStack.StatePlayback; + } + public void Undo() { OffsetChangeMap map = change.OffsetChangeMapOrNull; diff --git a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/IUndoableOperation.cs b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/IUndoableOperation.cs index d8653fc1f9..f1792fbb03 100644 --- a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/IUndoableOperation.cs +++ b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/IUndoableOperation.cs @@ -25,4 +25,10 @@ namespace ICSharpCode.AvalonEdit.Document /// void Redo(); } + + interface IUndoableOperationWithContext : IUndoableOperation + { + void Undo(UndoStack stack); + void Redo(UndoStack stack); + } } diff --git a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoOperationGroup.cs b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoOperationGroup.cs index 07be7e8bc4..a0f66217ac 100644 --- a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoOperationGroup.cs +++ b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoOperationGroup.cs @@ -15,7 +15,7 @@ namespace ICSharpCode.AvalonEdit.Document /// This class stacks the last x operations from the undostack and makes /// one undo/redo operation from it. /// - sealed class UndoOperationGroup : IUndoableOperation + sealed class UndoOperationGroup : IUndoableOperationWithContext { IUndoableOperation[] undolist; @@ -42,11 +42,25 @@ namespace ICSharpCode.AvalonEdit.Document } } + public void Undo(UndoStack stack) + { + for (int i = 0; i < undolist.Length; ++i) { + stack.RunUndo(undolist[i]); + } + } + public void Redo() { for (int i = undolist.Length - 1; i >= 0; --i) { undolist[i].Redo(); } } + + public void Redo(UndoStack stack) + { + for (int i = undolist.Length - 1; i >= 0; --i) { + stack.RunRedo(undolist[i]); + } + } } } diff --git a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoStack.cs b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoStack.cs index 57d9967302..ac4ff7ab59 100644 --- a/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoStack.cs +++ b/src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoStack.cs @@ -19,18 +19,32 @@ namespace ICSharpCode.AvalonEdit.Document [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix")] public sealed class UndoStack : INotifyPropertyChanged { + /// undo stack is listening for changes + internal const int StateListen = 0; + /// undo stack is reverting/repeating a set of changes + internal const int StatePlayback = 1; + // undo stack is reverting/repeating a set of changes and modifies the document to do this + internal const int StatePlaybackModifyDocument = 2; + /// state is used for checking that noone but the UndoStack performs changes + /// during Undo events + internal int state = StateListen; + Deque undostack = new Deque(); Deque redostack = new Deque(); - int sizeLimit = int.MaxValue; - bool acceptChanges = true; + + int undoGroupDepth; + int actionCountInUndoGroup; + int optionalActionCount; + object lastGroupDescriptor; + bool allowContinue; /// /// Gets if the undo stack currently accepts changes. /// Is false while an undo action is running. /// public bool AcceptChanges { - get { return acceptChanges; } + get { return state == StateListen; } } /// @@ -77,11 +91,6 @@ namespace ICSharpCode.AvalonEdit.Document redostack.PopFront(); } - int undoGroupDepth; - int actionCountInUndoGroup; - int optionalActionCount; - object lastGroupDescriptor; - /// /// If an undo group is open, gets the group descriptor of the current top-level /// undo group. @@ -122,7 +131,7 @@ namespace ICSharpCode.AvalonEdit.Document } /// - /// Starts grouping changes, continuing with the previously closed undo group. + /// Starts grouping changes, continuing with the previously closed undo group if possible. /// Maintains a counter so that nested calls are possible. /// If the call to StartContinuedUndoGroup is a nested call, it behaves exactly /// as , only top-level calls can continue existing undo groups. @@ -132,7 +141,7 @@ namespace ICSharpCode.AvalonEdit.Document public void StartContinuedUndoGroup(object groupDescriptor) { if (undoGroupDepth == 0) { - actionCountInUndoGroup = undostack.Count > 0 ? 1 : 0; + actionCountInUndoGroup = (allowContinue && undostack.Count > 0) ? 1 : 0; optionalActionCount = 0; lastGroupDescriptor = groupDescriptor; } @@ -155,21 +164,26 @@ namespace ICSharpCode.AvalonEdit.Document undostack.PopBack(); } } else if (actionCountInUndoGroup > 1) { + // combine all actions within the group into a single grouped action undostack.PushBack(new UndoOperationGroup(undostack, actionCountInUndoGroup)); } EnforceSizeLimit(); + allowContinue = true; } } /// /// Throws an InvalidOperationException if an undo group is current open. /// - void VerifyNoUndoGroupOpen() + void ThrowIfUndoGroupOpen() { if (undoGroupDepth != 0) { undoGroupDepth = 0; throw new InvalidOperationException("No undo group should be open at this point"); } + if (state != StateListen) { + throw new InvalidOperationException("This method cannot be called while an undo operation is being performed"); + } } /// @@ -177,14 +191,19 @@ namespace ICSharpCode.AvalonEdit.Document /// public void Undo() { - VerifyNoUndoGroupOpen(); + ThrowIfUndoGroupOpen(); if (undostack.Count > 0) { - lastGroupDescriptor = null; - acceptChanges = false; + // disallow continuing undo groups after undo operation + lastGroupDescriptor = null; allowContinue = false; + // fetch operation to undo and move it to redo stack IUndoableOperation uedit = undostack.PopBack(); redostack.PushBack(uedit); - uedit.Undo(); - acceptChanges = true; + state = StatePlayback; + try { + RunUndo(uedit); + } finally { + state = StateListen; + } if (undostack.Count == 0) NotifyPropertyChanged("CanUndo"); if (redostack.Count == 1) @@ -192,19 +211,31 @@ namespace ICSharpCode.AvalonEdit.Document } } + internal void RunUndo(IUndoableOperation op) + { + IUndoableOperationWithContext opWithCtx = op as IUndoableOperationWithContext; + if (opWithCtx != null) + opWithCtx.Undo(this); + else + op.Undo(); + } + /// /// Call this method to redo the last undone operation /// public void Redo() { - VerifyNoUndoGroupOpen(); + ThrowIfUndoGroupOpen(); if (redostack.Count > 0) { - lastGroupDescriptor = null; - acceptChanges = false; + lastGroupDescriptor = null; allowContinue = false; IUndoableOperation uedit = redostack.PopBack(); undostack.PushBack(uedit); - uedit.Redo(); - acceptChanges = true; + state = StatePlayback; + try { + RunRedo(uedit); + } finally { + state = StateListen; + } if (redostack.Count == 0) NotifyPropertyChanged("CanRedo"); if (undostack.Count == 1) @@ -212,6 +243,15 @@ namespace ICSharpCode.AvalonEdit.Document } } + internal void RunRedo(IUndoableOperation op) + { + IUndoableOperationWithContext opWithCtx = op as IUndoableOperationWithContext; + if (opWithCtx != null) + opWithCtx.Redo(this); + else + op.Redo(); + } + /// /// Call this method to push an UndoableOperation on the undostack. /// The redostack will be cleared if you use this method. @@ -241,7 +281,7 @@ namespace ICSharpCode.AvalonEdit.Document throw new ArgumentNullException("operation"); } - if (acceptChanges && sizeLimit > 0) { + if (state == StateListen && sizeLimit > 0) { bool wasEmpty = undostack.Count == 0; StartUndoGroup(); @@ -272,15 +312,16 @@ namespace ICSharpCode.AvalonEdit.Document /// public void ClearAll() { - VerifyNoUndoGroupOpen(); + ThrowIfUndoGroupOpen(); + actionCountInUndoGroup = 0; + optionalActionCount = 0; if (undostack.Count != 0) { lastGroupDescriptor = null; + allowContinue = false; undostack.Clear(); NotifyPropertyChanged("CanUndo"); } ClearRedoStack(); - actionCountInUndoGroup = 0; - optionalActionCount = 0; } internal void AttachToDocument(TextDocument document) @@ -302,6 +343,8 @@ namespace ICSharpCode.AvalonEdit.Document void document_Changing(object sender, DocumentChangeEventArgs e) { + if (state == StatePlayback) + throw new InvalidOperationException("Document changes during undo/redo operations are not allowed."); TextDocument document = (TextDocument)sender; Push(new DocumentChangeOperation(document, e)); }