From d2fce1fa2b4c7d320cc0c65cc29e540dcf869712 Mon Sep 17 00:00:00 2001 From: genriquez Date: Sun, 2 Dec 2018 21:28:25 +0200 Subject: [PATCH] [iOS] Recover from popping page after the page has already been popped by back button (#2873) * Cleanup navigation task on Pop exception * Bugzilla 59172 + recovery repro tests * Trap for Pop race * [Core] add check on OnPopToRootAsync * Added test scenario description - fixes #4129 --- .../Bugzilla59172.cs | 114 +++++++++++++++++++++ .../Xamarin.Forms.Controls.Issues.Shared.projitems | 1 + Xamarin.Forms.Core/NavigationPage.cs | 28 +++-- Xamarin.Forms.Core/NavigationProxy.cs | 7 ++ 4 files changed, 141 insertions(+), 9 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla59172.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla59172.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla59172.cs new file mode 100644 index 0000000..c66bec7 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla59172.cs @@ -0,0 +1,114 @@ +using System; +using System.Threading.Tasks; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Bugzilla, + 59172, "[iOS] Popped page does not appear on top of current navigation stack, please file a bug.", + PlatformAffected.iOS)] + public class Bugzilla59172 : TestNavigationPage + { + protected override void Init() + { + var firstPage = new TestPage(); + Navigation.PushAsync(firstPage); + } + + [Preserve(AllMembers = true)] + public class TestPage : ContentPage + { + TestPage parent; + Label navigationErrorLabel = new Label(); + + public TestPage(TestPage parentPage = null) + { + this.parent = parentPage; + + var layout = new StackLayout(); + + var forwardButton = new Button { Text = "Forward", AutomationId = "GoForward" }; + layout.Children.Add(forwardButton); + forwardButton.Clicked += Forward_OnClicked; + + if (parentPage != null) + { + var backButton = new Button { Text = "Back (Delayed)", AutomationId = "GoBackDelayed" }; + layout.Children.Add(backButton); + backButton.Clicked += (a, b) => BackButtonPress(false); + + var backButtonSafe = new Button { Text = "Back (Delayed; Safe)", AutomationId = "GoBackDelayedSafe" }; + layout.Children.Add(backButtonSafe); + backButtonSafe.Clicked += (a, b) => BackButtonPress(true); + } + + layout.Children.Add(navigationErrorLabel); + + Content = layout; + } + + void Forward_OnClicked(object sender, EventArgs e) + { + Navigation.PushAsync(new TestPage(this)); + } + + async void BackButtonPress(bool safe) + { + try + { + // Assume some workload that delays the back navigation + await Task.Delay(2500); + + await Navigation.PopAsync(); + } + catch (Exception ex) + { + if (!safe) { throw; } + + parent.navigationErrorLabel.Text = ex.Message; + } + } + } + +#if UITEST + + // Test scenario: Tapping the GoBack link triggers a PopAsync 2500ms after the tap event. + // Right before PopAsync is triggered, manually navigate back pressing the back arrow in the navigation bar + + [Test] + public async void Issue59172Test() + { + RunningApp.Tap(q => q.Marked("GoForward")); + RunningApp.Tap(q => q.Marked("GoBackDelayed")); + RunningApp.Back(); + + await Task.Delay(1000); + + // App should not have crashed + RunningApp.WaitForElement(q => q.Marked("GoForward")); + } + + [Test] + public async void Issue59172RecoveryTest() + { + RunningApp.Tap(q => q.Marked("GoForward")); + RunningApp.Tap(q => q.Marked("GoBackDelayedSafe")); + RunningApp.Back(); + + await Task.Delay(1000); + + RunningApp.Tap(q => q.Marked("GoForward")); + + // App should navigate + RunningApp.WaitForElement(q => q.Marked("GoBackDelayedSafe")); + } +#endif + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index acf5f3f..50c7c9f 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -9,6 +9,7 @@ Xamarin.Forms.Controls.Issues + Bugzilla60787.xaml Code diff --git a/Xamarin.Forms.Core/NavigationPage.cs b/Xamarin.Forms.Core/NavigationPage.cs index cf1669d..9f695ac 100644 --- a/Xamarin.Forms.Core/NavigationPage.cs +++ b/Xamarin.Forms.Core/NavigationPage.cs @@ -154,18 +154,28 @@ namespace Xamarin.Forms public async Task PopAsync(bool animated) { var tcs = new TaskCompletionSource(); - if (CurrentNavigationTask != null && !CurrentNavigationTask.IsCompleted) + try { - var oldTask = CurrentNavigationTask; - CurrentNavigationTask = tcs.Task; - await oldTask; + if (CurrentNavigationTask != null && !CurrentNavigationTask.IsCompleted) + { + var oldTask = CurrentNavigationTask; + CurrentNavigationTask = tcs.Task; + await oldTask; + } + else + CurrentNavigationTask = tcs.Task; + + var result = await PopAsyncInner(animated, false); + tcs.SetResult(true); + return result; } - else - CurrentNavigationTask = tcs.Task; + catch (Exception) + { + CurrentNavigationTask = null; + tcs.SetCanceled(); - var result = await PopAsyncInner(animated, false); - tcs.SetResult(true); - return result; + throw; + } } public event EventHandler Popped; diff --git a/Xamarin.Forms.Core/NavigationProxy.cs b/Xamarin.Forms.Core/NavigationProxy.cs index c0d9a77..2c289a4 100644 --- a/Xamarin.Forms.Core/NavigationProxy.cs +++ b/Xamarin.Forms.Core/NavigationProxy.cs @@ -179,6 +179,9 @@ namespace Xamarin.Forms.Internals INavigation currentInner = Inner; if (currentInner == null) { + if (_pushStack.Value.Count == 0) + return Task.FromResult(null); + Page root = _pushStack.Value.Last(); _pushStack.Value.Clear(); _pushStack.Value.Add(root); @@ -225,6 +228,8 @@ namespace Xamarin.Forms.Internals Page Pop() { List list = _pushStack.Value; + if (list.Count == 0) + return null; Page result = list[list.Count - 1]; list.RemoveAt(list.Count - 1); return result; @@ -233,6 +238,8 @@ namespace Xamarin.Forms.Internals Page PopModal() { List list = _modalStack.Value; + if (list.Count == 0) + return null; Page result = list[list.Count - 1]; list.RemoveAt(list.Count - 1); return result; -- 2.7.4