[Net 11]Fix SwipeItemView command leak - #35891
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35891Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35891" |
AI code review refresh for net11.0 target
Prior review reconciliationThere are no prior reviews, review comments, or fleet markers on this PR (only the standard What changed (independent read of
|
| File | Δ | Notes |
|---|---|---|
src/Controls/src/Core/SwipeView/SwipeItemView.cs |
+16 / −33 | Switches to the shared ICommandElement / CommandElement / WeakCommandSubscription infrastructure; replaces direct IsEnabled = writes with a coerced IsEnabledCore override; adds CommandProperty.DependsOn(CommandParameterProperty) for binding-order safety. |
PublicAPI/{net,net-android,net-ios,net-maccatalyst,net-windows,netstandard}/PublicAPI.Unshipped.txt |
+1 each | Declares override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool. |
tests/Core.UnitTests/CommandTests.cs |
+5 | Adds SwipeItemView to the parameterized CommandsSubscribedToCanExecuteCollect GC test (both true/false weak-event paths). |
tests/Core.UnitTests/SwipeViewTests.cs |
+35 | SwipeItemViewCommandCanExecuteUpdatesIsEnabled — covers initial false-CanExecute, ChangeCanExecute, CommandParameter swaps, and explicit IsEnabled=false overlay winning over CanExecute=true. |
Correctness deep-dive — command/event lifecycle
- Subscription model is correct.
CommandElement.OnCommandChangedconstructs aWeakCommandSubscription, which uses aDependentHandle(bindableObject, handler)to bridgeICommand.CanExecuteChangedback to the element. The element is only weakly reachable via the handle, so a long-livedICommandno longer pins theSwipeItemViewgraph — directly addressing the issueSwipeItemView.Commandleaks row views and command parameters throughCanExecuteChanged#35498 retention path (ICommand → CanExecuteChanged → SwipeItemView → CommandParameter / Content / BindingContext). - Cleanup paths.
OnCommandChangingdisposes the existing tracker before the new command is assigned;OnCommandChangedalso disposes when the new command isnull.Disposeremoves theCanExecuteChangedhandler from the old command and disposes theDependentHandle. No leak of the handle/proxy itself — and the new GC test asserts thatCleanupTrackerandCleanupTracker.Proxyare collectible alongside the control. - Pattern parity. Implementation mirrors
CheckBoxline-for-line (ICommandElementdeclaration,IsEnabledCore => base.IsEnabledCore && CommandElement.GetCanExecute(this, CommandProperty),CanExecuteChanged → RefreshIsEnabledProperty(),CleanupTracker { get; set; }). The implicit interface satisfaction forCommand/CommandParameter(via existing public properties) works because the file is#nullable disable, so the nullable-annotated interface signatures are satisfied. ✅ DependsOn(CommandParameterProperty). MatchesMenuItemand the issue CommandParameter TemplateBinding Lost During ControlTemplate Reparenting #31939 timing fix —GetCanExecutewill force a pendingCommandParameterbinding to apply before evaluatingCanExecute, which is the right call now thatCommandParameterno longer triggers a direct property write.- Behavioral semantics. Old code wrote
IsEnableddirectly (clobbering_isEnabledExplicit); new code overridesIsEnabledCore, so a user's explicitIsEnabled = falsenow correctly survivesCommand.ChangeCanExecute()even whenCanExecutereturns true. The added unit test pins exactly that. Net visible behavior matches the predecessor PR that already shipped oninflight/current. - Threading. No new threading surface.
RefreshIsEnabledProperty()andBindableObject.SetValuekeep the same UI-thread contract as the previous directIsEnabled =write. - AOT / trim. No reflection introduced; no new
RequiresUnreferencedCodecallees. ✅ - Partial class / static ctor.
SwipeItemViewonly has one .cs partial insrc/Controls/src/Core, so the newly addedstatic SwipeItemView()cannot collide with another partial declaration. OnInvokedpath. Still independently checksCommand.CanExecute(CommandParameter)beforeExecute, so executing a disabled command remains gated even ifIsEnabledcoercion is bypassed by a caller invoking the method directly.
Blast radius
Scope is intentionally tiny and surgical:
- One control surface (
SwipeItemView), one virtual override added, and theCommand/CommandParameterproperty metadata redirected to shared static handlers. - No handler, no platform code, no XAML/source-gen change. All
Microsoft.Maui.Controls.Handlers.SwipeItemViewHandler.*files are untouched. - Risk vectors I probed and didn't find: (a) consumers depending on
IsEnabledbeing written (vs coerced) — covered by the explicit-IsEnabled test; (b) ordering between command and parameter bindings during reparenting — covered byDependsOn; (c) double-subscribe from re-enteringOnCommandChanged—OnCommandChangingalways disposes first.
Findings
| Sev | Finding |
|---|---|
| nit | PublicAPI/net-tizen/PublicAPI.Unshipped.txt is not updated with the new override … IsEnabledCore.get entry, even though net-tizen ships SwipeItemView (see PublicAPI/net-tizen/PublicAPI.Shipped.txt). Today this is harmless on the net11.0 branch because Directory.Build.props has <IncludeTizenTargetFrameworks>false</IncludeTizenTargetFrameworks> ("Disabled until net10.0-tizen is available"), so the analyzer never sees the gap. Worth adding for consistency before Tizen is re-enabled — same one-line entry as the other TFMs. |
No other ❌ findings.
CI status (maui-pr build 1461088)
| Lane | Status | Classification |
|---|---|---|
Helix Unit Tests — osx.15.arm64.maui.open (Debug & Release) |
✅ pass (4644 / 4635 ran, 0 failed) | New SwipeItemViewCommandCanExecuteUpdatesIsEnabled and the parameterized CommandsSubscribedToCanExecuteCollect(SwipeItemView,*) cases ran here. |
Helix Unit Tests — Windows.10.Amd64.Open (Debug & Release) |
✅ pass (0 failed) | Same. |
| Build .NET MAUI macOS Debug, Pack macOS, Pack Windows, Build Windows Debug | ✅ pass | — |
| Integration: Build/macOS, Blazor/macOS, Samples/macOS, RunOnAndroid, RunOniOS Debug/CoreCLR/NativeAOT/BlazorDebug/BlazorRelease, win MultiProject/AOT/Helix | ✅ pass | — |
| Build .NET MAUI macOS (Release) | ❌ fail | Infra: src/Maui.InTree.props(16,5): error MSB4019: imported project ".buildtasks/Microsoft.Maui.Resizetizer.props" was not found — BuildTasks staging race on the macOS Release agent. Not produced by this diff. |
| Run Integration Tests AOT macOS | ❌ fail | Cascades from missing BuildTasks + same template trim error as below. |
| RunOniOS_MauiRelease ARM64 / RunOniOS_MauiReleaseTrimFull (CoreCLR) ARM64 | ❌ fail | Pre-existing net11.0 issue: template build fails with ILLink IL2026: HybridWebView … RequiresUnreferencedCodeAttribute … uses dynamic System.Text.Json serialization features, escalated by TreatWarningsAsErrors=true. Originates in Microsoft.Maui.Handlers.HybridWebViewHandler (SchemeHandler, WebViewScriptMessageHandler) — wholly unrelated to SwipeItemView. |
maui-pr (Windows Integration, Build Analysis, Bump global.json) |
⏳ pending | Not failures; still in flight at review time. |
Pipeline-wide search for SwipeItemView in the timeline returned 0 matches, confirming no test/build step failed because of this change.
Confidence
- High on correctness, lifecycle, and parity with the existing
ICommandElementcontrols. - High on CI-failure classification (infra + a known HybridWebView trim warning, both reproducible on unrelated branches).
- Medium that the Tizen
PublicAPI.Unshipped.txtomission is a nit and not a blocker — based onIncludeTizenTargetFrameworks=falsein this branch.
Suggested next step before un-[WIP]-ing
- Append
override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> booltosrc/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txtfor parity with the other six TFM folders. - Re-run
maui-pronce the macOS Release infra flake clears, or wait for an unrelated PR to confirm the HybridWebView trim error is a known issue onnet11.0.
(Automated round-19 refresh comment. Re-runs will edit this comment in place via its copilot-cli-net11-fleet-review-round19 marker.)
Multi-model review synthesis — PR #35891 (WIP draft)Verdict: NEEDS_DISCUSSION • confidence: medium • inline findings: 0 Models: gpt-5.5 ND, opus-4.8 ND, gemini ND, opus-4.6 LGTM → 3 NEEDS_DISCUSSION / 1 LGTM. Leak-fix assessment. The migration of Open question (why discussion, not LGTM). Purely process: it's still CI: Multi-model review (gpt-5.5 · opus-4.8 · opus-4.6 · gemini-3.1-pro). Comments only — not a formal approval. |
b3ae1cd to
c8f43cb
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
kubaflo
left a comment
There was a problem hiding this comment.
Synthesized review — PR #35891 (re-review @ c8f43cb)
Verdict: LGTM • inline findings: 1 (suggestion) • models: gpt-5.5 LGTM · opus-4.8 NEEDS_DISCUSSION · opus-4.6 LGTM · gemini LGTM (3 LGTM / 1 ND)
PR #35891 migrates SwipeItemView onto the shared ICommandElement / CommandElement / WeakCommandSubscription infrastructure to fix the #35498 memory leak, where a long-lived ICommand's CanExecuteChanged subscription strongly retained each row (its Content, CommandParameter, BindingContext, and row view models) after the page closed. The strong subscription is replaced by a DependentHandle-backed weak proxy (line-for-line parity with CheckBox/RefreshView), enablement moves into a coerced IsEnabledCore override (so an explicit IsEnabled = false now wins over Command.CanExecute = true), and CommandProperty.DependsOn(CommandParameterProperty) preserves binding-order safety. New unit tests cover both the GC-collection path (CommandsSubscribedToCanExecuteCollect) and the IsEnabled coercion semantics (SwipeItemViewCommandCanExecuteUpdatesIsEnabled). The most recent commit adds the previously-missing net-windows PublicAPI entry.
Consensus: LGTM. Validating SwipeItemView.cs against HEAD confirms the fix is correct and complete, and all four models agree the leak — the prior NEEDS_DISCUSSION concern at b3ae1cd — is fully resolved. Three of four models returned LGTM; opus-4.8's lone NEEDS_DISCUSSION was driven by process items (a [WIP]/draft title and red CI) plus one cosmetic finding. Those process items are now resolved: per meta.json the PR is no longer a draft, the [WIP] marker is gone, and CI was re-triggered (/azp run, build pending — the earlier red lanes were infra-only BuildTasks staging races and a pre-existing HybridWebView IL2026 trim warning, neither caused by this diff). There are zero error- or warning-severity findings across the fleet.
Remaining finding (suggestion). Byte inspection at HEAD confirms net-ios and net-maccatalyst PublicAPI.Unshipped.txt gained an unintended UTF-8 BOM (ef bb bf) that the other four TFM files lack; it's analyzer-tolerant and harmless, but worth normalizing for consistency. The previously-noted net-tizen omission of the IsEnabledCore entry is still present, but it is already documented in an existing PR comment and is harmless on net11.0 (IncludeTizenTargetFrameworks=false), so it is not re-posted inline to avoid duplicating prior feedback.
| @@ -1,4 +1,4 @@ | |||
| #nullable enable | |||
| #nullable enable | |||
There was a problem hiding this comment.
This hunk introduces a UTF-8 BOM (U+FEFF) before #nullable enable (net-maccatalyst/PublicAPI.Unshipped.txt gets the identical edit), while the other four PublicAPI.Unshipped.txt files have no BOM — an unintended, inconsistent encoding change unrelated to the leak fix. The Roslyn analyzer tolerates it so it's harmless; re-save both files as UTF-8 without a BOM for consistency.
✅ LGTM — no blocking issues foundSynthesized review — PR #35891 (re-review @ c8f43cb)Verdict: LGTM • inline findings: 1 (suggestion) • models: gpt-5.5 LGTM · opus-4.8 NEEDS_DISCUSSION · opus-4.6 LGTM · gemini LGTM (3 LGTM / 1 ND) PR #35891 migrates Consensus: LGTM. Validating Remaining finding (suggestion). Byte inspection at HEAD confirms Multi-model review (gpt-5.5 · opus-4.8 · opus-4.6 · gemini-3.1-pro). Comments only — not a formal approval. |
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| ~static readonly Microsoft.Maui.Controls.ToolbarItem.BadgeColorProperty -> Microsoft.Maui.Controls.BindableProperty | ||
| ~static readonly Microsoft.Maui.Controls.ToolbarItem.BadgeTextColorProperty -> Microsoft.Maui.Controls.BindableProperty | ||
| ~static readonly Microsoft.Maui.Controls.ToolbarItem.BadgeTextProperty -> Microsoft.Maui.Controls.BindableProperty | ||
| override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool |
There was a problem hiding this comment.
[major] Public API Surface — This shared SwipeItemView.IsEnabledCore override was added to most public API baselines, but src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt was not updated. Because SwipeItemView is present in the Tizen Controls API and this override is not platform-guarded, the Tizen API validation baseline should include the same entry or API checks may fail for that TFM.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@devanathan-vaithiyanathan — new AI review results are available based on this last commit:
c8f43cb. To request a fresh review after new comments or commits, comment/review rerun.
Review Sessions — click to expand
Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: ANDROID · Base: net11.0 · Merge base: 9e0f1e97
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🧪 CommandTests CommandTests |
✅ FAIL — 83s | ✅ PASS — 55s |
🧪 SwipeViewTests SwipeViewTests |
✅ FAIL — 19s | ✅ PASS — 18s |
🔴 Without fix — 🧪 CommandTests: FAIL ✅ · 83s
Determining projects to restore...
Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 4.25 sec).
Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 112 ms).
Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 2.17 sec).
Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 2.03 sec).
Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 8.74 sec).
Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 66 ms).
Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 61 ms).
Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 57 ms).
Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 1.78 sec).
1 of 10 projects are up-to-date for restore.
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net11.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net11.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net11.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net11.0/Microsoft.Maui.Maps.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net11.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net11.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net11.0/Microsoft.Maui.Controls.Maps.dll
TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v11.0)
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 11.0.0-preview.5.26256.105)
[xUnit.net 00:00:00.16] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.40] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.41] Starting: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.63] CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SwipeItemView), useWeakEventHandler: False) [FAIL]
[xUnit.net 00:00:02.67] Assert.False() Failure
[xUnit.net 00:00:02.67] Expected: False
[xUnit.net 00:00:02.67] Actual: True
[xUnit.net 00:00:02.68] Stack Trace:
[xUnit.net 00:00:02.68] /_/src/Controls/tests/Core.UnitTests/CommandTests.cs(354,0): at Microsoft.Maui.Controls.Core.UnitTests.CommandTests.CommandsSubscribedToCanExecuteCollect(Type controlType, Boolean useWeakEventHandler)
[xUnit.net 00:00:02.68] --- End of stack trace from previous location ---
Passed CanExecuteReturnsFalseIfParameterIsWrongValueType [9 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchBar), useWeakEventHandler: False) [81 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.MenuItem), useWeakEventHandler: True) [83 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.ImageButton), useWeakEventHandler: False) [77 ms]
Failed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SwipeItemView), useWeakEventHandler: False) [846 ms]
Error Message:
Assert.False() Failure
Expected: False
Actual: True
Stack Trace:
at Microsoft.Maui.Controls.Core.UnitTests.CommandTests.CommandsSubscribedToCanExecuteCollect(Type controlType, Boolean useWeakEventHandler) in /_/src/Controls/tests/Core.UnitTests/CommandTests.cs:line 354
--- End of stack trace from previous location ---
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.Button), useWeakEventHandler: False) [105 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.ImageButton), useWeakEventHandler: True) [67 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.RefreshView), useWeakEventHandler: False) [76 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.TextCell), useWeakEventHandler: False) [98 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchHandler), useWeakEventHandler: True) [73 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SwipeItemView), useWeakEventHandler: True) [72 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.MenuItem), useWeakEventHandler: False) [79 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.RefreshView), useWeakEventHandler: True) [78 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.TextCell), useWeakEventHandler: True) [69 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.Button), useWeakEventHandler: True) [83 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchBar), useWeakEventHandler: True) [105 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchHandler), useWeakEventHandler: False) [75 ms]
Passed ExecuteDoesNotRunIfParameterIsWrongReferenceType [1 ms]
Passed ExecuteRunsIfReferenceTypeAndSetToNull [< 1 ms]
Passed CanExecuteIgnoresParameterIfValueTypeAndSetToNull [< 1 ms]
Passed CanExecute(expected: True) [5 ms]
Passed CanExecute(expected: False) [< 1 ms]
Passed GenericExecuteWithCanExecute [1 ms]
Passed GenericThrowsWithNullExecute [< 1 ms]
Passed Execute [< 1 ms]
Passed GenericCanExecute(expected: True) [< 1 ms]
Passed GenericCanExecute(expected: False) [< 1 ms]
Passed ThrowsWithNullConstructor [< 1 ms]
Passed ExecuteDoesNotRunIfParameterIsWrongValueType [< 1 ms]
Passed GenericThrowsWithNullExecuteAndCanExecuteValid [< 1 ms]
Passed ThrowsWithNullExecuteValidCanExecute [< 1 ms]
Passed CanExecuteUsesParameterIfReferenceTypeAndSetToNull [< 1 ms]
Passed ChangeCanExecute [< 1 ms]
Passed GenericExecute [< 1 ms]
Passed ThrowsWithNullCanExecute [< 1 ms]
Passed ThrowsWithNullParameterizedConstructor [< 1 ms]
[xUnit.net 00:00:03.63] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed CanExecuteUsesParameterIfNullableAndSetToNull [< 1 ms]
Passed ExecuteDoesNotRunIfValueTypeAndSetToNull [< 1 ms]
Passed Constructor [< 1 ms]
Passed ThrowsWithNullParameterizedCanExecute [< 1 ms]
Passed ExecuteRunsIfNullableAndSetToNull [< 1 ms]
Passed ExecuteParameterized [3 ms]
Passed GenericThrowsWithValidExecuteAndCanExecuteNull [< 1 ms]
Passed ExecuteWithCanExecute [< 1 ms]
Passed CanExecuteReturnsFalseIfParameterIsWrongReferenceType [< 1 ms]
Test Run Failed.
Total tests: 45
Passed: 44
Failed: 1
Total time: 4.2159 Seconds
🟢 With fix — 🧪 CommandTests: PASS ✅ · 55s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net11.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net11.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net11.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net11.0/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net11.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net11.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net11.0/Microsoft.Maui.Controls.Maps.dll
TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v11.0)
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 11.0.0-preview.5.26256.105)
[xUnit.net 00:00:00.16] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.17] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.20] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed CanExecuteReturnsFalseIfParameterIsWrongValueType [5 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchBar), useWeakEventHandler: False) [83 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.MenuItem), useWeakEventHandler: True) [72 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.ImageButton), useWeakEventHandler: False) [95 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SwipeItemView), useWeakEventHandler: False) [65 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.Button), useWeakEventHandler: False) [75 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.ImageButton), useWeakEventHandler: True) [116 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.RefreshView), useWeakEventHandler: False) [70 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.TextCell), useWeakEventHandler: False) [68 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchHandler), useWeakEventHandler: True) [73 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SwipeItemView), useWeakEventHandler: True) [76 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.MenuItem), useWeakEventHandler: False) [75 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.RefreshView), useWeakEventHandler: True) [76 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.TextCell), useWeakEventHandler: True) [72 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.Button), useWeakEventHandler: True) [86 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchBar), useWeakEventHandler: True) [124 ms]
Passed CommandsSubscribedToCanExecuteCollect(controlType: typeof(Microsoft.Maui.Controls.SearchHandler), useWeakEventHandler: False) [83 ms]
Passed ExecuteDoesNotRunIfParameterIsWrongReferenceType [< 1 ms]
Passed ExecuteRunsIfReferenceTypeAndSetToNull [< 1 ms]
Passed CanExecuteIgnoresParameterIfValueTypeAndSetToNull [< 1 ms]
Passed CanExecute(expected: True) [4 ms]
Passed CanExecute(expected: False) [< 1 ms]
Passed GenericExecuteWithCanExecute [1 ms]
Passed GenericThrowsWithNullExecute [1 ms]
Passed Execute [< 1 ms]
Passed GenericCanExecute(expected: True) [< 1 ms]
Passed GenericCanExecute(expected: False) [< 1 ms]
Passed ThrowsWithNullConstructor [< 1 ms]
Passed ExecuteDoesNotRunIfParameterIsWrongValueType [< 1 ms]
Passed GenericThrowsWithNullExecuteAndCanExecuteValid [< 1 ms]
Passed ThrowsWithNullExecuteValidCanExecute [< 1 ms]
Passed CanExecuteUsesParameterIfReferenceTypeAndSetToNull [< 1 ms]
Passed ChangeCanExecute [< 1 ms]
Passed GenericExecute [< 1 ms]
Passed ThrowsWithNullCanExecute [< 1 ms]
Passed ThrowsWithNullParameterizedConstructor [< 1 ms]
Passed CanExecuteUsesParameterIfNullableAndSetToNull [< 1 ms]
Passed ExecuteDoesNotRunIfValueTypeAndSetToNull [< 1 ms]
Passed Constructor [< 1 ms]
[xUnit.net 00:00:02.63] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ThrowsWithNullParameterizedCanExecute [< 1 ms]
Passed ExecuteRunsIfNullableAndSetToNull [< 1 ms]
Passed ExecuteParameterized [3 ms]
Passed GenericThrowsWithValidExecuteAndCanExecuteNull [< 1 ms]
Passed ExecuteWithCanExecute [< 1 ms]
Passed CanExecuteReturnsFalseIfParameterIsWrongReferenceType [< 1 ms]
Test Run Successful.
Total tests: 45
Passed: 45
Total time: 3.1181 Seconds
🔴 Without fix — 🧪 SwipeViewTests: FAIL ✅ · 19s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net11.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net11.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net11.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net11.0/Microsoft.Maui.Maps.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net11.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net11.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net11.0/Microsoft.Maui.Controls.Maps.dll
TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v11.0)
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 11.0.0-preview.5.26256.105)
[xUnit.net 00:00:00.29] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.51] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.58] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed TestSwipeItemView [105 ms]
Passed TestContentBindingContextPropagatesToPassedInSwipeItem [23 ms]
Passed BindingContextTransfersToNewSetOfSwipeItems [12 ms]
Passed TestRightItems [3 ms]
Passed TestContentBindingContextPropagatesToAddedSwipeItems [< 1 ms]
Passed SwipeViewFindsScrollParentDirectlyWhenTreeIsConnected [7 ms]
Passed TestBottomItems [< 1 ms]
Passed TestDefaultSwipeItems [6 ms]
Passed SwipeViewResubscribesToScrollParentAfterRemovalAndReaddition [1 ms]
[xUnit.net 00:00:04.01] SwipeItemViewCommandCanExecuteUpdatesIsEnabled [FAIL]
[xUnit.net 00:00:04.01] Assert.False() Failure
[xUnit.net 00:00:04.01] Expected: False
[xUnit.net 00:00:04.01] Actual: True
[xUnit.net 00:00:04.01] Stack Trace:
[xUnit.net 00:00:04.02] /_/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(449,0): at Microsoft.Maui.Controls.Core.UnitTests.SwipeViewTests.SwipeItemViewCommandCanExecuteUpdatesIsEnabled()
[xUnit.net 00:00:04.02] at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:04.02] at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Passed TestConstructor [2 ms]
Passed TestSwipeViewBindingContextChangedEvent [< 1 ms]
Passed TestProgrammaticallyClose [6 ms]
Passed TestSwipeItemsSwipeBehaviorOnInvoked [< 1 ms]
Passed SwipeItemsRemainInLogicalTreeWhenContentIsSet [67 ms]
Passed TestContentBindingContextChangedEvent [< 1 ms]
Passed TestTemplatedContentBindingContextChangedEvent [4 ms]
Failed SwipeItemViewCommandCanExecuteUpdatesIsEnabled [14 ms]
Error Message:
Assert.False() Failure
Expected: False
Actual: True
Stack Trace:
at Microsoft.Maui.Controls.Core.UnitTests.SwipeViewTests.SwipeItemViewCommandCanExecuteUpdatesIsEnabled() in /_/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs:line 449
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Passed TestProgrammaticallyOpen [< 1 ms]
[xUnit.net 00:00:04.03] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ClearRemovesLogicalChildren [2 ms]
Passed TestLeftItems [< 1 ms]
Passed TestContentBindingContextPropagatesToNewSwipeItems [< 1 ms]
Passed TestSwipeItemsExecuteMode [2 ms]
Passed TestTopItems [< 1 ms]
Passed SwipeViewRediscoversScrollParentWhenTemplateRootIsReparented [< 1 ms]
Passed SwipeViewFindsScrollParentAfterTemplateParentConnected [< 1 ms]
Test Run Failed.
Total tests: 25
Passed: 24
Failed: 1
Total time: 5.0894 Seconds
🟢 With fix — 🧪 SwipeViewTests: PASS ✅ · 18s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net11.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net11.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net11.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net11.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net11.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
##vso[build.updatebuildnumber]11.0.0-ci+azdo.14417344
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net11.0/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net11.0/Microsoft.Maui.Controls.Xaml.dll
TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net11.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v11.0)
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 11.0.0-preview.5.26256.105)
[xUnit.net 00:00:00.53] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.60] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.64] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed TestSwipeItemView [60 ms]
Passed TestContentBindingContextPropagatesToPassedInSwipeItem [17 ms]
Passed BindingContextTransfersToNewSetOfSwipeItems [8 ms]
Passed TestRightItems [< 1 ms]
Passed TestContentBindingContextPropagatesToAddedSwipeItems [< 1 ms]
Passed SwipeViewFindsScrollParentDirectlyWhenTreeIsConnected [5 ms]
Passed TestBottomItems [< 1 ms]
Passed TestDefaultSwipeItems [7 ms]
Passed SwipeViewResubscribesToScrollParentAfterRemovalAndReaddition [1 ms]
Passed TestConstructor [1 ms]
Passed TestSwipeViewBindingContextChangedEvent [2 ms]
Passed TestProgrammaticallyClose [4 ms]
Passed TestSwipeItemsSwipeBehaviorOnInvoked [< 1 ms]
Passed SwipeItemsRemainInLogicalTreeWhenContentIsSet [35 ms]
Passed TestContentBindingContextChangedEvent [< 1 ms]
Passed TestTemplatedContentBindingContextChangedEvent [3 ms]
Passed SwipeItemViewCommandCanExecuteUpdatesIsEnabled [6 ms]
Passed TestProgrammaticallyOpen [< 1 ms]
[xUnit.net 00:00:03.90] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ClearRemovesLogicalChildren [< 1 ms]
Passed TestLeftItems [< 1 ms]
Passed TestContentBindingContextPropagatesToNewSwipeItems [< 1 ms]
Passed TestSwipeItemsExecuteMode [< 1 ms]
Passed TestTopItems [< 1 ms]
Passed SwipeViewRediscoversScrollParentWhenTemplateRootIsReparented [1 ms]
Passed SwipeViewFindsScrollParentAfterTemplateParentConnected [< 1 ms]
Test Run Successful.
Total tests: 25
Passed: 25
Total time: 4.8870 Seconds
📁 Fix files reverted (7 files)
src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txtsrc/Controls/src/Core/SwipeView/SwipeItemView.cs
UI Tests — SwipeView
Detected UI test categories: SwipeView
Pre-Flight — Context & Validation
Issue: #35498 - SwipeItemView.Command leaks row views and command parameters through CanExecuteChanged
PR: #35891 - [Net 11]Fix SwipeItemView command leak
Platforms Affected: Android, iOS (repro validated on both; fix is shared Controls code)
Files Changed: 7 implementation/API, 2 test
Key Findings
SwipeItemView.Commandpreviously subscribed directly toICommand.CanExecuteChanged, allowing long-lived commands to retainSwipeItemView, content, command parameters, binding contexts, and row view models after page disposal.- PR fix changes
SwipeItemViewto the sharedICommandElement/CommandElement/WeakCommandSubscriptionpattern used by other command-backed controls, and composes commandCanExecutewithbase.IsEnabledCore. - Added tests cover command-subscriber collection and
IsEnabledupdates fromCanExecute; linked issue comments confirm reproducibility on Android and iOS. - Prior reviews found no blocking errors; one suggestion notes unintended UTF-8 BOMs in iOS/MacCatalyst PublicAPI files.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 1
Key code review findings:
- 💡
src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt:1andsrc/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt:1gained a UTF-8 BOM; harmless but inconsistent.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35891 | Replace direct CanExecuteChanged subscription with ICommandElement / WeakCommandSubscription, move enablement to IsEnabledCore, and add PublicAPI entries/tests. |
✅ PASSED (Gate) | SwipeItemView.cs, PublicAPI files, CommandTests.cs, SwipeViewTests.cs |
Original PR |
Code Review — Deep Analysis
Code Review — PR #35891
Independent Assessment
What this changes: SwipeItemView now uses shared ICommandElement / WeakCommandSubscription infrastructure instead of directly subscribing to ICommand.CanExecuteChanged. It also moves command enablement into IsEnabledCore and adds unit coverage.
Inferred motivation: Avoid retaining SwipeItemView instances and their row content through long-lived command event subscriptions.
Reconciliation with PR Narrative
Author claims: Fixes #35498 memory leak from SwipeItemView.Command, with tests for GC and CanExecute/IsEnabled behavior.
Agreement/disagreement: Matches the code and linked issue. The implementation follows existing Button/CheckBox command patterns.
Prior Review Reconciliation
No prior ❌ Error findings found. Prior comments only noted low-severity PublicAPI consistency/BOM suggestions.
Blast Radius Assessment
- Runs for all instances: Only
SwipeItemView; no handler/platform changes. - Startup impact: No app startup path; static ctor runs on first
SwipeItemViewuse only. - Static/shared state: No new mutable shared state.
CI Status
- Required-check result:
gh pr checks --requiredunavailable due unauthenticatedgh; public Checks API showsmaui-pr,maui-pr-devicetests,maui-pr-uitests, and Build Analysis failing. - Classification: undetermined / likely unrelated infrastructure or existing test failures, but not fully verified.
- Action taken: Invoked
azdo-build-investigator; its requiredci-analysisskill was unavailable. Confidence capped low; no LGTM while CI is red/undetermined.
Findings
💡 Suggestion — Normalize PublicAPI file encoding
src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt:1 and net-maccatalyst/PublicAPI.Unshipped.txt:1 gained a UTF-8 BOM while peer files did not. Harmless for analyzers, but worth normalizing.
Failure-Mode Probing
- Long-lived command: weak subscription breaks the retain cycle.
- Command replacement/null:
OnCommandChanging/OnCommandChanged(null)dispose prior tracker. - Explicit
IsEnabled=false: preserved bybase.IsEnabledCore; covered by test. - Handler disconnect/reconnect: no handler subscriptions changed.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code change looks sound and well-tested, with only a minor encoding suggestion. Verdict is not LGTM solely because required CI is red/undetermined and could not be fully classified here.
Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Local WeakCommandSubscription field inside SwipeItemView, without adopting ICommandElement / CommandElement. |
✅ PASS | SwipeItemView.cs, PublicAPI files |
Lighter than PR, but less consistent with established MAUI command-element pattern. |
| 2 | try-fix | Keep strong direct subscription and clean up deterministically from Loaded/Unloaded lifecycle. |
❌ FAIL | SwipeItemView.cs, PublicAPI files |
69/70 passed; GC test fails because a never-attached SwipeItemView never unloads, so strong command delegate still retains it. |
| 3 | try-fix | SwipeItemView-specific weak listener registry/proxy with weak references to views. | ✅ PASS | SwipeItemView.cs, PublicAPI files |
Works, but adds bespoke per-command registry complexity instead of using existing infrastructure. |
| 4 | try-fix | Custom per-instance nested weak event proxy for SwipeItemView. |
✅ PASS | SwipeItemView.cs, PublicAPI files |
Works, but duplicates existing WeakCommandSubscription behavior locally. |
| PR | PR #35891 | Shared ICommandElement / CommandElement / WeakCommandSubscription migration plus IsEnabledCore. |
✅ PASSED (Gate) | 9 files | Original PR; uses proven shared command infrastructure and passed the prior gate. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | No new ideas; remaining options collapse into weak command subscription semantics or worse alternatives. |
| claude-opus-4.7 | 2 | No | No new ideas; PR fix is the shared idiomatic version, alternatives either leak or duplicate it. |
| gpt-5.3-codex | 2 | No | No new ideas; strong handlers reintroduce lifetime risk, weak alternatives duplicate WeakCommandSubscription. |
| gpt-5.5 | 2 | No | No new ideas; lifecycle cleanup failed and all viable weak strategies were explored. |
Candidate Narrative
try-fix-1 — Local WeakCommandSubscription
Passed all targeted unit tests. This proves a local field around the existing weak subscription helper can fix the leak without adopting ICommandElement, but it is less aligned with existing controls such as Button, CheckBox, SearchBar, MenuItem, and RefreshView.
try-fix-2 — Deterministic lifecycle cleanup
Failed one targeted unit test: CommandsSubscribedToCanExecuteCollect(SwipeItemView, useWeakEventHandler:false). This is the key learning from the loop: lifecycle cleanup cannot be the root fix because a SwipeItemView can be assigned a command without ever being loaded/unloaded, so the long-lived command still strongly retains it.
try-fix-3 — Per-command weak listener registry
Passed all targeted unit tests. It avoids strong retention but introduces a bespoke registry/proxy mechanism, which is more complex than reusing the repository existing weak command infrastructure.
try-fix-4 — Custom per-instance weak proxy
Passed all targeted unit tests. It is simpler than the registry, but still duplicates the behavior already provided by WeakCommandSubscription and therefore is not preferable to the PR fix.
Exhausted: Yes
Selected Fix: PR fix — it is not the smallest passing diff, but it is the best MAUI-style solution: it uses shared, proven ICommandElement / CommandElement / WeakCommandSubscription infrastructure; handles the never-attached leak scenario that broke Candidate #2; avoids bespoke registries/proxies from Candidates #3/#4; and preserves enabled-state composition through IsEnabledCore.
Report — Final Recommendation
Comparative Report — PR #35891
Candidates compared
| Rank | Candidate | Test result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Inherits PR gate pass; sandbox feedback is API-baseline-only | Best candidate. Keeps the PR's shared ICommandElement / CommandElement / WeakCommandSubscription solution and adds the missing Tizen PublicAPI baseline entry identified by expert review. |
| 2 | pr |
✅ Gate passed | Correct behavioral fix and best architectural approach among raw implementations, but expert review found one missing platform API baseline entry. |
| 3 | try-fix-1 |
✅ Passed targeted tests | Uses existing WeakCommandSubscription locally and is behaviorally viable, but is less consistent with MAUI's established command-element pattern than the PR. |
| 4 | try-fix-4 |
✅ Passed targeted tests | Per-instance weak proxy fixes the leak, but duplicates behavior already provided by shared infrastructure. |
| 5 | try-fix-3 |
✅ Passed targeted tests | Weak listener registry fixes the leak, but adds bespoke global registry/proxy complexity that is riskier than using existing infrastructure. |
| 6 | try-fix-2 |
❌ Failed targeted regression test | Ranked last because it keeps a strong subscription and fails the never-attached SwipeItemView leak scenario. Failed candidates must rank below passing candidates. |
Analysis
All passing candidates converge on the same essential root cause: SwipeItemView must not be strongly retained by a long-lived ICommand.CanExecuteChanged subscription. try-fix-2 proves lifecycle cleanup alone is insufficient because the view can be assigned a command and never loaded/unloaded, leaving the strong command delegate as a leak.
Among the passing candidates, the PR's approach is strongest because it reuses MAUI's existing command infrastructure rather than introducing a SwipeItemView-specific weak listener or proxy. The expert review did not find a behavioral flaw in that approach; it found a completeness issue in PublicAPI baselines. Applying that feedback produces pr-plus-reviewer, which preserves the proven PR behavior and removes the baseline risk.
Winner
pr-plus-reviewer wins. It is the PR's idiomatic shared weak-command fix plus the expert reviewer baseline correction, while all alternative try-fixes are either less aligned with established MAUI patterns or empirically failed regression coverage.
Future Action — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
#35677) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary - Posts MauiBot AI Summary output as a pull request review with parsed `APPROVE`, `REQUEST_CHANGES`, or safe `COMMENT` fallback. - Uses the new AI Review Summary layout with segmented status chips, collapsed review sessions, and merged Future Action content. - Keeps PR finalization out of the automated review process; AI Summary updates no longer preserve or merge `SECTION:PR-FINALIZE` blocks. - Adds visible AI Summary guidance telling users to comment `/review rerun` after new comments or commits when they want a fresh review. The command implementation is intentionally split into a follow-up PR. - Hides stale MauiBot AI Summary / try-fix artifacts with GitHub minimization instead of deleting them, while preserving same-run try-fix and AI Summary reviews. - Updates the Copilot pipeline to pass review IDs and patch review bodies after deep UI tests. - Hardens gate setup/retry handling by committing squashed PR changes before verification, resetting the review branch before gate retries, and detecting BlazorWebView unit-test project paths. ## Validation - Parsed changed PowerShell scripts with `System.Management.Automation.Language.Parser`. - Parsed `.github/workflows/review-trigger.yml` as YAML. - `Invoke-Pester .github/scripts/Post-AISummaryComment.Tests.ps1,.github/scripts/Remove-StaleMauiBotComments.Tests.ps1 -CI` - Dry-run AI Summary generation verified the rerun note, segmented chips, and collapsed review session layout. - Verified `Detect-TestsInDiff.ps1` maps `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/UriExtensions_Tests.cs` to `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/MauiBlazorWebView.UnitTests.csproj`. ## net11.0-targeting PR support This branch also makes the Copilot review pipeline handle **`net11.0`-targeting PRs** as well as `main`/net10, from a single pipeline branch (consolidates and supersedes #35994): - **Runtime base-branch auto-detection** (`eng/pipelines/ci-copilot.yml`): the `CopilotReview` and `DeepUITests` stages read the PR's `baseRefName` via `gh`, allowlist-validate it (`^(main|net[0-9]+\.0)$`), and check out that base **before** workload install and the squash-merge — so workloads (net11 `11.0.100` vs net10 `10.0.100`) and the merge base follow the PR. Trusted scripts are captured from the pipeline ref first (security rule 3); Task 1 Setup runs from `$TRUSTED`. - **Branch-aware test TargetFramework**: `BuildAndRunHostApp.ps1` and `run-device-tests/Run-DeviceTests.ps1` now derive the TFM from `Directory.Build.props` (`Get-MauiTfmVersion` in `shared-utils.ps1`) instead of hardcoding `net10.0`, so deep-UI/device tests build `net11.0-android` on net11. The `DeepUITests` stage restores the reviewed pipeline-branch scripts over the worktree before the per-category loop. `main`/net10 behavior is unchanged by design (base-detection checks out `main` = the original flow; `global.json` stays `10.0.108`). `review-trigger.yml` and `Review-PR.ps1` are intentionally untouched. **Validation:** 8 live net11 runs with 100% correct base detection (e.g. #35891 gate ran net11 Core tests 44/45 → 45/45, conflict-free squash-merge onto net11.0); reproduced and root-caused a separate net11.0-baseline `MAUIX2017` Xaml.UnitTests build break (unrelated to this change). Confirmation runs from this branch cover one net11 PR and one main PR. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com> Co-authored-by: Copilot <Copilot@users.noreply.github.com> Co-authored-by: Copilot CI <copilot-ci@microsoft.com> Co-authored-by: kubaflo <kubaflo@users.noreply.github.com>
dotnet#35677) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary - Posts MauiBot AI Summary output as a pull request review with parsed `APPROVE`, `REQUEST_CHANGES`, or safe `COMMENT` fallback. - Uses the new AI Review Summary layout with segmented status chips, collapsed review sessions, and merged Future Action content. - Keeps PR finalization out of the automated review process; AI Summary updates no longer preserve or merge `SECTION:PR-FINALIZE` blocks. - Adds visible AI Summary guidance telling users to comment `/review rerun` after new comments or commits when they want a fresh review. The command implementation is intentionally split into a follow-up PR. - Hides stale MauiBot AI Summary / try-fix artifacts with GitHub minimization instead of deleting them, while preserving same-run try-fix and AI Summary reviews. - Updates the Copilot pipeline to pass review IDs and patch review bodies after deep UI tests. - Hardens gate setup/retry handling by committing squashed PR changes before verification, resetting the review branch before gate retries, and detecting BlazorWebView unit-test project paths. ## Validation - Parsed changed PowerShell scripts with `System.Management.Automation.Language.Parser`. - Parsed `.github/workflows/review-trigger.yml` as YAML. - `Invoke-Pester .github/scripts/Post-AISummaryComment.Tests.ps1,.github/scripts/Remove-StaleMauiBotComments.Tests.ps1 -CI` - Dry-run AI Summary generation verified the rerun note, segmented chips, and collapsed review session layout. - Verified `Detect-TestsInDiff.ps1` maps `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/UriExtensions_Tests.cs` to `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/MauiBlazorWebView.UnitTests.csproj`. ## net11.0-targeting PR support This branch also makes the Copilot review pipeline handle **`net11.0`-targeting PRs** as well as `main`/net10, from a single pipeline branch (consolidates and supersedes dotnet#35994): - **Runtime base-branch auto-detection** (`eng/pipelines/ci-copilot.yml`): the `CopilotReview` and `DeepUITests` stages read the PR's `baseRefName` via `gh`, allowlist-validate it (`^(main|net[0-9]+\.0)$`), and check out that base **before** workload install and the squash-merge — so workloads (net11 `11.0.100` vs net10 `10.0.100`) and the merge base follow the PR. Trusted scripts are captured from the pipeline ref first (security rule 3); Task 1 Setup runs from `$TRUSTED`. - **Branch-aware test TargetFramework**: `BuildAndRunHostApp.ps1` and `run-device-tests/Run-DeviceTests.ps1` now derive the TFM from `Directory.Build.props` (`Get-MauiTfmVersion` in `shared-utils.ps1`) instead of hardcoding `net10.0`, so deep-UI/device tests build `net11.0-android` on net11. The `DeepUITests` stage restores the reviewed pipeline-branch scripts over the worktree before the per-category loop. `main`/net10 behavior is unchanged by design (base-detection checks out `main` = the original flow; `global.json` stays `10.0.108`). `review-trigger.yml` and `Review-PR.ps1` are intentionally untouched. **Validation:** 8 live net11 runs with 100% correct base detection (e.g. dotnet#35891 gate ran net11 Core tests 44/45 → 45/45, conflict-free squash-merge onto net11.0); reproduced and root-caused a separate net11.0-baseline `MAUIX2017` Xaml.UnitTests build break (unrelated to this change). Confirmation runs from this branch cover one net11 PR and one main PR. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com> Co-authored-by: Copilot <Copilot@users.noreply.github.com> Co-authored-by: Copilot CI <copilot-ci@microsoft.com> Co-authored-by: kubaflo <kubaflo@users.noreply.github.com>
Fixes a memory leak in
SwipeItemView.Commandwhere assigning a long-livedICommandcaused eachSwipeItemViewto be retained through a directCanExecuteChangedsubscription.The retained graph could keep row content, command parameters, binding contexts, and row view models alive after the containing page was closed.
This changes
SwipeItemViewto use the existingICommandElement/WeakCommandSubscriptioninfrastructure used by other command-backed controls. It also moves command enablement intoIsEnabledCore, so explicitIsEnabled=false, parent disabled state, andCommand.CanExecuteare composed consistently.Fixes #35498
Testing
dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~CommandTests|FullyQualifiedName~SwipeViewTests"dotnet build Microsoft.Maui.BuildTasks.slnfSwipeItemView.Commandleaks row views and command parameters throughCanExecuteChanged#35498 iOS repro after forced rebuild:SwipeItemViewCommand:commandSubscribers=0,aliveRows=0,retainedPayloadBytes=0SwipeItemView.Commandleaks row views and command parameters throughCanExecuteChanged#35498 Android repro after forced rebuild:SwipeItemViewCommandno longer retains all 1,000 rows / 125 MiB and drops to the Android platform baseline