Fix AdaptiveTrigger memory leak when VisualStateGroups are replaced after attach - #36240
Conversation
The official `ci-official.yml` pack pipeline only builds and packs NuGet packages — no tests run, so simulator runtimes are not needed. The `Install Simulator Runtimes` step has been timing out on macOS agents, blocking official builds. Adds `skipSimulatorSetup: true` to the provision parameters in the pack stage. Backport of the same fix from `release/11.0.1xx-preview3` (dotnet#34801). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Problem The `Merge net11.0 to next release` workflow silently does nothing when triggered via `workflow_dispatch` or `schedule` because both run against `main` (the default branch). The arcade merge script uses `GITHUB_REF_NAME` as the config lookup key, finds no `"main"` entry in `github-merge-flow-release-11.jsonc`, and exits with "There was no configuration found for main" — but reports success. ## Fix Add `if: github.ref_name == 'net11.0'` guard to the job so it skips immediately when triggered from the wrong branch, instead of silently succeeding. **For `workflow_dispatch`**: Select `net11.0` from the branch dropdown in the GitHub UI before clicking Run. **For `schedule`**: The cron trigger always runs on the default branch (`main`), so it will be skipped. The workflow on the `net11.0` branch (triggered by `push`) handles the actual merges. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#36188) <!-- 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! ### Description Fixes the broken `net11.0` → release down-merge automation. The `Merge net11.0 to next release` workflow (`.github/workflows/merge-net11-to-release.yml`) delegates to the shared arcade `inter-branch-merge-base.yml`, which reads its merge-flow configuration from the **default branch (`main`)** — the `configuration_file_branch` input [defaults to `'main'`](https://github.com/dotnet/arcade/blob/main/.github/workflows/inter-branch-merge-base.yml). On `main`, `github-merge-flow-release-11.jsonc` still pointed at the stale `release/11.0.1xx-preview4`. The bump to `preview6` (dotnet#36110) was applied only to the copy on the `net11.0` branch, which the workflow never reads. As a result the workflow targeted `preview4`, tried to update the leftover stale `merge/net11.0-to-release/11.0.1xx-preview4` branch, hit a non-fast-forward push rejection, and exited 1 on every `push`-from-`net11.0` run — so no `preview6` down-merge PR was ever created. This one-line change updates the **main** copy to `release/11.0.1xx-preview6` so the automation targets the current preview release branch. ### Evidence - Run [`28241084988`](https://github.com/dotnet/maui/actions/runs/28241084988) Merge step: `read-configuration.ps1 -ConfigurationFileBranch main … github-merge-flow-release-11.jsonc` → `[MergeToBranch, release/11.0.1xx-preview4]`, then non-fast-forward push rejection → `WARNING: Failed to update existing PR` → exit code 1. - `git show origin/main:github-merge-flow-release-11.jsonc` → `preview4`; `git show origin/net11.0:…` → `preview6` (unread by the workflow). - `release/11.0.1xx-preview6` exists on origin, so this was a stale-config issue, not a missing-branch issue. ### Follow-up (not in this PR) Stale leftover branches `merge/net11.0-to-release/11.0.1xx-preview4` and `…preview3` (and stale PR dotnet#34875) can be cleaned up separately. The misleading comments in `merge-net11-to-release.yml` (claiming the config is read from `net11.0`, and that the daily schedule works from `net11.0`) are also worth revisiting, since scheduled workflows and config reads both happen from the default branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36240Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36240" |
|
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. |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@Shalini-Ashokan — new AI review results are available based on this last commit:
b1996b3. 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: main · Merge base: 6e107357
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🧪 AdaptiveTriggerTests AdaptiveTriggerTests |
✅ FAIL — 108s | ✅ PASS — 68s |
🔴 Without fix — 🧪 AdaptiveTriggerTests: FAIL ✅ · 108s
Determining projects to restore...
Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 5.47 sec).
Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 9.17 sec).
Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 3.06 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 7 ms).
Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 10 ms).
Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 20 ms).
Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 51 ms).
Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 1.71 sec).
Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 1.87 sec).
1 of 10 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.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]10.0.90-ci+azdo.14529387
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.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/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
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 10.0.0)
[xUnit.net 00:00:00.14] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.37] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.38] Starting: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.51] OldAdaptiveTriggerDetachedWhenVSGsReplacedAfterAttach [FAIL]
[xUnit.net 00:00:01.51] Old AdaptiveTrigger should be detached after VSGs are replaced
[xUnit.net 00:00:01.51] Stack Trace:
[xUnit.net 00:00:01.51] /_/src/Controls/tests/Core.UnitTests/AdaptiveTriggerTests.cs(63,0): at Microsoft.Maui.Controls.Core.UnitTests.AdaptiveTriggerTests.OldAdaptiveTriggerDetachedWhenVSGsReplacedAfterAttach()
[xUnit.net 00:00:01.51] at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:01.51] at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:01.52] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ValidateAdaptiveTriggerDisconnects [60 ms]
Passed ResizingWindowPageActivatesTrigger [11 ms]
Failed OldAdaptiveTriggerDetachedWhenVSGsReplacedAfterAttach [1 ms]
Error Message:
Old AdaptiveTrigger should be detached after VSGs are replaced
Stack Trace:
at Microsoft.Maui.Controls.Core.UnitTests.AdaptiveTriggerTests.OldAdaptiveTriggerDetachedWhenVSGsReplacedAfterAttach() in /_/src/Controls/tests/Core.UnitTests/AdaptiveTriggerTests.cs:line 63
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Test Run Failed.
Total tests: 3
Passed: 2
Failed: 1
Total time: 2.0313 Seconds
🟢 With fix — 🧪 AdaptiveTriggerTests: PASS ✅ · 68s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.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]10.0.90-ci+azdo.14529387
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14529387
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.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/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
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 10.0.0)
[xUnit.net 00:00:00.22] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.39] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.41] Starting: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.64] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ValidateAdaptiveTriggerDisconnects [106 ms]
Passed ResizingWindowPageActivatesTrigger [20 ms]
Passed OldAdaptiveTriggerDetachedWhenVSGsReplacedAfterAttach [1 ms]
Test Run Successful.
Total tests: 3
Passed: 3
Total time: 3.4235 Seconds
📁 Fix files reverted (2 files)
src/Controls/src/Core/AdaptiveTrigger.cssrc/Controls/src/Core/VisualStateManager.cs
📱 UI Tests — VisualStateManager
Detected UI test categories: VisualStateManager
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
VisualStateManager |
0/100 (setup failed; 100 marked failed) | — |
⚠️ VisualStateManager — fixture setup failed for 100 tests
NUnit reported a OneTimeSetUp/fixture setup failure before test bodies ran; the TRX marked each affected test failed.
Multiple setup failure signatures were present; showing the first one. See the TRX artifact for all details.
OneTimeSetUp: System.TimeoutException : Timed out waiting for Go To Test button to appear (the app did not recover after crash-recovery attempts)
at Microsoft.Maui.TestCases.Tests.UtilExtensions.WaitForGoToTestButtonWithRecovery(IApp app, String timeoutMessage) in /_/src/Controls/tests/TestCases.Shared.Tests/UtilExtensions.cs:line 91
at Microsoft.Maui.TestCases.Tests.UtilExtensions.NavigateToGallery(IApp app, String page) in /_/src/Controls/tests/TestCases.Shared.Tests/UtilExtensions.cs:line 37
at Microsoft.Maui.TestCases.Tests._GalleryUITest.FixtureSetup() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/_GalleryUITest.cs:line 57
at UITest.Appium.NUnit.UITestBase.OneTimeSetup() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 221
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)
📋 Pre-Flight — Context & Validation
Issue: #36032 - AdaptiveTrigger leaks VisualElement when VisualStateGroups are replaced after attach
PR: #36240 - [WIP] Fix AdaptiveTrigger memory leak when VisualStateGroups are replaced after attach
Platforms Affected: Android, iOS, macOS/MacCatalyst (issue labels/repro); logic is shared Controls code
Files Changed: 2 implementation, 1 test
Key Findings
- #36032 reports a retention chain
Window.SizeChanged -> old AdaptiveTrigger -> VisualElement -> BindingContextafter visible controls replaceVisualStateManager.VisualStateGroupsat runtime. - The PR fix detaches triggers from the old group list in
VisualStateGroupsPropertyChanged, re-attaches incoming triggers when the element is already in aWindow, and weakensAdaptiveTrigger's element reference as a safety net. - Gate was already completed separately:
AdaptiveTriggerTestsfail without the fix and pass with the PR fix. Per instructions, gate verification was not re-run. - PR discussion and inline review comments available through public API contained no prior error-level review findings.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- ℹ No code-review defects found in the PR diff.
- ℹ Blast radius is limited to runtime replacement of
VisualStateGroups; no startup/static state impact. - ℹ CI status could not be fully determined because
ghis unauthenticated in this environment, so confidence is capped low.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36240 | In VisualStateManager.VisualStateGroupsPropertyChanged, call SendDetached() on old triggers, update new groups, and call InvalidateStateTriggers(true) if already windowed; also change AdaptiveTrigger's element reference to WeakReference<VisualElement>. |
✅ PASSED (Gate) | AdaptiveTrigger.cs, VisualStateManager.cs, AdaptiveTriggerTests.cs |
Original PR fix; gate output is in gate/content.md. |
🔬 Code Review — Deep Analysis
Code Review — PR #36240
Independent Assessment
What this changes: Fixes AdaptiveTrigger lifecycle when VisualStateGroups are replaced on an already-windowed element: old triggers detach, new triggers attach, and AdaptiveTrigger no longer strongly retains the VisualElement.
Inferred motivation: Prevent stale Window.SizeChanged subscriptions from retaining old visual trees/view models after runtime VSM replacement.
Reconciliation with PR Narrative
Author claims: Fixes #36032 memory leak by detaching old triggers and attaching replacement triggers.
Agreement/disagreement: Matches the code and linked issue’s retention chain.
Prior Review Reconciliation
No prior ❌ Error findings found.
Blast Radius Assessment
- Runs for all instances: only when
VisualStateGroupschanges. - Startup impact: no.
- Static/shared state: no.
CI Status
- Required-check result: undetermined via
gh(gh auth loginrequired). Public check-runs showed at least onemaui-prleg still in progress when reviewed. - Classification: pending/undetermined.
- Action taken: confidence capped low. Gate already passed for
AdaptiveTriggerTestsand was not re-run.
Findings
No ❌ Error,
Failure-Mode Probing
- Replacing VSM groups while windowed: old triggers receive
SendDetached; new triggers attach. - Window detach/reconnect: existing
InvalidateStateTriggerspath remains intact. - Null/default window: attach skips subscription safely.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code review found no correctness issues, and targeted gate tests passed. However, CI is pending/undetermined in this unauthenticated environment, so the skill rules prevent an LGTM verdict.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Move trigger lifecycle ownership into VisualStateGroupList.VisualElement and mutation APIs. |
✅ PASS | 1 file (VisualStateManager.cs) |
Broader abstraction-level fix; passes but is more invasive than necessary. |
| 2 | try-fix | Put attach/detach on VisualStateGroup.VisualElement; list setter only propagates element reassignment. |
✅ PASS | 1 file (VisualStateManager.cs) |
Selected alternative: narrower than Candidate 1 and fewer files than PR while fixing the lifecycle boundary. |
| PR | PR #36240 | Detach old triggers in VisualStateGroupsPropertyChanged, reattach new triggers with InvalidateStateTriggers(true), and make AdaptiveTrigger hold a weak element reference. |
✅ PASSED (Gate) | 2 implementation files + tests | Original PR fix; gate already proved fail-without/pass-with. |
Iterative Expert Review and Test Loop
| # | Expert guidance | Outcome | Lesson |
|---|---|---|---|
| 1 | MAUI expert suggested moving ownership to VisualStateGroupList for association/disassociation and list mutations. |
✅ PASS | Correct but broader than the PR's replacement-path scope. |
| 2 | MAUI expert suggested moving lifecycle to VisualStateGroup.VisualElement instead of list mutation APIs. |
✅ PASS | Better balance: fixes at group/element association boundary without changing AdaptiveTrigger. |
Candidate Details
Candidate 1 — VisualStateGroupList owns trigger lifecycle
Analysis
Result: Pass
What happened: The targeted AdaptiveTriggerTests regression suite passed: 3 passed, 0 failed.
Why it worked: Replacing VisualStateGroups sets the old list's VisualElement to null, which now detaches all triggers in the old list before clearing group element references. Setting the new list's VisualElement attaches its triggers immediately when the target element already has a Window, covering the replacement-after-attach scenario without modifying AdaptiveTrigger.
Self-review: 0 findings; reviewed against MAUI lifecycle/logic/regression dimensions. Main risk is that the fix broadens lifecycle ownership into all VisualStateGroupList mutation APIs, making it more invasive than the PR's targeted replacement-path fix.
Insights: This abstraction-level approach passes the gate and handles additional group-list mutations, but it touches more surface area than the PR and is not clearly simpler.
See ../try-fix-1/content.md and ../try-fix-1/fix.diff for the full diff and captured test output.
Candidate 2 — VisualStateGroup attachment-boundary lifecycle
Analysis
Result: Pass
What happened: The targeted AdaptiveTriggerTests regression suite passed: 3 passed, 0 failed.
Why it worked: VisualStateGroupsPropertyChanged already clears the old list's VisualElement and assigns the new list's VisualElement. Propagating that assignment to each VisualStateGroup, and making each group detach/attach its own triggers at that boundary, removes the old trigger's Window.SizeChanged subscription and attaches replacement triggers for already-windowed elements.
Self-review: 0 findings; reviewed against MAUI lifecycle/logic/regression dimensions. The main watchpoint is trigger additions after group attachment, which this candidate intentionally does not broaden into because it matches the PR's replacement-path scope.
Insights: This candidate passes the gate while changing only VisualStateManager.cs and avoiding the PR's AdaptiveTrigger weak-reference safety-net change. It is a plausible better alternative because it fixes lifecycle at the existing group/element association boundary with fewer files than the PR.
See ../try-fix-2/content.md and ../try-fix-2/fix.diff for the full diff and captured test output.
Selection
Exhausted: No — stopped because Candidate 2 passed all targeted regression tests and is demonstrably preferable to both Candidate 1 and the PR fix for this review criterion.
Selected Fix: Candidate #2 — It changes only VisualStateManager.cs, avoids adding a weak-reference safety net to AdaptiveTrigger, and places trigger attach/detach at the VisualStateGroup.VisualElement lifecycle boundary that is already exercised by VisualStateGroupsPropertyChanged during replacement. Compared with the PR fix, it uses fewer implementation files and addresses the root lifecycle association rather than special-casing replacement in the attached-property callback.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current title still has a [WIP] prefix and the description omits the AdaptiveTrigger weak-reference safety-net change.
Recommended title
[Controls] VisualStateManager: Detach AdaptiveTriggers when replacing VisualStateGroups
Recommended description
### Issue Details
AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit.
### Root Cause
VisualStateGroupsPropertyChanged in VisualStateManager.cs discards the old VisualStateGroups without calling SendDetached() on its triggers. The old AdaptiveTrigger stays subscribed to Window.SizeChanged, holding a strong reference to the VisualElement and its BindingContext forever.
### Description of Change
In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached.
After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change.
AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed.
Validated the behavior in the following platforms
- [x] Android
- [x] Windows
- [x] iOS
- [x] Mac
### Issues Fixed
Fixes #36032
### Output ScreenShot
|Before|After|
|--|--|
| <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> |
🏁 Report — Final Recommendation
Comparative Report — PR #36240
Candidates Compared
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr |
✅ PASSED | Best overall. Directly fixes the submitted replacement-after-attach leak, attaches incoming triggers, keeps behavior localized to the affected attached-property callback, and adds the AdaptiveTrigger weak-reference safety net. Expert review found no actionable issues. |
| 2 | pr-plus-reviewer |
✅ PASSED | Equivalent to pr; the expert reviewer produced no actionable feedback, so there is no modified sandbox diff to prefer over the raw PR. |
| 3 | try-fix-2 |
✅ PASS | Strong alternative that moves attach/detach to the VisualStateGroup.VisualElement lifecycle boundary and avoids changing AdaptiveTrigger. It is elegant, but it shifts lifecycle behavior into a broader internal setter and removes the PR's weak-reference safety net, so it is not clearly safer than the submitted fix. |
| 4 | try-fix-1 |
✅ PASS | Correct but more invasive. It changes VisualStateGroupList collection mutation behavior in addition to replacement, which increases blast radius beyond the issue's proven failing path. |
Decision
Winning candidate: pr
All candidates with available test evidence passed the targeted regression suite, so no failed candidate outranks a passing one. Between the passing options, the PR fix is the best merge candidate because it is already present in the PR, is narrowly targeted to VisualStateGroupsPropertyChanged, retains existing window lifecycle behavior, and includes a weak-reference safety net in AdaptiveTrigger for any missed detach path. try-fix-2 is a plausible architectural alternative, but the expert review did not find a defect that justifies replacing the submitted PR with a different lifecycle model.
Notes
pr-plus-reviewerdid not produce a distinct candidate becauseinline-findings.jsonis[].- Required GitHub CI and review surfaces were unavailable due missing
ghauthentication in this environment. The gate result supplied for this review was used as the regression evidence and was not rerun.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
There was a problem hiding this comment.
Pull request overview
Fixes a memory-leak scenario where AdaptiveTrigger instances can remain subscribed to Window.SizeChanged (and retain view models) when VisualStateManager.VisualStateGroups is replaced on an already-attached VisualElement.
Changes:
- Detach state triggers from the old visual state groups during
VisualStateGroupsPropertyChangedto ensure event unsubscription occurs before the old VSM associations are cleared. - When visual state groups are replaced while the element is already in a
Window, immediately attach triggers for the new groups viaInvalidateStateTriggers(true). - Add a regression unit test verifying that the old trigger detaches and the new trigger attaches when VSGs are replaced after attach.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/tests/Core.UnitTests/AdaptiveTriggerTests.cs | Adds regression test covering detach/attach behavior when replacing VSGs after window attach. |
| src/Controls/src/Core/VisualStateManager.cs | Ensures old triggers are detached on VSG replacement and new triggers are attached immediately when already in a window. |
| src/Controls/src/Core/AdaptiveTrigger.cs | Switches the stored visual element reference to a weak reference to avoid a strong retention path if detach is missed. |
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…fter attach (#36240) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit. ### Root Cause In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. ### Description of Change In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached. After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change. AdaptiveTrigger now stores its associated VisualElement through a WeakReference<VisualElement> while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36032 ### Output ScreenShot |Before|After| |--|--| | <img width="638" height="576" alt="image" src="https://github.com/user-attachments/assets/177dd499-9351-4edb-bd46-19b9c8d7c3ea" /> | <img width="642" height="613" alt="image" src="https://github.com/user-attachments/assets/136be499-da4b-4f4c-a357-ab7a21316a69" /> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
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!
Issue Details
AdaptiveTrigger leaks memory when VisualStateGroups are replaced at runtime on a visible element. This causes entire view models (data) to stay in memory permanently even after navigation, growing by ~3 MB per page visit.
Root Cause
In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached.
Description of Change
In VisualStateGroupsPropertyChanged, detach triggers from the old VisualStateGroups before unapplying old setters and clearing the old VisualElement association. This unsubscribes old AdaptiveTrigger instances from Window.SizeChanged when VisualStateGroups are replaced after the element is already attached.
After assigning the new VisualStateGroups, if the element is already in a Window, call visualElement.InvalidateStateTriggers(true) so the new triggers attach and activate immediately instead of waiting for a future Window change.
AdaptiveTrigger now stores its associated VisualElement through a WeakReference while attached. This avoids a strong AdaptiveTrigger -> VisualElement retention path if a detach path is missed.
Validated the behavior in the following platforms
Issues Fixed
Fixes #36032
Output ScreenShot