diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 2dd1ed0159ee..3764318aafb0 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -43,6 +43,19 @@ public Rect LayoutAreaOverride ScrollToRequestedEventArgs _pendingScrollToRequested; bool _replayPendingScrollToRequestedEvent; + // A parked element request lives exactly as long as the task the caller is awaiting. + // It leaves the park in one of two ways, and no other: the arrange arrives and the + // request is replayed against real geometry (the task completes with the scroll), + // or there is nothing left that could ever satisfy it and the request is dropped + // (the task completes without a scroll). The latter happens when the view's + // lifecycle ends — its handler goes away or it is removed from the tree — and when + // the target is orphaned from this ScrollView's content (see IsElementTargetOrphaned). + // Nothing releases the task while the request is still parked, so a completed await + // never scrolls later; and nothing but one of those terminal conditions drops the + // request, so a view that is merely hidden (a collapsed branch, an unselected tab) + // still scrolls to the element when it is eventually 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 nothing can make it happen. private protected override void OnHandlerChangedCore() { base.OnHandlerChangedCore(); @@ -50,20 +63,55 @@ private protected override void OnHandlerChangedCore() if (Handler is null) { // The handler went away with a request still queued, so nothing will ever - // dispatch it. Release the caller rather than leaving its task pending - // forever; Core does the same for its own pending request on disconnect. - if (_pendingScrollToRequested is not null) - { - _pendingScrollToRequested = null; - SendScrollFinished(); - } - + // dispatch it; Core does the same for its own pending request on disconnect + DropPendingScrollToRequest(); return; } DispatchPendingScrollToRequest(); } + private protected override void OnParentChangedCore() + { + base.OnParentChangedCore(); + + // Removed from the tree — the handler is not necessarily disconnected by the + // removal, so this is the other lifecycle end that drops a parked request. This + // includes the transient removal of a reparent: a request made against a tree + // position that no longer exists is cancelled and its task completes at the + // removal, and it is not carried over to the new parent (its arrange does not + // replay it). The drop is deliberately not deferred to "see whether a re-add + // follows": that would put a window between the drop and the completion, in + // which a newer request could be released in the old one's place. A caller that + // moves a ScrollView with a pending element scroll re-requests it in the new + // location. + if (RealParent is null) + { + DropPendingScrollToRequest(); + } + } + + void DropPendingScrollToRequest() + { + if (_pendingScrollToRequested is null) + { + return; + } + + _pendingScrollToRequested = null; + // A stale replay flag from a pre-handler park must not carry over to a later request + _replayPendingScrollToRequestedEvent = false; + + // Complete inline, at the moment the request is dropped. Deferring the completion + // would open a window in which a newer ScrollToAsync could swap the completion + // source, so a deferred completion would release the wrong task and orphan this + // one. Completing here binds the release to the request being dropped by + // construction. This is also the convention already in place: the handler-detach + // drop and Core's own DisconnectHandler both complete the task inline from their + // lifecycle hooks. + SendScrollFinished(); + } + void DispatchPendingScrollToRequest() { if (Handler is null || _pendingScrollToRequested is not { } pending) @@ -73,17 +121,27 @@ void DispatchPendingScrollToRequest() if (pending.Mode == ScrollToMode.Element) { - if (!IsElementTargetGeometryReady()) + if (IsElementTargetOrphaned(pending.Element)) + { + // The target no longer hangs off this ScrollView's content (the content + // was replaced or removed under a parked request), so no arrange can ever + // give it a position: nothing is left to wait for. Terminal, like a + // lifecycle end. + DropPendingScrollToRequest(); + return; + } + + if (!IsElementTargetGeometryReady(pending.Element)) { // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. return; } - // Those callbacks run while the pass that produced the sizes is still arranging - // children, so resolve on the next tick, once positions are final. Posting on - // every retry is deliberate: SendPendingScrollToRequest is a no-op once the - // request has been sent or superseded, so a dropped callback cannot wedge the - // request the way an "already queued" flag would. + // Those callbacks run while the pass that produced the sizes is still + // arranging children, so resolve on the next tick, once positions are final. + // Posting on every retry is deliberate: SendPendingScrollToRequest is a no-op + // once the request has been sent or superseded, so a dropped callback cannot + // wedge the request the way an "already queued" flag would. Dispatcher.Dispatch(SendPendingScrollToRequest); return; } @@ -91,15 +149,31 @@ void DispatchPendingScrollToRequest() SendPendingScrollToRequest(); } - // An element target is resolved against this ScrollView's geometry and the element's - // position inside the arranged content. Before the first layout pass Width/Height are - // still -1 (the never-arranged sentinel, for the content too), so a target computed - // then is garbage. The content check must be "not yet arranged" rather than "arranged - // to nothing": content can legitimately arrange to a zero size (a collapsed container), - // and that raises no further callbacks — gating on the size would hang the caller's - // task forever, while dispatching just clamps the target to the origin. - bool IsElementTargetGeometryReady() => - Width >= 0 && Height >= 0 && Content is not ({ Width: < 0 } or { Height: < 0 }); + // An element target is resolved against the geometry it actually depends on. Before + // the first layout pass Width/Height are still -1 (the never-arranged sentinel), so a + // target computed then is garbage. The ScrollView itself is a valid target and needs + // only its own geometry; any other target sits inside the content, so its position + // is meaningful only once the content has been arranged too. That content check must + // be "not yet arranged" rather than "arranged to nothing": content can legitimately + // arrange to a zero size (a collapsed container), and that raises no further + // callbacks — gating on the size would hang the caller's task forever, while + // dispatching just clamps the target to the origin. + bool IsElementTargetGeometryReady(Element target) + { + if (Width < 0 || Height < 0) + { + return false; + } + + return target == this || Content is { Width: >= 0, Height: >= 0 }; + } + + // A target validated at request time as belonging to this ScrollView can stop + // belonging to it while parked: the content it hung off was replaced or removed. Its + // coordinates then no longer relate to this ScrollView and no arrange of this + // ScrollView can change that, so waiting would be waiting for nothing. + bool IsElementTargetOrphaned(Element target) => + target != this && !CheckElementBelongsToScrollViewer(target); void SendPendingScrollToRequest() { @@ -120,7 +194,18 @@ void SendPendingScrollToRequest() if (_replayPendingScrollToRequestedEvent) { _replayPendingScrollToRequestedEvent = false; + + // A subscriber may issue a new ScrollToAsync from inside the event (a + // compatibility renderer scrolling, say). That newer request wins: it has + // already been sent, and sending the stale replay after it would land the + // scroll on the old target. Every request creates a fresh completion source, + // so a changed source is the exact signal that one was made. + var replayed = _scrollCompletionSource; ScrollToRequested?.Invoke(this, pending); + if (!ReferenceEquals(_scrollCompletionSource, replayed)) + { + return; + } } Handler.Invoke(nameof(IScrollView.RequestScrollTo), ConvertRequestMode(pending).ToRequest()); @@ -279,6 +364,11 @@ public View Content OnPropertyChanged(); Handler?.UpdateValue(nameof(Content)); + + // A parked element request may have just been orphaned (its target hung off + // the old content) — resolve that now rather than at the next arrange, which + // may not come if this ScrollView is already laid out + DispatchPendingScrollToRequest(); } } @@ -527,12 +617,14 @@ void OnScrollToRequested(ScrollToRequestedEventArgs e) _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = true; } - else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) + else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady(e.Element)) { // The handler exists but layout has not run yet (e.g. ScrollToAsync from // OnAppearing): resolving the element target now would compute against the -1 // never-arranged sentinels. Park it for the layout callbacks instead — the // subscribers were already notified above, so the replay must not re-raise. + // It stays parked until the arrange arrives or the view's lifecycle ends + // (see OnHandlerChangedCore); a hidden view scrolls once it is shown. _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = false; } diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 9c1e8e23f72f..d9d3bcb909de 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -487,6 +487,456 @@ public void ElementRequestWithHandlerAttachedWaitsForArrange() Assert.True(task.IsCompleted); } + [Fact] + public void ElementRequestOnHiddenScrollViewWaitsAndScrollsOnceShown() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout, IsVisible = false }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // A hidden view is skipped by layout, so no arrange is coming yet. The request + // stays parked and the task stays pending: completing it here would either + // scroll to a target computed from unarranged geometry or leave a request alive + // after its await returned — the task means "the scroll happened or the view is + // gone", nothing in between. + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Center, false); + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // Showing the view arranges it; the first arrange replays the request against + // real geometry so the scroll lands on the element: Center = 450 - 100/2 + 50/2 + scrollView.IsVisible = true; + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(425, request.VerticalOffset); + + scrollView.SendScrollFinished(); + Assert.True(task.IsCompleted); + } + + [Fact] + public void ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent = new StackLayout { Children = { scrollView } }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // Parked with the handler attached, waiting for the arrange + var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.False(task.IsCompleted); + + // Removing the view from the tree ends its lifecycle for layout purposes: no + // arrange will ever come from a parent it no longer has, and the removal does + // not disconnect its handler — so the request must be dropped and the caller + // released here, at the moment of removal, rather than left pending forever + parent.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // And dropped means dropped: re-attaching and arranging later must not resurrect + // the stale request into a scroll the caller no longer expects + parent.Children.Add(scrollView); + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void DropCompletesTheDroppedTaskAndOnlyThatTask() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + // T1 parks (handler attached, geometry not ready) + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task1.IsCompleted); + + // The view leaves the tree: T1's request is dropped and T1 completes at that + // moment — bound to the request being dropped, with no window in which anything + // else could be released in its place + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + + // Re-attached, a new request T2 parks. It is a different request with its own + // task: nothing about the earlier drop may touch it + parent.Children.Add(scrollView); + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.False(task2.IsCompleted); + + // T2 stays pending until its own scroll: the arrange replays it and lands on the + // element (End = 450 - 100 + 50), and only then does its task complete + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + var handler = (ViewportProviderHandlerStub)scrollView.Handler; + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + Assert.False(task2.IsCompleted); + + scrollView.SendScrollFinished(); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void ConsecutiveDropsEachCompleteTheirOwnTask() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + + // A second park-and-drop cycle on the same view: the second drop must complete + // the second task, and the first drop must have had no effect on it + parent.Children.Add(scrollView); + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task2.IsCompleted); + + parent.Children.Remove(scrollView); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void NewerRequestSupersedesAGeometryParkedElementRequest() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // T1 parks for geometry with the handler attached + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.Empty(handler.ScrollToRequests); + + // A direct request while T1 is parked wins: it is sent now, and T1's request is + // cleared so the arrange cannot replay the stale target on top of it + var task2 = scrollView.ScrollToAsync(0, 100, false); + var direct = Assert.Single(handler.ScrollToRequests); + Assert.Equal(100, direct.VerticalOffset); + + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Single(handler.ScrollToRequests); + + // The scroll completes the current (latest) request's task + scrollView.SendScrollFinished(); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void HandlerDetachDropCompletesOnlyTheDroppedTaskAndDoesNotResurrect() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task1.IsCompleted); + + // Handler detach is the other lifecycle end: T1 completes at that moment + scrollView.Handler = null; + Assert.True(task1.IsCompleted); + + // A new handler and a new request: T2 is its own request, unaffected by the drop + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.False(task2.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // The arrange replays exactly one request — T2's — and the dropped T1 request is + // not resurrected alongside it + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + Assert.False(task2.IsCompleted); + + scrollView.SendScrollFinished(); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void DoubleLifecycleEndOnOneRequestCompletesItOnceAndLeavesLaterRequestsAlone() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + + // Both lifecycle ends fire for the same parked request: the first drops and + // completes it, the second must be a no-op (nothing left to drop) + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + scrollView.Handler = null; + + // A later request on the revived view is untouched by either earlier end + parent.Children.Add(scrollView); + scrollView.Handler = new ViewportProviderHandlerStub(); + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task2.IsCompleted); + } + + [Fact] + public void PreHandlerParkedRequestIsDroppedWhenTheViewLeavesTheTree() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + + // Parked because there is no handler yet + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Leaving the tree is a lifecycle end whether or not a handler ever attached + parent.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + + // And a handler attaching afterwards must find nothing to replay + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void PreHandlerParkedOffsetRequestIsDroppedWhenTheViewLeavesTheTree() + { + var scrollView = new ScrollView { Content = new StackLayout() }; + var parent = new StackLayout { Children = { scrollView } }; + + // The contract is about parked requests of any mode, not only element mode + var task = scrollView.ScrollToAsync(0, 100, false); + Assert.False(task.IsCompleted); + + parent.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void ParkedElementRequestSurvivesContentReplacementAndDrainsOnTheNewContent() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // The content is swapped while the request is parked. The retry is hooked to + // the content's SizeChanged, so it must be re-hooked to the new content — the + // element still belongs to the tree via the old layout, but the ScrollView's + // geometry callbacks now come from the new one + var newLayout = new StackLayout { Children = { item } }; + scrollView.Content = newLayout; + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + newLayout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(450, request.VerticalOffset); + } + + [Fact] + public void ParkedElementRequestWhoseTargetIsOrphanedByContentRemovalIsDroppedAndCompleted() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Removing the content orphans the target: it no longer hangs off this ScrollView, + // so no arrange of this ScrollView can ever give it a position. There is nothing + // left to wait for — the request is dropped and the caller released at that + // moment (not at some later arrange that may never come), and no target computed + // against nothing is dispatched + scrollView.Content = null; + Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // A later arrange has nothing to replay + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void ParkedElementRequestWhoseTargetIsOrphanedByContentReplacementIsDroppedAndCompleted() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Replacing the content with a layout that does not contain the target orphans it + // just as removal does: dropped and completed at the replacement. (Replacement + // with content that still contains the target instead keeps waiting for that + // content's arrange — see ParkedElementRequestSurvivesContentReplacement...) + var unrelated = new StackLayout(); + scrollView.Content = unrelated; + Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + unrelated.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void SelfTargetOnAContentlessScrollViewNeedsOnlyItsOwnGeometry() + { + // ScrollToAsync(scrollView, ...) is a valid request that resolves to the origin + // without touching Content, so a ScrollView with no content must not park it + // forever waiting for content that may never come + var scrollView = new ScrollView(); + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(scrollView, ScrollToPosition.Start, false); + + // Waits only for the ScrollView's own arrange... + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // ...then dispatches against its own geometry + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(0, request.VerticalOffset); + + scrollView.SendScrollFinished(); + Assert.True(task.IsCompleted); + } + + [Fact] + public void ReparentingCancelsAPendingElementScroll() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent1 = new StackLayout { Children = { scrollView } }; + var parent2 = new StackLayout(); + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Moving the ScrollView passes through a removal. A request made against a tree + // position that no longer exists is cancelled at that removal: the task completes + // (without a scroll) and the request is not carried into the new parent — a + // caller that moves a ScrollView with a pending scroll re-requests it there + parent1.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + parent2.Children.Add(scrollView); + + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void ReentrantRequestFromTheDropContinuationParksCleanly() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent = new StackLayout { Children = { scrollView } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // The caller re-requests from its own continuation, which the inline drop runs + // synchronously in the middle of the removal. The mechanism must already have + // cleared its state by then, so the re-entrant request parks as a fresh T2 + // instead of being dropped by the same removal, double-completed, or lost. + Task task2 = null; + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + task1.ContinueWith(_ => task2 = scrollView.ScrollToAsync(item, ScrollToPosition.End, false), + TaskContinuationOptions.ExecuteSynchronously); + + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + Assert.NotNull(task2); + Assert.False(task2.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // T2 belongs to the new lifecycle: re-attached and arranged, it scrolls + parent.Children.Add(scrollView); + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + } + + [Fact] + public void ReentrantRequestFromTheReplayedEventSupersedesTheReplay() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + + // Parked before the handler exists, so the attach replays ScrollToRequested + _ = scrollView.ScrollToAsync(0, 100, false); + + // A subscriber (as a compatibility renderer would be) that issues a new request + // from inside the replayed event. The replay has already cleared the pending + // request before raising, so the re-entrant request is not clobbered — and it + // must win: the replay must not send the stale request to the handler on top of it. + Task reentrant = null; + var reentered = false; + ((IScrollViewController)scrollView).ScrollToRequested += (_, _) => + { + if (reentered) return; + reentered = true; + reentrant = scrollView.ScrollToAsync(0, 250, false); + }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + Assert.NotNull(reentrant); + // Latest request wins; exactly what was sent, in order: the re-entrant one first + // (sent immediately, handler present) — the stale replay must not follow it + Assert.Equal(new[] { 250d }, handler.ScrollToRequests.ConvertAll(r => r.VerticalOffset)); + } + [Fact] public void DeferredRequestReplaysEventForSubscribersAttachedWithTheHandler() { diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs index 0a3ee58aa362..632010939786 100644 --- a/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs @@ -167,10 +167,7 @@ string CheckResolvedMode(UIKit.UIScrollView nativeScrollView, string kind) var expected = edges.Equals(new SafeAreaEdges(SafeAreaRegions.Container)) ? UIKit.UIScrollViewContentInsetAdjustmentBehavior.Always : edges.Equals(SafeAreaEdges.None) || edges.Equals(SafeAreaEdges.All) ? UIKit.UIScrollViewContentInsetAdjustmentBehavior.Never : - // Default on a vertical scroll view resolves to Never since the landscape-notch - // fix (#35533): MAUI owns all edges there, and Automatic remains in use only for - // horizontal scroll views - UIKit.UIScrollViewContentInsetAdjustmentBehavior.Never; + UIKit.UIScrollViewContentInsetAdjustmentBehavior.Automatic; return nativeScrollView.ContentInsetAdjustmentBehavior == expected ? null diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs index 0c072373ceca..2847e5d412e9 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs @@ -66,14 +66,14 @@ public void ScrollToElementEndLandsInsideVisibleViewport() "Probe label should be fully visible after ScrollToAsync(element, End)"); } - // The reachable ContentInsetAdjustmentBehavior modes bake the safe area into the content + // The three ContentInsetAdjustmentBehavior modes bake the safe area into the content // differently, so each is exercised. The page asserts the resolved native behavior, so a - // mode that silently drifted fails instead of quietly testing a different branch. - // Default on a vertical scroll view resolves to Never since the landscape-notch fix - // (#35533); Automatic remains in use only for horizontal scroll views. + // mode that silently drifted fails instead of quietly testing a different branch — + // exactly what happened when the landscape-notch fix (#35533, since reverted in #36580) + // briefly remapped Default to Never. [Test] [Category(UITestCategories.ScrollView)] - [TestCase("ModeDefaultButton", "Never")] + [TestCase("ModeDefaultButton", "Automatic")] [TestCase("ModeNoneButton", "Never")] // Also resolves to Never, but bakes the safe area into the arranged content, which None // does not — so it is the case that actually exercises the measured extent's baked padding @@ -97,13 +97,13 @@ public void ScrollToExtremesInEachInsetMode(string modeButton, string expectedMo } // Element targets resolve against the effective viewport, and each inset mode obscures it - // differently: Always through AdjustedContentInset, Default/SafeAreaEdges.All by baking + // differently: Automatic/Always through AdjustedContentInset, SafeAreaEdges.All by baking // the safe area into the content where AdjustedContentInset never reports it. The page's // oracle measures the probe's bottom edge against the unobscured viewport bottom in window // coordinates, so the mode where MAUI itself obscures the viewport is proven too. [Test] [Category(UITestCategories.ScrollView)] - [TestCase("ModeDefaultButton", "Never")] + [TestCase("ModeDefaultButton", "Automatic")] [TestCase("ModeNoneButton", "Never")] // Also resolves to Never, but bakes the safe area into the content — the case where the // viewport shrink comes from MAUI's own arrange instead of a UIKit inset diff --git a/src/Core/src/Platform/iOS/MauiScrollView.cs b/src/Core/src/Platform/iOS/MauiScrollView.cs index 764905c4d090..4ca1573c43f1 100644 --- a/src/Core/src/Platform/iOS/MauiScrollView.cs +++ b/src/Core/src/Platform/iOS/MauiScrollView.cs @@ -415,7 +415,7 @@ bool ValidateSafeArea() // it can push ContentSize over the Bounds, causing AdjustedContentInset to become non-zero and SafeAreaInsets on the child to reset to zero. // This can result in a loop of invalidations as the layout toggles between these states. // To prevent this, we ignore safe area calculations on child views when they are inside a scroll view. - if (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentBehavior.Never) + if (!UIKitCompensatesForSafeArea) _safeArea = GetInset(SafeAreaInsets).ToSafeAreaInsets(); else _safeArea = GetInset(SystemAdjustedContentInset).ToSafeAreaInsets(); @@ -510,25 +510,44 @@ internal CGSize ScrollableContentSize CGRect? _arrangedContentRect; /// - /// The safe area baked into the content's coordinate - /// space: when it applies the safe area while UIKit is not compensating through - /// , the content is arranged inside - /// safe-area-inset bounds, so element positions carry the padding and the trailing - /// padding still obscures the viewport without ever appearing in the adjusted inset. + /// Whether UIKit is compensating for the safe area through + /// . When it is not + /// (, or a zero system + /// inset), bakes the safe area into the content's + /// coordinate space instead. Shared by the arrange branch and the _safeArea + /// source selection in so the two cannot desynchronize; + /// the arrange records the outcome in for + /// readers between arranges. + /// + bool UIKitCompensatesForSafeArea => + SystemAdjustedContentInset != UIEdgeInsets.Zero + && ContentInsetAdjustmentBehavior != UIScrollViewContentInsetAdjustmentBehavior.Never; + + /// + /// The safe area the last baked into the content's + /// coordinate space: when it applies the safe area while UIKit is not compensating + /// through , the content is arranged + /// inside safe-area-inset bounds, so element positions carry the padding and the + /// trailing padding still obscures the viewport without ever appearing in the + /// adjusted inset. /// /// - /// Mirrors the arrange-side branch exactly: bounds are inset only while - /// _appliesSafeAreaAdjustments, and the inset origin is kept only when UIKit - /// contributes nothing (, - /// or a zero system inset). In the remaining case () - /// the content is re-based at the origin and UIKit's inset owns the compensation, so the - /// arranged rect measures naturally excludes it (issue #36801). + /// Captured at arrange time rather than derived live from UIKit state, because the + /// two can legitimately disagree between arranges: under + /// the arrange + /// that bakes the padding can push the content size past the bounds, at which point + /// UIKit turns the adjusted inset non-zero — the content still carries the padding + /// until the next arrange re-bases it, and this must keep saying so. Recorded + /// alongside , which does the same for the rect + /// (issue #36801). /// - internal SafeAreaPadding SafeAreaBakedIntoContent => - _appliesSafeAreaAdjustments && - (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentBehavior.Never) - ? _safeArea - : SafeAreaPadding.Empty; + internal SafeAreaPadding SafeAreaBakedIntoContent => _bakedSafeArea; + + /// + /// The value reports, set by + /// from the branch it actually took. + /// + SafeAreaPadding _bakedSafeArea = SafeAreaPadding.Empty; UIEdgeInsets SystemAdjustedContentInset { @@ -564,12 +583,15 @@ Size CrossPlatformArrange(CGRect bounds) Size contentSize; CGPoint contentOrigin; + SafeAreaPadding bakedSafeArea; double width; double height; - if (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentBehavior.Never) + if (!UIKitCompensatesForSafeArea) { contentSize = CrossPlatformLayout?.CrossPlatformArrange(bounds.ToRectangle()) ?? Size.Zero; contentOrigin = bounds.Location; + // The inset bounds put the safe area into the content's coordinate space + bakedSafeArea = _appliesSafeAreaAdjustments ? _safeArea : SafeAreaPadding.Empty; width = contentSize.Width; height = contentSize.Height; @@ -578,14 +600,20 @@ Size CrossPlatformArrange(CGRect bounds) { contentSize = CrossPlatformLayout?.CrossPlatformArrange(new Rect(new Point(), bounds.Size.ToSize())) ?? Size.Zero; contentOrigin = CGPoint.Empty; + // Re-based at the origin: UIKit's adjusted inset owns the compensation + bakedSafeArea = SafeAreaPadding.Empty; width = contentSize.Width; height = contentSize.Height; } - // Record where the content was actually arranged, before the ContentSize adjustments - // below: ScrollableContentSize measures the scrollable extent from this rect + // Record what this arrange actually did, before the ContentSize adjustments below: + // ScrollableContentSize measures the scrollable extent from the rect, and + // SafeAreaBakedIntoContent reports the padding the content now carries. Both are + // captured rather than re-derived later, since UIKit's inset state can move + // between arranges (see SafeAreaBakedIntoContent). _arrangedContentRect = new CGRect(contentOrigin, contentSize.ToCGSize()); + _bakedSafeArea = bakedSafeArea; // When using ContentInsetAdjustmentBehavior.Automatic, UIKit dynamically decides whether to apply