[Android] Fix FlyoutPage detail-swap fragment crash causing SwappingDetailPageWorksForSplitFlyoutBehavior device test failure on candidate branch

[praveenkumarkarunanithi]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue.
Thank you!

Root Cause

A FlyoutPage's Detail view is hosted inside a fragment container (navigationlayout_content) within the FlyoutPage's DrawerLayout. Changing Detail swaps that view using an asynchronous fragment transaction (Replace(...).Commit()), which is queued to the main thread and not executed immediately.

PR #35372 added containerView.CurrentView = null to NavigationRootManager.ClearPlatformParts() — a teardown path. That line synchronously removes the DrawerLayout, along with navigationlayout_content.

The crash is caused by an ordering issue: if a detail Replace transaction is still queued when teardown runs, PR #35372 removes its container first; the queued transaction then executes, cannot find navigationlayout_content, and Android throws:

java.lang.IllegalArgumentException: No view found for id 0x7f080156
(...:id/navigationlayout_content) for fragment ScopedFragment{...}
    at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2052)

This terminates the app process; in CI, the run appears hung with no per-test result.

In SwappingDetailPageWorksForSplitFlyoutBehavior, teardown is not triggered by setting Detail. Instead, it happens when the test harness disconnects the Window after CreateHandlerAndAddToWindow(...) completes its action block. The detail swap inside that block leaves a transaction queued; window teardown then removes the container before it executes.

Before PR #35372, teardown did not remove the DrawerLayout, so the queued transaction still found its container and no crash occurred.

Description of Change

In NavigationRootManager.ClearPlatformParts() (Android), pending fragment transactions are drained before clearing the container’s CurrentView, while the container is still attached.

This ensures any in-flight detail replace completes against a valid container, so nothing remains queued when the DrawerLayout is detached.

PR #35372’s leak fix (clearing CurrentView) remains unchanged — this only hardens the ordering by adding the drain immediately before it.

The drain:

• Runs only when the window root is a FlyoutPage
• Safely no-ops when the fragment manager is unavailable
• Safely no-ops when the context is destroyed
• Safely no-ops when instance state has already been saved

Regression Introduced By

PR #35372

Issues Fixed

SwappingDetailPageWorksForSplitFlyoutBehavior

Tested the behaviour in the following platforms

• Android
• Windows
• iOS
• Mac

[sheiksyedm]

/azp run maui-pr-devicetests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[PureWeen]

Adversarial multi-model review — 3 independent reviewers with consensus

Methodology: three independent reviewers analyzed this PR in parallel; findings below survived cross-validation. Severity reflects calibrated consensus, not any single reviewer.

The regression diagnosis in the PR is correct and well-evidenced: FlyoutViewHandler.UpdateDetailsFragmentView() commits the detail Replace(navigationlayout_content, …) asynchronously, and #35372 made the container teardown (CurrentView = null) synchronous — so a committed-but-not-executed transaction can target a container that teardown already removed. Draining does resolve the immediate crash for the failing device test.

That said, the consensus view (and the answer to the explicit "symptom vs. root cause" question) is that this treats the ordering symptom from the wrong layer, and the specific tool chosen (ExecutePendingTransactions on the shared activity FragmentManager) carries real residual hazards.

🔑 Symptom vs. root cause

The true root cause is an ownership/lifecycle mismatch:

• FlyoutViewHandler owns the detail fragment and commits its Replace with async .Commit(). Once committed, FlyoutViewHandler.CancelPendingFragment() cannot cancel it (it only disposes the RunOrWaitForResume handle).
• NavigationRootManager.ClearPlatformParts() — which does not own that fragment — then removes the container synchronously (#35372).

The fix lives in NavigationRootManager and reaches across into another component's fragment work via a global flush. It happens to work because, for the root flyout, both resolve to the same activity FragmentManager — an implicit coupling that isn't guaranteed and that pulls in unrelated transactions.

Recommended root-cause direction: resolve the detail transaction in the layer that owns it — e.g. have FlyoutViewHandler flush/CommitNow (or remove) its detail fragment during DisconnectHandler/CancelPendingFragment, and/or ensure the FlyoutView handler is disconnected before the container is detached. That fixes the ordering at the owning layer and avoids the global-flush and recursion hazards below. If the team needs to unblock the candidate branch now, this patch is an acceptable guarded stopgap — but it should be guarded (see inline) and tracked with a follow-up to move the flush into FlyoutViewHandler.

Findings (ranked)

• ⚠️ Reliability / Re-entrancy (3/3 reviewers; one rated must-fix) — ExecutePendingTransactions() throws IllegalStateException: Recursive entry to executePendingTransactions when called while the FragmentManager is already executing — a hazard this repo already tracks (Bugzilla40333). The IsDestroyed/IsStateSaved guards don't cover it. See inline at line 165.
• ⚠️ Scope / global side effects (3/3 reviewers) — GetFragmentManager() returns the activity-level SupportFragmentManager shared with StackNavigationManager, modal, and tabbed navigation, so the drain flushes every pending transaction app-wide, not just this FlyoutView's detail Replace. See inline at line 165.
• 💡 Resource handling (2/3 reviewers) — when a new detail Replace is pending, draining forces that fragment's view to be created and attached to navigationlayout_content, only for the very next line (CurrentView = null) to tear the whole subtree down. Not a hard leak, but wasted create-then-destroy work during teardown — and a smell that the flush is happening at the wrong moment.

Considered and discarded

• "IsStateSaved guard no-ops the fix in the production teardown case" (1/3) — not applicable: the detail Replace uses .Commit() via RunOrWaitForResume, which defers (does not commit) when state is saved, so there is no committed transaction to crash in that case; skipping the drain is correct.
• "transaction re-queued between the drain and CurrentView = null" (1/3, low-probability) — the detail ScopedFragment.OnViewCreated does not re-trigger Detail mapping, so re-queue in the synchronous window is not a realistic path.

Test coverage

This is a regression fix validated by an existing device test (SwappingDetailPageWorksForSplitFlyoutBehavior), which exercises the synchronous pre-state-save teardown path. No new test is added. The residual concerns above (re-entrancy, global-flush side effects) are not covered by any test — appropriate to leave to the follow-up if the root-cause refactor is deferred.

Prior reviews

No prior reviews on this PR; existing comments are only /azp run triggers. Nothing duplicated.

This is a COMMENT-only review — severity is expressed via ❌/⚠️/💡 markers, not a blocking state.

[PureWeen]

⚠️ Android fragment correctness / Reliability — two concerns flagged by all three reviewers on this ExecutePendingTransactionsEx() call:

1. Re-entrancy crash. executePendingTransactions() throws IllegalStateException: Recursive entry to executePendingTransactions if the FragmentManager is already executing transactions — a hazard this repo already tracks (Bugzilla40333). ClearPlatformParts() runs from both Connect() and Disconnect(), which are reachable from fragment-driven teardown (e.g. a ModalNavigationManager dismiss whose modal content is a FlyoutPage). The IsDestroyed/IsStateSaved guards do not protect against this. If you keep the drain here, guard it:

try
{
    fragmentManager.ExecutePendingTransactionsEx();
}
catch (Java.Lang.IllegalStateException)
{
    // Already executing transactions on this FragmentManager;
    // the pending detail Replace will complete on its own.
}

2. Global blast radius. _mauiContext.GetFragmentManager() resolves to the activity-level SupportFragmentManager, which is shared with StackNavigationManager, modal, and tabbed navigation. ExecutePendingTransactions() synchronously flushes every pending transaction app-wide — not just this FlyoutView's detail Replace — forcing unrelated fragment lifecycle work (e.g. an in-flight animated navigation) to run mid-teardown.

The lower-risk, root-cause-aligned alternative is to settle the detail transaction in its owner (FlyoutViewHandler), scoped to that one transaction, rather than a global drain from NavigationRootManager.

Flagged by: 3/3 reviewers (one rated the re-entrancy must-fix).

[praveenkumarkarunanithi]

@PureWeen

1. Re-entrancy: I tried to reproduce the recursive ExecutePendingTransactions crash across multiple paths — modal FlyoutPage open/swap/dismiss, nested modals, and FlyoutPage → FlyoutPage root swaps (instrumented with a drain-depth counter). The drain was never re-entered, and I could not reproduce the crash. Based on this, no additional handling is being added for now.

2. Global blast radius: Verified through FragmentManager identity logging that the modal path uses a scoped child FragmentManager, while StackNavigationManager uses its own. In all tested scenarios, the drain never flushed unrelated navigation and only settled the FlyoutPage’s own pending detail Replace, so the effective scope remains limited.

Based on the above validation, both concerns are currently considered low risk and are not being addressed in this fix.

[kubaflo]

Could you check Shane's suggestions?

[AdamEssenmacher]

@kubaflo looked over this like you asked. I think the targeted fix here is probably the right incremental correctness improvement.

One small hardening thought; It's probably not necessary, but wrapping

var fragmentManager = _mauiContext?.GetFragmentManager();

in a narrow try/catch might be good in case GetFragmentManager() throws an InvalidOperationException (making it a true no-op)

[kubaflo]

Alright, I think we should revert this PR: #35372 and create a new one that includes enhancement added in this PR. The reason is that we shouldn’t rush this change, and it feels risky to include it in the next release.

[praveenkumarkarunanithi]

Closing this in favor of #36152, which reverts #35372 (the source of this regression).