[iOS] ScrollView: Complete element scroll requests inside collapsed branches

[albyrock87]

Description of Change

Follow-up to #37060, carrying two pieces the merged PR is missing:

1. The final review-round commit that missed the merge. #37060 was merged at e3f386a811 a few hours before its last round landed, so the two findings adopted from the final AI review (and referenced in the inline replies there) never shipped:

• A parked element scroll request has one contract: it lives exactly as long as the task the caller is awaiting. An element-mode request parked for arranged geometry (issued before the first layout, e.g. from OnAppearing) leaves the park in exactly two ways and no other: the arrange arrives and the request is replayed against real geometry (the task completes with the scroll), or the view's lifecycle ends — its handler goes away or it is removed from the tree — and the request is dropped (the task completes without a scroll). Nothing releases the task while a request is still parked, so a completed await can never scroll later; and nothing but a lifecycle end drops the request, so a view that is merely hidden (a collapsed branch, an unselected Shell tab) still scrolls to the element when it is eventually shown and arranged. A view that stays attached and is never arranged keeps the task pending — that is the contract, not a leak: the task completes when the scroll happens or the view is torn down. Removal from the tree is the newly-covered lifecycle end: LayoutHandler.Remove detaches only the platform view and does not disconnect the child's handler, so without it a removed ScrollView held a parked request nobody would ever retry.
• A dropped request completes its task inline, at the moment it is dropped, binding the release to that request by construction — there is no window in which a newer ScrollToAsync could swap the completion source and be released in its place. This is the convention already in place (the shipped handler-detach drop and Core's DisconnectHandler both complete inline from their lifecycle hooks); the completion source construction is unchanged from before this PR, so ordinary await ScrollToAsync(...) timing is untouched on every platform.
• The geometry gate requires arranged content. Enumerating the deferral state machine for coverage surfaced a latent hole: with Content set to null while an element request was parked, the gate was trivially satisfied and the ScrollView's own layout dispatched a target computed against no content. With no content there is nothing to resolve the element against, so the request now waits for content or a lifecycle end.
• SafeAreaBakedIntoContent is captured at arrange time, alongside the arranged rect, instead of being re-derived live from UIKit state: under Automatic the arrange that bakes the padding can push the content size past the bounds, at which point UIKit turns the adjusted inset non-zero while the content still carries the padding — a live read got that window wrong. The arrange branch and the _safeArea source selection share one predicate (UIKitCompensatesForSafeArea) where live evaluation is the correct semantics.

2. Repair of the Issue36801 inset-mode expectations crossed by the landscape-notch revert. The fixtures merged with #37060 asserted SafeAreaEdges.Default resolves to Never, matching the landscape-notch fix (#35533) present in inflight/current at the time. That fix has since been reverted (#36580), so Default resolves to Automatic again and the shipped resolved-mode sentinel currently fails the suite on this base. This restores the Automatic expectations in the shared tests and the HostApp oracle — the sentinel failing loudly on both transitions is it doing exactly what it was designed for (never silently testing a different clamp branch).

No new public API; WillArrange and UIKitCompensatesForSafeArea are private.

Issues Fixed

Follow-up to #37060 (fixes for #36801). Review threads addressed: the collapsed-branch finding and the shared-predicate finding.

Tests

Verified on the current inflight/current base on the iOS simulator:

• ScrollViewUnitTests: 34/34 — including tests pinning each edge of the contract: a hidden view waits and scrolls once shown; a view leaving the tree drops the request, completes the task, and does not resurrect the request if re-attached; drop-then-new-request on both lifecycle ends completes only the dropped task and leaves the new one to scroll (reproduced failing on the deferred design); consecutive and doubled lifecycle ends; pre-handler parks in both modes leaving the tree; supersede while geometry-parked; content replacement; content removal
• Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent UI suites: 12/12 — note these fail on the base without the expectation repair in this PR.

🤖 Generated with Claude Code

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[MauiBot]

Expert Review — 3 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness / Regression Prevention — WillArrange() is evaluated only at the two entry points that park or re-dispatch a request (here, and DispatchPendingScrollToRequest on handler attach). It is a point-in-time snapshot: nothing re-evaluates it when visibility changes later. Concrete scenario that still hangs: ScrollToAsync(item, ScrollToPosition.End, false) is called from OnAppearing while the branch is visible and geometry is not ready, so WillArrange() returns true and the request parks; the same code path then sets container.IsVisible = false (a loading/empty-state toggle) before the first arrange. The collapsed branch is skipped by layout, so neither OnSizeAllocated nor ContentSizeChanged ever fires, the handler stays non-null (so the OnHandlerChangedCore release path at line 55 does not run), and the caller's task stays pending forever — the exact failure mode this PR set out to remove, only with the collapse happening after the park instead of before. Reparenting a ScrollView with a parked request into a collapsed branch is the same class. A re-check of the parked request when IsVisible changes (or on the propagated visibility change) would close it; as written neither added test (ElementRequestOnCollapsedScrollViewCompletesInsteadOfHanging collapses before the request, DeferredElementRequestOnCollapsedAncestorDispatchesOnAttach collapses before attach) exercises the collapse-after-park ordering, so the gap is untested as well.

[albyrock87]

Not adopted — the collapse-after-park window is real but bounded, and every fix shape for it costs more than the residual it closes. The bounds: a request parked while visible self-heals on both exits — if the branch is ever shown again, the arrange fires and the request drains correctly (the desired outcome, not a hang), and if the view goes away, the handler detach drains and completes the task. A permanent hang therefore requires the app to keep an invisible, handler-attached ScrollView alive indefinitely while awaiting its scroll. Closing that window would mean PropertyChanged subscriptions across the entire ancestor chain with re-walks on reparenting — a mutable subscription web whose own failure modes (leaks, stale handlers, re-entrancy during layout) have a larger expected cost than the marginal case it removes; a partial guard (self-only IsVisible watch) would cover a slice while conceding the class, which is worse than a clear boundary. The boundary as shipped: visibility is evaluated at park time and at every retry (now across all visual ancestors, per the sibling thread); transitions during the parked window heal on re-show or detach. Worth noting this point-in-time semantics is also what the previous round's winning candidate validated.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — the ancestor walk terminates at the first non-VisualElement in the chain (element is VisualElement visual is the loop condition), so it never sees visibility state carried by non-visual ancestors. A ScrollView hosted in a Shell tab stops the walk at ShellContent/ShellSection (BaseShellItem : NavigableElement : Element, not a VisualElement) even though BaseShellItem exposes its own IsVisible; a hidden or not-yet-realized ShellContent therefore yields WillArrange() == true, the request parks, and if that branch is never arranged the caller's task never completes. Same for any custom non-visual container in the chain. The helper's comment claims the check covers "a view anywhere inside a collapsed branch", which is only true for chains that are VisualElement all the way up — worth either handling the BaseShellItem.IsVisible hop or narrowing the comment to the guarantee actually provided. No test covers a non-VisualElement boundary in the chain.

[albyrock87]

Adopted in 7306000, taking the narrowing option for the non-visual half: the walk now traverses the whole RealParent chain and checks IsVisible on every VisualElement it crosses, skipping over non-visual links instead of stopping — so a collapsed Shell (or any visual ancestor beyond a non-visual container) is seen. A new unit test (CollapsedShellAncestorBeyondNonVisualLinksIsDetected) exercises exactly the ShellContent/ShellSection boundary and fails under the old walk. BaseShellItem.IsVisible is deliberately not consulted, per the comment now stating the precise guarantee: for a hidden tab, parking keeps the scroll correct if the tab is ever shown, while early-completing would execute a garbage scroll — and a handler detach still completes the task either way.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Complexity Reduction — the new property's summary states it is "shared by the arrange branch and SafeAreaBakedIntoContent so the two can never desynchronize", but a third occurrence of the identical predicate remains inline at line 395 (if (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == ...Never)), and that one selects the source of _safeArea — the very value SafeAreaBakedIntoContent returns. Leaving it duplicated means a future change to UIKitCompensatesForSafeArea silently diverges from the _safeArea computation it feeds, which is the desynchronization the refactor is meant to prevent. Converting line 395 to !UIKitCompensatesForSafeArea is behavior-preserving (the De Morgan negation matches exactly, as it does for the two sites already converted).

[albyrock87]

Adopted in 9f439ed — the _safeArea source selection in ValidateSafeArea now uses !UIKitCompensatesForSafeArea too, and the property doc lists all three sites. You're right that leaving it inline kept alive exactly the desynchronization the refactor exists to prevent.

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness / Regression Prevention — WillArrange() treats "currently collapsed" as "will never arrange", but a collapsed branch that is later made visible does arrange. Concrete scenario: scrollView.IsVisible = false (or a collapsed ancestor layout), caller does await ScrollToAsync(item, ScrollToPosition.End, false), then the branch is set visible. Before this change the request parked and was replayed from OnSizeAllocated/ContentSizeChanged on the first real arrange, so it landed on item. Now it is dispatched immediately against the never-arranged sentinels (Width/Height == -1 → GetScrollPositionForElement yields ~(-1,-1), clamped to the origin), so when the branch is shown the scroll position is wrong and the request is gone (_pendingScrollToRequested = null). The same decision is made at line 559 in OnScrollToRequested. The three new unit tests all cover the never-shown case only; there is no test for collapsed-then-shown, so this behavior change is untested. Consider parking and completing the caller's task (or re-parking on the IsVisible false→true transition) so the hang is fixed without losing the correct target.

[albyrock87]

Adopted in 7f625b3, with your park-and-complete shape: both sites now release the caller's task immediately when the branch is collapsed (no hang) while leaving the request parked — the first arrange after the branch is shown replays it against real geometry, so the scroll lands on the element exactly like a visible pre-arrange park. Latest-request-wins supersedes it as usual and a handler detach still drains it. The three collapsed-branch unit tests now assert the full sequence (caller released → no premature dispatch → shown-and-arranged replay landing on the element's computed position), closing the collapsed-then-shown coverage gap you noted.

One deliberate distinction, to head off a symmetry question with the drain-on-detach thread on #37060: showing a hidden view is the same living view continuing its lifecycle, so honoring the last requested target is the expected outcome — whereas replaying across a handler detach/reattach would resurrect a request from a completed lifecycle. The two behaviors are intentionally different for that reason.

[MauiBot]

Expert Review — 2 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — WillArrange() is only consulted at the moment a request is parked (OnScrollToRequested) and inside DispatchPendingScrollToRequest, which runs solely from OnHandlerChangedCore, OnSizeAllocated, and ContentSizeChanged. Nothing re-evaluates it when IsVisible changes, so the reverse ordering still hangs: ScrollToAsync(element, ...) from OnAppearing (handler attached, Width/Height still -1) parks the request with WillArrange() == true, and if the app then collapses the ScrollView or an ancestor before the first arrange (e.g. hiding content behind a loading overlay), no arrange callback ever fires, DispatchPendingScrollToRequest is never re-entered, and the awaited task never completes — the exact hang this PR fixes for the collapsed-at-request-time ordering. A retry from the IsVisible property-changed path (or completing the parked request when the branch becomes collapsed) would close the gap; the added unit tests only cover collapsed-before-request.

[albyrock87]

Adopted in a3323ee, taking your "complete the parked request when the branch becomes collapsed" shape — and reversing my earlier rebuttal, honestly: park-and-complete changed the cost. The objection was that closing this ordering meant subscription machinery to replay the request on visibility transitions; now the request stays parked regardless and all a collapse has to do is release the caller, which is a one-line re-check. Both transitions with a real hook are covered: the ScrollView's own collapse (OnIsVisibleChanged) and reparenting into a collapsed branch (OnParentSet, the case your verdict also named). Two new unit tests cover exactly the parked-while-visible → collapsed-before-arrange ordering, including the later re-show replay landing on the element.

One boundary stated in the code rather than papered over: an ancestor collapsing after the park sends no signal down the tree — layout managers simply skip the collapsed child and nothing reaches descendants (I checked; MAUI has no downward visibility propagation) — so that request is released on re-show or handler detach. Closing it would need per-ancestor PropertyChanged subscriptions with re-walks on every reparent, whose lifecycle failure modes cost more than the ordering they'd cover.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Safe Area and Window Insets — The extracted predicate is behavior-preserving (!(SystemAdjustedContentInset == Zero || Behavior == Never) is exactly the negation of both original inline conditions), but the doc claim that sharing it means the sites "can never desynchronize" and that SafeAreaBakedIntoContent "always describes what the last arrange actually did" is not what the change guarantees. Both CrossPlatformArrange and SafeAreaBakedIntoContent evaluate this property live against mutable UIKit state, and under Automatic UIKit turns AdjustedContentInset non-zero once ContentSize exceeds Bounds — i.e. as a result of the very arrange that just baked the inset. A later read of SafeAreaBakedIntoContent then returns Empty while the content coordinate space still carries the baked padding, which is the mismatch element-target resolution (#36801) depends on. Sharing a live expression removes textual divergence only; capturing the value the arrange actually used (as _arrangedContentRect already does for the rect) would deliver the stated invariant.

[albyrock87]

Adopted in 7e4f1bf — you're right that sharing a live expression only removes textual divergence, and the Automatic transition you describe (the arrange that bakes the padding pushes ContentSize past Bounds, UIKit flips the inset non-zero, a later live read says Empty while the content still carries the padding) is exactly the window that mattered. CrossPlatformArrange now records the baked padding from the branch it actually took, alongside _arrangedContentRect which already did the same for the rect, and SafeAreaBakedIntoContent returns that captured value — so the doc claim is now literally what the code does. UIKitCompensatesForSafeArea remains shared between the arrange branch and the _safeArea source selection, where the live evaluation is the correct semantics.

[MauiBot]

Expert Review — 4 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Async and Threading Safety — SendScrollFinished() completes _scrollCompletionSource (created in CheckTaskCompletionSource without TaskCreationOptions.RunContinuationsAsynchronously, line 573), so the awaiting caller's continuation now runs inline on the mutation stack that triggered the release. Concrete scenario: await scrollView.ScrollToAsync(item, ScrollToPosition.Center, false) is parked while visible, then parentLayout.Children.Add(scrollView) under a collapsed parent (the exact sequence in ElementRequestParkedWhileVisibleIsReleasedWhenReparentedIntoCollapsedBranch) — the user's await continuation resumes inside OnParentSet / the child-add, before the parent finishes wiring the child. Same for the IsVisible = false setter path via OnIsVisibleChanged (line 145). If that continuation mutates the tree (navigates, adds/removes children, toggles IsVisible again) it re-enters a half-completed parenting/property-change operation. The pre-existing SendScrollFinished call sites are platform scroll callbacks and handler-change, so this widens the re-entrancy surface to ordinary tree mutation. Consider creating the TCS with RunContinuationsAsynchronously, or releasing via Dispatcher.Dispatch(SendScrollFinished) from ReleaseParkedRequestIfCollapsed.

[albyrock87]

Adopted in fd0c039 with your first suggestion: the completion source is created with TaskCreationOptions.RunContinuationsAsynchronously. That's the stronger of the two options because it's a construction guarantee rather than a call-site discipline — it protects every existing SendScrollFinished site (platform scroll callbacks, handler change) as well as the new tree-removal drop, and it holds regardless of whether a dispatcher is present. The task itself still transitions to completed synchronously, so nothing polling IsCompleted changes. Test: ScrollCompletionNeverResumesTheCallerInlineOnTheMutationStack attaches an ExecuteSynchronously continuation before the removal and asserts it lands on a different thread than the mutating one — verified to fail without the flag.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — WillArrange() uses "no collapsed VisualElement ancestor" as a proxy for "an arrange callback is coming", but the two are not equivalent, so the hang this PR fixes survives in adjacent states that the PR's own hooks nearly reach:

1. Reparent to null. OnParentSet (line 156) fires when the ScrollView is removed from the tree, but WillArrange() then walks this → RealParent == null with IsVisible == true and returns true, so ReleaseParkedRequestIfCollapsed does nothing. A ScrollView removed from its parent does not automatically null its Handler, so OnHandlerChangedCore's release (line 55) does not fire either and the parked request's task stays pending forever — the original bug, one hook away from being covered.
2. Unselected Shell tab. The comment at lines 120–123 states this is deliberate, but the consequence is the reported failure mode: for a page in a non-current ShellSection whose view controller's view is never loaded, no arrange occurs while Shell.IsVisible is true, so WillArrange() returns true and await ScrollToAsync(element, …) never completes.

No test covers either case — the five new unit tests all express "no arrange coming" as IsVisible = false on a VisualElement, which is precisely the subset the guard can detect, so a green suite cannot discriminate the guard's premise from the actual condition. Either widen the release condition (e.g. also release when RealParent is null while a handler is attached) or state in the PR that the remaining hang cases are out of scope.

[albyrock87]

Both cases resolved in fd0c039, by removing the proxy rather than widening it. (1) Reparent-to-null is real — I verified LayoutHandler.Remove detaches only the platform view and leaves the child's handler connected, so a removed ScrollView held a parked request nobody would ever retry. Removal from the tree is now treated as what it is: a lifecycle end (OnParentSet with a null parent drops the request and completes the task), the same category as handler detach — not a visibility proxy. Test: ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree, which also asserts the dropped request is not resurrected if the view is re-attached and arranged later. (2) The unselected Shell tab is covered by the contract instead of a special case: an attached, never-arranged view keeps its task pending until the arrange happens or the view is torn down — which for a tab means the scroll lands correctly on the element the moment the tab is shown, strictly better than releasing early with a lost target. WillArrange() and its IsVisible walk no longer exist, so there is no premise left for a green suite to fail to discriminate.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency — Releasing the caller while leaving the request parked means await ScrollToAsync(element, …) can report completion and then perform the scroll an arbitrary amount of time later, when the branch is next shown. Concrete scenario: a collapsed detail panel whose ScrollToAsync(item, Center) is awaited during OnAppearing; the user expands the panel minutes later and the ScrollView jumps to a target chosen for stale content. Only a subsequent ScrollToAsync clears the park (_pendingScrollToRequested = null, line 621) — the caller that was already released has no way to cancel it, and ScrollX/ScrollY have no public setter to override it. Worth confirming this deferred replay is the intended contract versus dropping the parked request at release time (which would make Task completion mean "this request is over" on every path).

[albyrock87]

Adopted in fd0c039 — and this finding is the one that resolved the whole series, so thank you for it. You're right that releasing the task while leaving the request parked meant a completed await could scroll arbitrarily later with no way to cancel it; that is a ghost scroll, and it's the exact hazard I'd argued against on the detach thread in #37060. The park-and-complete design was internally contradictory and every prior round was one end of that contradiction being pulled.

Replaced with a single invariant, now stated in the code and the PR description: a parked element request lives exactly as long as its task. It leaves the park in exactly two ways and no other — the arrange arrives and it's replayed against real geometry (task completes with the scroll), or the view's lifecycle ends (handler goes away / removed from the tree) and it's dropped (task completes without a scroll). Nothing releases the task while a request is parked, so completion means "this request is over" on every path; nothing but a lifecycle end drops it, so a merely hidden view still scrolls to the element once shown. The visibility-proxy machinery (WillArrange, OnIsVisibleChanged, the collapse releases) is gone entirely — every proxy for "no arrange is coming" leaks, and each leak was a hang.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Complexity Reduction — This edit adds a UTF-8 BOM and re-sorts every pre-existing line in addition to inserting override Microsoft.Maui.Controls.ScrollView.OnParentSet() -> void. The functional delta is one line; the whole-file rewrite guarantees a textual merge conflict for every other in-flight PR touching this file (which is essentially every PR adding public API), and the sort is not even carried through — Microsoft.Maui.Controls.ShellContent.~ShellContent() -> void remains at the tail. Reducing this to the single added entry would avoid the conflict surface.

[albyrock87]

Fixed in fd0c039 — you were right, and it was worse than described: besides the BOM and the re-sort, the net-ios file had three upstream entries duplicated. Both files are now the current base plus exactly the one added OnParentSet entry each (a formatter had been allowed to touch them; it no longer is).

[MauiBot]

Expert Review — 2 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Public API Surface / Cross-Platform Consistency — RunContinuationsAsynchronously is applied to the completion source used by every ScrollToAsync call, not just the new lifecycle-drop path. Previously, when the platform raised SendScrollFinished() on the UI thread, an await ScrollToAsync(...) continuation resumed inline (the awaiter inlines when the captured SynchronizationContext matches the completing one). With this flag the continuation is always re-posted to the dispatcher, so user code after await ScrollToAsync(...) now runs one dispatcher turn later and is reordered behind any work already queued on the UI thread (e.g. a Dispatcher.Dispatch posted before the scroll completed, or the layout pass triggered by the scroll itself). Chained sequences like await ScrollToAsync(...); await AnimateAsync(...) observe different interleaving than before on all platforms.

The requirement that motivates this change is narrow — only DropPendingScrollToRequest() completes the task from inside a lifecycle mutation. Scoping the asynchrony to that path (e.g. posting the drop's SendScrollFinished() via Dispatcher.Dispatch, or having only the drop path use a completion source created with the flag) would keep the reentrancy guarantee without changing completion semantics for the normal platform-callback path.

[albyrock87]

Adopted in c56e736 — you're right, and I'd overstated the flag as "strictly stronger": I reasoned only about re-entrancy safety and missed the observable timing change on the normal platform-callback path (continuations re-posted a turn later, reordering user code behind queued UI work, on every platform). The requirement is narrow, so the mechanism is now scoped to it: DropPendingScrollToRequest posts SendScrollFinished through the dispatcher, and the completion source construction is back to its pre-PR form verbatim — ordinary await ScrollToAsync(...) continuations keep their existing timing.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Regression Prevention / Async and Threading — This assertion tests thread identity, not the property under test ("the continuation did not run inline on the mutation stack"). The continuation is queued to the thread pool by parent.Children.Remove(...); the test thread then blocks on await continuation.WaitAsync(...). If the pool is saturated (parallel test collections) the queued work item can still be pending when the test thread yields at the await, and — when the test itself runs on a pool thread — that same thread can dequeue and execute the continuation, making continuationThread == mutatingThread and failing a correct implementation. A deterministic formulation (e.g. setting a mutating = true flag around the Remove call and asserting the continuation observed mutating == false, or asserting continuationThread was still unset immediately after Remove returned via a captured-on-completion snapshot) proves "not inline" without depending on scheduler timing.

[albyrock87]

Adopted in c56e736, with one note on the suggested shape. A mutating flag observed by the continuation is the formulation I had tried first, and it flaked 5/6 under load: an off-stack continuation can run concurrently with the mutation on a pool thread and truthfully read the flag as still set — so it's not deterministic either. Both formulations were fragile because they were observing a thread-pool mechanism. With the deferral now scoped to a dispatcher post (sibling thread), the property is directly and deterministically testable: the test captures dispatcher posts (DispatcherProviderStubOptions.InvokeOnMainThread, restored in finally), asserts the task is still pending after Remove returns with exactly one post queued, and that running the post completes it. No timing, no thread pool — and it's verified to fail when the drop completes inline.

[MauiBot]

Expert Review — 2 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — The deferred completion is posted as a bare method group, so it completes whatever _scrollCompletionSource exists when the dispatcher turn runs, not the one that was pending when the request was dropped. SendScrollFinished (line 238) calls _scrollCompletionSource?.TrySetResult(true), and CheckTaskCompletionSource (line 537) replaces _scrollCompletionSource with a fresh instance on every ScrollToAsync.

Concrete failing sequence, all inside one dispatcher turn:

1. ScrollToAsync(element, ...) parks the request and returns task T1.
2. The view's lifecycle ends (parent.Children.Remove(sv), or handler churn) → DropPendingScrollToRequest clears _pendingScrollToRequested and posts SendScrollFinished.
3. Still before the post runs, the view is re-attached (otherParent.Children.Add(sv) / a new handler is assigned) and the caller issues a second ScrollToAsync → CheckTaskCompletionSource swaps in T2, and the new request parks.
4. The posted SendScrollFinished runs and calls TrySetResult on T2.

Two consequences, both of which are exactly what this change set out to prevent:

• T1 is never completed. The dropped caller’s task stays pending forever — the original leak survives the fix on this interleaving.
• T2 completes while its request is still parked. The caller’s await returns with no scroll, and the parked request then replays on the next arrange and scrolls afterwards — directly contradicting the contract asserted in the class comment at lines 46–56 (“Nothing releases the task while the request is still parked, so a completed await never scrolls later”).

Bind the completion to the source captured at drop time so a superseding request cannot be released by an older drop, e.g. capture var completion = _scrollCompletionSource; before the post and dispatch () => completion?.TrySetResult(true);.

[albyrock87]

Confirmed and fixed in fa03bc0 — reproduced your exact sequence on the previous head (t1.completed=false, t2.completed=true), so this was a real bug I introduced when scoping the deferral last round. Rather than bind a captured source to the post, I removed the deferral: the drop now completes inline at the moment the request is dropped, which binds the release to that request by construction — no window, no captured identity, no interleaving to reason about. That is also the convention already shipping in this exact code: the handler-detach drop merged in #37060 and Core's DisconnectHandler both complete the task inline from their lifecycle hooks, so the deferral was protecting a property nothing here had while adding a real state hazard. The completion source construction is back to its pre-PR form. Regression tests pin the interleaving on both lifecycle ends (DropCompletesTheDroppedTaskAndOnlyThatTask, HandlerDetachDropCompletesOnlyTheDroppedTaskAndDoesNotResurrect), plus consecutive and doubled ends.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — This test only exercises the benign ordering: drop → post → nothing else happens → run the post. Because posted is captured and replayed manually, the test has full control of the window between the drop and the completion, yet never exercises the dangerous case in that window — a new ScrollToAsync arriving before completion() is invoked.

That is the one interleaving where deferring the completion changes which task gets released (see the finding on ScrollView.cs:102): CheckTaskCompletionSource swaps _scrollCompletionSource, so the post lands on the new caller’s task while the old one is orphaned. The current assertions (Assert.False(task.IsCompleted), Assert.Single(posted), Assert.True(task.IsCompleted)) all pass either way, so they cannot discriminate correct from incorrect behavior here.

Add a case that, between parent.Children.Remove(scrollView) and completion(), re-attaches the view and issues a second ScrollToAsync, then asserts that the first task completes and the second stays pending until its own scroll or lifecycle end.

[albyrock87]

Adopted in fa03bc0 and taken further. The interleaving you asked for is pinned (DropCompletesTheDroppedTaskAndOnlyThatTask: drop → re-attach → new ScrollToAsync → arrange ⇒ T1 completed at the drop, T2 untouched, then replayed to the correct target and completed only by its own scroll). Since the previous test could only pass either way, I also enumerated the deferral state machine (initial disposition × lifecycle events) instead of testing the one path a finding names — nine new tests, and the enumeration surfaced a real latent hole in the geometry gate: with Content = null while an element request was parked, Content is not ({Width:<0} or {Height:<0}) was trivially true for null, so the ScrollView's own layout dispatched a target computed against no content. The gate now requires arranged content and the request waits for content or a lifecycle end (ParkedElementRequestWithContentRemovedWaitsThenCompletesOnLifecycleEnd).

[MauiBot]

AI Review Summary

@albyrock87 — new AI review results are available based on commit c56e736.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: IOS · Base: inflight/current · Merge base: 2a5dfca5

✅ Fix verified — 1 test(s) reproduce the bug (FAIL without the fix → PASS with it). 1 test(s) pass in both states and are not bug-reproducing; under the "at least one test reproduces the bug and none regress" rule they don't block the gate.

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 ScrollViewUnitTests ScrollViewUnitTests	✅ FAIL — 59s	✅ PASS — 23s
🖥️ Issue36801 Issue36801	❌ PASS — 870s	✅ PASS — 198s

🔴 Without fix — 🧪 ScrollViewUnitTests: FAIL ✅ · 59s

Error-relevant lines (filtered from the build log):

     at Microsoft.Maui.Controls.Core.UnitTests.ScrollViewUnitTests.DroppedRequestCompletesThroughTheDispatcherNotInlineOnTheMutationStack() in /_/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs:line 582
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
     at Microsoft.Maui.Controls.Core.UnitTests.ScrollViewUnitTests.ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() in /_/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs:line 545

🟢 With fix — 🧪 ScrollViewUnitTests: PASS ✅ · 23s

(no coded error found; showing last 1200 chars)

crollTo [< 1 ms]
  Passed ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree [< 1 ms]
  Passed TestScrollToElement [1 ms]
  Passed TestChildChanged [< 1 ms]
  Passed InsetRefreshUpdatesOffsetsWithoutRaisingScrolled [< 1 ms]
  Passed DeferredRequestReplaysEventForSubscribersAttachedWithTheHandler [< 1 ms]
  Passed TestScrollToElementNotAnimated [< 1 ms]
  Passed DeferredElementScrollDispatchesOnceContentIsArranged [< 1 ms]
  Passed SetScrollPosition [< 1 ms]
  Passed TestConstructor [1 ms]
  Passed ElementRequestOnHiddenScrollViewWaitsAndScrollsOnceShown [< 1 ms]
[xUnit.net 00:00:00.82]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed TestBackToBackBiDirectionalScroll [< 1 ms]
  Passed DeferredElementScrollCompletesWhenTheHandlerGoesAway [4 ms]
  Passed DeferredElementScrollCompletesWhenContentArrangesToZero [< 1 ms]
  Passed TestScrollWasNotFiredOnNeither [< 1 ms]
  Passed DirectRequestSupersedesDeferredElementRequest [< 1 ms]
  Passed TestScrollToNotAnimated [< 1 ms]
  Passed TestChildDoubleSet [< 1 ms]
  Passed ElementTargetsAccountForViewportAndContentCoordinateInsets [< 1 ms]
Test Run Successful.
Total tests: 26
     Passed: 26
 Total time: 1.1240 Seconds

🔴 Without fix — 🖥️ Issue36801: PASS ❌ · 870s

(no coded error found; showing last 1200 chars)

setMode Start
>>>>> 8/16/2026 9:39:05 AM ScrollToExtremesInEachInsetMode Stop
  Passed ScrollToExtremesInEachInsetMode("ModeContainerButton","Always") [6 s]
>>>>> 8/16/2026 9:39:05 AM ScrollToAsyncReachesInsetAwareExtremes Start
>>>>> 8/16/2026 9:39:13 AM ScrollToAsyncReachesInsetAwareExtremes Stop
  Passed ScrollToAsyncReachesInsetAwareExtremes [7 s]
>>>>> 8/16/2026 9:39:13 AM ScrollToElementEndLandsInsideVisibleViewport Start
>>>>> 8/16/2026 9:39:17 AM ScrollToElementEndLandsInsideVisibleViewport Stop
  Passed ScrollToElementEndLandsInsideVisibleViewport [4 s]
>>>>> 8/16/2026 9:39:23 AM FixtureSetup for Issue36801DeferredElement(iOS)
>>>>> 8/16/2026 9:39:31 AM DeferredElementScrollLandsInsideVisibleViewport Start
>>>>> 8/16/2026 9:39:32 AM DeferredElementScrollLandsInsideVisibleViewport Stop
  Passed DeferredElementScrollLandsInsideVisibleViewport [1 s]
NUnit Adapter 4.5.0.0: Test execution complete
Results File: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36801.trx
Test Run Successful.
Total tests: 11
     Passed: 11
 Total time: 3.6917 Minutes
>>> TRX_RESULT_FILE: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36801.trx

🟢 With fix — 🖥️ Issue36801: PASS ✅ · 198s

(no coded error found; showing last 1200 chars)

setMode Start
>>>>> 8/16/2026 9:49:24 AM ScrollToExtremesInEachInsetMode Stop
  Passed ScrollToExtremesInEachInsetMode("ModeContainerButton","Always") [7 s]
>>>>> 8/16/2026 9:49:24 AM ScrollToAsyncReachesInsetAwareExtremes Start
>>>>> 8/16/2026 9:49:32 AM ScrollToAsyncReachesInsetAwareExtremes Stop
  Passed ScrollToAsyncReachesInsetAwareExtremes [8 s]
>>>>> 8/16/2026 9:49:32 AM ScrollToElementEndLandsInsideVisibleViewport Start
>>>>> 8/16/2026 9:49:37 AM ScrollToElementEndLandsInsideVisibleViewport Stop
  Passed ScrollToElementEndLandsInsideVisibleViewport [4 s]
>>>>> 8/16/2026 9:49:43 AM FixtureSetup for Issue36801DeferredElement(iOS)
>>>>> 8/16/2026 9:49:51 AM DeferredElementScrollLandsInsideVisibleViewport Start
>>>>> 8/16/2026 9:49:52 AM DeferredElementScrollLandsInsideVisibleViewport Stop
  Passed DeferredElementScrollLandsInsideVisibleViewport [1 s]
NUnit Adapter 4.5.0.0: Test execution complete
Results File: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36801.trx
Test Run Successful.
Total tests: 11
     Passed: 11
 Total time: 1.7646 Minutes
>>> TRX_RESULT_FILE: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36801.trx

⚠️ Failure Details

• ❌ Issue36801 PASSED without fix (should fail) — tests don't catch the bug

📁 Fix files reverted (2 files)

• src/Controls/src/Core/ScrollView/ScrollView.cs
• src/Core/src/Platform/iOS/MauiScrollView.cs

---

📋 Pre-Flight — Context & Validation

PR #37409 Pre-Flight

Pull Request

• Title: [iOS] ScrollView: Complete element scroll requests inside collapsed branches
• Base / head: inflight/current (2a5dfca5) -> c56e736b
• Issue context: Follow-up to #37060 / #36801. On iOS, programmatic ScrollView offsets must account for AdjustedContentInset; this follow-up also defines the lifetime of element requests parked before usable arranged geometry exists.
• Gate: Passed previously. Do not rerun the gate and do not modify gate/content.md.

Current PR Approach

The PR changes five files:

• src/Controls/src/Core/ScrollView/ScrollView.cs
• src/Core/src/Platform/iOS/MauiScrollView.cs
• src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs
• src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs
• src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs

The production fix has two related parts:

1. ScrollView parks an element request until arranged geometry exists. A hidden-but-attached branch keeps the request pending and replays it after layout. Handler disconnection or removal from the visual tree drops the request and posts SendScrollFinished through the dispatcher, preventing an awaiting continuation from resuming inside a lifecycle mutation.
2. MauiScrollView shares UIKitCompensatesForSafeArea between safe-area validation and arrange, and captures the safe area actually baked into content at arrange time rather than re-deriving it from UIKit state later.

The test-only expectation repair changes SafeAreaEdges.Default back to native Automatic, matching the current base after #35533 was reverted by #36580.

Required Alternative-Fix Scope

Each candidate must propose one root-cause-level alternative to the PR implementation, not a cosmetic relocation of the same checks. Preserve the behavioral contract proved by the existing PR tests:

• A hidden attached ScrollView keeps a parked element request pending and scrolls once shown and arranged.
• Removing the ScrollView from the tree drops the parked request, completes the caller asynchronously through the dispatcher, and never resurrects the stale request after reattachment.
• Handler teardown also releases a parked request.
• The iOS inset-aware ScrollView behavior and current Automatic expectations remain intact.

Do not alter the PR tests merely to make an alternative pass. Do not add public API.

Bounded Test Contract

Run only these tests, in this order:

1. Primary reproducer

   dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~ScrollViewUnitTests"

2. Mandatory iOS regression (only after the primary test succeeds)

   pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform ios -TestFilter "Issue36801"

The prior gate observed ScrollViewUnitTests fail without the production fix (specifically the removal and dispatcher-completion tests) and pass with it; Issue36801 passed in both states and is regression coverage. Do not rerun fail-without-fix gate verification.

Each candidate gets one implementation/test pass and at most one focused correction/retest. A device or environment failure is Blocked, not Pass.

Repository Safety

The branch contains unrelated pre-existing working-tree changes under .github/, eng/, and skill infrastructure. They are not part of PR #37409. Preserve them exactly; do not restore, reset, stage, or include them in candidate diffs. The two production fix files were clean at pre-flight. Use only .github/scripts/EstablishBrokenBaseline.ps1 and its -Restore mode for baseline transitions as required by the try-fix skill.

Applicable repository guidance includes public-api.instructions.md (no API change), threading-async.instructions.md, and safe-area-ios.instructions.md.

---

🔬 Code Review — Deep Analysis

Expert Code Review — PR #37409

Verdict: NEEDS_CHANGES
Confidence: high for the correctness finding; medium overall because this is platform-specific handler/UI lifecycle plumbing.

Independent assessment

The submitted fix gives parked element-mode ScrollToAsync requests two terminal paths: replay after arrange, or cancellation-like completion when the handler or logical parent goes away. It defers lifecycle-drop completion through the dispatcher to prevent continuations from re-entering a parenting or handler mutation. Separately, the iOS native scroll view captures baked safe-area state at arrange time and shares one predicate for UIKit compensation.

The safe-area changes are internally consistent: the extracted predicate is logically equivalent to the old branches, and capturing the baked inset alongside arranged geometry prevents later UIKit state changes from misreporting what was included in that arrange. The lifecycle work has one blocking race.

Findings

❌ Error — Deferred drop can complete the wrong request

At src/Controls/src/Core/ScrollView/ScrollView.cs:102, Dispatcher.Dispatch(SendScrollFinished) resolves _scrollCompletionSource only when the dispatcher callback runs. A new ScrollToAsync can replace that field after an old parked request is dropped but before the callback runs. The callback then completes the new task, leaves the dropped task pending forever, and permits the new parked request to scroll after its task has already completed.

Capture the completion source being released before dispatching and complete that captured source inside the callback.

⚠️ Warning — Deferral test misses the superseding-request interleaving

At src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs:585, the test controls the dispatcher window but only runs the callback without issuing another request. It therefore passes whether the callback is bound to the dropped request or incorrectly resolves the mutable field. Reattach the view and issue a second request before running the captured callback; assert that only the first task completes.

Blast radius and failure-mode probes

• The change is per-ScrollView instance and iOS-specific for native inset bookkeeping; it adds no static/shared state and does not affect application startup.
• Handler disconnect and logical-parent removal both reach the new drop path. Non-null-to-non-null reparenting does not drop a request; remove-and-readd does, intentionally.
• The dispatcher race is concrete: drop T1, reattach, park T2, then run the queued completion. Current code completes T2 and strands T1.
• _bakedSafeArea remains per-platform-view state and is consumed with the arranged rectangle, so handler instances do not share stale safe-area state.

Prior-review reconciliation

The Issue36801 expectation change matches the current SafeAreaEdges.Default resolution to Automatic; it is not an unrelated behavioral regression. No unresolved prior error was found that supersedes the blocking race above.

Consolidated reviewer improvement

Bind deferred completion to the captured task source and strengthen the existing dispatcher test with a superseding request inside the deferred window. No other source change is recommended.

---

🛠️ Try-Fix — Analysis & Comparison

Try-Fix Aggregate — PR #37409

Bounded STEP 5a: at most two independent candidates. The gate was not rerun.

Candidate 1 — Completion-Source-Owned Park

Model: claude-opus-5
Result: Pass
Full record: ../try-fix-1/content.md
Attempt artifacts: attempt-1/

Approach

Bind each parked element request to the TaskCompletionSource created by the ScrollToAsync call that parked it. Losing the handler or parent schedules one dispatcher review rather than synchronously dropping the request. On the next turn, the review keeps a request whose view has been reattached; otherwise it clears the request and completes only its recorded owner.

This differs from the PR's transition-based lifetime model. The PR treats detachment as an irreversible lifecycle end, clears the park immediately, and later calls SendScrollFinished. Candidate 1 treats detachment as potentially transient and defers the drop decision itself. It therefore preserves a request across same-turn reparenting/handler recreation and avoids completing a newer caller if another ScrollToAsync occurs before the posted release runs.

Changed Code

• src/Controls/src/Core/ScrollView/ScrollView.cs (+59/-27 relative to PR head)
• No test or public API changes
• src/Core/src/Platform/iOS/MauiScrollView.cs intentionally retained the PR's orthogonal arrange-time safe-area capture

The candidate adds _parkedRequestOwner and _parkedRequestReviewQueued, replaces DropPendingScrollToRequest with a deferred ReleaseParkedRequestIfUnserviceable, validates request ownership before replay, records ownership whenever a request is parked, and clears ownership when a request is sent or superseded. The complete tested diff is preserved in try-fix-1/content.md and attempt-1/fix.diff.

Validation

Command	Result
dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~ScrollViewUnitTests"	Pass — 26/26
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform ios -TestFilter "Issue36801"	Pass

The existing PR tests were not modified. Hidden attached content remained pending until arrange; removal posted exactly one asynchronous release; reattachment did not resurrect a released request; and handler teardown released its caller.

Failure / Blocker Analysis

EstablishBrokenBaseline.ps1 refused to create state because unrelated pre-existing tracked changes exist under .github/ and eng/, and those changes could not be reset or stashed safely. No baseline state was created. The candidate was therefore applied as an in-place replacement relative to clean PR-head ScrollView.cs, and both bounded commands tested exactly that candidate diff. The edit was then reverted to PR head; all unrelated working-tree changes were preserved.

Inline Expert Self-Review

Two findings: 0 critical, 0 major, 1 moderate, 1 minor.

• Moderate: serviceability requires RealParent != null, so a never-parented ScrollView can be released after attachment loss; the PR has the same exposure.
• Minor: an undelivered dispatcher post leaves the de-duplication flag latched and the caller pending; the PR's posted completion has the same dependency.

One focused pre-test correction removed a completed-task clause that could have allowed a stray platform completion to swallow a live park. No test-loop correction was needed.

Candidate Diff

See the complete fenced diff in try-fix-1/content.md lines 61-239 or attempt-1/fix.diff; it is the authoritative PR-head-to-candidate patch.

Candidate 2 — Lifecycle-Epoch Tombstone

Model: gpt-5.6-sol
Result: Pass
Full record: ../try-fix-2/content.md
Attempt artifacts: attempt-2/

Approach

Give each parked request the current attachment epoch. Parent removal or handler teardown atomically increments that epoch, immediately and irreversibly invalidating the old request. Replay paths reject an epoch mismatch. A dispatcher callback later clears the invalidated tombstone and completes the caller, keeping cleanup and completion outside the lifecycle mutation.

Hidden-but-attached views never change attachment epochs, so their request remains parked until arranged. Reattachment cannot revive an invalid request because the old epoch cannot equal the incremented epoch.

This differs from both earlier strategies:

• Unlike the PR, lifecycle hooks do not synchronously clear the parked request; they create a monotonic logical tombstone, with physical cleanup and completion deferred together.
• Unlike candidate 1, there is no completion-source ownership, attachment revalidation, same-turn reparent survival, or queued-review/de-duplication state. Detachment is immediately final.

Changed Code

• src/Controls/src/Core/ScrollView/ScrollView.cs (+38/-33 relative to PR head)
• No test or public API changes
• src/Core/src/Platform/iOS/MauiScrollView.cs intentionally retained the PR's orthogonal arrange-time safe-area capture

The candidate adds _attachmentEpoch and _pendingScrollToRequestEpoch, advances the epoch with Interlocked.Increment on lifecycle loss, rejects stale epochs in both dispatch paths, and asynchronously releases the tombstone. The complete tested diff is preserved in try-fix-2/content.md and attempt-2/fix.diff.

Validation

Command	Result
dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~ScrollViewUnitTests"	Pass — 26/26
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform ios -TestFilter "Issue36801"	Pass

The existing tests were unchanged. Hidden attached content retained its epoch until arrange; removal/handler teardown invalidated the epoch; the posted callback completed outside mutation; and a later reattachment could not replay the tombstone.

Failure / Blocker Analysis

As in candidate 1, EstablishBrokenBaseline.ps1 refused to create state because of unrelated pre-existing dirty tracked files. Both production targets were first proven clean against PR HEAD; only ScrollView.cs was replaced for the bounded candidate run. The script's -Restore mode reported that no baseline state existed, after which the exact PR blob was restored without resetting, restoring, stashing, staging, or touching unrelated files. Both targets were verified equal to PR HEAD.

Inline Expert Self-Review

Clean: 0 findings. The review covered overarching and always-active dimensions plus async/threading safety. One focused pre-test correction added the required System.Threading import; the diff remained unchanged during the test loop.

Candidate Diff

See the complete fenced diff in try-fix-2/content.md lines 24-167 or attempt-2/fix.diff; it is the authoritative PR-head-to-candidate patch.

Aggregate Outcome

Two candidates were attempted, two passed, and no further candidates were launched. Both retained the PR's iOS safe-area capture while replacing its parked-request lifecycle strategy:

Candidate	Distinguishing strategy	Result	Self-review
1	Completion-source ownership with deferred attachment revalidation	Pass	2 non-blocking findings
2	Monotonic attachment-epoch tombstone	Pass	Clean

---

🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Winner: pr-plus-reviewer

The submitted PR has the right lifecycle and safe-area model, and the trusted Gate passed, but its deferred drop posts SendScrollFinished against a mutable _scrollCompletionSource. A newer ScrollToAsync can replace that field before the callback runs, causing the old task to remain pending and the newer task to complete before its parked scroll. The minimal reviewer refinement binds the callback to the source being dropped and adds the missing interleaving regression; both required targeted validations pass.

Comparative ranking

Rank	Candidate	Validation	Assessment
1	pr-plus-reviewer	ScrollView 26/26; Issue36801 iOS 11/11	Fixes the concrete expert-review race with a three-line production delta, directly tests the superseding-request window, and preserves the PR's intended detach-is-final contract.
2	try-fix-1	Recorded pass: ScrollView 26/26; Issue36801 iOS pass	Completion ownership is the right direction, but ownership is stored in the mutable _parkedRequestOwner and read only when the deferred review runs. A newer request can overwrite that owner; same-turn reattachment also makes the callback return without releasing the original caller. It additionally changes reparenting semantics and adds substantially more state.
3	try-fix-2	Recorded pass: ScrollView 26/26; Issue36801 iOS pass	Epoch invalidation prevents stale replay, but a newer request records the current epoch before the old callback runs. The callback then returns as though nothing is stale, leaving the original completion source unaddressed. The epoch machinery is larger than the winner and does not bind release to a caller.
4	pr	Trusted Gate passed	Correctly handles hidden branches, lifecycle drop, and arrange-time safe-area capture, but has the confirmed wrong-task completion race and no discriminating regression test.

All candidates retained the PR's iOS safe-area work and all recorded bounded tests passed; none is demoted for a recorded regression-test failure. Under the hard execution contract, the try-fix candidates were compared from their preserved diffs and prior validation records rather than rerun with the newly strengthened test. That leaves uncertainty about their runtime result on the new interleaving, but their state transitions expose the ownership gap directly.

Why pr-plus-reviewer wins

The winner addresses the exact failing interleaving without replacing the PR's lifecycle policy:

1. T1 parks and is dropped.
2. The drop captures T1's completion source and posts its release.
3. T2 parks before the post runs.
4. The post completes T1 only; T2 remains pending until its own scroll or lifecycle end.

This is both simpler and better evidenced than the alternative lifetime models. Because the winning changes are not present in the submitted PR HEAD, the recommendation is REQUEST CHANGES.

---

📱 UI Tests — ScrollView,ViewBaseTests

Detected UI test categories: ScrollView,ViewBaseTests

✅ Deep UI tests — 285 passed, 0 failed, 1 skipped across 2 categories on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category	Tests	Snapshot diffs
ScrollView	173/174 (1 skipped) ✓	—
ViewBaseTests	112/112 ✓	—
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

🧭 Next Steps — reviewer patch required (pr-plus-reviewer)

The reviewer-enhanced candidate won, so the submitted PR still needs those changes.

Why: The reviewer-refined PR preserves the submitted lifecycle and safe-area design while binding deferred completion to the task source actually being dropped. Its strengthened superseding-request regression and the required iOS regression both pass, whereas the raw PR has a confirmed wrong-task completion race.

Apply PRAgent/pr-plus-reviewer/reviewer.patch from the CopilotLogs
artifact (or follow the report's Required submitted-PR change), push the update, and run
the review again.