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] fix memory leak in Image #15062

Merged
merged 8 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
40 changes: 40 additions & 0 deletions src/Controls/tests/DeviceTests/Elements/Image/ImageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,45 @@ await InvokeOnMainThreadAsync(async () =>
await handler.ToPlatform().AssertContainsColor(Colors.Red, MauiContext);
});
}

[Fact("Image Does Not Leak")]
public async Task DoesNotLeak()
{
SetupBuilder();
WeakReference platformViewReference = null;
WeakReference handlerReference = null;

await InvokeOnMainThreadAsync(async () =>
{
var layout = new VerticalStackLayout();
var image = new Image
{
Background = Colors.Black,
Source = "red.png",
};
layout.Add(image);

var handler = CreateHandler<LayoutHandler>(layout);
handlerReference = new WeakReference(image.Handler);
platformViewReference = new WeakReference(image.Handler.PlatformView);
await image.Wait();
});

Assert.NotNull(handlerReference);
Assert.NotNull(platformViewReference);

// Several GCs required on iOS
for (int i = 0; i < 5; i++)
{
if (!handlerReference.IsAlive && !platformViewReference.IsAlive)
break;
await Task.Yield();
GC.Collect();
GC.WaitForPendingFinalizers();
}

Assert.False(handlerReference.IsAlive, "Handler should not be alive!");
Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!");
}
}
}
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Button/ButtonHandler.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public static Task MapImageSourceAsync(IButtonHandler handler, IImage image)
return handler.ImageSourceLoader.UpdateImageSourceAsync();
}

void OnSetImageSource(Drawable? obj)
void IImageSourcePartSetter.SetImageSource(Drawable? obj)
{
PlatformView.Icon = obj;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Button/ButtonHandler.Standard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ public static void MapFont(IButtonHandler handler, ITextStyle button) { }
public static void MapPadding(IButtonHandler handler, IButton button) { }
public static void MapImageSource(IButtonHandler handler, IImage image) { }

void OnSetImageSource(object? obj) { }
void IImageSourcePartSetter.SetImageSource(object? obj) { }
}
}
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Button/ButtonHandler.Tizen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ void OnButtonPressed(object? sender, EventArgs e)
VirtualView?.Pressed();
}

void OnSetImageSource(MauiImageSource? image)
void IImageSourcePartSetter.SetImageSource(MauiImageSource? image)
{
if (image == null)
return;
Expand Down
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Button/ButtonHandler.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public static void MapImageSource(IButtonHandler handler, IImage image) =>
.UpdateImageSourceAsync()
.FireAndForget(handler);

void OnSetImageSource(ImageSource? platformImageSource)
void IImageSourcePartSetter.SetImageSource(ImageSource? platformImageSource)
{
PlatformView.UpdateImageSource(platformImageSource);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Core/src/Handlers/Button/ButtonHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@

namespace Microsoft.Maui.Handlers
{
public partial class ButtonHandler : IButtonHandler
public partial class ButtonHandler : IButtonHandler, IImageSourcePartSetter
{
ImageSourcePartLoader? _imageSourcePartLoader;
public ImageSourcePartLoader ImageSourceLoader =>
_imageSourcePartLoader ??= new ImageSourcePartLoader(this, () => (VirtualView as IImageButton), OnSetImageSource);
_imageSourcePartLoader ??= new ImageSourcePartLoader(this);

public static IPropertyMapper<IImage, IButtonHandler> ImageButtonMapper = new PropertyMapper<IImage, IButtonHandler>()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Button/ButtonHandler.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public static void MapFormatting(IButtonHandler handler, IText button)
handler.PlatformView?.UpdateCharacterSpacing(button);
}

void OnSetImageSource(UIImage? image)
void IImageSourcePartSetter.SetImageSource(UIImage? image)
{
if (image != null)
{
Expand Down
4 changes: 4 additions & 0 deletions src/Core/src/Handlers/Image/IImageHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,9 @@ public partial interface IImageHandler : IViewHandler
new IImage VirtualView { get; }
ImageSourcePartLoader SourceLoader { get; }
new PlatformView PlatformView { get; }

#if IOS || MACCATALYST
void OnWindowChanged() { }
#endif
}
}
20 changes: 20 additions & 0 deletions src/Core/src/Handlers/Image/IImageSourcePartSetter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#if __IOS__ || MACCATALYST
using PlatformImage = UIKit.UIImage;
#elif MONOANDROID
using PlatformImage = Android.Graphics.Drawables.Drawable;
#elif WINDOWS
using PlatformImage = Microsoft.UI.Xaml.Media.ImageSource;
#elif TIZEN
using PlatformImage = Microsoft.Maui.Platform.MauiImageSource;
#elif (NETSTANDARD || !PLATFORM) || (NET6_0_OR_GREATER && !IOS && !ANDROID && !TIZEN)
using PlatformImage = System.Object;
#endif

namespace Microsoft.Maui.Handlers
{
public interface IImageSourcePartSetter : IElementHandler
{
void SetImageSource(PlatformImage? obj);
}
}

2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Image/ImageHandler.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static void MapSource(IImageHandler handler, IImage image) =>
public static Task MapSourceAsync(IImageHandler handler, IImage image) =>
handler.SourceLoader.UpdateImageSourceAsync();

void OnSetImageSource(Drawable? obj) =>
void IImageSourcePartSetter.SetImageSource(Drawable? obj) =>
PlatformView.SetImageDrawable(obj);

public override void PlatformArrange(Graphics.Rect frame)
Expand Down
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Image/ImageHandler.Standard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ public partial class ImageHandler : ViewHandler<IImage, object>
public static void MapAspect(IImageHandler handler, IImage image) { }
public static void MapIsAnimationPlaying(IImageHandler handler, IImage image) { }
public static void MapSource(IImageHandler handler, IImage image) { }
void OnSetImageSource(object? obj) => throw new NotImplementedException();
void IImageSourcePartSetter.SetImageSource(object? obj) => throw new NotImplementedException();
}
}
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Image/ImageHandler.Tizen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static void MapSource(IImageHandler handler, IImage image) =>
public static Task MapSourceAsync(IImageHandler handler, IImage image) =>
handler.SourceLoader.UpdateImageSourceAsync();

void OnSetImageSource(MauiImageSource? obj)
void IImageSourcePartSetter.SetImageSource(MauiImageSource? obj)
{
if (obj == null)
return;
Expand Down
2 changes: 1 addition & 1 deletion src/Core/src/Handlers/Image/ImageHandler.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static void MapSource(IImageHandler handler, IImage image) =>
public static Task MapSourceAsync(IImageHandler handler, IImage image) =>
handler.SourceLoader.UpdateImageSourceAsync();

void OnSetImageSource(ImageSource? obj) =>
void IImageSourcePartSetter.SetImageSource(ImageSource? obj) =>
PlatformView.Source = obj;
}
}
4 changes: 2 additions & 2 deletions src/Core/src/Handlers/Image/ImageHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

namespace Microsoft.Maui.Handlers
{
public partial class ImageHandler : IImageHandler
public partial class ImageHandler : IImageHandler, IImageSourcePartSetter
{
public static IPropertyMapper<IImage, IImageHandler> Mapper = new PropertyMapper<IImage, IImageHandler>(ViewHandler.ViewMapper)
{
Expand All @@ -32,7 +32,7 @@ public partial class ImageHandler : IImageHandler

ImageSourcePartLoader? _imageSourcePartLoader;
public ImageSourcePartLoader SourceLoader =>
_imageSourcePartLoader ??= new ImageSourcePartLoader(this, () => VirtualView, OnSetImageSource);
_imageSourcePartLoader ??= new ImageSourcePartLoader(this);

public ImageHandler() : base(Mapper)
{
Expand Down
17 changes: 3 additions & 14 deletions src/Core/src/Handlers/Image/ImageHandler.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,12 @@ namespace Microsoft.Maui.Handlers
{
public partial class ImageHandler : ViewHandler<IImage, UIImageView>
{
protected override UIImageView CreatePlatformView() => new MauiImageView();

protected override void ConnectHandler(UIImageView platformView)
{
base.ConnectHandler(platformView);

if (PlatformView is MauiImageView imageView)
imageView.WindowChanged += OnWindowChanged;
}
protected override UIImageView CreatePlatformView() => new MauiImageView(this);

protected override void DisconnectHandler(UIImageView platformView)
{
base.DisconnectHandler(platformView);

if (platformView is MauiImageView imageView)
imageView.WindowChanged -= OnWindowChanged;

SourceLoader.Reset();
}

Expand All @@ -52,10 +41,10 @@ public static void MapSource(IImageHandler handler, IImage image) =>
public static Task MapSourceAsync(IImageHandler handler, IImage image) =>
handler.SourceLoader.UpdateImageSourceAsync();

void OnSetImageSource(UIImage? obj) =>
void IImageSourcePartSetter.SetImageSource(UIImage? obj) =>
PlatformView.Image = obj;

void OnWindowChanged(object? sender, EventArgs e)
public void OnWindowChanged()
Copy link
Member

Choose a reason for hiding this comment

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

Do we want this to be a public API ? doesn't seem consistent with other Handlers

{
if (SourceLoader.SourceManager.IsResolutionDependent)
UpdateValue(nameof(IImage.Source));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ protected override ShapeableImageView CreatePlatformView()
return platformView;
}

void OnSetImageSource(Drawable? obj)
void IImageSourcePartSetter.SetImageSource(Drawable? obj)
{
PlatformView.SetImageDrawable(obj);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public static void MapStrokeThickness(IImageButtonHandler handler, IButtonStroke
public static void MapCornerRadius(IImageButtonHandler handler, IButtonStroke buttonStroke) { }
public static void MapPadding(IImageButtonHandler handler, IImageButton imageButton) { }

void OnSetImageSource(object? obj)
void IImageSourcePartSetter.SetImageSource(object? obj)
{
throw new NotImplementedException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ void OnClicked(object? sender, EventArgs e)
VirtualView?.Clicked();
}

void OnSetImageSource(MauiImageSource? img)
void IImageSourcePartSetter.SetImageSource(MauiImageSource? img)
{
if (img == null)
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public static void MapPadding(IImageButtonHandler handler, IImageButton imageBut
(handler.PlatformView as Button)?.UpdatePadding(imageButton);
}

void OnSetImageSource(ImageSource? nativeImageSource)
void IImageSourcePartSetter.SetImageSource(ImageSource? nativeImageSource)
{
PlatformView.UpdateImageSource(nativeImageSource);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Core/src/Handlers/ImageButton/ImageButtonHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

namespace Microsoft.Maui.Handlers
{
public partial class ImageButtonHandler : IImageButtonHandler
public partial class ImageButtonHandler : IImageButtonHandler, IImageSourcePartSetter
{
public static IPropertyMapper<IImage, IImageHandler> ImageMapper = new PropertyMapper<IImage, IImageHandler>(ImageHandler.Mapper);

Expand All @@ -44,7 +44,7 @@ public partial class ImageButtonHandler : IImageButtonHandler

ImageSourcePartLoader? _imageSourcePartLoader;
public ImageSourcePartLoader SourceLoader =>
_imageSourcePartLoader ??= new ImageSourcePartLoader(this, () => VirtualView, OnSetImageSource);
_imageSourcePartLoader ??= new ImageSourcePartLoader(this);

public ImageButtonHandler() : base(Mapper)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ protected override UIButton CreatePlatformView()
return platformView;
}

void OnSetImageSource(UIImage? obj)
void IImageSourcePartSetter.SetImageSource(UIImage? obj)
{
PlatformView.SetImage(obj?.ImageWithRenderingMode(UIImageRenderingMode.AlwaysOriginal), UIControlState.Normal);
PlatformView.HorizontalAlignment = UIControlContentHorizontalAlignment.Fill;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ void UpdateSize()
var icons = textView.GetCompoundDrawables();
if (icons.Length > 1 && icons[1] != null)
{
OnSetImageSource(icons[1]);
((IImageSourcePartSetter)this).SetImageSource(icons[1]);
}
}

Expand All @@ -143,7 +143,7 @@ void UpdateSize()
PlatformView.SetPadding(0, buttonPadding, 0, buttonPadding);
}

void OnSetImageSource(Drawable? drawable)
void IImageSourcePartSetter.SetImageSource(Drawable? drawable)
{
if (drawable != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public static void MapBackground(ISwipeItemMenuItemHandler handler, ISwipeItemMe

public static void MapVisibility(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem view) { }

void OnSetImageSource(object? obj)
void IImageSourcePartSetter.SetImageSource(object? obj)
{
throw new NotImplementedException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static void MapVisibility(ISwipeItemMenuItemHandler handler, ISwipeItemMe

}

void OnSetImageSource(MauiImageSource? obj)
void IImageSourcePartSetter.SetImageSource(MauiImageSource? obj)
{
if (obj != null)
PlatformView.Icon.ResourceUrl = obj.ResourceUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
namespace Microsoft.Maui.Handlers
{
public partial class SwipeItemMenuItemHandler : ISwipeItemMenuItemHandler
#if !WINDOWS
, IImageSourcePartSetter
#endif
{
public static IPropertyMapper<ISwipeItemMenuItem, ISwipeItemMenuItemHandler> Mapper =
new PropertyMapper<ISwipeItemMenuItem, ISwipeItemMenuItemHandler>(ViewHandler.ElementMapper)
Expand Down Expand Up @@ -56,7 +59,7 @@ protected SwipeItemMenuItemHandler(IPropertyMapper? mapper, CommandMapper? comma
#if !WINDOWS
ImageSourcePartLoader? _imageSourcePartLoader;
public ImageSourcePartLoader SourceLoader =>
_imageSourcePartLoader ??= new ImageSourcePartLoader(this, () => VirtualView, OnSetImageSource);
_imageSourcePartLoader ??= new ImageSourcePartLoader(this);


public static void MapSource(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem image) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void OnSwipeItemFrameChanged(object? sender, EventArgs e)
swipeItemMenuItemHandler.UpdateValue(nameof(ISwipeItemMenuItem.Source));
}

void OnSetImageSource(UIImage? image)
void IImageSourcePartSetter.SetImageSource(UIImage? image)
{
if (PlatformView == null || PlatformView.Frame == CGRect.Empty)
return;
Expand Down
Loading