Browse Source

Raise TopLevel.Closed from managed Dispose() paths (#22045)

TopLevel teardown (HandleClosed) was only ever triggered by the backend
invoking ITopLevelImpl.Closed. Browser, iOS, Android, macOS and the offscreen
(designer previewer) impls never raise it from Dispose(), so
EmbeddableControlRoot.Closed was never raised there and StopRendering() was
never reached - the top level stayed registered in MediaContext forever.

Make teardown idempotent behind EnsureClosed() and call it from the managed
Dispose() paths (EmbeddableControlRoot, OffscreenTopLevel, PopupRoot). The
guard lives in a new non-virtual entry point because WindowBase and Window
override HandleClosed and run side effects before calling base.

Also drop ChoreographerTimer's view-visibility gate on Android. It predates
the render timer rewrite and vetoed ticks that DefaultRenderLoop had explicitly
asked for, so the synchronous compositor round-trip in HandleClosed
(Renderer.Dispose -> MediaContext.SyncDisposeCompositionTarget) could never
complete once the view had unsubscribed - deadlocking every activity destroy.
DefaultRenderLoop already owns the sleep/wake state machine, driven by
StartRendering/StopRendering, which makes the extra gate redundant.

Verified at runtime on X11, Wayland, Win32, macOS, Headless, Browser (WASM),
Android and iOS: Closed fires exactly once per teardown, MediaContext returns
to baseline, and platform-initiated closes still fire exactly once.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
pull/22069/head
Nikita Tsukanov 1 month ago
committed by GitHub
parent
commit
7f456057cd
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 15
      src/Android/Avalonia.Android/AvaloniaView.cs
  2. 22
      src/Android/Avalonia.Android/ChoreographerTimer.cs
  3. 2
      src/Avalonia.Controls/Embedding/EmbeddableControlRoot.cs
  4. 1
      src/Avalonia.Controls/Embedding/Offscreen/OffscreenTopLevel.cs
  5. 1
      src/Avalonia.Controls/Primitives/PopupRoot.cs
  6. 15
      src/Avalonia.Controls/TopLevel.cs
  7. 76
      tests/Avalonia.Controls.UnitTests/TopLevelTests.cs

15
src/Android/Avalonia.Android/AvaloniaView.cs

@ -23,7 +23,7 @@ namespace Avalonia.Android
private readonly ViewImpl _view;
private readonly ExploreByTouchHelper _accessHelper;
private IDisposable? _timerSubscription;
private bool _isRendering;
private bool _surfaceCreated;
public AvaloniaView(Context context) : base(context)
@ -106,13 +106,9 @@ namespace Avalonia.Android
if (_root == null || !_surfaceCreated)
return;
if (isVisible && _timerSubscription == null)
if (isVisible && !_isRendering)
{
if (AndroidPlatform.Timer is { } timer)
{
_timerSubscription = timer.SubscribeView(this);
}
_isRendering = true;
_root.StartRendering();
if (_view.TryGetFeature<IInsetsManager>(out var insetsManager) == true)
@ -120,11 +116,10 @@ namespace Avalonia.Android
(insetsManager as AndroidInsetsManager)?.ApplyStatusBarState();
}
}
else if (!isVisible && _timerSubscription != null)
else if (!isVisible && _isRendering)
{
_isRendering = false;
_root.StopRendering();
_timerSubscription?.Dispose();
_timerSubscription = null;
}
}

22
src/Android/Avalonia.Android/ChoreographerTimer.cs

@ -1,9 +1,7 @@
using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using Avalonia.Reactive;
using Avalonia.Rendering;
using static Avalonia.Android.Platform.SkiaPlatform.AndroidFramebuffer;
@ -17,7 +15,6 @@ namespace Avalonia.Android
private readonly TaskCompletionSource<IntPtr> _choreographer = new();
private readonly AutoResetEvent _event = new(false);
private readonly GCHandle _timerHandle;
private readonly HashSet<AvaloniaView> _views = new();
private Action<TimeSpan>? _tick;
private bool _pendingCallback;
private long _lastTime;
@ -49,23 +46,6 @@ namespace Avalonia.Android
}
}
internal IDisposable SubscribeView(AvaloniaView view)
{
lock (_lock)
{
_views.Add(view);
PostFrameCallbackIfNeeded();
}
return Disposable.Create(
() =>
{
lock (_lock)
_views.Remove(view);
}
);
}
private void Loop()
{
Looper.Prepare();
@ -94,7 +74,7 @@ namespace Avalonia.Android
if(_pendingCallback)
return;
if (_tick == null || _views.Count == 0)
if (_tick == null)
return;
_pendingCallback = true;

2
src/Avalonia.Controls/Embedding/EmbeddableControlRoot.cs

@ -69,7 +69,7 @@ namespace Avalonia.Controls.Embedding
public void Dispose()
{
PlatformImpl?.Dispose();
LayoutManager?.Dispose();
EnsureClosed();
}
}
}

1
src/Avalonia.Controls/Embedding/Offscreen/OffscreenTopLevel.cs

@ -35,6 +35,7 @@ namespace Avalonia.Controls.Embedding.Offscreen
public void Dispose()
{
PlatformImpl?.Dispose();
EnsureClosed();
}
}
}

1
src/Avalonia.Controls/Primitives/PopupRoot.cs

@ -126,6 +126,7 @@ namespace Avalonia.Controls.Primitives
public void Dispose()
{
PlatformImpl?.Dispose();
EnsureClosed();
}
private void UpdatePosition()

15
src/Avalonia.Controls/TopLevel.cs

@ -123,6 +123,7 @@ namespace Avalonia.Controls
private readonly IDisposable? _backGestureSubscription;
private readonly Dictionary<AvaloniaProperty, Action> _platformImplBindings = new();
private double _scaling;
private bool _isClosed;
private Size _clientSize;
private Size? _frameSize;
private WindowTransparencyLevel _actualTransparencyLevel;
@ -233,7 +234,7 @@ namespace Avalonia.Controls
impl.Closed = HandleClosed;
impl.Closed = EnsureClosed;
impl.Paint = HandlePaint;
impl.Resized = HandleResized;
impl.ScalingChanged += HandleScalingChanged;
@ -661,6 +662,18 @@ namespace Avalonia.Controls
private protected void StopRendering() => MediaContext.Instance.RemoveTopLevel(this);
/// <summary>
/// Runs the top level teardown exactly once, no matter whether it was initiated by the
/// platform via <see cref="ITopLevelImpl.Closed"/> or by a managed <c>Dispose()</c>.
/// </summary>
private protected void EnsureClosed()
{
if (_isClosed)
return;
_isClosed = true;
HandleClosed();
}
/// <summary>
/// Handles a closed notification from <see cref="ITopLevelImpl.Closed"/>.
/// </summary>

76
tests/Avalonia.Controls.UnitTests/TopLevelTests.cs

@ -1,4 +1,5 @@
using System;
using Avalonia.Controls.Embedding;
using Avalonia.Controls.Presenters;
using Avalonia.Controls.Templates;
using Avalonia.Input;
@ -166,6 +167,81 @@ namespace Avalonia.Controls.UnitTests
}
}
[Fact]
public void Impl_Close_Should_Raise_Closed_Event_Only_Once()
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var impl = CreateMockTopLevelImpl(true);
var raised = 0;
var target = new TestTopLevel(impl.Object);
target.Closed += (s, e) => raised++;
impl.Object.Closed!();
impl.Object.Closed!();
Assert.Equal(1, raised);
}
}
[Fact]
public void EmbeddableControlRoot_Dispose_Should_Raise_Closed_Event()
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var impl = CreateMockTopLevelImpl(true);
var raised = 0;
var target = new EmbeddableControlRoot(impl.Object);
target.Closed += (s, e) => raised++;
target.Dispose();
Assert.Equal(1, raised);
}
}
[Fact]
public void EmbeddableControlRoot_Dispose_After_Impl_Close_Should_Raise_Closed_Event_Only_Once()
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var impl = CreateMockTopLevelImpl(true);
var raised = 0;
var target = new EmbeddableControlRoot(impl.Object);
target.Closed += (s, e) => raised++;
impl.Object.Closed!();
target.Dispose();
Assert.Equal(1, raised);
}
}
[Fact]
public void EmbeddableControlRoot_Dispose_Should_Dispose_Impl_Before_Teardown()
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var impl = CreateMockTopLevelImpl(true);
var closedRaised = false;
var implDisposedBeforeClosed = false;
impl.Setup(x => x.Dispose()).Callback(() => implDisposedBeforeClosed = !closedRaised);
var target = new EmbeddableControlRoot(impl.Object);
target.Closed += (s, e) => closedRaised = true;
target.Dispose();
impl.Verify(x => x.Dispose(), Times.Once);
Assert.True(implDisposedBeforeClosed);
Assert.True(closedRaised);
}
}
[Fact]
public void Impl_Close_Should_Raise_DetachedFromLogicalTree_Event()
{

Loading…
Cancel
Save