[iOS] Recover from popping page after the page has already been popped by back button...
authorgenriquez <genriquez@users.noreply.github.com>
Sun, 2 Dec 2018 19:28:25 +0000 (21:28 +0200)
committerShane Neuville <shane94@hotmail.com>
Sun, 2 Dec 2018 19:28:25 +0000 (12:28 -0700)
* 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

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla59172.cs [new file with mode: 0644]
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
Xamarin.Forms.Core/NavigationPage.cs
Xamarin.Forms.Core/NavigationProxy.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 (file)
index 0000000..c66bec7
--- /dev/null
@@ -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
index acf5f3f..50c7c9f 100644 (file)
@@ -9,6 +9,7 @@
     <Import_RootNamespace>Xamarin.Forms.Controls.Issues</Import_RootNamespace>
   </PropertyGroup>
   <ItemGroup>
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla59172.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla60787.xaml.cs">
       <DependentUpon>Bugzilla60787.xaml</DependentUpon>
       <SubType>Code</SubType>
index cf1669d..9f695ac 100644 (file)
@@ -154,18 +154,28 @@ namespace Xamarin.Forms
                public async Task<Page> PopAsync(bool animated)
                {
                        var tcs = new TaskCompletionSource<bool>();
-                       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<NavigationEventArgs> Popped;
index c0d9a77..2c289a4 100644 (file)
@@ -179,6 +179,9 @@ namespace Xamarin.Forms.Internals
                        INavigation currentInner = Inner;
                        if (currentInner == null)
                        {
+                               if (_pushStack.Value.Count == 0)
+                                       return Task.FromResult<Page>(null);
+
                                Page root = _pushStack.Value.Last();
                                _pushStack.Value.Clear();
                                _pushStack.Value.Add(root);
@@ -225,6 +228,8 @@ namespace Xamarin.Forms.Internals
                Page Pop()
                {
                        List<Page> 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<Page> list = _modalStack.Value;
+                       if (list.Count == 0)
+                               return null;
                        Page result = list[list.Count - 1];
                        list.RemoveAt(list.Count - 1);
                        return result;