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

Bring back some aspect of ConvertView on TableView and avoid AT_MOST Measure #20130

Merged
merged 6 commits into from Feb 7, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -102,6 +102,9 @@ protected override void Init()
#if UITEST
[Test]
[Compatibility.UITests.FailsOnMauiIOS]
#if ANDROID
[Compatibility.UITests.MovedToAppium]
#endif
public void Issue5555Test()
{
RunningApp.Tap(q => q.Marked("Push page"));
Expand Down
123 changes: 123 additions & 0 deletions src/Controls/samples/Controls.Sample.UITests/Issues/Issue5555.cs
@@ -0,0 +1,123 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.Maui.Controls;

namespace Maui.Controls.Sample.Issues
{
[Issue(IssueTracker.None, 5555, "Memory leak when SwitchCell or EntryCell", PlatformAffected.iOS)]
public class Issue5555 : TestContentPage
PureWeen marked this conversation as resolved.
Show resolved Hide resolved
{
public static Label DestructorCount = new Label() { Text = "0" };
protected override void Init()
{
var instructions = new Label
{
FontSize = 16,
Text = "Click 'Push page' twice"
};

var result = new Label
{
Text = "Success",
AutomationId = "SuccessLabel",
IsVisible = false
};

var list = new List<WeakReference>();

var checkButton = new Button
{
Text = "Check Result",
AutomationId = "CheckResult",
IsVisible = false,
Command = new Command(async () =>
{
if (list.Count < 2)
{
instructions.Text = "Click 'Push page' again";
return;
}

try
{
await GarbageCollectionHelper.WaitForGC(2500, list.ToArray());
result.Text = "Success";
result.IsVisible = true;
instructions.Text = "";
}
catch (Exception)
{
instructions.Text = "Failed";
result.IsVisible = false;
return;
}
})
};

Content = new StackLayout
{
Children = {
DestructorCount,
instructions,
result,
new Button
{
Text = "Push page",
AutomationId = "PushPage",
Command = new Command(async() => {
if (list.Count >= 2)
list.Clear();

var wref = new WeakReference(new LeakPage());

await Navigation.PushAsync(wref.Target as Page);
await (wref.Target as Page).Navigation.PopAsync();

list.Add(wref);
if (list.Count > 1)
{
checkButton.IsVisible = true;
instructions.Text = "You can check result";
}
else
{
instructions.Text = "Again";
}
})
},
checkButton
}
};
}

class LeakPage : ContentPage
{
public LeakPage()
{
Content = new StackLayout
{
Children = {
new Entry { Text = "LeakPage" },
new TableView
{
Root = new TableRoot
{
new TableSection
{
new SwitchCell { Text = "switch cell", On = true },
new EntryCell { Text = "entry cell" }
}
}
}
}
};
}

~LeakPage()
{
System.Diagnostics.Debug.WriteLine("LeakPage Finalized");
}
}
}
}
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Maui.Controls.Sample.Issues.Issue5924"
xmlns:ns="clr-namespace:Maui.Controls.Sample.Issues">
<TableView Intent="Settings">
<TableRoot>
<TableSection Title="Locations">
<ViewCell>
<AbsoluteLayout>
<Label AutomationId="label" Text="Enter text into the Entry field and it shouldn't disappear" HorizontalOptions="Start" VerticalOptions="Center" AbsoluteLayout.LayoutBounds="0,0.5" AbsoluteLayout.LayoutFlags="PositionProportional"/>
<Entry AutomationId="entry" HorizontalOptions="End" VerticalOptions="Center" AbsoluteLayout.LayoutBounds="1,0.5" AbsoluteLayout.LayoutFlags="PositionProportional"/>
</AbsoluteLayout>
</ViewCell>
</TableSection>
</TableRoot>
</TableView>
</ContentPage>
@@ -0,0 +1,18 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Xaml;

namespace Maui.Controls.Sample.Issues
{
[XamlCompilation(XamlCompilationOptions.Compile)]
[Issue(IssueTracker.Github, 5924, "TableView ViewCell vanishes after content is updated", PlatformAffected.Android)]
public partial class Issue5924 : ContentPage
{
public Issue5924()
{
InitializeComponent();
}
}
}
@@ -0,0 +1,54 @@
using System;
using System.Collections.Generic;
using System.Text;
using System.Threading.Tasks;

namespace Maui.Controls.Sample
{
public static class GarbageCollectionHelper
{
public static async Task WaitForGC(params WeakReference[] references) => await WaitForGC(5000, references);

public static async Task WaitForGC(int timeout, params WeakReference[] references)
{
bool referencesCollected()
{
GC.Collect();
GC.WaitForPendingFinalizers();

foreach (var reference in references)
{
if (reference.IsAlive)
{
return false;
}
}

return true;
}

await AssertEventually(referencesCollected, timeout);
}

public static async Task AssertEventually(this Func<bool> assertion, int timeout = 1000, int interval = 100, string message = "Assertion timed out")
{
do
{
if (assertion())
{
return;
}

await Task.Delay(interval);
timeout -= interval;

}
while (timeout >= 0);

if (!assertion())
{
throw new Exception(message);
}
}
}
}
Expand Up @@ -12,13 +12,20 @@ public static class CellFactory
{
public static AView GetCell(Cell item, AView convertView, ViewGroup parent, Context context, View view)
{
// If the convert view coming in is null that means this cell is going to need a new view generated for it
// This should probably be copied over to ListView once all sets of these TableView changes are propagated There
if (item.Handler is IElementHandler handler && convertView is null && view is TableView)
{
handler.DisconnectHandler();
}

CellRenderer renderer = CellRenderer.GetRenderer(item);
if (renderer == null)
{
var mauiContext = view.FindMauiContext() ?? item.FindMauiContext();
item.ConvertView = convertView;

_ = item.ToPlatform(mauiContext);
convertView = item.ToPlatform(mauiContext);
item.ConvertView = null;

renderer = CellRenderer.GetRenderer(item);
Expand Down
Expand Up @@ -15,8 +15,6 @@ public class CellRenderer : ElementHandler<Cell, AView>, IRegisterable
{
static readonly PropertyChangedEventHandler PropertyChangedHandler = OnGlobalCellPropertyChanged;

EventHandler _onForceUpdateSizeRequested;

public static PropertyMapper<Cell, CellRenderer> Mapper =
new PropertyMapper<Cell, CellRenderer>(ElementHandler.ElementMapper);

Expand Down Expand Up @@ -48,10 +46,13 @@ public AView GetCell(Cell item, AView convertView, ViewGroup parent, Context con

Performance.Start(out string reference);

if (Cell is ICellController cellController)
cellController.ForceUpdateSizeRequested -= OnForceUpdateSizeRequested;

Cell = item;
Cell.PropertyChanged -= PropertyChangedHandler;

if (convertView != null)
if (convertView is not null)
{
Object tag = convertView.Tag;
CellRenderer renderer = (tag as RendererHolder)?.Renderer;
Expand Down Expand Up @@ -112,20 +113,35 @@ protected virtual void OnCellPropertyChanged(object sender, PropertyChangedEvent
protected void WireUpForceUpdateSizeRequested(Cell cell, AView platformCell)
{
ICellController cellController = cell;
cellController.ForceUpdateSizeRequested -= _onForceUpdateSizeRequested;
cellController.ForceUpdateSizeRequested -= OnForceUpdateSizeRequested;
cellController.ForceUpdateSizeRequested += OnForceUpdateSizeRequested;
}

protected override void DisconnectHandler(AView platformView)
{
if (Cell is ICellController cellController)
cellController.ForceUpdateSizeRequested -= OnForceUpdateSizeRequested;

base.DisconnectHandler(platformView);
}

_onForceUpdateSizeRequested = (sender, e) =>
static void OnForceUpdateSizeRequested(object sender, EventArgs e)
{
if (sender is not Cell cellInner)
return;

if (cellInner.Handler is not IElementHandler elementHandler ||
elementHandler.PlatformView is not AView pCell ||
!pCell.IsAlive())
{
if (platformCell.Handle == IntPtr.Zero)
return;
// RenderHeight may not be changed, but that's okay, since we
// don't actually use the height argument in the OnMeasure override.
platformCell.Measure(platformCell.Width, (int)cell.RenderHeight);
platformCell.SetMinimumHeight(platformCell.MeasuredHeight);
platformCell.SetMinimumWidth(platformCell.MeasuredWidth);
};

cellController.ForceUpdateSizeRequested += _onForceUpdateSizeRequested;
return;
}

// RenderHeight may not be changed, but that's okay, since we
// don't actually use the height argument in the OnMeasure override.
pCell.Measure(pCell.Width, (int)cellInner.RenderHeight);
pCell.SetMinimumHeight(pCell.MeasuredHeight);
pCell.SetMinimumWidth(pCell.MeasuredWidth);
}

internal static CellRenderer GetRenderer(Cell cell)
Expand Down
Expand Up @@ -13,20 +13,10 @@ public class EntryCellRenderer : CellRenderer

protected override global::Android.Views.View GetCellCore(Cell item, global::Android.Views.View convertView, ViewGroup parent, Context context)
{
if (item?.Parent is TableView && item.Handler?.PlatformView is EntryCellView entryCellView)
{
// TableView doesn't use convertView
_view = entryCellView;
return _view;
}

Disconnect();
if ((_view = convertView as EntryCellView) == null)
_view = new EntryCellView(context, item);
else
{
_view.TextChanged = null;
_view.FocusChanged = null;
_view.EditingCompleted = null;
_view = new EntryCellView(context, item);
}

UpdateLabel();
Expand Down Expand Up @@ -166,5 +156,15 @@ void UpdateText()

_view.EditText.Text = entryCell.Text;
}

void Disconnect()
{
if (_view is null)
return;

_view.TextChanged = null;
_view.FocusChanged = null;
_view.EditingCompleted = null;
}
}
}
Expand Up @@ -19,13 +19,6 @@ protected override AView GetCellCore(Cell item, AView convertView, ViewGroup par
{
var cell = (SwitchCell)Cell;

if (item?.Parent is TableView && item.Handler?.PlatformView is SwitchCellView switchCellView)
{
// TableView doesn't use convertView
_view = switchCellView;
return _view;
}

if ((_view = convertView as SwitchCellView) == null)
_view = new SwitchCellView(context, item);

Expand Down
Expand Up @@ -13,13 +13,6 @@ public class TextCellRenderer : CellRenderer

protected override AView GetCellCore(Cell item, AView convertView, ViewGroup parent, Context context)
{
if (item?.Parent is TableView && item.Handler?.PlatformView is TextCellView textCellView)
{
// TableView doesn't use convertView
View = textCellView;
return View;
}

if ((View = convertView as TextCellView) == null)
View = new TextCellView(context, item);

Expand Down