-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix NRE if user immediately completes Deferral #13202
Conversation
Made the deferral flow more linear and easy to follow Moved the navigation parts of Shell out to a separate class
|
||
public ShellNavigatingEventArgs(ShellNavigationState current, ShellNavigationState target, ShellNavigationSource source, bool canCancel) | ||
{ | ||
|
||
#if NETSTANDARD2_0 | ||
_deferralFinishedTask = () => Task.CompletedTask; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main fix. Just initializing the deferral finished task to a completed task so if no delayed task gets registered the code doesn't crash with an NRE
|
||
namespace Xamarin.Forms | ||
{ | ||
internal class ShellNavigationManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is mainly a copy paste of the code that was already inside Shell
|
||
bool accept = !navigatingArgs.NavigationDelayedOrCancelled; | ||
if (navigatingArgs.DeferredTask != null) | ||
accept = await navigatingArgs.DeferredTask; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this PR a deferred GotoAsync would always call GotoAsync
again once the operation was completed but with these changes the same GotoAsync
just continues on after the user has completed the deferral
|
||
Func<Task> navigationTask = () => GoToAsync(navArgs.Target, navArgs.Animate, false, navArgs); | ||
|
||
if (Device.IsInvokeRequired) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user completes the deferral off the UI Thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't reproduce the NRE anymore
Test failures unrelated |
Description of Change
The deferral code was naively assuming there would be a delay between
GetDeferral
andComplete
so the DeferralTask was never getting created.Issues Resolved
Platforms Affected
Testing Procedure
PR Checklist