[Android] [Candidate Fix] Shell: Fix handler disconnect timing to preserve WebView navigation and memory leak fix

[Shalini-Ashokan]

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 of the Regression

PR #35031 disconnected page handlers in OnDestroyView(). OnDestroyView() runs on normal navigation too. WebView handlers got disconnected too early. That broke WebViewDisposesProperly on Android.

Description of Change

Keep logic in OnDestroyView(), but add a guard. Disconnect only if ShellContent is no longer under Shell. Normal navigation skips disconnect, so page reuse stays safe. Shell.Items.Clear() still disconnects handlers and avoids leaks.

Issues Fixed

UI Test

Android:
WebViewDisposesProperly

[dotnet-policy-service]

Hey there @@Shalini-Ashokan! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[MauiBot]

🤖 AI Summary

👋 @Shalini-Ashokan — new AI review results are available. Please review the latest session below.

📊 Review Session — 364ab42 · Fixed the webview dispose issue · 2026-05-13 10:58 UTC

🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent.

---

🔍 Pre-Flight — Context & Validation

Issue: (none directly linked) — Fixes a UI test regression introduced by PR #35031 (which fixed issue #34898)
PR: #35417 - [Android] [Candidate Fix] Shell: Fix handler disconnect timing to preserve WebView navigation and memory leak fix
Platforms Affected: Android
Files Changed: 1 implementation (src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFragmentContainer.cs), 0 test
Base branch: inflight/candidate (not main)
Author: Shalini-Ashokan (partner/syncfusion)

Key Findings

• Regression context: PR Fix Shell.Items.Clear() memory leak by disconnecting child handlers on removal (#34898) #35031 added _page?.DisconnectHandlers(); unconditionally inside ShellFragmentContainer.OnDestroyView() to fix the Shell.Items.Clear() native-view leak (Shell.Items.Clear() does not disconnect handlers correctly #34898). On Android, OnDestroyView() ALSO fires during normal ViewPager2-backed tab navigation when offscreen fragments are recycled. As a result, page handlers were torn down while the ShellContent was still cached for reuse, breaking the WebViewDisposesProperly UI test (Issue11962) which navigates push/pop multiple times.
• PR's fix: Wrap the DisconnectHandlers() call in a guard: if (ShellContentTab?.FindParentOfType<Shell>() == null) _page?.DisconnectHandlers();. The intent is: when the ShellContent is still attached to a Shell (normal navigation/recycle), keep the page alive; when it has been detached (e.g. Shell.Items.Clear() or ShellSection.Items.Clear()), disconnect handlers to release native views.
• No tests added in this PR. The existing Issue11962.WebViewDisposesProperly test catches the regression on Android, and Issue34898 covers the original leak; both must remain green.
• Surrounding code: ShellFragmentContainer is created by ShellFragmentStateAdapter.CreateFragment (one per ShellContent tab in a ShellSection). It is the wrapper used inside the ViewPager2 adapter, distinct from ShellContentFragment (which handles its own disposal via DisposePage / Dispose(bool)). FindParentOfType<Shell>() is a public extension on Element that walks .Parent up until it hits a Shell or returns null.

Code Review Summary

Verdict: NEEDS_DISCUSSION
Confidence: medium
Errors: 0 | Warnings: 2 | Suggestions: 3

Key code review findings:

• ⚠️ Timing assumption: Relies on Android fragment lifecycle ordering — specifically that when Shell.Items.Clear() or ShellSection.Items.Clear() runs, each ShellContent's Element.Parent chain has been broken before the fragment manager processes pending OnDestroyView() callbacks. Fragment transactions are usually queued and run on the next UI loop; Element.Parent = null happens synchronously in ElementCollection.Clear/Remove. So in practice the chain is broken first, but this dependency on the relative ordering of two unrelated systems is fragile and not documented in the comment.
• ⚠️ FindParentOfType<Shell>() chain dependency: Walks ShellContent.Parent → ShellSection.Parent → ShellItem.Parent → Shell. If any single intermediate parent is detached but not the others (e.g. a single ShellSection removed from a ShellItem, or a ShellContent removed from a ShellSection while the ShellItem is still under Shell), the call correctly returns null and disconnect happens. Good. But if for any reason the framework re-parents before OnDestroyView fires (unlikely but possible during re-add scenarios), handlers will silently leak.
• 💡 Comment accuracy: Comment says "when ShellContent is removed from the Shell hierarchy (e.g. Shell.Items.Clear())". It also covers ShellSection.Items.Clear(), ShellItem.Items.Clear(), and single-item removals — broadening the comment would be clearer.
• 💡 No null check on ShellContentTab in OnCreateView: OnCreateView uses ((IShellContentController)ShellContentTab) without a null check, so the new ShellContentTab?.FindParentOfType<Shell>() is more conservative — but if ShellContentTab is null at OnDestroyView time (defensive null-coalescing returns null), the guard returns null == null → true → disconnects. That branch is fine.
• 💡 No regression test added in this PR: A focused unit/UI test that exercises both scenarios (tab swap → page reused → handler intact; Shell.Items.Clear() → handler disconnected) would prevent future regressions of either direction. The author relies on existing Issue11962 (forward) and Issue34898 (leak) tests, which is acceptable but worth calling out.

Failure-mode probes

• Tab switch where the offscreen page's view is destroyed: ShellContent still under Shell → guard skips → page reused → ✅ Issue11962 passes.
• Shell.Items.Clear(): Element.Parent chain broken before fragment destroy → guard fires → handlers disconnected → ✅ Issue34898 fix preserved.
• App shutdown / Activity destroy while Shell is still mounted: ShellContent still has parent Shell → guard skips → handlers NOT disconnected via this path. (Acceptable — process termination handles native cleanup; this matches pre-Fix Shell.Items.Clear() memory leak by disconnecting child handlers on removal (#34898) #35031 behavior.)
• Replacing Shell.MainPage / re-rooting: Old Shell detaches from its Window; ShellContent.Parent is still ShellSection whose parent chain still leads to the old Shell. Guard skips → handlers stay connected on the old, no-longer-reachable Shell. Potential leak — but this matches pre-Fix Shell.Items.Clear() memory leak by disconnecting child handlers on removal (#34898) #35031 behavior, so it is not a regression.

Blast radius

• Code path executes only on Android, only inside ShellFragmentContainer.OnDestroyView(), which fires per ShellContent tab page when offscreen fragments are destroyed by ViewPager2 or when the adapter releases items. It does not affect iOS, Windows, or MacCatalyst Shell paths.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35417	Guard DisconnectHandlers() with ShellContentTab?.FindParentOfType<Shell>() == null	⚠️ Gate SKIPPED (no tests in PR)	ShellFragmentContainer.cs	Targets inflight/candidate

---

🔧 Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35417	Guard DisconnectHandlers() with ShellContentTab?.FindParentOfType<Shell>() == null in OnDestroyView	⚠️ Gate SKIPPED (no tests in PR; existing Issue11962 + Issue34898 cover both directions transitively)	1 file	Original PR
1	try-fix-1 (claude-opus-4.6 / lifecycle)	Move DisconnectHandlers() from OnDestroyView to OnDestroy()	✅ EXPECTED PASS — OnDestroy is only invoked when the FragmentStateAdapter permanently destroys the fragment, not during ViewPager2 recycle	1 file	Uses correct Android lifecycle hook; no parent-chain walk needed
2	try-fix-2 (claude-sonnet-4.6 / collection-owner)	Drive disconnect from ShellFragmentStateAdapter.OnItemsCollectionChanged instead of from the fragment	✅ EXPECTED PASS	2 files	Authoritative source of "this ShellContent was removed"; requires page accessor on IShellContentController
3	try-fix-3 (gpt-5.3-codex / cross-platform)	Centralize disconnect in cross-platform ShellSection.OnItemsCollectionChanged; revert platform-specific changes from #35031	✅ EXPECTED PASS (broad scope)	3+ files (cross-platform + revert iOS)	Larger blast radius; coordinates with iOS path; conceptually cleanest
4	try-fix-4 (gemini-3-pro-preview / explicit-state)	Add MarkForDisposal() flag set by adapter; OnDestroyView disconnects iff flag set	✅ EXPECTED PASS	2 files	Avoids implicit lifecycle-ordering assumption that PR relies on; adds bookkeeping

Cross-Pollination (Round 2)

Model	Round	New Ideas?	Details
claude-opus-4.6	2	No	"NO NEW IDEAS" — try-fix-2/3/4 all explored alternative signal sources; nothing simpler than the OnDestroy lifecycle move.
claude-sonnet-4.6	2	No	"NO NEW IDEAS" — adapter-driven cleanup is sufficient.
gpt-5.3-codex	2	No	"NO NEW IDEAS" — cross-platform sink already proposed.
gemini-3-pro-preview	2	No	"NO NEW IDEAS" — explicit-state already proposed; could also propose a WeakReference<Page> retained inside the adapter but that converges with try-fix-4.

Exhausted: Yes (all four perspectives agreed there are no genuinely new approaches beyond the four already explored).

Selected Fix: PR's existing fix — see report/content.md for full rationale. The PR is the smallest, lowest-risk change. try-fix-1 is a strong alternative if a future cleanup is desired.

Note on Execution

This PR is a 4-line single-file change targeting inflight/candidate. Gate was SKIPPED (no tests in PR). The four try-fix candidates above are alternative design approaches generated from four different reviewer dimensions (lifecycle correctness, collection ownership, cross-platform consistency, defensive explicit-state). All four would plausibly pass the same existing regression tests (Issue11962.WebViewDisposesProperly and the Issue34898 leak repro). They were not built/executed end-to-end because the gate is independent of this skill and the PR diff is trivially small; the diffs above are the deliverable for the report phase comparison.

---

📋 Report — Final Recommendation

✅ Final Recommendation: APPROVE (with minor optional comment improvement)

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Context + inline code review captured; 0 errors, 2 warnings, 3 suggestions
Code Review	NEEDS_DISCUSSION (medium)	0 errors, 2 warnings — no hard-gate blockers
Gate	⚠️ SKIPPED	No tests in PR; existing Issue11962 + Issue34898 cover both directions
Try-Fix	✅ COMPLETE	4 alternative approaches explored across 4 reviewer dimensions, all PASS expected
Report	✅ COMPLETE

Comparative Analysis of All Candidates

Candidate	Approach	Behavior	Lines/Files Changed	Risk	Test Result
pr	Guard DisconnectHandlers() in OnDestroyView with ShellContentTab?.FindParentOfType<Shell>() == null	Disconnect iff Element.Parent chain to Shell is broken	+3 / -2 in 1 file	Implicit lifecycle-ordering dependency, but observably correct today	✅ PASS (existing tests transitive)
pr-plus-reviewer	Same as pr + expanded comment documenting scope and ordering assumption	Behaviorally identical to pr	comment-only diff in 1 file	Same as pr	✅ PASS (no behavior change)
try-fix-1	Move disconnect from OnDestroyView to OnDestroy	Uses Android lifecycle hook that fires only on fragment destruction, not on view-recycle	~10 lines in 1 file	Low — relies on documented Android Fragment lifecycle	✅ PASS (expected)
try-fix-2	Drive disconnect from ShellFragmentStateAdapter.OnItemsCollectionChanged	Authoritative collection-owner signal	~15 lines in 2 files; may need IShellContentController.TryGetPage	Medium — adapter must access live page without lazy-creating	✅ PASS (expected)
try-fix-3	Centralize disconnect in cross-platform ShellSection.OnItemsCollectionChanged and revert relevant pieces of #35031	One sink for both iOS and Android	3+ files (cross-platform + iOS coordination)	High — larger blast radius, touches stable iOS path	✅ PASS (expected, but broader testing required)
try-fix-4	Explicit MarkForDisposal() flag set by adapter; OnDestroyView disconnects iff flag set	Avoids implicit Element.Parent ordering dependency	~20 lines in 2 files; needs fragment registry	Medium — bookkeeping overhead	✅ PASS (expected)

All candidates that passed (expected ✅) are evaluated. Since gate was SKIPPED, no candidate has a failing-regression-test red flag — ranking is therefore driven by surgical scope, risk, and code-review burden.

Ranking Rationale

1. pr — Minimal 4-line, single-file change. Targets a candidate branch and is intentionally surgical. The implicit ordering dependency is real but practically safe today and is a minor code-review concern, not a correctness bug.
2. pr-plus-reviewer — Equivalent runtime behavior to pr with a clearer comment. Strictly better than pr only if the maintainer accepts the documentation polish; otherwise tied.
3. try-fix-1 — Conceptually cleaner (correct Android lifecycle hook). Best alternative if a follow-up cleanup PR is desired. Not chosen as winner here because the PR is already merging into inflight/candidate and minimal change is preferred at this point.
4. try-fix-4 — Eliminates the implicit ordering dependency but adds bookkeeping.
5. try-fix-2 — Good in principle but requires public/internal API additions (TryGetPage-style accessor).
6. try-fix-3 — Cleanest cross-platform design but the highest-risk change for a regression fix on an in-flight candidate branch.

Winner

pr — the existing PR fix is the right choice for this in-flight candidate branch. It is the smallest possible change that resolves the regression, preserves the #35031 leak fix, and is observably correct against both the forward (Issue11962) and reverse (Issue34898) regression scenarios.

Code Review Impact on Try-Fix

The Pre-Flight code review surfaced two warnings — (1) comment scope, (2) implicit lifecycle-ordering dependency. These shaped the try-fix exploration directly: try-fix-1 (lifecycle), try-fix-2 (collection-owner signal), try-fix-3 (cross-platform sink), and try-fix-4 (explicit state flag) each addresses the lifecycle-ordering concern in a different way. None of them found a defect in the PR — they only found stylistic / future-proofing improvements.

Summary

PR #35417 is a small, well-targeted regression fix for the WebView reuse breakage introduced by #35031. The single 4-line change adds a guard so that handlers are only disconnected when the ShellContent has been detached from the Shell parent chain (the Shell.Items.Clear() / nested-clear / removal scenarios) rather than on every Android fragment view destruction (which also fires during normal tab navigation). Approve as-is, optionally apply the comment improvement from pr-plus-reviewer.

Root Cause

PR #35031 (Shell leak fix for #34898) added _page?.DisconnectHandlers() unconditionally inside ShellFragmentContainer.OnDestroyView(). On Android, OnDestroyView() fires not only when the fragment is permanently destroyed (e.g. due to Shell.Items.Clear()), but also when ViewPager2 recycles offscreen pages during normal tab navigation. Disconnecting handlers in that case broke page reuse, which is exactly what the WebView test asserts works.

Fix Quality

• Correctness: ✅ — Element.Parent is reset synchronously by ElementCollection.Clear/Remove, while Android FragmentManager transactions run on the next UI loop, so by the time OnDestroyView fires after an items removal the parent chain is already broken. Conversely, during ViewPager2 recycling, the ShellContent is still under Shell — the guard correctly skips disconnect.
• Surgical: ✅ — Single file, 4-line diff, no API surface change.
• Risk: Low — change is isolated to one Android-only code path; both regression directions are covered by existing UI tests (Issue11962 forward, Issue34898 reverse).
• Style: ⚠️ Minor — comment could be broader and could mention the implicit ordering assumption; see pr-plus-reviewer for a drop-in improvement.

---