[ios] fix memory leaks in various controls (#18434)

Fixes:

* `ActivityIndicator`
* `BoxView`
* `Polygon`
* `Polyline`
* `IndicatorView`

Some of these were based on `PlatformGraphicsView`, which had a circular
reference, for example:

* `BoxView` -> `BoxViewHandler`/`PlatformGraphicsViewHandler` ->
  `PlatformGraphicsView` -> `BoxView`

* It appears both `IDrawable` and `IGraphicsRenderer` values in
  `PlatformGraphicsView` were circular references.

Various shapes were based on `PlatformGraphicsView`. We don't have
the iOS memory analyzer running on `Microsoft.Maui.Graphics.dll` yet.

Similarly:

* `ActivityIndicator` -> `ActivityIndicatorHandler` ->
  `MauiActivityIndicator` -> `ActivityIndicator`

* `IndicatorView` -> `IndicatorViewHandler` ->
  `MauiPageControl` -> `IndicatorView`

*Open question*: Why didn't the analyzer catch the issues with
`ActivityIndicator` and `IndicatorView`? Investigating why.
This commit is contained in:
Jonathan Peppers 2023-10-30 17:22:53 -05:00 коммит произвёл GitHub
Родитель 6fa96e9f02
Коммит a589b120ce
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
4 изменённых файлов: 64 добавлений и 46 удалений

Просмотреть файл

@ -1,7 +1,9 @@
using System; using System;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.Maui.Controls; using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Handlers;
using Microsoft.Maui.Controls.Handlers.Compatibility; using Microsoft.Maui.Controls.Handlers.Compatibility;
using Microsoft.Maui.Controls.Shapes;
using Microsoft.Maui.Handlers; using Microsoft.Maui.Handlers;
using Microsoft.Maui.Hosting; using Microsoft.Maui.Hosting;
using Xunit; using Xunit;
@ -17,7 +19,9 @@ public class MemoryTests : ControlsHandlerTestBase
{ {
builder.ConfigureMauiHandlers(handlers => builder.ConfigureMauiHandlers(handlers =>
{ {
handlers.AddHandler<ActivityIndicator, ActivityIndicatorHandler>();
handlers.AddHandler<Border, BorderHandler>(); handlers.AddHandler<Border, BorderHandler>();
handlers.AddHandler<BoxView, BoxViewHandler>();
handlers.AddHandler<CheckBox, CheckBoxHandler>(); handlers.AddHandler<CheckBox, CheckBoxHandler>();
handlers.AddHandler<DatePicker, DatePickerHandler>(); handlers.AddHandler<DatePicker, DatePickerHandler>();
handlers.AddHandler<Entry, EntryHandler>(); handlers.AddHandler<Entry, EntryHandler>();
@ -26,18 +30,24 @@ public class MemoryTests : ControlsHandlerTestBase
handlers.AddHandler<Label, LabelHandler>(); handlers.AddHandler<Label, LabelHandler>();
handlers.AddHandler<ListView, ListViewRenderer>(); handlers.AddHandler<ListView, ListViewRenderer>();
handlers.AddHandler<Picker, PickerHandler>(); handlers.AddHandler<Picker, PickerHandler>();
handlers.AddHandler<Polygon, PolygonHandler>();
handlers.AddHandler<Polyline, PolylineHandler>();
handlers.AddHandler<IContentView, ContentViewHandler>(); handlers.AddHandler<IContentView, ContentViewHandler>();
handlers.AddHandler<Image, ImageHandler>(); handlers.AddHandler<Image, ImageHandler>();
handlers.AddHandler<IndicatorView, IndicatorViewHandler>();
handlers.AddHandler<RefreshView, RefreshViewHandler>(); handlers.AddHandler<RefreshView, RefreshViewHandler>();
handlers.AddHandler<IScrollView, ScrollViewHandler>(); handlers.AddHandler<IScrollView, ScrollViewHandler>();
handlers.AddHandler<SwipeView, SwipeViewHandler>(); handlers.AddHandler<SwipeView, SwipeViewHandler>();
handlers.AddHandler<TimePicker, TimePickerHandler>(); handlers.AddHandler<TimePicker, TimePickerHandler>();
handlers.AddHandler<WebView, WebViewHandler>();
}); });
}); });
} }
[Theory("Handler Does Not Leak")] [Theory("Handler Does Not Leak")]
[InlineData(typeof(ActivityIndicator))]
[InlineData(typeof(Border))] [InlineData(typeof(Border))]
[InlineData(typeof(BoxView))]
[InlineData(typeof(ContentView))] [InlineData(typeof(ContentView))]
[InlineData(typeof(CheckBox))] [InlineData(typeof(CheckBox))]
[InlineData(typeof(DatePicker))] [InlineData(typeof(DatePicker))]
@ -45,12 +55,16 @@ public class MemoryTests : ControlsHandlerTestBase
[InlineData(typeof(Editor))] [InlineData(typeof(Editor))]
[InlineData(typeof(GraphicsView))] [InlineData(typeof(GraphicsView))]
[InlineData(typeof(Image))] [InlineData(typeof(Image))]
[InlineData(typeof(IndicatorView))]
[InlineData(typeof(Label))] [InlineData(typeof(Label))]
[InlineData(typeof(Picker))] [InlineData(typeof(Picker))]
[InlineData(typeof(Polygon))]
[InlineData(typeof(Polyline))]
[InlineData(typeof(RefreshView))] [InlineData(typeof(RefreshView))]
[InlineData(typeof(ScrollView))] [InlineData(typeof(ScrollView))]
[InlineData(typeof(SwipeView))] [InlineData(typeof(SwipeView))]
[InlineData(typeof(TimePicker))] [InlineData(typeof(TimePicker))]
[InlineData(typeof(WebView))]
public async Task HandlerDoesNotLeak(Type type) public async Task HandlerDoesNotLeak(Type type)
{ {
SetupBuilder(); SetupBuilder();

Просмотреть файл

@ -8,18 +8,21 @@ namespace Microsoft.Maui.Platform
{ {
public class MauiActivityIndicator : UIActivityIndicatorView, IUIViewLifeCycleEvents public class MauiActivityIndicator : UIActivityIndicatorView, IUIViewLifeCycleEvents
{ {
IActivityIndicator? _virtualView; readonly WeakReference<IActivityIndicator>? _virtualView;
bool IsRunning => _virtualView is not null && _virtualView.TryGetTarget(out var a) ? a.IsRunning : false;
public MauiActivityIndicator(CGRect rect, IActivityIndicator? virtualView) : base(rect) public MauiActivityIndicator(CGRect rect, IActivityIndicator? virtualView) : base(rect)
{ {
_virtualView = virtualView; if (virtualView is not null)
_virtualView = new(virtualView);
} }
public override void Draw(CGRect rect) public override void Draw(CGRect rect)
{ {
base.Draw(rect); base.Draw(rect);
if (_virtualView?.IsRunning == true) if (IsRunning)
StartAnimating(); StartAnimating();
else else
StopAnimating(); StopAnimating();
@ -29,7 +32,7 @@ namespace Microsoft.Maui.Platform
{ {
base.LayoutSubviews(); base.LayoutSubviews();
if (_virtualView?.IsRunning == true) if (IsRunning)
StartAnimating(); StartAnimating();
else else
StopAnimating(); StopAnimating();
@ -38,10 +41,10 @@ namespace Microsoft.Maui.Platform
protected override void Dispose(bool disposing) protected override void Dispose(bool disposing)
{ {
base.Dispose(disposing); base.Dispose(disposing);
_virtualView = null;
} }
readonly WeakEventManager _weakEventManager = new();
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = IUIViewLifeCycleEvents.UnconditionalSuppressMessage)] [UnconditionalSuppressMessage("Memory", "MA0002", Justification = IUIViewLifeCycleEvents.UnconditionalSuppressMessage)]
EventHandler? _movedToWindow; EventHandler? _movedToWindow;
event EventHandler IUIViewLifeCycleEvents.MovedToWindow event EventHandler IUIViewLifeCycleEvents.MovedToWindow

Просмотреть файл

@ -1,6 +1,5 @@
using System; using System;
using System.Diagnostics.CodeAnalysis; using System.Diagnostics.CodeAnalysis;
using System.Reflection.Metadata;
using CoreGraphics; using CoreGraphics;
using ObjCRuntime; using ObjCRuntime;
using UIKit; using UIKit;
@ -11,7 +10,7 @@ namespace Microsoft.Maui.Platform
{ {
const int DefaultIndicatorSize = 6; const int DefaultIndicatorSize = 6;
IIndicatorView? _indicatorView; WeakReference<IIndicatorView>? _indicatorView;
bool _updatingPosition; bool _updatingPosition;
public MauiPageControl() public MauiPageControl()
@ -30,7 +29,7 @@ namespace Microsoft.Maui.Platform
{ {
ValueChanged -= MauiPageControlValueChanged; ValueChanged -= MauiPageControlValueChanged;
} }
_indicatorView = indicatorView; _indicatorView = indicatorView is null ? null : new(indicatorView);
} }
@ -81,11 +80,11 @@ namespace Microsoft.Maui.Platform
int GetCurrentPage() int GetCurrentPage()
{ {
if (_indicatorView == null) if (_indicatorView is null || !_indicatorView.TryGetTarget(out var indicatorView))
return -1; return -1;
var maxVisible = _indicatorView.GetMaximumVisible(); var maxVisible = indicatorView.GetMaximumVisible();
var position = _indicatorView.Position; var position = indicatorView.Position;
var index = position >= maxVisible ? maxVisible - 1 : position; var index = position >= maxVisible ? maxVisible - 1 : position;
return index; return index;
} }
@ -93,9 +92,9 @@ namespace Microsoft.Maui.Platform
public void UpdateIndicatorCount() public void UpdateIndicatorCount()
{ {
if (_indicatorView == null) if (_indicatorView is null || !_indicatorView.TryGetTarget(out var indicatorView))
return; return;
this.UpdatePages(_indicatorView.GetMaximumVisible()); this.UpdatePages(indicatorView.GetMaximumVisible());
UpdatePosition(); UpdatePosition();
} }
@ -136,10 +135,10 @@ namespace Microsoft.Maui.Platform
void MauiPageControlValueChanged(object? sender, System.EventArgs e) void MauiPageControlValueChanged(object? sender, System.EventArgs e)
{ {
if (_updatingPosition || _indicatorView == null) if (_updatingPosition || _indicatorView is null || !_indicatorView.TryGetTarget(out var indicatorView))
return; return;
_indicatorView.Position = (int)CurrentPage; indicatorView.Position = (int)CurrentPage;
//if we are iOS13 or lower and we are using a Square shape //if we are iOS13 or lower and we are using a Square shape
//we need to update the CornerRadius of the new shape. //we need to update the CornerRadius of the new shape.
if (IsSquare && !(OperatingSystem.IsIOSVersionAtLeast(14) || OperatingSystem.IsTvOSVersionAtLeast(14))) if (IsSquare && !(OperatingSystem.IsIOSVersionAtLeast(14) || OperatingSystem.IsTvOSVersionAtLeast(14)))

Просмотреть файл

@ -1,5 +1,4 @@
using System; using System;
using System.Drawing;
using CoreGraphics; using CoreGraphics;
using Foundation; using Foundation;
using UIKit; using UIKit;
@ -9,9 +8,9 @@ namespace Microsoft.Maui.Graphics.Platform
[Register(nameof(PlatformGraphicsView))] [Register(nameof(PlatformGraphicsView))]
public class PlatformGraphicsView : UIView public class PlatformGraphicsView : UIView
{ {
private IGraphicsRenderer _renderer; private WeakReference<IGraphicsRenderer> _renderer;
private CGColorSpace _colorSpace; private CGColorSpace _colorSpace;
private IDrawable _drawable; private WeakReference<IDrawable> _drawable;
private CGRect _lastBounds; private CGRect _lastBounds;
public PlatformGraphicsView(CGRect frame, IDrawable drawable = null, IGraphicsRenderer renderer = null) : base(frame) public PlatformGraphicsView(CGRect frame, IDrawable drawable = null, IGraphicsRenderer renderer = null) : base(frame)
@ -35,57 +34,53 @@ namespace Microsoft.Maui.Graphics.Platform
public IGraphicsRenderer Renderer public IGraphicsRenderer Renderer
{ {
get => _renderer; get => _renderer is not null && _renderer.TryGetTarget(out var r) ? r : null;
set set
{ {
if (_renderer != null) var renderer = Renderer;
if (renderer != null)
{ {
_renderer.Drawable = null; renderer.Drawable = null;
_renderer.GraphicsView = null; renderer.GraphicsView = null;
_renderer.Dispose(); renderer.Dispose();
} }
_renderer = value ?? new DirectRenderer(); renderer = value ?? new DirectRenderer();
_renderer = new(renderer);
_renderer.GraphicsView = this; renderer.GraphicsView = this;
_renderer.Drawable = _drawable; renderer.Drawable = Drawable;
var bounds = Bounds; var bounds = Bounds;
_renderer.SizeChanged((float)bounds.Width, (float)bounds.Height); renderer.SizeChanged((float)bounds.Width, (float)bounds.Height);
} }
} }
public IDrawable Drawable public IDrawable Drawable
{ {
get => _drawable; get => _drawable is not null && _drawable.TryGetTarget(out var d) ? d : null;
set set
{ {
_drawable = value; _drawable = new(value);
if (_renderer != null) if (Renderer is IGraphicsRenderer renderer)
{ {
_renderer.Drawable = _drawable; renderer.Drawable = value;
_renderer.Invalidate(); renderer.Invalidate();
} }
} }
} }
public void InvalidateDrawable() public void InvalidateDrawable() => Renderer?.Invalidate();
{
_renderer.Invalidate();
}
public void InvalidateDrawable(float x, float y, float w, float h) public void InvalidateDrawable(float x, float y, float w, float h) => Renderer?.Invalidate(x, y, w, h);
{
_renderer.Invalidate(x, y, w, h);
}
public override void WillMoveToSuperview(UIView newSuperview) public override void WillMoveToSuperview(UIView newSuperview)
{ {
base.WillMoveToSuperview(newSuperview); base.WillMoveToSuperview(newSuperview);
if (newSuperview == null) if (newSuperview == null && Renderer is IGraphicsRenderer renderer)
{ {
_renderer.Detached(); renderer.Detached();
} }
} }
@ -107,7 +102,10 @@ namespace Microsoft.Maui.Graphics.Platform
coreGraphics.SetStrokeColorSpace(_colorSpace); coreGraphics.SetStrokeColorSpace(_colorSpace);
coreGraphics.SetPatternPhase(PatternPhase); coreGraphics.SetPatternPhase(PatternPhase);
_renderer.Draw(coreGraphics, dirtyRect.AsRectangleF()); if (Renderer is IGraphicsRenderer renderer)
{
renderer.Draw(coreGraphics, dirtyRect.AsRectangleF());
}
} }
public override CGRect Bounds public override CGRect Bounds
@ -120,8 +118,12 @@ namespace Microsoft.Maui.Graphics.Platform
if (_lastBounds.Width != newBounds.Width || _lastBounds.Height != newBounds.Height) if (_lastBounds.Width != newBounds.Width || _lastBounds.Height != newBounds.Height)
{ {
base.Bounds = value; base.Bounds = value;
_renderer.SizeChanged((float)newBounds.Width, (float)newBounds.Height);
_renderer.Invalidate(); if (Renderer is IGraphicsRenderer renderer)
{
renderer.SizeChanged((float)newBounds.Width, (float)newBounds.Height);
renderer.Invalidate();
}
_lastBounds = newBounds; _lastBounds = newBounds;
} }