Skip to content

Commit

Permalink
[ios/catalyst] fix memory leak in DatePicker
Browse files Browse the repository at this point in the history
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiDatePicker.cs(37,27): error MA0003: Subscribing to events with instance method 'OnValueChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in `MemoryTests.cs`:

    ++[InlineData(typeof(DatePicker))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by fixing two places:

* `MauiDatePicker` now uses a `UIDatePickerProxy` type, for iOS

* `DatePickerHandler.MacCatalyst.cs` now uses a `UIDatePickerProxy` type,
  for MacCatalyst.
  • Loading branch information
jonathanpeppers committed Aug 3, 2023
1 parent 9079f91 commit a1fa07e
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 31 deletions.
2 changes: 2 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ void SetupBuilder()
{
handlers.AddHandler<Border, BorderHandler>();
handlers.AddHandler<CheckBox, CheckBoxHandler>();
handlers.AddHandler<DatePicker, DatePickerHandler>();
handlers.AddHandler<Entry, EntryHandler>();
handlers.AddHandler<Editor, EditorHandler>();
handlers.AddHandler<Label, LabelHandler>();
Expand All @@ -33,6 +34,7 @@ void SetupBuilder()
[InlineData(typeof(Border))]
[InlineData(typeof(ContentView))]
[InlineData(typeof(CheckBox))]
[InlineData(typeof(DatePicker))]
[InlineData(typeof(Entry))]
[InlineData(typeof(Editor))]
[InlineData(typeof(Image))]
Expand Down
73 changes: 48 additions & 25 deletions src/Core/src/Handlers/DatePicker/DatePickerHandler.MacCatalyst.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ namespace Microsoft.Maui.Handlers
{
public partial class DatePickerHandler : ViewHandler<IDatePicker, UIDatePicker>
{
readonly UIDatePickerProxy _proxy = new();

protected override UIDatePicker CreatePlatformView()
{
return new UIDatePicker { Mode = UIDatePickerMode.Date, TimeZone = new NSTimeZone("UTC") };
Expand All @@ -16,9 +18,7 @@ protected override UIDatePicker CreatePlatformView()

protected override void ConnectHandler(UIDatePicker platformView)
{
platformView.EditingDidBegin += OnStarted;
platformView.EditingDidEnd += OnEnded;
platformView.ValueChanged += OnValueChanged;
_proxy.Connect(this, VirtualView, platformView);

var date = VirtualView?.Date;
if (date is DateTime dt)
Expand All @@ -31,9 +31,7 @@ protected override void ConnectHandler(UIDatePicker platformView)

protected override void DisconnectHandler(UIDatePicker platformView)
{
platformView.EditingDidBegin -= OnStarted;
platformView.EditingDidEnd -= OnEnded;
platformView.ValueChanged -= OnValueChanged;
_proxy.Disconnect(platformView);

base.DisconnectHandler(platformView);
}
Expand Down Expand Up @@ -77,33 +75,58 @@ public static partial void MapFlowDirection(DatePickerHandler handler, IDatePick

}

void OnValueChanged(object? sender, EventArgs? e)
void SetVirtualViewDate()
{
if (UpdateImmediately) // Platform Specific
SetVirtualViewDate();
if (VirtualView == null)
return;

if (VirtualView != null)
VirtualView.IsFocused = true;
VirtualView.Date = PlatformView.Date.ToDateTime().Date;
}

void OnStarted(object? sender, EventArgs eventArgs)
class UIDatePickerProxy
{
if (VirtualView != null)
VirtualView.IsFocused = true;
}
WeakReference<DatePickerHandler>? _handler;
WeakReference<IDatePicker>? _virtualView;

void OnEnded(object? sender, EventArgs eventArgs)
{
if (VirtualView != null)
VirtualView.IsFocused = false;
}
IDatePicker? VirtualView => _virtualView is not null && _virtualView.TryGetTarget(out var v) ? v : null;

void SetVirtualViewDate()
{
if (VirtualView == null)
return;
public void Connect(DatePickerHandler handler, IDatePicker virtualView, UIDatePicker platformView)
{
_handler = new(handler);
_virtualView = new(virtualView);

VirtualView.Date = PlatformView.Date.ToDateTime().Date;
platformView.EditingDidBegin += OnStarted;
platformView.EditingDidEnd += OnEnded;
platformView.ValueChanged += OnValueChanged;
}

public void Disconnect(UIDatePicker platformView)
{
platformView.EditingDidBegin -= OnStarted;
platformView.EditingDidEnd -= OnEnded;
platformView.ValueChanged -= OnValueChanged;
}

void OnValueChanged(object? sender, EventArgs? e)
{
if (_handler is not null && _handler.TryGetTarget(out var handler) && handler.UpdateImmediately)
handler.SetVirtualViewDate();

if (VirtualView is IDatePicker virtualView)
virtualView.IsFocused = true;
}

void OnStarted(object? sender, EventArgs eventArgs)
{
if (VirtualView is IDatePicker virtualView)
virtualView.IsFocused = true;
}

void OnEnded(object? sender, EventArgs eventArgs)
{
if (VirtualView is IDatePicker virtualView)
virtualView.IsFocused = false;
}
}
}
}
19 changes: 13 additions & 6 deletions src/Core/src/Platform/iOS/MauiDatePicker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ namespace Microsoft.Maui.Platform
public class MauiDatePicker : NoCaretField
{
#if !MACCATALYST
readonly UIDatePickerProxy _proxy = new();

public MauiDatePicker()
{
BorderStyle = UITextBorderStyle.RoundedRect;
Expand Down Expand Up @@ -34,7 +36,7 @@ public MauiDatePicker()

this.EditingDidBegin += OnStarted;
this.EditingDidEnd += OnEnded;
picker.ValueChanged += OnValueChanged;
picker.ValueChanged += _proxy.OnValueChanged;
}

static void OnDoneClicked(object obj)
Expand All @@ -43,9 +45,6 @@ static void OnDoneClicked(object obj)
mdp.MauiDatePickerDelegate?.DoneClicked();
}

void OnValueChanged(object? sender, EventArgs e) =>
MauiDatePickerDelegate?.DatePickerValueChanged();

void OnEnded(object? sender, EventArgs e) =>
MauiDatePickerDelegate?.DatePickerEditingDidEnd();

Expand All @@ -54,11 +53,19 @@ void OnStarted(object? sender, EventArgs e) =>

internal MauiDatePickerDelegate? MauiDatePickerDelegate
{
get;
set;
get => _proxy.MauiDatePickerDelegate;
set => _proxy.MauiDatePickerDelegate = value;
}

internal UIDatePicker? DatePickerDialog { get { return InputView as UIDatePicker; } }

class UIDatePickerProxy
{
internal MauiDatePickerDelegate? MauiDatePickerDelegate { get; set; }

public void OnValueChanged(object? sender, EventArgs e) =>
MauiDatePickerDelegate?.DatePickerValueChanged();
}
#else
public MauiDatePicker()
{
Expand Down

0 comments on commit a1fa07e

Please sign in to comment.