Skip to content

Commit

Permalink
Bring back some aspect of ConvertView on TableView and avoid AT_MOST …
Browse files Browse the repository at this point in the history
…Measure (#20130)

* Bring back some aspect of convertView on TableView

* Update TableViewRenderer.cs

* - add more tests

* - add comments fix double create of views

* - add extra disconnect code to GetCell

* - fix events
  • Loading branch information
PureWeen authored Feb 7, 2024
1 parent f6d4b94 commit 08d0388
Show file tree
Hide file tree
Showing 16 changed files with 404 additions and 56 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
{
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");
}
}
}
}
18 changes: 18 additions & 0 deletions src/Controls/samples/Controls.Sample.UITests/Issues/Issue5924.xaml
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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;
}
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 08d0388

Please sign in to comment.