Skip to content

Commit

Permalink
Make sure extension window closes with Dynamo (#10557)
Browse files Browse the repository at this point in the history
View extensions that add a Window to the extension bar, run the risk of
leaving a window open when Dynamo closes, resulting in a zombie process
. The extension could avoid the problem in either of these two ways:

1. Setting the Owner property of the Window they create to the one
obtained from ViewLoadedParam.DynamoWindow.

2. Calling the Close method on the Window they created when the
Shutdown life-time method is called on the extension.

However, Dynamo should not rely on view extensions doing this. In order
to be able to still close no matter what, we assign the Owner property
explicitly in DynamoView.AddExtensionTabItem.
  • Loading branch information
mmisol authored Apr 9, 2020
1 parent 7e1ee46 commit 85c4120
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
6 changes: 6 additions & 0 deletions src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,12 @@ internal TabItem AddExtensionTabItem(IViewExtension viewExtension, ContentContro
else
{
tab.Content = contentControl.Content;
var extensionWindow = contentControl as Window;
if (extensionWindow != null)
{
// Make sure the extension window closes with Dynamo
extensionWindow.Owner = this;
}
}

//Insert the tab at the end
Expand Down
32 changes: 27 additions & 5 deletions test/DynamoCoreWpfTests/ViewExtensions/ViewExtensionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@ public void OnWorkspaceChangedExtensionIsNotified()
Assert.AreEqual(2, viewExtension.Counter);
}

[Test]
public void ExtensionWindowIsClosedWithDynamo()
{
var dummyExtension = new DummyViewExtension()
{
SetOwner = false
};

RaiseLoadedEvent(this.View);
var extensionManager = View.viewExtensionManager;
extensionManager.Add(dummyExtension);

View.Close();

Assert.IsTrue(dummyExtension.WindowClosed);
}

[Test]
public void ExtensionsSideBarExtensionsTest()
{
Expand Down Expand Up @@ -63,7 +80,9 @@ public static void RaiseLoadedEvent(FrameworkElement element)

public class DummyViewExtension : IViewExtension
{
public int Counter = 0;
public int Counter { get; set; }
public bool SetOwner { get; set; } = true;
public bool WindowClosed { get; set; }

public string UniqueId
{
Expand All @@ -87,11 +106,15 @@ public void Loaded(ViewLoadedParams p)
Counter++;
};

var window = new Window
var window = new Window();
window.Closed += (sender, args) =>
{
// Set the owner of the window to tuIDhe Dynamo window.
Owner = p.DynamoWindow
WindowClosed = true;
};
if (SetOwner)
{
window.Owner = p.DynamoWindow;
}

p.AddToExtensionsSideBar(this, window);
}
Expand Down Expand Up @@ -138,7 +161,6 @@ public void Loaded(ViewLoadedParams p)

var window = new Window
{
// Set the owner of the window to tuIDhe Dynamo window.
Owner = p.DynamoWindow
};

Expand Down

0 comments on commit 85c4120

Please sign in to comment.