[Android] Fix CollectionView with an EmptyView or EmptyViewTemplate gets potentially corrupted after being cleared and repopulated a number of times

[davidlilwall]

Description of Change

Added guards to the property setters for HeaderView, EmptyView etc in EmptyViewAdapter to prevent _headerViewType, _emptyItemViewType, and _footerViewType getting repeatedly unnecessarily incremented. Also ensured that the corresponding integer ViewType variables do not collide with each other.

Issues Fixed

Fixes #34207

[davidlilwall]

@dotnet-policy-service agree

[kubaflo]

Could you please review the AI's summary?

[davidlilwall]

The AI recommended approach of just toggling the values rather than incrementing them seems to be a poor suggestion. These values are effectively Ids and are presumably being used for caching (which is why they are being changed in the first place). If the Ids are just toggled then after the first toggle, every other toggle is going to collide with a previously used Id resulting in stale versions of the views potentially being used.

[kubaflo]

Multimodal Code Review — Codex + Gemini

🟡 Codex (GPT-5.1-Codex): 1 issue (false positive)

int.Max does not compile (Critical — ❌ False Positive)
Codex flagged int.Max(int, int) as non-existent on System.Int32. However, this API is available in .NET 7+ via the generic math interface INumber<int>. Since this file lives under Handlers/Items/Android/ and only compiles for net9.0-android+, it builds fine.

---

🔴 Gemini (Gemini 3 Pro): 3 issues

1. Unbounded ViewType growth → RecycledViewPool fragmentation (High)
Each property set generates a new monotonically-increasing view type ID via Max + 1. Android's RecycledViewPool maps each unique viewType to a distinct ViewHolder bucket. Old IDs are abandoned but their cached ViewHolders are not cleaned up, causing:

• Memory leak: Orphaned ViewHolders for stale IDs linger in the pool.
• Performance hit: Views are never recycled — every change forces expensive OnCreateViewHolder (inflation) instead of cheap OnBindViewHolder.

Example: EmptyView = A (type=4) → EmptyView = B (type=5) → EmptyView = A again (type=6, not 4). The pool for type 4 is wasted.

Suggestion: Use a fixed set of stable IDs for Header/Footer/Empty slots and handle content changes through OnBindViewHolder / NotifyItemChanged instead of forcing re-creation via new view types. If re-creation is truly required, consider explicitly clearing the RecycledViewPool.

2. Reference equality fails for strings and boxed value types (Medium)
The guards use == on object-typed fields:

if (_headerView == value) return;

This is reference equality. If the consumer sets Header = "Title" twice with two different string instances of the same content, the guard fails and the view type is needlessly incremented.

Suggestion: Use Object.Equals(_headerView, value) for correct value-based comparison.

3. int.Max TFM availability (Low)
int.Max(int, int) requires .NET 7+ (generic math). This is fine today since the file is Android-only, but worth a comment or using Math.Max for broader compatibility if the code is ever shared.

---

Consensus

Aspect	Codex	Gemini
int.Max validity	❌ False positive	⚠️ Low (TFM note)
ViewType collision fix	✅ Correct	✅ Correct
ViewType unbounded growth	No concern	🔴 Pool fragmentation
Equality guards (==)	No concern	🟡 Fails for strings
Missing tests	Noted	Noted

Overall: The fix correctly prevents view type collisions, which was the original bug. However, it introduces a new concern: unbounded view type growth causing RecycledViewPool fragmentation and defeating view recycling. The reference equality guards also won't work for string/value-type Header/Footer content. Consider using stable view type IDs with proper bind-time updates instead of ever-increasing IDs.

[davidlilwall]

The fix does not introduce the new concern of 'unbounded view type growth' - that was a concern which very obviously existed in the original code. The fix mitigates this concern rather than introduces it.

'Using stable view type IDs with proper binding-time updates instead of ever-increasing IDs' is of course the preferred solution but that would involve changing the code in other files as well.

This fix simply fixes two bugs in the EmptyViewAdapter so that the code works in the way the original coder presumably intended it to. Consequently it fixes the Issue that the pull-request is linked to. However it does not seek to fix more general issues with the code.

[MauiBot]

🚦 Gate — Test Before and After Fix

👋 @davidlilwall — new gate results are available. Please review the latest session below.

🚦 Gate Session — d755a05 · Added guards to the View and ViewTemplate setters in EmptyViewAdapter and ensured that the corresponding integer ViewType variables do not collide with each other · 2026-04-17 09:20 UTC

---

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

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

@copilot write tests for this PR

The agent will analyze the issue, determine the appropriate test type (UI test, device test, unit test, or XAML test), and create tests that verify the fix.

---

[MauiBot]

🤖 AI Summary

👋 @davidlilwall — new AI review results are available. Please review the latest session below.

📊 Review Session — d755a05 · Added guards to the View and ViewTemplate setters in EmptyViewAdapter and ensured that the corresponding integer ViewType variables do not collide with each other · 2026-04-17 10:56 UTC

---

🔍 Pre-Flight — Context & Validation

---

Issue: #34207 - [Android] CollectionView with an EmptyView or EmptyViewTemplate gets potentially corrupted after being cleared and repopulated a number of times
PR: #34208 - [Android] Fix CollectionView with an EmptyView or EmptyViewTemplate gets potentially corrupted after being cleared and repopulated a number of times
Platforms Affected: Android
Files Changed: 1 implementation, 0 test

Key Findings

• Root cause: EmptyViewAdapter's six property setters each increment their respective view type ID counter (_headerViewType, _emptyItemViewType, _footerViewType) unconditionally. The call site MauiRecyclerView.UpdateEmptyView() sets all six properties on every invocation regardless of whether values changed, causing the counters to overflow and eventually collide with DataTemplate IDs used by the main RecyclerView adapter.
• The bug is deterministic: corrupts after ~9 reloads because _emptyItemViewType eventually equals a DataTemplate view type, causing RecyclerView to feed the wrong ViewHolder for data positions.
• PR fix: (1) adds equality guard (if (_x == value) return;) to each setter and (2) uses GetCurrentMaxViewTypeValue() + 1 to ensure all three IDs remain distinct after any change.
• Prior agent review (kubaflo, March 24) flagged unbounded ViewType growth and reference equality issues.
• Contributor explicitly clarified the reference equality guard is appropriate because .NET MAUI DataTemplate/View assignments are not mutable after assignment in this context.

Code Review Summary

Verdict: NEEDS_CHANGES
Confidence: high
Errors: 0 | Warnings: 2 | Suggestions: 1

Key code review findings:

• ⚠️ EmptyViewAdapter.cs — No regression test for the repopulation corruption scenario (10× reload). The bug has a deterministic repro and CollectionView is the highest-regression component. A UI test (Issue34207.cs) should be added.
• ⚠️ EmptyViewAdapter.cs:36,50 — Both Header and HeaderTemplate share _headerViewType. On the first UpdateEmptyView() call with both set, both setters fire and consume two ID slots instead of one. The first ID is immediately orphaned. Harmless but wasteful; could be addressed with a combined UpdateHeader(view, template) method.
• 💡 EmptyViewAdapter.cs:130 — int.Max(...) (generic-math, .NET 7+) works but Math.Max(...) is the codebase-conventional spelling.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #34208	Add equality guards to all 6 setters; use GetCurrentMaxViewTypeValue()+1 to prevent inter-type ID collisions	⚠️ SKIPPED (no tests)	EmptyViewAdapter.cs	Original PR

---

🔬 Code Review — Deep Analysis

---

Code Review — PR #34208

Independent Assessment

What this changes: Adds reference-equality guards (if (_x == value) return;) to all six property setters in EmptyViewAdapter (Header, HeaderTemplate, Footer, FooterTemplate, EmptyView, EmptyViewTemplate), and changes the view type ID assignment from independent increments (+= 1) to GetCurrentMaxViewTypeValue() + 1 — a new helper that returns int.Max of all three type fields so every new ID is guaranteed to be strictly greater than all currently-in-use IDs.

Inferred motivation: Every call to UpdateEmptyView() in MauiRecyclerView.cs sets all six properties unconditionally, even when values haven't changed. With the old += 1 pattern, repeated calls cause the three view-type integers to converge and collide. For example: after a few calls _emptyItemViewType can reach the same value as _headerViewType, causing RecyclerView to feed the wrong ViewHolder type for the empty or header position — leading to visible corruption (missing items, headers/footers appearing in wrong positions).

---

Reconciliation with PR Narrative

Author claims: Guards prevent unnecessary increments. GetCurrentMaxViewTypeValue() ensures the three type IDs never collide with each other.

Agreement: The analysis is correct. The call site in MauiRecyclerView.UpdateEmptyView() sets all six properties on every invocation, regardless of whether they changed. Without the guards, every UpdateEmptyView() call bumps each type counter once per setter, so three calls (× 6 setters) can push _emptyItemViewType to 2 + 6 = 8, which would collide with a separate adapter's view types in the shared RecycledViewPool. The fix is correctly targeted.

---

Findings

⚠️ Warning — No regression test for the repopulation corruption scenario

The bug has a precise, deterministic repro ("corrupts on the 9th reload") that is automatable. The review rules for CollectionView are explicit: "Any change to layout, scroll, Header/Footer, or EmptyView must be tested across: empty collection, single item, many items, and with grouping". An existing parallel exists in CollectionViewUITests.EmptyView.cs and CollectionView_DynamicChangesFeatureTests.cs. A test that loads data, clears it, reloads it 10+ times, and asserts the EmptyView and items render correctly would provide a permanent regression guard. Without it, a future refactor of the type-ID logic could silently re-introduce the same bug.

Suggested location: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34207.cs + matching TestCases.HostApp/Issues/Issue34207.cs, with the test tapping a "Reload" button 10 times and asserting the CollectionView still shows the correct item count / EmptyView state on Android.

⚠️ Warning — Both Header and HeaderTemplate share _headerViewType; both setters run on every UpdateEmptyView() call

When UpdateEmptyView() is called for the first time with both Header and HeaderTemplate set to non-null values, both setters fire and bump _headerViewType twice:

// MauiRecyclerView.cs:190-191
_emptyViewAdapter.Header = structuredItemsView.Header;      // _headerViewType → max(1,2,3)+1 = 4
_emptyViewAdapter.HeaderTemplate = structuredItemsView.HeaderTemplate;  // _headerViewType → max(4,2,3)+1 = 5

The first assignment's type ID (4) is immediately orphaned — GetItemViewType will only ever return 5 for the header position after the second setter fires. This wastes one ID slot per UpdateEmptyView() call for the initial setup (same applies to Footer/FooterTemplate). On subsequent calls, the equality guards correctly short-circuit both setters. This is harmless for correctness but worth being aware of. If it becomes a concern, the two setters per slot could be merged into a single UpdateHeader(object view, DataTemplate template) method.

💡 Suggestion — Math.Max is the conventional spelling here

int GetCurrentMaxViewTypeValue() => int.Max(int.Max(_headerViewType, _emptyItemViewType), _footerViewType);

int.Max(a, b) (static generic-math API, added in .NET 7) is valid, but Math.Max(Math.Max(a, b), c) is the form used throughout the rest of the MAUI codebase and will be instantly recognizable to all contributors. A minor consistency preference.

---

Devil's Advocate

On the equality guards: Could a same-reference DataTemplate be mutated after assignment, making the guard's early return incorrect? In practice MAUI doesn't support mutating an in-use DataTemplate and expecting automatic layout refresh — that would require a separate NotifyDataSetChanged() trigger. The guard is appropriate.

On GetCurrentMaxViewTypeValue(): Does the invariant hold that all three IDs remain distinct? Starting at {1, 2, 3}, each update sets one field to max(all three) + 1, which is always greater than the other two. The invariant is maintained by construction. ✓

On unbounded growth: Each property change permanently increments the max. Apps that cycle EmptyView between null ↔ view repeatedly will see the IDs grow. RecyclerView's RecycledViewPool uses a SparseArray keyed by view type, so large integers are fine — no array-bounds concern. In practice the IDs will never grow large enough to matter.

Am I sure about the missing-test issue? Yes. The bug's repro is deterministic (step count is known), and CollectionView is the highest-regression component in the codebase. The warning is warranted.

---

Verdict: NEEDS_CHANGES

Confidence: high

Summary: The core fix is correct and well-reasoned. The equality guards prevent redundant view-type bumps, and GetCurrentMaxViewTypeValue() + 1 guarantees the three IDs remain distinct across all update sequences. The one concrete ask before merge is a regression UI test for the repopulation cycle (10× reload → assert correct render), given that CollectionView is the highest-regression component in this codebase and the bug has a precise, automatable repro. The double-setter issue for Header/HeaderTemplate is a minor design note, not a blocker.

---

🔧 Fix — Analysis & Comparison

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	Call-site equality guards in MauiRecyclerView.UpdateEmptyView()	⚠️ Blocked (no test)	MauiRecyclerView.cs	Keeps adapter unchanged; less robust to new call sites
2	try-fix	UpdateAll() API with Object.Equals guards in EmptyViewAdapter	⚠️ Blocked (no test)	EmptyViewAdapter.cs, MauiRecyclerView.cs	Adds public API; fixes Object.Equals concern; bigger scope
3	try-fix	Stable fixed const IDs (1/2/3) — remove all += 1	⚠️ Blocked (no test)	EmptyViewAdapter.cs	May be BROKEN: removes mechanism forcing ViewHolder re-creation
4	try-fix	Dirty flags + reserved negative view-type range	⚠️ Blocked (no test)	EmptyViewAdapter.cs	More complex than needed
5	try-fix	SwapAdapter(_, false) — prevent cross-adapter pool contamination	⚠️ Blocked (no test)	MauiRecyclerView.cs	Addresses root cause; may impact performance on adapter swaps
PR	PR #34208	Equality guards in all 6 setters + GetCurrentMaxViewTypeValue()+1	⚠️ Gate SKIPPED (no tests)	EmptyViewAdapter.cs	Focused, safe, correct

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	2	Yes	Clear RecycledViewPool AFTER SwapAdapter
claude-sonnet-4.6	2	Yes	Change SwapAdapter recycleScrap=true → false (implemented as Attempt 5)
gpt-5.3-codex	2	Yes	Remove EmptyViewAdapter entirely (too large, not attempted)
claude-opus-4.6	3	Yes	Apply SetAdapter(null)+SwapAdapter pattern from UpdateAdapter() to all swap sites (not attempted — max rounds reached)

Exhausted: Yes (max 3 rounds reached)
Selected Fix: PR #34208 — Reason: Most focused change (adapter-level, 1 file), logically correct, guards prevent collision AND unnecessary increments. Attempt 5 (recycleScrap=false) is an interesting alternative addressing the root cause but may have performance trade-offs and lacks regression tests. Without tests to validate either approach on-device, the PR's targeted adapter fix is preferred as simpler and lower-risk.

---

📋 Report — Final Recommendation

---

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Android-only, 1 implementation file, 0 test files
Code Review	NEEDS_CHANGES (high)	0 errors, 2 warnings, 1 suggestion
Gate	⚠️ SKIPPED	No tests detected in PR
Try-Fix	✅ COMPLETE	5 attempts, 0 passing (all Blocked — no regression test exists)
Report	✅ COMPLETE

Code Review Impact on Try-Fix

The code review warning about unbounded ViewType growth directly motivated Attempt 3 (stable fixed IDs) and Attempts 4-5 (pool contamination root-cause strategies). The warning about the Header/HeaderTemplate double-bump motivated Attempt 2 (UpdateAll() API treating them as a single logical slot). No code review ❌ Errors were found — all findings were ⚠️ Warnings. The most productive cross-pollination insight came from questioning WHY GetRecycledViewPool().Clear() was already in the code — leading to Attempt 5's recycleScrap=false hypothesis, a genuine alternative root-cause fix.

Summary

The PR fixes a deterministic Android CollectionView corruption bug where _headerViewType, _emptyItemViewType, and _footerViewType in EmptyViewAdapter get incremented on every UpdateEmptyView() call even when values haven't changed, eventually colliding with DataTemplate type IDs in the main RecyclerView adapter. The fix (equality guards + GetCurrentMaxViewTypeValue()+1) is correct and well-targeted. The recommendation to REQUEST CHANGES is driven by the missing regression test — not by any correctness error in the code.

Try-Fix also surfaced a noteworthy alternative: changing SwapAdapter(_, recycleScrap:true) → SwapAdapter(_, recycleScrap:false) in MauiRecyclerView.cs (Attempt 5). This prevents cross-adapter pool contamination at the source — a more direct root-cause fix — but may affect performance and has not been empirically validated.

Root Cause

MauiRecyclerView.UpdateEmptyView() sets all 6 EmptyViewAdapter properties unconditionally on every invocation. Each setter unconditionally does _xxxViewType += 1. After ~9 reload cycles, _emptyItemViewType reaches a value matching a DataTemplate view type in the main adapter's RecycledViewPool, causing RecyclerView to serve the wrong ViewHolder for data positions.

Fix Quality

The PR's fix is correct and minimal:

• Equality guards in all 6 setters prevent unnecessary type increments — idiomatic, clear
• GetCurrentMaxViewTypeValue() + 1 ensures all three IDs remain distinct after any change — the invariant holds by construction (proved in code review)
• Only 1 file changed (32 lines)
• int.Max is valid (.NET 7+ generic math); Math.Max would be marginally more conventional
• The one open concern (Header+HeaderTemplate double-bump on first call) is harmless — the orphaned ID is never used after the second setter fires

Missing test is the primary blocker. The bug has a deterministic repro (9 reload cycles) that is automatable. A UI test that reloads the CollectionView 10+ times and verifies correct layout would provide a permanent regression guard for the highest-regression component in the MAUI codebase.

Noteworthy alternative (Attempt 5): SwapAdapter(_, false) is a 4-line change to MauiRecyclerView.cs that addresses the root cause at the pool management level. Worth the team evaluating whether this is the right long-term fix.

---

[kubaflo]

Could you please review the ai's summary?

[davidlilwall]

It looks like the change made in #34452 means that this pull request is no longer the appropriate solution.

I've not tested it - but it would seem that _headerViewType, _emptyItemViewType, and _footerViewType in EmptyViewAdapter can now just have fixed values because whenever EmptyViewAdapter is used then GetRecycledViewPool().Clear(); gets called first.

I can't look into this further right now - but I'm closing this pull request because it look like there is a much better alternative now in just fixing these values.