From 6aa96a43915edaa8fa03ab9bf5abf00fd424f3f1 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Tue, 27 Sep 2016 04:51:22 -0600 Subject: Clean up listeners and tag on ButtonRenderer during disposal (#360) Clean up OnFocusChangeListener on ViewRenderer during disposal Prevent memory leak of PageContainer/FragmentContainer when animating fragment transitions Call Destroy() on Map during disposal Rebasing --- .../AppCompat/ButtonRenderer.cs | 7 ++++ .../AppCompat/FragmentContainer.cs | 14 ++++---- .../AppCompat/NavigationPageRenderer.cs | 37 +++++++++++++++------- Xamarin.Forms.Platform.Android/ViewRenderer.cs | 3 +- .../VisualElementRenderer.cs | 5 +++ 5 files changed, 45 insertions(+), 21 deletions(-) (limited to 'Xamarin.Forms.Platform.Android') diff --git a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs index 2d175860..4c33df85 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs @@ -71,6 +71,13 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (disposing) { + if (Control != null) + { + Control.SetOnClickListener(null); + Control.RemoveOnAttachStateChangeListener(this); + Control.Tag = null; + _textColorSwitcher = null; + } } base.Dispose(disposing); diff --git a/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs index 7c7b5250..62e43809 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs @@ -1,9 +1,9 @@ using System; using Android.OS; using Android.Runtime; -using Android.Support.V4.App; using Android.Views; using AView = Android.Views.View; +using Fragment = Android.Support.V4.App.Fragment; namespace Xamarin.Forms.Platform.Android.AppCompat { @@ -75,7 +75,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat return null; } - + public override void OnDestroyView() { if (Page != null) @@ -90,18 +90,16 @@ namespace Xamarin.Forms.Platform.Android.AppCompat _visualElementRenderer.Dispose(); } - if (_pageContainer != null && _pageContainer.Handle != IntPtr.Zero) - { - _pageContainer.RemoveFromParent(); - _pageContainer.Dispose(); - } + // We do *not* eagerly dispose of the _pageContainer here; doing so causes a memory leak + // if animated fragment transitions are enabled (it removes some info that the animation's + // onAnimationEnd handler requires to properly clean things up) + // Instead, we let the garbage collector pick it up later, when we can be sure it's safe Page?.ClearValue(Android.Platform.RendererProperty); } _onCreateCallback = null; _visualElementRenderer = null; - _pageContainer = null; base.OnDestroyView(); } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs index b2b9108a..61d0b322 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs @@ -41,6 +41,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat ToolbarTracker _toolbarTracker; bool _toolbarVisible; + // The following is based on https://android.googlesource.com/platform/frameworks/support/+/refs/heads/master/v4/java/android/support/v4/app/FragmentManager.java#849 + const int TransitionDuration = 220; + public NavigationPageRenderer() { AutoPackage = false; @@ -71,7 +74,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } } - FragmentManager FragmentManager => _fragmentManager ?? (_fragmentManager = ((FormsAppCompatActivity)Context).SupportFragmentManager); + FragmentManager FragmentManager => _fragmentManager ?? (_fragmentManager = ((FormsAppCompatActivity)Context).SupportFragmentManager); IPageController PageController => Element as IPageController; @@ -132,14 +135,14 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (Element != null) { - foreach(Element element in PageController.InternalChildren) + foreach (Element element in PageController.InternalChildren) { var child = element as VisualElement; if (child == null) { continue; } - + IVisualElementRenderer renderer = Android.Platform.GetRenderer(child); renderer?.Dispose(); } @@ -320,7 +323,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat else transaction.SetTransition((int)FragmentTransit.FragmentClose); } - + internal int GetNavBarHeight() { if (!ToolbarVisible) @@ -501,7 +504,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat ToolbarNavigationClickListener = new ClickListener(Element) }; - if (_drawerListener != null) + if (_drawerListener != null) { _drawerLayout.RemoveDrawerListener(_drawerListener); } @@ -569,6 +572,12 @@ namespace Xamarin.Forms.Platform.Android.AppCompat var tcs = new TaskCompletionSource(); Fragment fragment = FragmentContainer.CreateInstance(view); FragmentManager fm = FragmentManager; + +#if DEBUG + // Enables logging of moveToState operations to logcat + FragmentManager.EnableDebugLogging(true); +#endif + List fragments = _fragmentStack; Current = view; @@ -599,7 +608,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat transaction.Remove(currentToRemove); popPage = popToRoot; } - + Fragment toShow = fragments.Last(); // Execute pending transactions so that we can be sure the fragment list is accurate. fm.ExecutePendingTransactions(); @@ -621,7 +630,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat // The fragment transitions don't really SUPPORT telling you when they end // There are some hacks you can do, but they actually are worse than just doing this: - + if (animated) { if (!removed) @@ -633,12 +642,15 @@ namespace Xamarin.Forms.Platform.Android.AppCompat else if (_drawerToggle != null && ((INavigationPageController)Element).StackDepth == 2) AnimateArrowOut(); - Device.StartTimer(TimeSpan.FromMilliseconds(200), () => + Device.StartTimer(TimeSpan.FromMilliseconds(TransitionDuration), () => { tcs.TrySetResult(true); - fragment.UserVisibleHint = true; + fragment.UserVisibleHint = !removed; if (removed) + { UpdateToolbar(); + } + return false; }); } @@ -647,8 +659,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Device.StartTimer(TimeSpan.FromMilliseconds(1), () => { tcs.TrySetResult(true); - fragment.UserVisibleHint = true; + fragment.UserVisibleHint = !removed; UpdateToolbar(); + return false; }); } @@ -656,7 +669,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Context.HideKeyboard(this); ((Platform)Element.Platform).NavAnimationInProgress = false; - // 200ms is how long the animations are, and they are "reversible" in the sense that starting another one slightly before it's done is fine + // TransitionDuration is how long the built-in animations are, and they are "reversible" in the sense that starting another one slightly before it's done is fine return tcs.Task; } @@ -821,4 +834,4 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } } } -} \ No newline at end of file +} diff --git a/Xamarin.Forms.Platform.Android/ViewRenderer.cs b/Xamarin.Forms.Platform.Android/ViewRenderer.cs index f285d6d9..057fbcee 100644 --- a/Xamarin.Forms.Platform.Android/ViewRenderer.cs +++ b/Xamarin.Forms.Platform.Android/ViewRenderer.cs @@ -76,7 +76,8 @@ namespace Xamarin.Forms.Platform.Android { if (Control != null && ManageNativeControlLifetime) { - Control.RemoveFromParent(); + Control.OnFocusChangeListener = null; + RemoveView(Control); Control.Dispose(); Control = null; } diff --git a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs index 404dd23e..745d2cdd 100644 --- a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs +++ b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs @@ -229,6 +229,9 @@ namespace Xamarin.Forms.Platform.Android if (disposing) { + SetOnClickListener(null); + SetOnTouchListener(null); + if (Tracker != null) { Tracker.Dispose(); @@ -273,6 +276,8 @@ namespace Xamarin.Forms.Platform.Android if (Platform.GetRenderer(Element) == this) Platform.SetRenderer(Element, null); + (Element as IElementController).EffectControlProvider = null; + Element = null; } } -- cgit v1.2.3