Address API and code review on the modal navigation seam

API review and code review found blocking issues in the seam. This applies
all of them.

Recursion (blocking): IModalNavigationHost.IsModalReady folded
IModalNavigationPlatform.IsReady back in, so the natural implementation
`IsReady => host.IsModalReady && ...` recursed into an uncatchable
StackOverflowException. Replaced with IsWindowReady, which is framework
readiness only (window handler + page handler) and is documented as safe to
consult from IsReady.

Threading (blocking): IModalNavigationHost.RequestSync documented itself as
callable from any thread but ran readiness checks, page lifecycle events and
platform presentation on the calling thread. It now marshals the whole
reconciliation entry through the window scope's IDispatcher when a dispatch
is required, and the remaining synchronous reentrancy on the UI thread is
documented.

Failure semantics: a faulting custom PopModalAsync removed the page from
PlatformModalStack and never restored it, so a still-visible native modal
became absent from both stacks and unreachable. Push and pop now follow one
rule: the requested stack is intent, PlatformModalStack is reality, and a
faulted operation restores reality to "did not take effect" so the next
reconciliation pass retries. Push fault removes, pop fault restores.

Deferred pop animation: the pop request leaves the logical stack before the
deferred reconciliation runs, so the reconciled pop always got animated:false
and the caller's flag was silently dropped. Pending pop animation metadata is
now retained until the platform actually dismisses the modal.

Deferred completion: the docs promised faults are rethrown to the navigation
caller, but a deferred operation completes that caller before the platform
runs and the later sync is fire-and-forget. Rather than change when
navigation completes, the contract now states exactly when a fault reaches
the caller (inline application) and when it is logged instead (deferred).

Lifecycle: teardown is now terminal for the override so it cannot be lazily
recreated or leaked after Window.Destroying; only attaching a new handler,
which brings a new service scope, re-enables resolution. A throwing factory
is logged and falls back to the built-in platform permanently instead of
escaping from an arbitrary call site or retrying on every access.
PageAttached is now keyed on the page handler instance, which fixes a real
doubling: the page-change sync can resolve the override before
PageAttachedHandler runs.

Also documents that teardown does not pop still-presented modals and Dispose
must not assume MauiContext is usable, that native dismissals must be routed
back through Navigation.PopModalAsync and PopModalAsync must be idempotent,
and that IsBatchPopping is a Shell-only optimization hint. Adds IsBatchPushing
for symmetry. The factory example no longer uses IPlatformViewHandler, which
external backends cannot compile against, and uses the public TFM-neutral
DisconnectHandlers instead.

Adds regressions for recursion avoidance, background-thread RequestSync
marshalling, inline RequestSync, push and pop fault rollback, fault recovery
by reconciliation, deferred pop animation in both directions, platform
dismissal round-trip, throwing factory, no resurrection after destroy,
recreation on new handler, teardown with presented modals, and late
resolution delivering PageAttached exactly once. Each was verified to fail
against a mutation of the corresponding fix.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: Redth

src/Controls/src/Core/Platform/ModalNavigationManager/IModalNavigationHost.cs

@@ -22,6 +22,9 @@ namespace Microsoft.Maui.Controls.Platform
 	/// reconciliation loop that keeps the platform stack in sync with the requested stack. A platform
 	/// implementation is only responsible for the visual presentation of a single push or pop.
 	/// </para>
+	/// <para>
+	/// Every member except <see cref="RequestSync"/> must be read on the UI thread.
+	/// </para>
 	/// </remarks>
 	public interface IModalNavigationHost
 	{
@@ -33,6 +36,12 @@ public interface IModalNavigationHost
 		/// <summary>
 		/// Gets the <see cref="IMauiContext"/> scoped to <see cref="Window"/>.
 		/// </summary>
+		/// <remarks>
+		/// This context is tied to the window's current handler. It is not guaranteed to be usable once
+		/// the window has been torn down, so do not read it from
+		/// <see cref="System.IDisposable.Dispose"/>. Capture whatever cleanup needs while the platform
+		/// instance is still live.
+		/// </remarks>
 		/// <exception cref="System.InvalidOperationException">
 		/// Thrown when the window does not currently have a handler and therefore has no context.
 		/// </exception>
@@ -67,29 +76,73 @@ public interface IModalNavigationHost
 		Page CurrentPlatformPage { get; }
 
 		/// <summary>
-		/// Gets a value indicating whether the framework considers the window ready to present modals
-		/// (the window and its page both have handlers, and <see cref="IModalNavigationPlatform.IsReady"/>
-		/// returned <see langword="true"/>).
+		/// Gets a value indicating whether the framework side of the window is ready for modal
+		/// presentation: the window and its page both have handlers.
 		/// </summary>
-		bool IsModalReady { get; }
+		/// <remarks>
+		/// <para>
+		/// This intentionally does <b>not</b> include
+		/// <see cref="IModalNavigationPlatform.IsReady"/>. Implementations are expected to combine it
+		/// with their own conditions, for example
+		/// <c>public bool IsReady =&gt; _host.IsWindowReady &amp;&amp; _nativeWindowIsRealized;</c>.
+		/// Because the framework folds <see cref="IModalNavigationPlatform.IsReady"/> into its own
+		/// overall readiness separately, a host property that already included it would recurse into an
+		/// uncatchable <see cref="System.StackOverflowException"/>.
+		/// </para>
+		/// <para>
+		/// Consulting this from <see cref="IModalNavigationPlatform.IsReady"/> is optional. The framework
+		/// never calls <see cref="IModalNavigationPlatform.PushModalAsync(Page, bool)"/> or
+		/// <see cref="IModalNavigationPlatform.PopModalAsync(Page, bool)"/> unless this is already
+		/// <see langword="true"/>.
+		/// </para>
+		/// </remarks>
+		bool IsWindowReady { get; }
 
 		/// <summary>
-		/// Gets a value indicating whether several modals are being dismissed as a single batch, for
-		/// example during a <see cref="Shell"/> pop-to-root. Implementations can use this to dismiss
-		/// without animation or intermediate layout so that the modals in between do not flash on screen.
+		/// Gets a hint indicating that several modals are being dismissed as a single batch, for example
+		/// during a <see cref="Shell"/> pop-to-root.
 		/// </summary>
+		/// <remarks>
+		/// This is an optional optimization hint, not a contract. Implementations can use it to dismiss
+		/// without animation so the modals in between do not flash on screen. It is only ever
+		/// <see langword="true"/> while <see cref="Controls.Window.Page"/> is a <see cref="Shell"/> that
+		/// is popping its modal stack; other batch dismissals report <see langword="false"/>. An
+		/// implementation that ignores it stays correct.
+		/// </remarks>
 		bool IsBatchPopping { get; }
 
+		/// <summary>
+		/// Gets a hint indicating that several modals are being presented as a single batch, for example
+		/// while a <see cref="Shell"/> applies a navigation state that contains multiple modals.
+		/// </summary>
+		/// <remarks>
+		/// The counterpart to <see cref="IsBatchPopping"/>, with the same caveats: it is an optional
+		/// optimization hint that is only ever <see langword="true"/> for <see cref="Shell"/>, and an
+		/// implementation that ignores it stays correct.
+		/// </remarks>
+		bool IsBatchPushing { get; }
+
 		/// <summary>
 		/// Asks the framework to re-run the reconciliation loop that compares the requested modal stack
 		/// with <see cref="PlatformModalStack"/> and issues any push or pop that is still outstanding.
 		/// </summary>
 		/// <remarks>
+		/// <para>
 		/// Call this when <see cref="IModalNavigationPlatform.IsReady"/> transitions from
 		/// <see langword="false"/> to <see langword="true"/>. The framework does not poll
 		/// <see cref="IModalNavigationPlatform.IsReady"/>, so a platform that defers readiness must
 		/// notify the framework through this method or queued modals will never be presented.
-		/// The call is safe to make from any thread and does not block.
+		/// </para>
+		/// <para>
+		/// This is safe to call from any thread: when the caller is not already on the window's UI
+		/// thread the work is marshalled there and this method returns immediately. When called
+		/// <b>on</b> the UI thread the reconciliation starts synchronously, which means
+		/// <see cref="IModalNavigationPlatform.IsReady"/> and possibly
+		/// <see cref="IModalNavigationPlatform.PushModalAsync(Page, bool)"/> or
+		/// <see cref="IModalNavigationPlatform.PopModalAsync(Page, bool)"/> can be re-entered before
+		/// this method returns. Do not call it while holding a lock, and finish mutating your own state
+		/// before calling it.
+		/// </para>
 		/// </remarks>
 		void RequestSync();
 	}

src/Controls/src/Core/Platform/ModalNavigationManager/IModalNavigationPlatform.cs

@@ -24,19 +24,54 @@ namespace Microsoft.Maui.Controls.Platform
 	/// <para>
 	/// All members are invoked on the UI thread.
 	/// </para>
+	/// <para>
+	/// <b>Dismissals that start on the platform.</b> When the user dismisses a modal natively — an
+	/// interactive swipe-to-dismiss, a hardware back press, a native close button — the framework does
+	/// not observe it. The implementation must route it back through
+	/// <c>host.Window.Navigation.PopModalAsync()</c> so the cross-platform modal stack and the page
+	/// lifecycle events stay in sync. The framework then calls
+	/// <see cref="PopModalAsync(Page, bool)"/> for that page as part of applying the pop, so
+	/// <see cref="PopModalAsync(Page, bool)"/> must be idempotent and complete successfully when the
+	/// native modal is already gone.
+	/// </para>
+	/// <para>
+	/// <b>Disposal.</b> <see cref="IDisposable.Dispose"/> is called when the window is destroyed and
+	/// when the window's handler changes. The framework does <b>not</b> call
+	/// <see cref="PopModalAsync(Page, bool)"/> for modals that are still presented at that point, so
+	/// <see cref="IDisposable.Dispose"/> is solely responsible for dismissing and releasing every
+	/// native modal the backend still owns. Do not assume
+	/// <see cref="IModalNavigationHost.MauiContext"/>, the window handler, or any platform view
+	/// obtained through them is still usable there — teardown may already have disposed the window's
+	/// service scope. Capture whatever cleanup needs while the instance is live and treat disposal as
+	/// best effort. A disposed instance is never reused; a new one is created from the factory if the
+	/// window gets a new handler.
+	/// </para>
 	/// </remarks>
 	public interface IModalNavigationPlatform : IDisposable
 	{
 		/// <summary>
 		/// Gets a value indicating whether the backend can present or dismiss a modal right now.
 		/// </summary>
 		/// <remarks>
+		/// <para>
 		/// While this returns <see langword="false"/> the framework still records pushes and pops on the
 		/// cross-platform modal stack and still raises the corresponding page lifecycle events, but it
 		/// does not call <see cref="PushModalAsync(Page, bool)"/> or <see cref="PopModalAsync(Page, bool)"/>.
 		/// When the backend becomes ready it must call
 		/// <see cref="IModalNavigationHost.RequestSync"/> so the queued operations are applied.
 		/// Return <see langword="true"/> unconditionally if the backend has no readiness requirement.
+		/// </para>
+		/// <para>
+		/// Deferring has an observable consequence for callers: <c>Navigation.PushModalAsync</c> and
+		/// <c>Navigation.PopModalAsync</c> complete as soon as the framework has updated its own state,
+		/// so they do not wait for the deferred presentation and cannot report a failure in it. See the
+		/// remarks on <see cref="PushModalAsync(Page, bool)"/> for how deferred faults are reported.
+		/// </para>
+		/// <para>
+		/// Implementations may consult <see cref="IModalNavigationHost.IsWindowReady"/> here. Do not try
+		/// to derive this from the framework's overall modal readiness, which already folds this
+		/// property in.
+		/// </para>
 		/// </remarks>
 		bool IsReady { get; }
 
@@ -53,9 +88,22 @@ public interface IModalNavigationPlatform : IDisposable
 		/// subsequent pop to race the presentation.
 		/// </returns>
 		/// <remarks>
-		/// If this task faults, the framework removes <paramref name="modal"/> from
-		/// <see cref="IModalNavigationHost.PlatformModalStack"/> again and rethrows to the caller of
-		/// <c>PushModalAsync</c>.
+		/// <para>
+		/// If this task faults the framework removes <paramref name="modal"/> from
+		/// <see cref="IModalNavigationHost.PlatformModalStack"/> again, because the modal is not on
+		/// screen. The cross-platform modal stack is left untouched, so
+		/// <c>Navigation.ModalStack</c> still contains the page and the next reconciliation pass retries
+		/// the presentation.
+		/// </para>
+		/// <para>
+		/// Where the fault surfaces depends on when the operation is applied. When
+		/// <see cref="IsReady"/> was <see langword="true"/> at request time the push is applied inline
+		/// and the fault is rethrown to the caller of <c>Navigation.PushModalAsync</c>. When the push was
+		/// deferred because <see cref="IsReady"/> was <see langword="false"/>, that caller has already
+		/// completed, so the fault is instead logged through the window's
+		/// <see cref="Microsoft.Extensions.Logging.ILogger"/> and cannot be observed by navigation code.
+		/// Backends that need deferred failures to be actionable should surface them themselves.
+		/// </para>
 		/// </remarks>
 		Task PushModalAsync(Page modal, bool animated);
 
@@ -67,23 +115,49 @@ public interface IModalNavigationPlatform : IDisposable
 		/// <see cref="IModalNavigationHost.PlatformModalStack"/> when this method is called, so
 		/// <see cref="IModalNavigationHost.CurrentPlatformPage"/> already refers to the page that is
 		/// about to be revealed.</param>
-		/// <param name="animated"><see langword="true"/> to animate the transition. The framework passes
-		/// the value that was supplied to the matching push when the pop is the result of stack
-		/// reconciliation, and the value supplied to <c>PopModalAsync</c> otherwise.</param>
+		/// <param name="animated"><see langword="true"/> to animate the transition. The framework preserves
+		/// the value supplied to <c>Navigation.PopModalAsync</c>, including when the dismissal has to be
+		/// deferred until <see cref="IsReady"/> becomes <see langword="true"/>. For a pop that comes from
+		/// reconciling a stack the app changed wholesale (for example a <see cref="Shell"/> pop-to-root),
+		/// the value recorded for the matching push is used.</param>
 		/// <returns>A task that completes once the modal is off screen.</returns>
 		/// <remarks>
+		/// <para>
 		/// Implementations are responsible for releasing the platform views created for
 		/// <paramref name="modal"/>. The framework detaches the page from its parent after this task
 		/// completes.
+		/// </para>
+		/// <para>
+		/// This must be idempotent. It is called for a page the backend already dismissed natively when
+		/// that dismissal is routed back through <c>Navigation.PopModalAsync</c>, and it must complete
+		/// successfully rather than throw in that case.
+		/// </para>
+		/// <para>
+		/// If this task faults the framework puts <paramref name="modal"/> back on
+		/// <see cref="IModalNavigationHost.PlatformModalStack"/>, because a failed dismissal means the
+		/// modal is presumed to still be on screen and dropping it would leave a visible modal that no
+		/// stack knows about. The cross-platform modal stack is left without the page, so the next
+		/// reconciliation pass retries the dismissal. As with
+		/// <see cref="PushModalAsync(Page, bool)"/>, the fault is rethrown to the caller of
+		/// <c>Navigation.PopModalAsync</c> only when the pop was applied inline; a deferred pop that
+		/// faults is logged instead.
+		/// </para>
 		/// </remarks>
 		Task PopModalAsync(Page modal, bool animated);
 
 		/// <summary>
 		/// Called when the window's page gets a handler, including when the page is replaced.
 		/// </summary>
 		/// <remarks>
+		/// <para>
 		/// Use this to attach platform hooks that depend on the window's content being realized, such as
-		/// a hardware back button handler. This may be called multiple times for the same window.
+		/// a hardware back button handler. This may be called multiple times for the same window. It is
+		/// also called once immediately after the instance is created when the window's page already has
+		/// a handler, so a backend that resolves late does not miss the notification.
+		/// </para>
+		/// <para>
+		/// The framework guarantees it is never delivered twice for the same page-handler attachment.
+		/// </para>
 		/// </remarks>
 		void PageAttached();
 	}

src/Controls/src/Core/Platform/ModalNavigationManager/IModalNavigationPlatformFactory.cs

@@ -19,9 +19,19 @@ namespace Microsoft.Maui.Controls.Platform
 	/// lifecycle events and the push/pop reconciliation loop.
 	/// </para>
 	/// <para>
-	/// The factory itself is resolved from the window's service scope, so it may be registered with any
-	/// lifetime. The framework always calls it once per window and disposes the returned instance, so
-	/// return a new <see cref="IModalNavigationPlatform"/> for each call.
+	/// The factory is resolved from the window's service scope, so it may be registered with any
+	/// lifetime. The framework calls it once per window and disposes the returned instance, so return a
+	/// new <see cref="IModalNavigationPlatform"/> for each call. It is called again for the same window
+	/// only if the window gets a new handler, which brings a new service scope (for example an Android
+	/// activity recreation).
+	/// </para>
+	/// <para>
+	/// If this method throws, the framework logs the exception and permanently falls back to the
+	/// built-in platform for that window rather than rethrowing from whatever call site happened to
+	/// trigger resolution. Creation is not retried, so a failed registration fails deterministically and
+	/// visibly in the log instead of throwing repeatedly from arbitrary navigation code. Prefer
+	/// returning <see langword="null"/> over throwing when a backend deliberately does not want to
+	/// handle a window.
 	/// </para>
 	/// <example>
 	/// Registering a backend that presents modals with its own native window stack:
@@ -35,30 +45,44 @@ namespace Microsoft.Maui.Controls.Platform
 	/// public sealed class MyModalNavigationPlatform : IModalNavigationPlatform
 	/// {
 	///     readonly IModalNavigationHost _host;
+	///     readonly MyNativeModalStack _stack;
 	///
-	///     public MyModalNavigationPlatform(IModalNavigationHost host) =&gt; _host = host;
+	///     public MyModalNavigationPlatform(IModalNavigationHost host)
+	///     {
+	///         _host = host;
+	///         // Captured while the instance is live: Dispose must not depend on the MauiContext.
+	///         _stack = MyNativeModalStack.For(host.MauiContext);
+	///     }
 	///
-	///     public bool IsReady =&gt; true;
+	///     public bool IsReady =&gt; _host.IsWindowReady &amp;&amp; _stack.IsRealized;
 	///
 	///     public async Task PushModalAsync(Page modal, bool animated)
 	///     {
 	///         var nativeView = modal.ToPlatform(_host.MauiContext);
-	///         await MyNativeModalStack.For(_host.Window).PushAsync(nativeView, animated);
+	///         await _stack.PushAsync(nativeView, animated &amp;&amp; !_host.IsBatchPushing);
 	///     }
 	///
 	///     public async Task PopModalAsync(Page modal, bool animated)
 	///     {
-	///         await MyNativeModalStack.For(_host.Window).PopAsync(animated &amp;&amp; !_host.IsBatchPopping);
-	///         (modal.Handler as IPlatformViewHandler)?.Dispose();
+	///         // Idempotent: the modal may already be gone when the user dismissed it natively.
+	///         if (_stack.Contains(modal))
+	///             await _stack.PopAsync(animated &amp;&amp; !_host.IsBatchPopping);
+	///
+	///         modal.DisconnectHandlers();
 	///     }
 	///
-	///     public void PageAttached()
+	///     public void PageAttached() =&gt;
+	///         _stack.BackButtonPressed = () =&gt; _host.CurrentPage?.SendBackButtonPressed() ?? false;
+	///
+	///     // A native dismissal the framework can't see has to be routed back through Navigation so the
+	///     // cross-platform stack and the page lifecycle events stay in sync.
+	///     void OnDismissedNatively() =&gt; _host.Window.Navigation.PopModalAsync(animated: false);
+	///
+	///     public void Dispose()
 	///     {
-	///         MyNativeWindow.For(_host.Window).BackButtonPressed =
-	///             () =&gt; _host.CurrentPage?.SendBackButtonPressed() ?? false;
+	///         // Still-presented modals are NOT popped by the framework during teardown.
+	///         _stack.DismissAll();
 	///     }
-	///
-	///     public void Dispose() =&gt; MyNativeModalStack.Release(_host.Window);
 	/// }
 	///
 	/// // In MauiProgram: