[Ide] Fix IWorkbenchWindow.Closing event handling with async operations

If a workbench window contains an unsaved document, we ask the user
to save it and run the save operation asynchronously. This is controlled
by the WorkbenchWindowEventArgs.Cancel property and the result
contols whether we set specific Gtk/GLib events as hanled or not
(i.e. in order to allow or disallow Gtk to close a window).

The first problem here was the async EventHandler for the Closing event
which was not awaited and the window was closed before the
Save operation had a chance to run to completion. This is solved by
an EventHandler delegate returning a Task, making it possible to
await the handlers subscribed to the event.

The second problem with this approach was that Gtk doesn't know
anything about awaiting Tasks and if one of its event handlers
leaves the synchronization context (like the asynchrounous
file saving), it will let the signal pass without waiting for its
handled state to be updated. This is solved by delaying window
closing/destruction (always set the initial delete/close event
handled state and close the window in a continuation task
synchronized with the UI thread)

(fix bug #53089)

6 files changed, 36 insertions, 24 deletions

diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/DefaultWorkbench.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/DefaultWorkbench.cs
index 3ee80e6944..056046ce66 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/DefaultWorkbench.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/DefaultWorkbench.cs
@@ -687,13 +687,21 @@ namespace MonoDevelop.Ide.Gui
 				}
 			}
 		}
-		
-		protected /*override*/ void OnClosing(object o, Gtk.DeleteEventArgs e)
+
+		bool closing;
+		protected /*override*/ async void OnClosing(object o, Gtk.DeleteEventArgs e)
 		{
-			if (Close()) {
+			// close the window in case DeleteEvent fires again after a successful close
+			if (closing) 
+				return;
+			
+			// don't allow Gtk to close the workspace, in case Close() leaves the synchronization context
+			// Gtk.Application.Quit () will handle it for us.
+			e.RetVal = true;
+			if (await Close ()) {
+				closing = true;
+				Destroy (); // default delete action
 				Gtk.Application.Quit ();
-			} else {
-				e.RetVal = true;
 			}
 		}
 		
@@ -720,7 +728,7 @@ namespace MonoDevelop.Ide.Gui
 			Destroy ();
 		}
 		
-		public bool Close() 
+		public async Task<bool> Close()
 		{
 			if (!IdeApp.OnExit ())
 				return false;
@@ -745,7 +753,7 @@ namespace MonoDevelop.Ide.Gui
 				}
 			}
 			
-			if (!IdeApp.Workspace.Close (false, false))
+			if (!await IdeApp.Workspace.Close (false, false))
 				return false;
 			
 			CloseAllViews ();
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Document.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Document.cs
index 56481b7d61..4bd8a5e6b2 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Document.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Document.cs
@@ -516,9 +516,9 @@ namespace MonoDevelop.Ide.Gui
 			await UpdateParseDocument ();
 		}
 		
-		public bool Close ()
+		public async Task<bool> Close ()
 		{
-			return ((SdiWorkspaceWindow)Window).CloseWindow (false, true);
+			return await ((SdiWorkspaceWindow)Window).CloseWindow (false, true);
 		}
 
 		protected override void OnSaved (EventArgs e)
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/HiddenWorkbenchWindow.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/HiddenWorkbenchWindow.cs
index 34d6fef15f..b5e3110a4e 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/HiddenWorkbenchWindow.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/HiddenWorkbenchWindow.cs
@@ -26,6 +26,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Threading.Tasks;
 using Mono.Addins;
 
 namespace MonoDevelop.Ide.Gui
@@ -74,9 +75,9 @@ namespace MonoDevelop.Ide.Gui
 			set {}
 		}
 		
-		public bool CloseWindow (bool force)
+		public Task<bool> CloseWindow (bool force)
 		{
-			return true;
+			return Task.FromResult (true);
 		}
 		
 		public void SelectWindow ()
@@ -112,7 +113,7 @@ namespace MonoDevelop.Ide.Gui
 
 		public event EventHandler TitleChanged { add {} remove {} }
 		public event EventHandler DocumentChanged { add {} remove {} }
-		public event MonoDevelop.Ide.Gui.WorkbenchWindowEventHandler Closing { add {} remove {} }
+		public event MonoDevelop.Ide.Gui.WorkbenchWindowAsyncEventHandler Closing { add {} remove {} }
 		public event MonoDevelop.Ide.Gui.WorkbenchWindowEventHandler Closed { add {} remove {} }
 		public event MonoDevelop.Ide.Gui.ActiveViewContentEventHandler ActiveViewContentChanged { add {} remove {} }
 		public event EventHandler ViewsChanged { add {} remove {} }
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/IWorkbenchWindow.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/IWorkbenchWindow.cs
index 8bcc0cde5c..4614f4fe51 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/IWorkbenchWindow.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/IWorkbenchWindow.cs
@@ -30,6 +30,7 @@ using System.ComponentModel;
 using System.Collections.Generic;
 using Mono.Addins;
 using MonoDevelop.Components.Docking;
+using System.Threading.Tasks;
 
 namespace MonoDevelop.Ide.Gui
 {
@@ -53,7 +54,7 @@ namespace MonoDevelop.Ide.Gui
 		void SwitchView (BaseViewContent subViewContent);
 		int FindView <T>();
 		
-		bool CloseWindow (bool force);
+		Task<bool> CloseWindow (bool force);
 		void SelectWindow ();
 
 		/// <summary>
@@ -63,11 +64,12 @@ namespace MonoDevelop.Ide.Gui
 
 		event EventHandler DocumentChanged;
 		event WorkbenchWindowEventHandler Closed;
-		event WorkbenchWindowEventHandler Closing;
+		event WorkbenchWindowAsyncEventHandler Closing;
 		event ActiveViewContentEventHandler ActiveViewContentChanged;
 		event EventHandler ViewsChanged;
 	}
 
+	public delegate Task WorkbenchWindowAsyncEventHandler (object o, WorkbenchWindowEventArgs e);
 	public delegate void WorkbenchWindowEventHandler (object o, WorkbenchWindowEventArgs e);
 	public class WorkbenchWindowEventArgs : CancelEventArgs
 	{
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/SdiWorkspaceWindow.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/SdiWorkspaceWindow.cs
index b6eeec99d7..20c876f248 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/SdiWorkspaceWindow.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/SdiWorkspaceWindow.cs
@@ -38,6 +38,7 @@ using MonoDevelop.Components.Commands;
 using MonoDevelop.Ide.Extensions;
 using MonoDevelop.Ide.Gui.Content;
 using MonoDevelop.Components.DockNotebook;
+using System.Threading.Tasks;
 
 namespace MonoDevelop.Ide.Gui
 {
@@ -443,17 +444,17 @@ namespace MonoDevelop.Ide.Gui
 			}
 		}
 		
-		public bool CloseWindow (bool force)
+		public Task<bool> CloseWindow (bool force)
 		{
 			return CloseWindow (force, false);
 		}
 
-		public bool CloseWindow (bool force, bool animate)
+		public async Task<bool> CloseWindow (bool force, bool animate)
 		{
 			bool wasActive = workbench.ActiveWorkbenchWindow == this;
 			WorkbenchWindowEventArgs args = new WorkbenchWindowEventArgs (force, wasActive);
 			args.Cancel = false;
-			OnClosing (args);
+			await OnClosing (args);
 			if (args.Cancel)
 				return false;
 			
@@ -808,10 +809,10 @@ namespace MonoDevelop.Ide.Gui
 			}
 		}
 
-		protected virtual void OnClosing (WorkbenchWindowEventArgs e)
+		protected virtual async System.Threading.Tasks.Task OnClosing (WorkbenchWindowEventArgs e)
 		{
 			if (Closing != null) {
-				Closing (this, e);
+				await Closing (this, e);
 			}
 		}
 
@@ -830,7 +831,7 @@ namespace MonoDevelop.Ide.Gui
 
 		public event EventHandler TitleChanged;
 		public event WorkbenchWindowEventHandler Closed;
-		public event WorkbenchWindowEventHandler Closing;
+		public event WorkbenchWindowAsyncEventHandler Closing;
 		public event ActiveViewContentEventHandler ActiveViewContentChanged;
 	}
 }
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Workbench.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Workbench.cs
index 324f2fc919..2840b11442 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Workbench.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui/Workbench.cs
@@ -142,9 +142,9 @@ namespace MonoDevelop.Ide.Gui
 			Present ();
 		}
 		
-		internal bool Close ()
+		internal async Task<bool> Close ()
 		{
-			return workbench.Close();
+			return await workbench.Close();
 		}
 
 		public ImmutableList<Document> Documents {
@@ -584,7 +584,7 @@ namespace MonoDevelop.Ide.Gui
 							}
 							return doc;
 						} else {
-							if (!doc.Close ())
+							if (!await doc.Close ())
 								return doc;
 							break;
 						}
@@ -841,7 +841,7 @@ namespace MonoDevelop.Ide.Gui
 			return pad;
 		}
 		
-		async void OnWindowClosing (object sender, WorkbenchWindowEventArgs args)
+		async Task OnWindowClosing (object sender, WorkbenchWindowEventArgs args)
 		{
 			var window = (IWorkbenchWindow) sender;
 			var viewContent = window.ViewContent;