[Android] Memory leak when MasterDetailPage Detail set to NavigationPage (#239)

* Create repro

* Remove unnecessary cast

* Add null checks on weak references in PageContainer

* Remove master/detail fragments from manager when switching master/detail pages
Separate renderer ViewGroup removal from renderer disposal in FragmentContainer
Separate PageContainer disposal from renderer disposal in FragmentContainer
Remove Drawer Listener for NavigationPageRenderer in Dispose

* Fix missing spaces;
Add explicit SPACE_BEFORE_IF_PARENTHESES settings to DotSettings file

* Remove javascript rules

* Remove usage of .ForEach()

4 files changed, 72 insertions, 23 deletions

diff --git a/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs
index 34abfda8..3d85ee07 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs
@@ -75,25 +75,31 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 			return null;
 		}
-
+		
 		public override void OnDestroyView()
 		{
 			if (Page != null)
 			{
-				IVisualElementRenderer renderer = _visualElementRenderer;
-				PageContainer container = _pageContainer;
-
-				if (container.Handle != IntPtr.Zero && renderer.ViewGroup.Handle != IntPtr.Zero)
+				if (_visualElementRenderer != null)
 				{
-					container.RemoveFromParent();
-					renderer.ViewGroup.RemoveFromParent();
-					Page.ClearValue(Android.Platform.RendererProperty);
+					if (_visualElementRenderer.ViewGroup.Handle != IntPtr.Zero)
+					{
+						_visualElementRenderer.ViewGroup.RemoveFromParent();
+					}
 
-					container.Dispose();
-					renderer.Dispose();
+					_visualElementRenderer.Dispose();
 				}
+
+				if (_pageContainer != null && _pageContainer.Handle != IntPtr.Zero)
+				{
+					_pageContainer.RemoveFromParent();
+					_pageContainer.Dispose();
+				}
+
+				Page?.ClearValue(Android.Platform.RendererProperty);
 			}
 
+			_onCreateCallback = null;
 			_visualElementRenderer = null;
 			_pageContainer = null;
 
@@ -108,9 +114,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				return;
 
 			if (hidden)
-				PageController.SendDisappearing();
+				PageController?.SendDisappearing();
 			else
-				PageController.SendAppearing();
+				PageController?.SendAppearing();
 		}
 
 		public override void OnPause()
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
index e001389f..102bac53 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
@@ -12,6 +12,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 		FragmentManager _fragmentManager;
 		readonly bool _isMaster;
 		readonly MasterDetailPage _parent;
+		Fragment _currentFragment;
 
 		public MasterDetailContainer(MasterDetailPage parent, bool isMaster, Context context) : base(parent, isMaster, context)
 		{
@@ -59,17 +60,30 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 			if (page == null)
 			{
-				// Not a NavigationPage or TabbedPage? Just do the normal thing
+				// The thing we're adding is not a NavigationPage or TabbedPage, so we can just use the old AddChildView 
+
+				if (_currentFragment != null)
+				{
+					// But first, if the previous occupant of this container was a fragment, we need to remove it properly
+					FragmentTransaction transaction = FragmentManager.BeginTransaction();
+					transaction.DisallowAddToBackStack();
+					transaction.Remove(_currentFragment);
+					transaction.SetTransition((int)FragmentTransit.None);
+					transaction.Commit();
+
+					_currentFragment = null;
+				}
+				
 				base.AddChildView(childView);
 			}
 			else
 			{
 				// The renderers for NavigationPage and TabbedPage both host fragments, so they need to be wrapped in a 
 				// FragmentContainer in order to get isolated fragment management
-
 				Fragment fragment = FragmentContainer.CreateInstance(page);
 				
 				var fc = fragment as FragmentContainer;
+
 				fc?.SetOnCreateCallback(pc =>
 				{
 					_pageContainer = pc;
@@ -78,9 +92,17 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 				FragmentTransaction transaction = FragmentManager.BeginTransaction();
 				transaction.DisallowAddToBackStack();
+
+				if (_currentFragment != null)
+				{
+					transaction.Remove(_currentFragment);
+				}
+
 				transaction.Add(Id, fragment);
 				transaction.SetTransition((int)FragmentTransit.FragmentOpen);
 				transaction.Commit();
+
+				_currentFragment = fragment;
 			}
 		}
 
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs
index 50e99e50..6740285c 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs
@@ -268,7 +268,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				UpdateMaster();
 			else if (e.PropertyName == "Detail")
 				UpdateDetail();
-			else if (e.PropertyName == "IsGestureEnabled")
+			else if (e.PropertyName == MasterDetailPage.IsGestureEnabledProperty.PropertyName)
 				SetGestureState();
 			else if (e.PropertyName == MasterDetailPage.IsPresentedProperty.PropertyName)
 			{
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs
index cb4fcdbb..c833e0c5 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs
@@ -132,11 +132,14 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 				if (Element != null)
 				{
-					for (var i = 0; i < PageController.InternalChildren.Count; i++)
+					foreach(Element element in PageController.InternalChildren)
 					{
-						var child = PageController.InternalChildren[i] as VisualElement;
+						var child = element as VisualElement;
 						if (child == null)
+						{
 							continue;
+						}
+							
 						IVisualElementRenderer renderer = Android.Platform.GetRenderer(child);
 						renderer?.Dispose();
 					}
@@ -148,7 +151,6 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 					navController.PopToRootRequested -= OnPoppedToRoot;
 					navController.InsertPageBeforeRequested -= OnInsertPageBeforeRequested;
 					navController.RemovePageRequested -= OnRemovePageRequested;
-					PageController.SendDisappearing();
 				}
 
 				if (_toolbarTracker != null)
@@ -165,6 +167,13 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 					_toolbar = null;
 				}
 
+				if (_drawerLayout != null && _drawerListener != null)
+				{
+					_drawerLayout.RemoveDrawerListener(_drawerListener);
+				}
+
+				_drawerToggle = null;
+
 				Current = null;
 
 				Device.Info.PropertyChanged -= DeviceInfoPropertyChanged;
@@ -237,7 +246,10 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				navController.RemovePageRequested += OnRemovePageRequested;
 
 				// If there is already stuff on the stack we need to push it
-				((INavigationPageController)e.NewElement).StackCopy.Reverse().ForEach(p => PushViewAsync(p, false));
+				foreach (Page page in navController.StackCopy.Reverse())
+				{
+					PushViewAsync(page, false);
+				}
 			}
 		}
 
@@ -447,6 +459,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			RemovePage(e.Page);
 		}
 
+		private DrawerMultiplexedListener _drawerListener;
+		private DrawerLayout _drawerLayout;
+
 		void RegisterToolbar()
 		{
 			Context context = Context;
@@ -478,13 +493,20 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			if (renderer == null)
 				return;
 
-			var drawerLayout = (DrawerLayout)renderer;
-			_drawerToggle = new ActionBarDrawerToggle((Activity)context, drawerLayout, bar, global::Android.Resource.String.Ok, global::Android.Resource.String.Ok)
+			_drawerLayout = (DrawerLayout)renderer;
+			_drawerToggle = new ActionBarDrawerToggle((Activity)context, _drawerLayout, bar, global::Android.Resource.String.Ok, global::Android.Resource.String.Ok)
 			{
 				ToolbarNavigationClickListener = new ClickListener(Element)
 			};
 
-			drawerLayout.AddDrawerListener(new DrawerMultiplexedListener { Listeners = { _drawerToggle, renderer } });
+			if (_drawerListener != null) 
+			{
+				_drawerLayout.RemoveDrawerListener(_drawerListener);
+			}
+
+			_drawerListener = new DrawerMultiplexedListener { Listeners = { _drawerToggle, renderer } };
+			_drawerLayout.AddDrawerListener(_drawerListener);
+
 			_drawerToggle.DrawerIndicatorEnabled = true;
 		}
 
@@ -542,7 +564,6 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 		Task<bool> SwitchContentAsync(Page view, bool animated, bool removed = false, bool popToRoot = false)
 		{
-			var activity = (FormsAppCompatActivity)Context;
 			var tcs = new TaskCompletionSource<bool>();
 			Fragment fragment = FragmentContainer.CreateInstance(view);
 			FragmentManager fm = FragmentManager;