Bring back some aspect of ConvertView on TableView and avoid AT_MOST 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
This commit is contained in:
Shane Neuville 2024-02-07 17:59:51 -06:00 коммит произвёл GitHub
Родитель f6d4b9489b
Коммит 08d0388fb4
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
16 изменённых файлов: 403 добавлений и 55 удалений

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

@ -102,6 +102,9 @@ namespace Microsoft.Maui.Controls.ControlGallery.Issues
#if UITEST
[Test]
[Compatibility.UITests.FailsOnMauiIOS]
#if ANDROID
[Compatibility.UITests.MovedToAppium]
#endif
public void Issue5555Test()
{
RunningApp.Tap(q => q.Marked("Push page"));

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

@ -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");
}
}
}
}

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

@ -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);
}
}
}
}

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

@ -12,13 +12,20 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
{
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);

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

@ -15,8 +15,6 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
{
static readonly PropertyChangedEventHandler PropertyChangedHandler = OnGlobalCellPropertyChanged;
EventHandler _onForceUpdateSizeRequested;
public static PropertyMapper<Cell, CellRenderer> Mapper =
new PropertyMapper<Cell, CellRenderer>(ElementHandler.ElementMapper);
@ -48,10 +46,13 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
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;
@ -112,20 +113,35 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
protected void WireUpForceUpdateSizeRequested(Cell cell, AView platformCell)
{
ICellController cellController = cell;
cellController.ForceUpdateSizeRequested -= _onForceUpdateSizeRequested;
cellController.ForceUpdateSizeRequested -= OnForceUpdateSizeRequested;
cellController.ForceUpdateSizeRequested += OnForceUpdateSizeRequested;
}
_onForceUpdateSizeRequested = (sender, e) =>
protected override void DisconnectHandler(AView platformView)
{
if (Cell is ICellController cellController)
cellController.ForceUpdateSizeRequested -= OnForceUpdateSizeRequested;
base.DisconnectHandler(platformView);
}
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);
};
return;
}
cellController.ForceUpdateSizeRequested += _onForceUpdateSizeRequested;
// 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)

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

@ -13,20 +13,10 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
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();
@ -166,5 +156,15 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
_view.EditText.Text = entryCell.Text;
}
void Disconnect()
{
if (_view is null)
return;
_view.TextChanged = null;
_view.FocusChanged = null;
_view.EditingCompleted = null;
}
}
}

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

@ -19,13 +19,6 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
{
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);

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

@ -13,13 +13,6 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
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);

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

@ -20,7 +20,7 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
var cell = (ViewCell)item;
var container = convertView as ViewCellContainer;
if (container != null)
if (container is not null)
{
container.Update(cell);
Performance.Stop(reference, "GetCellCore");
@ -47,16 +47,25 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
var view = (IPlatformViewHandler)cell.View.ToHandler(cell.FindMauiContext());
cell.View.IsPlatformEnabled = true;
ViewCellContainer c = view.PlatformView.GetParentOfType<ViewCellContainer>();
// If the convertView is null we don't want to return the same view, we need to return a new one.
// We should probably do this for ListView as well
if (ParentView is TableView)
{
view.ToPlatform().RemoveFromParent();
}
else
{
ViewCellContainer c = view.ToPlatform().GetParentOfType<ViewCellContainer>();
if (c != null)
return c;
if (c != null)
return c;
}
c = new ViewCellContainer(context, (IPlatformViewHandler)cell.View.Handler, cell, ParentView, unevenRows, rowHeight);
var newContainer = new ViewCellContainer(context, (IPlatformViewHandler)cell.View.Handler, cell, ParentView, unevenRows, rowHeight);
Performance.Stop(reference, "GetCellCore");
return c;
return newContainer;
}
protected override void DisconnectHandler(AView platformView)
@ -142,7 +151,7 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
_unevenRows = unevenRows;
_rowHeight = rowHeight;
_viewCell = viewCell;
AddView(view.PlatformView);
AddView(view.ToPlatform());
UpdateIsEnabled();
UpdateWatchForLongPress();
}
@ -185,7 +194,7 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
public void Update(ViewCell cell)
{
// This cell could have a handler that was used for the measure pass for the Listview height calculations
// This cell could have a handler that was used for the measure pass for the ListView height calculations
//cell.View.Handler.DisconnectHandler();
Performance.Start(out string reference);
@ -250,7 +259,7 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
public void DisconnectHandler()
{
var oldView = _currentView ?? _viewHandler.PlatformView;
var oldView = _currentView ?? _viewHandler.ToPlatform();
if (oldView != null)
RemoveView(oldView);
@ -274,7 +283,7 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
protected override void OnLayout(bool changed, int l, int t, int r, int b)
{
if (_viewHandler.PlatformView == null || Context == null)
if (_viewHandler.PlatformView is null || Context is null)
{
return;
}
@ -291,7 +300,7 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
if (ParentHasUnevenRows)
{
if (_viewHandler.PlatformView == null)
if (_viewHandler.PlatformView is null)
{
SetMeasuredDimension(0, 0);
return;

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

@ -139,6 +139,11 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
return new AView(Context);
}
if (convertView is ConditionalFocusLayout cfl && cfl.ChildCount > 0)
{
convertView = cfl.GetChildAt(0);
}
AView nativeCellContent = CellFactory.GetCell(item, convertView, parent, Context, _view);
// The cell content we get back might already be in a ConditionalFocusLayout; if it is,

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

@ -4,6 +4,7 @@ using Android.Views;
using AndroidX.Core.Widget;
using Microsoft.Maui.Controls.Platform;
using Microsoft.Maui.Graphics;
using Microsoft.Maui.Primitives;
using AListView = Android.Widget.ListView;
using AView = Android.Views.View;
@ -86,6 +87,36 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
}
}
protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec)
{
// If you perform an AtMost (or Unspecified) height measure of ListView, Android will essentially create
// a scrap copy of all the ListView cells to calculate the height.
// https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/widget/ListView.java;l=1314-1322?q=ListView
// This causes issues because if a TextCell already has a view that's attached to the visual tree, then `OnMeasure(AT_MOST)`
// will call "GetView" without a convert view. Android basically creates an in memory copy of the table to calculate the measure.
//
// Our problem is that we don't have a way of knowing if a view we are returning from getView will be the one we
// should track against our TextCellHandler or not.
// This all worked fine in XF because in XF we didn't really block against just creating as many renderers against a single
// VirtualView as you wanted. This led to a whole different set of hard-to-track issues.
// Fundamentally, the ListView control on Android is an old control and the TableView should really be converted to
// a BindableLayout or just generating cross-platform views against a VerticalStackLayout.
//
// We handle the Unspecified path inside "GetDesiredSize" by calculating the height of the cells ourselves and requesting an exact measure.
// Because another quirk of ListView is that if you give it an unspecified measure, it'll just size itself to the first row
// https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/widget/ListView.java;l=1289-1304?q=ListView
//
// There is a path here where we could make our structures play friendly with the ListView and then just let ListView do its scrapview thing
// But, for how we use TableView, converting to an Exactly measure seems good enough for us.
if (heightMeasureSpec.GetMode() == MeasureSpecMode.AtMost)
{
var size = MeasureSpec.GetSize(heightMeasureSpec);
heightMeasureSpec = MeasureSpec.MakeMeasureSpec(size, MeasureSpecMode.Exactly);
}
base.OnMeasure(widthMeasureSpec, heightMeasureSpec);
}
public override SizeRequest GetDesiredSize(double widthConstraint, double heightConstraint)
{
if (double.IsInfinity(heightConstraint))
@ -94,7 +125,11 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
{
heightConstraint = (int)(_adapter.Count * Element.RowHeight);
}
else if (_adapter != null)
else if (this is IViewHandler vh && vh.VirtualView is not null && Dimension.IsExplicitSet(vh.VirtualView.Height) && Context is not null)
{
heightConstraint = MeasureSpec.MakeMeasureSpec((int)Context.ToPixels(vh.VirtualView.Height), MeasureSpecMode.Exactly);
}
else if (_adapter is not null)
{
double totalHeight = 0;
int adapterCount = _adapter.Count;
@ -107,7 +142,15 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
continue;
}
AView listItem = _adapter.GetView(i, null, Control);
// We aren't using ToPlatform because we only want the platform view if it's been created
// Also the cells only implement `IElementHandler` so they don't have ContainerViews
var platformView = cell.Handler?.PlatformView as AView;
// If the view has a parent that means it's already been added to the `ConditionalFocusLayout`
// That's going to be the actual `convertView`
var convertView = platformView?.Parent as AView;
AView listItem = _adapter.GetView(i, convertView, Control);
int widthSpec;
if (double.IsInfinity(widthConstraint))
@ -154,4 +197,4 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
base.Dispose(disposing);
}
}
}
}

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

@ -64,6 +64,7 @@ Microsoft.Maui.Controls.PointerGestureRecognizer.PointerReleasedCommand.set -> v
Microsoft.Maui.Controls.PointerGestureRecognizer.PointerReleasedCommandParameter.get -> object!
Microsoft.Maui.Controls.PointerGestureRecognizer.PointerReleasedCommandParameter.set -> void
override Microsoft.Maui.Controls.Handlers.Compatibility.TableViewModelRenderer.GetItemViewType(int position) -> int
override Microsoft.Maui.Controls.Handlers.Compatibility.TableViewRenderer.OnMeasure(int widthMeasureSpec, int heightMeasureSpec) -> void
override Microsoft.Maui.Controls.Handlers.Items.MauiCarouselRecyclerView.OnAttachedToWindow() -> void
override Microsoft.Maui.Controls.Handlers.Items.MauiCarouselRecyclerView.OnDetachedFromWindow() -> void
static readonly Microsoft.Maui.Controls.KeyboardAccelerator.KeyProperty -> Microsoft.Maui.Controls.BindableProperty!
@ -128,6 +129,7 @@ override Microsoft.Maui.Controls.SearchBar.IsEnabledCore.get -> bool
~Microsoft.Maui.Controls.InputView.FontFamily.set -> void
~Microsoft.Maui.Controls.WebView.UserAgent.get -> string
~Microsoft.Maui.Controls.WebView.UserAgent.set -> void
~override Microsoft.Maui.Controls.Handlers.Compatibility.CellRenderer.DisconnectHandler(Android.Views.View platformView) -> void
~override Microsoft.Maui.Controls.Handlers.Items.ItemsViewHandler<TItemsView>.DisconnectHandler(AndroidX.RecyclerView.Widget.RecyclerView platformView) -> void
~override Microsoft.Maui.Controls.ImageButton.OnPropertyChanged(string propertyName = null) -> void
~override Microsoft.Maui.Controls.LayoutOptions.Equals(object obj) -> bool

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

@ -0,0 +1,35 @@
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;
namespace Microsoft.Maui.AppiumTests.Issues
{
public class Issue5555 : _IssuesUITest
{
public override string Issue => "Memory leak when SwitchCell or EntryCell";
public Issue5555(TestDevice device) : base(device)
{
}
[Test]
public void TableViewMemoryLeakWhenUsingSwitchCellOrEntryCell()
{
this.IgnoreIfPlatforms(new[]
{
TestDevice.Mac,
TestDevice.iOS,
});
App.WaitForElement("PushPage");
App.Click("PushPage");
App.WaitForElement("PushPage");
App.Click("PushPage");
App.WaitForElement("PushPage");
App.WaitForElement("CheckResult");
App.Click("CheckResult");
App.WaitForElement("SuccessLabel");
}
}
}

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

@ -0,0 +1,29 @@
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;
namespace Microsoft.Maui.AppiumTests.Issues
{
public class Issue5924 : _IssuesUITest
{
public override string Issue => "TableView ViewCell vanishes after content is updated";
public Issue5924(TestDevice device) : base(device)
{
}
[Test]
public void TableViewViewCellVanishesAfterContentIsUpdated()
{
App.WaitForElement("entry");
App.EnterText("entry", "I haven't disappeared");
var entry = App.WaitForElement("entry").GetRect();
var label = App.WaitForElement("label").GetRect();
Assert.Greater(entry.Height, 0);
Assert.Greater(entry.Width, 0);
Assert.Greater(label.Height, 0);
Assert.Greater(label.Width, 0);
}
}
}