Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ios/catalyst] fix memory leak in gestures #21887

Merged
merged 2 commits into from
Apr 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class GesturePlatformManager : IDisposable
bool? _defaultAccessibilityRespondsToUserInteraction;

double _previousScale = 1.0;
UITouchEventArgs? _shouldReceiveTouch;
ShouldReceiveTouchProxy? _proxy;
DragAndDropDelegate? _dragAndDropDelegate;

public GesturePlatformManager(IViewHandler handler)
Expand Down Expand Up @@ -340,19 +340,19 @@ weakEventTracker.Target is GesturePlatformManager eventTracker &&
switch (r.State)
{
case UIGestureRecognizerState.Began:
if (r.NumberOfTouches != panRecognizer.TouchPoints)
if (r.NumberOfTouches != ((PanGestureRecognizer)panGestureRecognizer).TouchPoints)
return;
panGestureRecognizer.SendPanStarted(view, PanGestureRecognizer.CurrentId.Value);
break;
case UIGestureRecognizerState.Changed:
if (r.NumberOfTouches != panRecognizer.TouchPoints)
if (r.NumberOfTouches != ((PanGestureRecognizer)panGestureRecognizer).TouchPoints)
{
r.State = UIGestureRecognizerState.Ended;
panGestureRecognizer.SendPanCompleted(view, PanGestureRecognizer.CurrentId.Value);
PanGestureRecognizer.CurrentId.Increment();
return;
}
var translationInView = r.TranslationInView(PlatformView);
var translationInView = r.TranslationInView(eventTracker?.PlatformView);
panGestureRecognizer.SendPan(view, translationInView.X, translationInView.Y, PanGestureRecognizer.CurrentId.Value);
break;
case UIGestureRecognizerState.Cancelled:
Expand All @@ -361,7 +361,7 @@ weakEventTracker.Target is GesturePlatformManager eventTracker &&
PanGestureRecognizer.CurrentId.Increment();
break;
case UIGestureRecognizerState.Ended:
if (r.NumberOfTouches != panRecognizer.TouchPoints)
if (r.NumberOfTouches != ((PanGestureRecognizer)panGestureRecognizer).TouchPoints)
{
panGestureRecognizer.SendPanCompleted(view, PanGestureRecognizer.CurrentId.Value);
PanGestureRecognizer.CurrentId.Increment();
Expand Down Expand Up @@ -564,12 +564,6 @@ void LoadRecognizers()
if (ElementGestureRecognizers == null)
return;

if (_shouldReceiveTouch == null)
{
// Cache this so we don't create a new UITouchEventArgs instance for every recognizer
_shouldReceiveTouch = ShouldReceiveTouch;
}
Comment on lines -567 to -571
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C# 11 and higher does this for you now: dotnet/roslyn#5835


UIDragInteraction? uIDragInteraction = null;
UIDropInteraction? uIDropInteraction = null;

Expand Down Expand Up @@ -669,7 +663,8 @@ _handler.VirtualView is View v &&
{
if (nativeRecognizer != null && PlatformView != null)
{
nativeRecognizer.ShouldReceiveTouch = _shouldReceiveTouch;
_proxy ??= new ShouldReceiveTouchProxy(this);
nativeRecognizer.ShouldReceiveTouch = _proxy.ShouldReceiveTouch;
PlatformView.AddGestureRecognizer(nativeRecognizer);

}
Expand Down Expand Up @@ -738,37 +733,47 @@ void OnTapGestureRecognizerPropertyChanged(object? sender, System.ComponentModel
LoadRecognizers();
}

bool ShouldReceiveTouch(UIGestureRecognizer recognizer, UITouch touch)
class ShouldReceiveTouchProxy
{
var platformView = PlatformView;
var virtualView = _handler?.VirtualView;
readonly WeakReference<GesturePlatformManager> _manager;

if (virtualView == null || platformView == null)
{
return false;
}
public ShouldReceiveTouchProxy(GesturePlatformManager manager) => _manager = new(manager);

if (virtualView.InputTransparent)
public bool ShouldReceiveTouch(UIGestureRecognizer recognizer, UITouch touch)
{
return false;
}
if (!_manager.TryGetTarget(out var manager))
return false;

if (touch.View == platformView)
{
return true;
}
var platformView = manager.PlatformView;
var virtualView = manager._handler?.VirtualView;

// If the touch is coming from the UIView our handler is wrapping (e.g., if it's
// wrapping a UIView which already has a gesture recognizer), then we should let it through
// (This goes for children of that control as well)
if (virtualView == null || platformView == null)
{
return false;
}

if (touch.View.IsDescendantOfView(platformView) &&
(touch.View.GestureRecognizers?.Length > 0 || platformView.GestureRecognizers?.Length > 0))
{
return true;
}
if (virtualView.InputTransparent)
{
return false;
}

if (touch.View == platformView)
{
return true;
}

return false;
// If the touch is coming from the UIView our handler is wrapping (e.g., if it's
// wrapping a UIView which already has a gesture recognizer), then we should let it through
// (This goes for children of that control as well)

if (touch.View.IsDescendantOfView(platformView) &&
(touch.View.GestureRecognizers?.Length > 0 || platformView.GestureRecognizers?.Length > 0))
{
return true;
}

return false;
}
}

void GestureRecognizersOnCollectionChanged(object? sender, NotifyCollectionChangedEventArgs notifyCollectionChangedEventArgs)
Expand Down
56 changes: 56 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Maui.Controls.Handlers;
using Microsoft.Maui.Controls.Handlers.Compatibility;
using Microsoft.Maui.Controls.Handlers.Items;
using Microsoft.Maui.Controls.Platform;
using Microsoft.Maui.Controls.Shapes;
using Microsoft.Maui.Handlers;
using Microsoft.Maui.Hosting;
Expand Down Expand Up @@ -51,6 +52,11 @@ void SetupBuilder()
handlers.AddHandler<TableView, TableViewRenderer>();
handlers.AddHandler<TimePicker, TimePickerHandler>();
handlers.AddHandler<WebView, WebViewHandler>();
#if IOS || MACCATALYST
handlers.AddHandler(typeof(NavigationPage), typeof(NavigationRenderer));
#else
handlers.AddHandler(typeof(NavigationPage), typeof(NavigationViewHandler));
#endif
});
});
}
Expand Down Expand Up @@ -140,6 +146,56 @@ await InvokeOnMainThreadAsync(async () =>
await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
}

[Theory("Gesture Does Not Leak")]
[InlineData(typeof(DragGestureRecognizer))]
[InlineData(typeof(DropGestureRecognizer))]
[InlineData(typeof(PanGestureRecognizer))]
[InlineData(typeof(PinchGestureRecognizer))]
[InlineData(typeof(PointerGestureRecognizer))]
[InlineData(typeof(SwipeGestureRecognizer))]
[InlineData(typeof(TapGestureRecognizer))]
public async Task GestureDoesNotLeak(Type type)
{
SetupBuilder();

WeakReference viewReference = null;
WeakReference handlerReference = null;

var observable = new ObservableCollection<int> { 1 };
var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
{
await navPage.Navigation.PushAsync(new ContentPage
{
Content = new CollectionView
{
ItemTemplate = new DataTemplate(() =>
{
var view = new Label
{
GestureRecognizers =
{
(GestureRecognizer)Activator.CreateInstance(type)
}
};
view.SetBinding(Label.TextProperty, ".");

viewReference = new WeakReference(view);
handlerReference = new WeakReference(view.Handler);

return view;
}),
ItemsSource = observable
}
});

await navPage.Navigation.PopAsync();
});

await AssertionExtensions.WaitForGC(viewReference, handlerReference);
}

#if IOS
[Fact]
public async Task ResignFirstResponderTouchGestureRecognizer()
Expand Down
Loading