From 340a705c5e2f912f97e8e276215de4ff20583e59 Mon Sep 17 00:00:00 2001 From: adrianknight89 Date: Wed, 15 Feb 2017 05:38:22 -0600 Subject: [PATCH] [iOS] Dispose pickers properly and automate GC checks (#632) * dispose datepicker * add dispose to other pickers * add ui test * add dispose to picker model * change class name --- .../Issue1023.cs | 162 +++++++++++++++++++++ .../Xamarin.Forms.Controls.Issues.Shared.projitems | 3 +- .../Renderers/DatePickerRenderer.cs | 46 +++++- .../Renderers/PickerRenderer.cs | 58 +++++++- .../Renderers/TimePickerRenderer.cs | 23 ++- 5 files changed, 277 insertions(+), 15 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1023.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1023.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1023.cs new file mode 100644 index 0000000..a09a49a --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1023.cs @@ -0,0 +1,162 @@ +using System; +using System.Collections.Generic; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; +using System.Threading; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.None, 1023, "Automate GC checks of picker disposals", PlatformAffected.iOS)] + public class Bugzilla1023 : TestNavigationPage + { + protected override void Init() + { + PushAsync(new LandingPage1023()); + } + +#if UITEST && __IOS__ + [Test] + public void Bugzilla1023Test() + { + for (var n = 0; n < 10; n++) + { + RunningApp.WaitForElement(q => q.Marked("Push")); + RunningApp.Tap(q => q.Marked("Push")); + + RunningApp.WaitForElement(q => q.Marked("ListView")); + RunningApp.Back(); + } + + // At this point, the counter can be any value, but it's most likely not zero. + // Invoking GC once is enough to clean up all garbage data and set counter to zero + RunningApp.WaitForElement(q => q.Marked("GC")); + RunningApp.Tap(q => q.Marked("GC")); + + RunningApp.WaitForElement(q => q.Marked("Counter: 0")); + } +#endif + } + + [Preserve(AllMembers = true)] + public class LandingPage1023 : ContentPage + { + public static int Counter; + public Label Label; + + public LandingPage1023() + { + Label = new Label + { + Text = "Counter: " + Counter, + HorizontalTextAlignment = TextAlignment.Center, + VerticalTextAlignment = TextAlignment.Center + }; + + Content = new StackLayout + { + Orientation = StackOrientation.Vertical, + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center, + Spacing = 15, + Children = + { + new Label + { + Text = "Click Push to show a ListView. When you hit the Back button, Counter will show the number of pages that have not been finalized yet." + + " If you click GC, the counter should be 0." + }, + Label, + new Button + { + Text = "GC", + AutomationId = "GC", + Command = new Command(o => + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + Label.Text = "Counter: " + Counter; + }) + }, + new Button + { + Text = "Push", + AutomationId = "Push", + Command = new Command(async o => + { + await Navigation.PushAsync(new ContentPage1023()); + }) + } + } + }; + } + + protected override void OnAppearing() + { + base.OnAppearing(); + + if (Label != null) + Label.Text = "Counter: " + Counter; + } + } + + [Preserve(AllMembers = true)] + public class ContentPage1023 : ContentPage + { + public ContentPage1023() + { + Interlocked.Increment(ref LandingPage1023.Counter); + System.Diagnostics.Debug.WriteLine("Page: " + LandingPage1023.Counter); + + Content = new ListView + { + HasUnevenRows = true, + ItemsSource = new List { "DatePicker", "Picker", "TimePicker" }, + ItemTemplate = new DataTemplateSelector1023(), + AutomationId = "ListView" + }; + } + + ~ContentPage1023() + { + Interlocked.Decrement(ref LandingPage1023.Counter); + System.Diagnostics.Debug.WriteLine("Page: " + LandingPage1023.Counter); + } + } + + [Preserve(AllMembers = true)] + public class DataTemplateSelector1023 : DataTemplateSelector + { + public DataTemplate DatePickerTemplate { get; set; } + public DataTemplate PickerTemplate { get; set; } + public DataTemplate TimePickerTemplate { get; set; } + + public DataTemplateSelector1023() + { + DatePickerTemplate = new DataTemplate(() => new ViewCell { View = new DatePicker() }); + PickerTemplate = new DataTemplate(() => new ViewCell { View = new Picker() }); + TimePickerTemplate = new DataTemplate(() => new ViewCell { View = new TimePicker() }); + } + + protected override DataTemplate OnSelectTemplate(object item, BindableObject container) + { + switch (item as string) + { + case "DatePicker": + return DatePickerTemplate; + case "Picker": + return PickerTemplate; + case "TimePicker": + return TimePickerTemplate; + } + + return null; + } + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index c69e1a3..754133d 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -177,6 +177,7 @@ + @@ -634,4 +635,4 @@ MSBuild:UpdateDesignTimeXaml - \ No newline at end of file + diff --git a/Xamarin.Forms.Platform.iOS/Renderers/DatePickerRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/DatePickerRenderer.cs index 710aa36..39e7ab7 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/DatePickerRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/DatePickerRenderer.cs @@ -22,6 +22,7 @@ namespace Xamarin.Forms.Platform.iOS { UIDatePicker _picker; UIColor _defaultTextColor; + bool _disposed; IElementController ElementController => Element as IElementController; @@ -29,7 +30,10 @@ namespace Xamarin.Forms.Platform.iOS { base.OnElementChanged(e); - if (e.OldElement == null) + if (e.NewElement == null) + return; + + if (Control == null) { var entry = new NoCaretField { BorderStyle = UITextBorderStyle.RoundedRect }; @@ -55,13 +59,10 @@ namespace Xamarin.Forms.Platform.iOS SetNativeControl(entry); } - if (e.NewElement != null) - { - UpdateDateFromModel(false); - UpdateMaximumDate(); - UpdateMinimumDate(); - UpdateTextColor(); - } + UpdateDateFromModel(false); + UpdateMaximumDate(); + UpdateMinimumDate(); + UpdateTextColor(); } protected override void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e) @@ -120,5 +121,34 @@ namespace Xamarin.Forms.Platform.iOS else Control.TextColor = textColor.ToUIColor(); } + + protected override void Dispose(bool disposing) + { + if (_disposed) + return; + + _disposed = true; + + if (disposing) + { + _defaultTextColor = null; + + if (_picker != null) + { + _picker.RemoveFromSuperview(); + _picker.ValueChanged -= HandleValueChanged; + _picker.Dispose(); + _picker = null; + } + + if (Control != null) + { + Control.EditingDidBegin -= OnStarted; + Control.EditingDidEnd -= OnEnded; + } + } + + base.Dispose(disposing); + } } } \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS/Renderers/PickerRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/PickerRenderer.cs index c0f10d1..0905dcf 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/PickerRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/PickerRenderer.cs @@ -11,6 +11,7 @@ namespace Xamarin.Forms.Platform.iOS { UIPickerView _picker; UIColor _defaultTextColor; + bool _disposed; IElementController ElementController => Element as IElementController; @@ -139,13 +140,51 @@ namespace Xamarin.Forms.Platform.iOS Control.TextColor = textColor.ToUIColor(); } + protected override void Dispose(bool disposing) + { + if (_disposed) + return; + + _disposed = true; + + if (disposing) + { + _defaultTextColor = null; + + if (_picker != null) + { + if (_picker.Model != null) + { + _picker.Model.Dispose(); + _picker.Model = null; + } + + _picker.RemoveFromSuperview(); + _picker.Dispose(); + _picker = null; + } + + if (Control != null) + { + Control.EditingDidBegin -= OnStarted; + Control.EditingDidEnd -= OnEnded; + } + + if(Element != null) + ((INotifyCollectionChanged)Element.Items).CollectionChanged -= RowsCollectionChanged; + } + + base.Dispose(disposing); + } + class PickerSource : UIPickerViewModel { - readonly PickerRenderer _renderer; + PickerRenderer _renderer; + bool _disposed; - public PickerSource(PickerRenderer model) + public PickerSource(PickerRenderer renderer) { - _renderer = model; + _renderer = renderer; } public int SelectedIndex { get; internal set; } @@ -183,6 +222,19 @@ namespace Xamarin.Forms.Platform.iOS if(_renderer.Element.On().UpdateMode() == UpdateMode.Immediately) _renderer.UpdatePickerFromModel(this); } + + protected override void Dispose(bool disposing) + { + if (_disposed) + return; + + _disposed = true; + + if (disposing) + _renderer = null; + + base.Dispose(disposing); + } } } } \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS/Renderers/TimePickerRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/TimePickerRenderer.cs index f263276..bfb96d8 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/TimePickerRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/TimePickerRenderer.cs @@ -10,17 +10,34 @@ namespace Xamarin.Forms.Platform.iOS { UIDatePicker _picker; UIColor _defaultTextColor; + bool _disposed; IElementController ElementController => Element as IElementController; protected override void Dispose(bool disposing) { + if (_disposed) + return; + + _disposed = true; + if (disposing) { - Control.EditingDidBegin -= OnStarted; - Control.EditingDidEnd -= OnEnded; + _defaultTextColor = null; - _picker.ValueChanged -= OnValueChanged; + if (_picker != null) + { + _picker.RemoveFromSuperview(); + _picker.ValueChanged -= OnValueChanged; + _picker.Dispose(); + _picker = null; + } + + if (Control != null) + { + Control.EditingDidBegin -= OnStarted; + Control.EditingDidEnd -= OnEnded; + } } base.Dispose(disposing); -- 2.7.4