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

Fix DatePicker on Android so that it can GC #8505

Merged
merged 4 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
19 changes: 17 additions & 2 deletions src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using Android.App;
using Android.Content.Res;
using Android.Graphics.Drawables;
using Android.Views;
using Microsoft.Maui.Devices;

namespace Microsoft.Maui.Handlers
Expand Down Expand Up @@ -31,7 +30,21 @@ protected override MauiDatePicker CreatePlatformView()
protected override void ConnectHandler(MauiDatePicker platformView)
{
base.ConnectHandler(platformView);
platformView.ViewAttachedToWindow += OnViewAttachedToWindow;
platformView.ViewDetachedFromWindow += OnViewDetachedFromWindow;

if (platformView.IsAttachedToWindow)
DeviceDisplay.MainDisplayInfoChanged += OnMainDisplayInfoChanged;
PureWeen marked this conversation as resolved.
Show resolved Hide resolved
}

void OnViewDetachedFromWindow(object? sender, View.ViewDetachedFromWindowEventArgs e)
{
// I tested and this is called when an activity is destroyed
DeviceDisplay.MainDisplayInfoChanged -= OnMainDisplayInfoChanged;
}

void OnViewAttachedToWindow(object? sender, View.ViewAttachedToWindowEventArgs e)
{
DeviceDisplay.MainDisplayInfoChanged += OnMainDisplayInfoChanged;
}

Expand All @@ -44,6 +57,8 @@ protected override void DisconnectHandler(MauiDatePicker platformView)
_dialog = null;
}

platformView.ViewAttachedToWindow -= OnViewAttachedToWindow;
platformView.ViewDetachedFromWindow -= OnViewDetachedFromWindow;
DeviceDisplay.MainDisplayInfoChanged -= OnMainDisplayInfoChanged;

base.DisconnectHandler(platformView);
Expand Down
9 changes: 9 additions & 0 deletions src/Core/tests/DeviceTests/Handlers/HandlerTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ protected TCustomHandler CreateHandler<THandler, TCustomHandler>(IElement view,
return handler;
}


protected IPlatformViewHandler CreateHandler(IElement view, Type handlerType)
{
var handler = (IPlatformViewHandler)Activator.CreateInstance(handlerType);
InitializeViewHandler(view, handler, MauiContext);
return handler;

}

public void Dispose()
{
((IDisposable)_mauiApp).Dispose();
Expand Down
55 changes: 1 addition & 54 deletions src/Core/tests/DeviceTests/Handlers/HandlerTestBaseOfT.Tests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Maui.DeviceTests.Stubs;
Expand All @@ -12,58 +11,6 @@ namespace Microsoft.Maui.DeviceTests
{
public abstract partial class HandlerTestBase<THandler, TStub>
{

// This way of testing leaks currently doesn't seem to work on WinAppSDK
// If you set a break point and the app breaks then the test passes?!?
// Not sure if you can test this type of thing with WinAppSDK or not
#if !WINDOWS
[Fact(DisplayName = "Handlers Deallocate When No Longer Referenced")]
public async Task HandlersDeallocateWhenNoLongerReferenced()
{
// Once this includes all handlers we can delete this
Type[] testedTypes = new[]
{
typeof(EditorHandler),
#if IOS
typeof(DatePickerHandler)
#endif
};

if (!testedTypes.Any(t => t.IsAssignableTo(typeof(THandler))))
return;

var handler = await CreateHandlerAsync(new TStub()) as IPlatformViewHandler;

WeakReference<THandler> weakHandler = new WeakReference<THandler>((THandler)handler);
WeakReference<TStub> weakView = new WeakReference<TStub>((TStub)handler.VirtualView);

handler = null;

await AssertionExtensions.Wait(() =>
{
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();

if (weakHandler.TryGetTarget(out THandler _) ||
weakView.TryGetTarget(out TStub _))
{
return false;
}

return true;

}, 1500);

if (weakHandler.TryGetTarget(out THandler _))
Assert.True(false, $"{typeof(THandler)} failed to collect");

if (weakView.TryGetTarget(out TStub _))
Assert.True(false, $"{typeof(TStub)} failed to collect");
}
#endif

[Fact]
public async Task DisconnectHandlerDoesntCrash()
{
Expand All @@ -73,7 +20,7 @@ await InvokeOnMainThreadAsync(() =>
handler.DisconnectHandler();
});
}

[Fact(DisplayName = "Automation Id is set correctly")]
public async Task SetAutomationId()
{
Expand Down
44 changes: 44 additions & 0 deletions src/Core/tests/DeviceTests/Memory/MemoryTestFixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System;
using System.Collections.Generic;


namespace Microsoft.Maui.Handlers.Memory
{
public class MemoryTestFixture : IDisposable
{
Dictionary<Type, (WeakReference handler, WeakReference view)> _handlers
= new Dictionary<Type, (WeakReference handler, WeakReference view)>();

public MemoryTestFixture()
{
}

public void AddReferences(Type handlerType, (WeakReference handler, WeakReference view) value) =>
_handlers.Add(handlerType, value);

public bool HasType(Type handlerType) => _handlers.ContainsKey(handlerType);

public bool DoReferencesStillExist(Type handlerType)
{
WeakReference weakHandler;
WeakReference weakView;
(weakHandler, weakView) = _handlers[handlerType];


if (weakHandler.Target != null ||
weakHandler.IsAlive ||
weakView.Target != null ||
weakView.IsAlive)
{
return true;
}

return false;
}

public void Dispose()
{
_handlers.Clear();
}
}
}
28 changes: 28 additions & 0 deletions src/Core/tests/DeviceTests/Memory/MemoryTestOrdering.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Xunit.Abstractions;
using Xunit.Sdk;


namespace Microsoft.Maui.Handlers.Memory
{
public class MemoryTestOrdering : ITestCaseOrderer
{
public IEnumerable<TTestCase> OrderTestCases<TTestCase>(IEnumerable<TTestCase> testCases)
where TTestCase : ITestCase
{
var result = testCases.ToList();


if (result.Count > 2)
throw new InvalidOperationException("Add new test to sort if you want it to run");

return new List<TTestCase>()
{
result.First(x=> x.TestMethod.Method.Name == nameof(MemoryTests.Allocate)),
result.First(x=> x.TestMethod.Method.Name == nameof(MemoryTests.CheckAllocation)),
};
}
}
}
18 changes: 18 additions & 0 deletions src/Core/tests/DeviceTests/Memory/MemoryTestTypes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using System.Collections;
using System.Collections.Generic;
using Microsoft.Maui.DeviceTests.Stubs;


namespace Microsoft.Maui.Handlers.Memory
{
public class MemoryTestTypes : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
{
yield return new object[] { (typeof(DatePickerStub), typeof(DatePickerHandler)) };
yield return new object[] { (typeof(EditorStub), typeof(EditorHandler)) };
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
88 changes: 88 additions & 0 deletions src/Core/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
using System;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Maui.DeviceTests;
using Microsoft.Maui.DeviceTests.Stubs;
using Microsoft.Maui.Graphics;
using Microsoft.Maui.Media;
using Xunit;
using Xunit.Abstractions;
using Xunit.Sdk;


namespace Microsoft.Maui.Handlers.Memory
{
/// <summary>
/// Trying to allocate and then rely on the GC to collect within the same test was a bit unreliable when running
/// tests from the xHarness CLI.
/// For example, if you call Thread.Sleep it will block the entire test run from moving forward whereas locally
/// it's able to still run in parallel. So, I broke the allocate and check allocate into two steps
/// which seems to make Android and WinUI happier. Low APIs on Android still have issues running via xHarness
/// which is why we only currently run these on API 30+
/// </summary>
[TestCaseOrderer("Microsoft.Maui.Handlers.Memory.MemoryTestOrdering", "Microsoft.Maui.Core.DeviceTests")]
public class MemoryTests : HandlerTestBase, IClassFixture<MemoryTestFixture>
{
MemoryTestFixture _fixture;
public MemoryTests(MemoryTestFixture fixture)
{
_fixture = fixture;
}

[Theory]
[ClassData(typeof(MemoryTestTypes))]
public async Task Allocate((Type ViewType, Type HandlerType) data)
{
if (!OperatingSystem.IsAndroidVersionAtLeast(30))
return;

var handler = await InvokeOnMainThreadAsync(() => CreateHandler((IElement)Activator.CreateInstance(data.ViewType), data.HandlerType));
WeakReference weakHandler = new WeakReference(handler);
_fixture.AddReferences(data.HandlerType, (weakHandler, new WeakReference(handler.VirtualView)));
handler = null;

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
}

[Theory]
[ClassData(typeof(MemoryTestTypes))]
public async Task CheckAllocation((Type ViewType, Type HandlerType) data)
{
if (!OperatingSystem.IsAndroidVersionAtLeast(30))
return;

// This is mainly relevant when running inside the visual runner as a single test
if (!_fixture.HasType(data.HandlerType))
await Allocate(data);

await AssertionExtensions.Wait(() =>
{
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();

if (_fixture.DoReferencesStillExist(data.HandlerType))
{
return false;
}

return true;

}, 5000);

if (_fixture.DoReferencesStillExist(data.HandlerType))
{
Assert.True(false, $"{data.HandlerType} failed to collect.");
}
}
}
}