Unhook drawer listeners so MDP renderer and pages can be collected (#412)

Null out page in custom MDP renderer in Control Gallery so it can be collected

Checkpoint

Checkpoint

Checkpoint

Checkpoint

Checkpoint

Checkpoint

Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs

@@ -34,6 +34,7 @@ namespace Xamarin.Forms.ControlGallery.Android
 	public class NativeDroidMasterDetail : Xamarin.Forms.Platform.Android.AppCompat.MasterDetailPageRenderer
 	{
 		MasterDetailPage _page;
+		bool _disposed;
 
 		protected override void OnElementChanged(VisualElement oldElement, VisualElement newElement)
 		{
@@ -61,10 +62,18 @@ namespace Xamarin.Forms.ControlGallery.Android
 
 		protected override void Dispose(bool disposing)
 		{
+			if (_disposed)
+			{
+				return;
+			}
+
+			_disposed = true;
+
 			if (disposing && _page != null)
 			{
 				_page.LayoutChanged -= Page_LayoutChanged;
 				_page.PropertyChanged -= Page_PropertyChanged;
+				_page = null;
 			}
 
 			base.Dispose(disposing);

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla44166.cs

@@ -0,0 +1,206 @@
+using System;
+using System.Diagnostics;
+using System.Threading;
+using System.Threading.Tasks;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Bugzilla, 44166, "MasterDetailPage instances do not get disposed upon GC")]
+	public class Bugzilla44166 : TestContentPage
+	{
+		protected override void Init()
+		{
+			var label = new Label() { Text = "Testing..." };
+
+			var goButton = new Button { Text = "Go" };
+			goButton.Clicked += (sender, args) => Application.Current.MainPage = new _44166MDP();
+
+			var gcButton = new Button { Text = "GC" };
+			gcButton.Clicked += (sender, args) =>
+			{
+				GC.Collect();
+				GC.WaitForPendingFinalizers();
+
+				if (_44166MDP.Counter > 0)
+				{
+					Debug.WriteLine($">>>>>>>> Post-GC, {_44166MDP.Counter} {nameof(_44166MDP)} allocated");
+				}
+
+				if (_44166Master.Counter > 0)
+				{
+					Debug.WriteLine($">>>>>>>> Post-GC, {_44166Master.Counter} {nameof(_44166Master)} allocated");
+				}
+
+				if (_44166Detail.Counter > 0)
+				{
+					Debug.WriteLine($">>>>>>>> Post-GC, {_44166Detail.Counter} {nameof(_44166Detail)} allocated");
+				}
+
+				if (_44166NavContent.Counter > 0)
+				{
+					Debug.WriteLine($">>>>>>>> Post-GC, {_44166NavContent.Counter} {nameof(_44166NavContent)} allocated");
+				}
+
+				if (_44166NavContent.Counter + _44166Detail.Counter + _44166Master.Counter + _44166MDP.Counter == 0)
+				{
+					label.Text = "Success";
+				}
+			};
+
+			Content = new StackLayout
+			{
+				Children = { label, goButton, gcButton }
+			};
+		}
+
+		#if UITEST
+		[Test]
+		public void Bugzilla44166Test()
+		{
+			RunningApp.WaitForElement(q => q.Marked("Go"));
+			RunningApp.Tap(q => q.Marked("Go"));
+
+			RunningApp.WaitForElement(q => q.Marked("Back"));
+			RunningApp.Tap(q => q.Marked("Back"));
+
+			for(int n = 0; n < 10; n++)
+			{
+				RunningApp.WaitForElement(q => q.Marked("GC"));
+				RunningApp.Tap(q => q.Marked("GC"));
+
+				if (RunningApp.Query(q => q.Marked("Success")).Length > 0)
+				{
+					return;
+				}
+			}
+
+			var pageStats = string.Empty;
+
+			if (_44166MDP.Counter > 0)
+			{
+				pageStats += $"{_44166MDP.Counter} {nameof(_44166MDP)} allocated; ";
+			}
+
+			if (_44166Master.Counter > 0)
+			{
+				pageStats += $"{_44166Master.Counter} {nameof(_44166Master)} allocated; ";
+			}
+
+			if (_44166Detail.Counter > 0)
+			{
+				pageStats += $"{_44166Detail.Counter} {nameof(_44166Detail)} allocated; ";
+			}
+
+			if (_44166NavContent.Counter > 0)
+			{
+				pageStats += $"{_44166NavContent.Counter} {nameof(_44166NavContent)} allocated; ";
+			}
+
+			Assert.Fail($"At least one of the pages was not collected: {pageStats}");
+		}
+		#endif
+	}
+
+	[Preserve(AllMembers = true)]
+	public class _44166MDP : MasterDetailPage
+	{
+		public static int Counter;
+
+		public _44166MDP()
+		{
+			Interlocked.Increment(ref Counter);
+			Debug.WriteLine($"++++++++ {nameof(_44166MDP)} constructor, {Counter} allocated");
+
+			Master = new _44166Master();
+			Detail = new _44166Detail();
+		}
+
+		~_44166MDP()
+		{
+			Interlocked.Decrement(ref Counter);
+			Debug.WriteLine($"-------- {nameof(_44166MDP)} destructor, {Counter} allocated");
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	public class _44166Master : ContentPage
+	{
+		public static int Counter;
+
+		public _44166Master()
+		{
+			Interlocked.Increment(ref Counter);
+			Debug.WriteLine($"++++++++ {nameof(_44166Master)} constructor, {Counter} allocated");
+
+			Title = "Master";
+			var goButton = new Button { Text = "Back" };
+			goButton.Clicked += (sender, args) => Application.Current.MainPage = new Bugzilla44166();
+
+			Content = new StackLayout
+			{
+				Children = { goButton }
+			};
+		}
+
+		~_44166Master()
+		{
+			Interlocked.Decrement(ref Counter);
+			Debug.WriteLine($"-------- {nameof(_44166Master)} destructor, {Counter} allocated");
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	public class _44166Detail : NavigationPage
+	{
+		public static int Counter;
+
+		public _44166Detail()
+		{
+			Interlocked.Increment(ref Counter);
+			Debug.WriteLine($"++++++++ {nameof(_44166Detail)} constructor, {Counter} allocated");
+
+			Title = "Detail";
+			PushAsync(new _44166NavContent());
+		}
+
+		~_44166Detail()
+		{
+			Interlocked.Decrement(ref Counter);
+			Debug.WriteLine($"-------- {nameof(_44166Detail)} destructor, {Counter} allocated");
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	public class _44166NavContent : ContentPage
+	{
+		public static int Counter;
+
+		public _44166NavContent ()
+		{
+			Interlocked.Increment(ref Counter);
+			Debug.WriteLine($"++++++++ {nameof(_44166NavContent)} constructor, {Counter} allocated");
+
+			var goButton = new Button { Text = "Back" };
+			goButton.Clicked += (sender, args) => Application.Current.MainPage = new Bugzilla44166();
+
+			Content = new StackLayout
+			{
+				Children = { goButton }
+			};
+		}
+
+		~_44166NavContent ()
+		{
+			Interlocked.Decrement(ref Counter);
+			Debug.WriteLine($"-------- {nameof(_44166NavContent)} destructor, {Counter} allocated");
+		}
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -125,6 +125,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42364.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42519.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla43516.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla44166.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)CarouselAsync.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34561.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34727.cs" />

Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs

@@ -11,8 +11,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 		PageContainer _pageContainer;
 		FragmentManager _fragmentManager;
 		readonly bool _isMaster;
-		readonly MasterDetailPage _parent;
+		MasterDetailPage _parent;
 		Fragment _currentFragment;
+		bool _disposed;
 
 		public MasterDetailContainer(MasterDetailPage parent, bool isMaster, Context context) : base(parent, isMaster, context)
 		{
@@ -106,6 +107,37 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			}
 		}
 
+		protected override void Dispose(bool disposing)
+		{
+			if (_disposed)
+			{
+				return;
+			}
+
+			_disposed = true;
+
+			if (disposing)
+			{
+				if (_currentFragment != null)
+				{
+					FragmentTransaction transaction = FragmentManager.BeginTransaction();
+					transaction.DisallowAddToBackStack();
+					transaction.Remove(_currentFragment);
+					transaction.SetTransition((int)FragmentTransit.None);
+					transaction.CommitAllowingStateLoss();
+					FragmentManager.ExecutePendingTransactions();
+
+					_currentFragment = null;
+				}
+
+				_parent = null;
+				_pageContainer = null;
+				_fragmentManager = null;
+			}
+
+			base.Dispose(disposing);
+		}
+
 		public void SetFragmentManager(FragmentManager fragmentManager)
 		{
 			if (_fragmentManager == null)

Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs

@@ -188,24 +188,29 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 				if (_detailLayout != null)
 				{
+					RemoveView(_detailLayout);
 					_detailLayout.Dispose();
 					_detailLayout = null;
 				}
 
 				if (_masterLayout != null)
 				{
+					RemoveView(_masterLayout);
 					_masterLayout.Dispose();
 					_masterLayout = null;
 				}
 
 				Device.Info.PropertyChanged -= DeviceInfoPropertyChanged;
 
+				RemoveDrawerListener(this);
+
 				if (Element != null)
 				{
 					MasterDetailPageController.BackButtonPressed -= OnBackButtonPressed;
 					Element.PropertyChanged -= HandlePropertyChanged;
 					Element.Appearing -= MasterDetailPageAppearing;
 					Element.Disappearing -= MasterDetailPageDisappearing;
+
 					Element.ClearValue(Android.Platform.RendererProperty);
 					Element = null;
 				}

Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs

@@ -173,6 +173,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				if (_drawerLayout != null && _drawerListener != null)
 				{
 					_drawerLayout.RemoveDrawerListener(_drawerListener);
+					_drawerListener = null;
 				}
 
 				_drawerToggle = null;