Skip to content

Commit b3b0a7c

Browse files
committed
Release refactoring presenters on the UI thread
This seems to fix an issue with unclean exits of Excel after canceling a refactoring dialog. The change guarantees that the dialog saved in the presenter gets disposed on the UI thread, no matter how the refactoring is called or exits.
1 parent b678254 commit b3b0a7c

18 files changed

+100
-32
lines changed

Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Rubberduck.Parsing.Grammar;
66
using Rubberduck.Parsing.Rewriter;
77
using Rubberduck.Parsing.Symbols;
8+
using Rubberduck.Parsing.UIContext;
89
using Rubberduck.Parsing.VBA;
910
using Rubberduck.Refactorings.Exceptions;
1011
using Rubberduck.VBEditor;
@@ -26,8 +27,9 @@ public EncapsulateFieldRefactoring(
2627
IRefactoringPresenterFactory factory,
2728
IRewritingManager rewritingManager,
2829
ISelectionProvider selectionProvider,
29-
ISelectedDeclarationProvider selectedDeclarationProvider)
30-
:base(rewritingManager, selectionProvider, factory)
30+
ISelectedDeclarationProvider selectedDeclarationProvider,
31+
IUiDispatcher uiDispatcher)
32+
:base(rewritingManager, selectionProvider, factory, uiDispatcher)
3133
{
3234
_declarationFinderProvider = declarationFinderProvider;
3335
_selectedDeclarationProvider = selectedDeclarationProvider;

Rubberduck.Refactorings/ExtractInterface/ExtractInterfaceRefactoring.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Rubberduck.Parsing.Grammar;
66
using Rubberduck.Parsing.Rewriter;
77
using Rubberduck.Parsing.Symbols;
8+
using Rubberduck.Parsing.UIContext;
89
using Rubberduck.Parsing.VBA;
910
using Rubberduck.Refactorings.Exceptions;
1011
using Rubberduck.Refactorings.ImplementInterface;
@@ -28,8 +29,9 @@ public ExtractInterfaceRefactoring(
2829
IParseManager parseManager,
2930
IRefactoringPresenterFactory factory,
3031
IRewritingManager rewritingManager,
31-
ISelectionProvider selectionProvider)
32-
:base(rewritingManager, selectionProvider, factory)
32+
ISelectionProvider selectionProvider,
33+
IUiDispatcher uiDispatcher)
34+
:base(rewritingManager, selectionProvider, factory, uiDispatcher)
3335
{
3436
_declarationFinderProvider = declarationFinderProvider;
3537
_parseManager = parseManager;

Rubberduck.Refactorings/InteractiveRefactoringBase.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Rubberduck.VBEditor.Utility;
33
using System;
44
using Rubberduck.Parsing.Symbols;
5+
using Rubberduck.Parsing.UIContext;
56
using Rubberduck.Refactorings.Exceptions;
67

78
namespace Rubberduck.Refactorings
@@ -15,10 +16,12 @@ public abstract class InteractiveRefactoringBase<TPresenter, TModel> : Refactori
1516
protected InteractiveRefactoringBase(
1617
IRewritingManager rewritingManager,
1718
ISelectionProvider selectionProvider,
18-
IRefactoringPresenterFactory factory)
19+
IRefactoringPresenterFactory factory,
20+
IUiDispatcher uiDispatcher)
1921
:base(rewritingManager, selectionProvider)
2022
{
21-
_presenterFactory = ((model) => DisposalActionContainer.Create(factory.Create<TPresenter, TModel>(model), factory.Release));
23+
Action<TPresenter> presenterDisposalAction = (TPresenter presenter) => uiDispatcher.Invoke(() => factory.Release(presenter));
24+
_presenterFactory = ((model) => DisposalActionContainer.Create(factory.Create<TPresenter, TModel>(model), presenterDisposalAction));
2225
}
2326

2427
public override void Refactor(Declaration target)
@@ -47,9 +50,9 @@ protected void Refactor(TModel initialModel)
4750
{
4851
throw new InvalidRefactoringModelException();
4952
}
50-
51-
RefactorImpl(model);
5253
}
54+
55+
RefactorImpl(model);
5356
}
5457

5558
protected abstract TModel InitializeModel(Declaration target);

Rubberduck.Refactorings/RemoveParameters/RemoveParametersRefactoring.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Rubberduck.Parsing.Grammar;
77
using Rubberduck.Parsing.Rewriter;
88
using Rubberduck.Parsing.Symbols;
9+
using Rubberduck.Parsing.UIContext;
910
using Rubberduck.Parsing.VBA;
1011
using Rubberduck.Refactorings.Exceptions;
1112
using Rubberduck.Refactorings.Exceptions.RemoveParameter;
@@ -25,8 +26,9 @@ public RemoveParametersRefactoring(
2526
IRefactoringPresenterFactory factory,
2627
IRewritingManager rewritingManager,
2728
ISelectionProvider selectionProvider,
28-
ISelectedDeclarationProvider selectedDeclarationProvider)
29-
:base(rewritingManager, selectionProvider, factory)
29+
ISelectedDeclarationProvider selectedDeclarationProvider,
30+
IUiDispatcher uiDispatcher)
31+
:base(rewritingManager, selectionProvider, factory, uiDispatcher)
3032
{
3133
_declarationFinderProvider = declarationFinderProvider;
3234
_selectedDeclarationProvider = selectedDeclarationProvider;

Rubberduck.Refactorings/Rename/RenameRefactoring.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Rubberduck.VBEditor.SafeComWrappers;
1515
using Rubberduck.VBEditor.Utility;
1616
using NLog;
17+
using Rubberduck.Parsing.UIContext;
1718

1819
namespace Rubberduck.Refactorings.Rename
1920
{
@@ -37,8 +38,9 @@ public RenameRefactoring(
3738
IRewritingManager rewritingManager,
3839
ISelectionProvider selectionProvider,
3940
ISelectedDeclarationProvider selectedDeclarationProvider,
40-
IParseManager parseManager)
41-
:base(rewritingManager, selectionProvider, factory)
41+
IParseManager parseManager,
42+
IUiDispatcher uiDispatcher)
43+
:base(rewritingManager, selectionProvider, factory, uiDispatcher)
4244
{
4345
_declarationFinderProvider = declarationFinderProvider;
4446
_selectedDeclarationProvider = selectedDeclarationProvider;

Rubberduck.Refactorings/ReorderParameters/ReorderParametersRefactoring.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Linq;
88
using Antlr4.Runtime;
99
using Rubberduck.Parsing.Binding;
10+
using Rubberduck.Parsing.UIContext;
1011
using Rubberduck.Parsing.VBA;
1112
using Rubberduck.Refactorings.Exceptions;
1213
using Rubberduck.VBEditor.Extensions;
@@ -24,8 +25,9 @@ public ReorderParametersRefactoring(
2425
IRefactoringPresenterFactory factory,
2526
IRewritingManager rewritingManager,
2627
ISelectionProvider selectionProvider,
27-
ISelectedDeclarationProvider selectedDeclarationProvider)
28-
:base(rewritingManager, selectionProvider, factory)
28+
ISelectedDeclarationProvider selectedDeclarationProvider,
29+
IUiDispatcher uiDispatcher)
30+
:base(rewritingManager, selectionProvider, factory, uiDispatcher)
2931
{
3032
_declarationFinderProvider = declarationFinderProvider;
3133
_selectedDeclarationProvider = selectedDeclarationProvider;

RubberduckTests/CodeExplorer/CodeExplorerViewModelTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ public bool AddUserDocument_CanExecuteBasedOnProjectType(ProjectType projectType
265265
public bool RefactorExtractInterface_CanExecuteBasedOnComponentType(ComponentType componentType)
266266
{
267267
using (var explorer = new MockedCodeExplorer(ProjectType.HostProject, componentType, @"Public Sub Foo(): MsgBox """":End Sub ")
268-
.ImplementExtractIntercaceCommand().SelectFirstModule())
268+
.ImplementExtractInterfaceCommand().SelectFirstModule())
269269
{
270270
return explorer.ViewModel.CodeExplorerExtractInterfaceCommand.CanExecute(explorer.ViewModel.SelectedItem);
271271
}

RubberduckTests/CodeExplorer/MockedCodeExplorer.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,11 +460,11 @@ public MockedCodeExplorer ImplementIndenterCommand()
460460
return this;
461461
}
462462

463-
public MockedCodeExplorer ImplementExtractIntercaceCommand()
463+
public MockedCodeExplorer ImplementExtractInterfaceCommand()
464464
{
465465
ViewModel.CodeExplorerExtractInterfaceCommand = new CodeExplorerExtractInterfaceCommand(
466466
new Rubberduck.Refactorings.ExtractInterface.ExtractInterfaceRefactoring(
467-
State, State, null, null, null),
467+
State, State, null, null, null, _uiDispatcher.Object),
468468
State, null, VbeEvents.Object);
469469
return this;
470470
}

RubberduckTests/Commands/RefactorCommands/EncapsulateFieldCommandTests.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
using Moq;
1+
using System;
2+
using Moq;
23
using NUnit.Framework;
34
using Rubberduck.Interaction;
45
using Rubberduck.Parsing.Rewriter;
6+
using Rubberduck.Parsing.UIContext;
57
using Rubberduck.Parsing.VBA;
68
using Rubberduck.Refactorings;
79
using Rubberduck.Refactorings.EncapsulateField;
@@ -57,7 +59,11 @@ protected override CommandBase TestCommand(IVBE vbe, RubberduckParserState state
5759
var msgBox = new Mock<IMessageBox>().Object;
5860
var factory = new Mock<IRefactoringPresenterFactory>().Object;
5961
var selectedDeclarationProvider = new SelectedDeclarationProvider(selectionService, state);
60-
var refactoring = new EncapsulateFieldRefactoring(state, null, factory, rewritingManager, selectionService, selectedDeclarationProvider);
62+
var uiDispatcherMock = new Mock<IUiDispatcher>();
63+
uiDispatcherMock
64+
.Setup(m => m.Invoke(It.IsAny<Action>()))
65+
.Callback((Action action) => action.Invoke());
66+
var refactoring = new EncapsulateFieldRefactoring(state, null, factory, rewritingManager, selectionService, selectedDeclarationProvider, uiDispatcherMock.Object);
6167
var notifier = new EncapsulateFieldFailedNotifier(msgBox);
6268
var selectedDeclarationService = new SelectedDeclarationProvider(selectionService, state);
6369
return new RefactorEncapsulateFieldCommand(refactoring, notifier, state, selectionService, selectedDeclarationService);

RubberduckTests/Commands/RefactorCommands/ExtractInterfaceCommandTests.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
using Moq;
1+
using System;
2+
using Moq;
23
using NUnit.Framework;
34
using Rubberduck.Interaction;
45
using Rubberduck.Parsing.Rewriter;
6+
using Rubberduck.Parsing.UIContext;
57
using Rubberduck.Parsing.VBA;
68
using Rubberduck.Refactorings;
79
using Rubberduck.Refactorings.ExtractInterface;
@@ -164,7 +166,11 @@ protected override CommandBase TestCommand(IVBE vbe, RubberduckParserState state
164166
{
165167
var factory = new Mock<IRefactoringPresenterFactory>().Object;
166168
var msgBox = new Mock<IMessageBox>().Object;
167-
var refactoring = new ExtractInterfaceRefactoring(state, state, factory, rewritingManager, selectionService);
169+
var uiDispatcherMock = new Mock<IUiDispatcher>();
170+
uiDispatcherMock
171+
.Setup(m => m.Invoke(It.IsAny<Action>()))
172+
.Callback((Action action) => action.Invoke());
173+
var refactoring = new ExtractInterfaceRefactoring(state, state, factory, rewritingManager, selectionService, uiDispatcherMock.Object);
168174
var notifier = new ExtractInterfaceFailedNotifier(msgBox);
169175
return new RefactorExtractInterfaceCommand(refactoring, notifier, state, selectionService);
170176
}

0 commit comments

Comments
 (0)