From 2c56d6211db41a1b105ac466630dca783d5bd1ec Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Mon, 23 Jan 2017 12:42:38 -0700 Subject: [PATCH] Reduce overhead of pushing existing navigation stack (#672) * Make StackCopy less awkward * Clean up comment * Update docs * Update docs * Replace SecondToLast with an arbitrarily deep Peek method * Update docs * Handle negative depths in Peek() --- .../NavigationUnitTest.cs | 91 ++++++++++++++++--- .../INavigationPageController.cs | 4 +- Xamarin.Forms.Core/NavigationPage.cs | 19 ++-- .../AppCompat/NavigationPageRenderer.cs | 4 +- .../Renderers/NavigationRenderer.cs | 4 +- .../NavigationPageRenderer.cs | 2 +- .../NavigationPageRenderer.cs | 8 +- .../Renderers/NavigationRenderer.cs | 4 +- .../INavigationPageController.xml | 52 +++++++---- .../Xamarin.Forms/NavigationPage.xml | 60 ++++++++---- 10 files changed, 182 insertions(+), 66 deletions(-) diff --git a/Xamarin.Forms.Core.UnitTests/NavigationUnitTest.cs b/Xamarin.Forms.Core.UnitTests/NavigationUnitTest.cs index 1cc3ac6a7..7118e1496 100644 --- a/Xamarin.Forms.Core.UnitTests/NavigationUnitTest.cs +++ b/Xamarin.Forms.Core.UnitTests/NavigationUnitTest.cs @@ -213,28 +213,93 @@ namespace Xamarin.Forms.Core.UnitTests } [Test] - public async Task TestStackCopy () + public async Task PeekOne() { - var nav = new NavigationPage (); + var nav = new NavigationPage(); bool signaled = false; nav.PoppedToRoot += (sender, args) => signaled = true; - var root = new ContentPage {Content = new View ()}; - var child1 = new ContentPage {Content = new View ()}; - var child2 = new ContentPage {Content = new View ()}; + var root = new ContentPage { Content = new View() }; + var child1 = new ContentPage { Content = new View() }; + var child2 = new ContentPage { Content = new View() }; - await nav.PushAsync (root); - await nav.PushAsync (child1); - await nav.PushAsync (child2); + await nav.PushAsync(root); + await nav.PushAsync(child1); + await nav.PushAsync(child2); - var copy = ((INavigationPageController)nav).StackCopy; - - Assert.AreEqual (child2, copy.Pop ()); - Assert.AreEqual (child1, copy.Pop ()); - Assert.AreEqual (root, copy.Pop ()); + Assert.AreEqual(((INavigationPageController)nav).Peek(1), child1); } + [Test] + public async Task PeekZero() + { + var nav = new NavigationPage(); + + bool signaled = false; + nav.PoppedToRoot += (sender, args) => signaled = true; + + var root = new ContentPage { Content = new View() }; + var child1 = new ContentPage { Content = new View() }; + var child2 = new ContentPage { Content = new View() }; + + await nav.PushAsync(root); + await nav.PushAsync(child1); + await nav.PushAsync(child2); + + Assert.AreEqual(((INavigationPageController)nav).Peek(0), child2); + } + + [Test] + public async Task PeekPastStackDepth() + { + var nav = new NavigationPage(); + + bool signaled = false; + nav.PoppedToRoot += (sender, args) => signaled = true; + + var root = new ContentPage { Content = new View() }; + var child1 = new ContentPage { Content = new View() }; + var child2 = new ContentPage { Content = new View() }; + + await nav.PushAsync(root); + await nav.PushAsync(child1); + await nav.PushAsync(child2); + + Assert.AreEqual(((INavigationPageController)nav).Peek(3), null); + } + + [Test] + public async Task PeekShallow() + { + var nav = new NavigationPage(); + + bool signaled = false; + nav.PoppedToRoot += (sender, args) => signaled = true; + + var root = new ContentPage { Content = new View() }; + var child1 = new ContentPage { Content = new View() }; + var child2 = new ContentPage { Content = new View() }; + + await nav.PushAsync(root); + await nav.PushAsync(child1); + await nav.PushAsync(child2); + + Assert.AreEqual(((INavigationPageController)nav).Peek(-1), null); + } + + [Test] + public async Task PeekEmpty([Range(0, 3)] int depth) + { + var nav = new NavigationPage(); + + bool signaled = false; + nav.PoppedToRoot += (sender, args) => signaled = true; + + Assert.AreEqual(((INavigationPageController)nav).Peek(depth), null); + } + + [Test] public void ConstructWithRoot () { diff --git a/Xamarin.Forms.Core/INavigationPageController.cs b/Xamarin.Forms.Core/INavigationPageController.cs index eddbe750b..d4ae8675b 100644 --- a/Xamarin.Forms.Core/INavigationPageController.cs +++ b/Xamarin.Forms.Core/INavigationPageController.cs @@ -7,7 +7,9 @@ namespace Xamarin.Forms { public interface INavigationPageController { - Stack StackCopy { get; } + Page Peek(int depth); + + IEnumerable Pages { get; } int StackDepth { get; } diff --git a/Xamarin.Forms.Core/NavigationPage.cs b/Xamarin.Forms.Core/NavigationPage.cs index 7b393b9cd..fdafec061 100644 --- a/Xamarin.Forms.Core/NavigationPage.cs +++ b/Xamarin.Forms.Core/NavigationPage.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; using System.Threading.Tasks; using Xamarin.Forms.Internals; @@ -61,17 +62,23 @@ namespace Xamarin.Forms internal Task CurrentNavigationTask { get; set; } - Stack INavigationPageController.StackCopy + Page INavigationPageController.Peek(int depth) { - get + if (depth < 0) { - var result = new Stack(PageController.InternalChildren.Count); - foreach (Page page in PageController.InternalChildren) - result.Push(page); - return result; + return null; } + + if (PageController.InternalChildren.Count <= depth) + { + return null; + } + + return (Page)PageController.InternalChildren[PageController.InternalChildren.Count - depth - 1]; } + IEnumerable INavigationPageController.Pages => PageController.InternalChildren.Cast(); + int INavigationPageController.StackDepth { get { return PageController.InternalChildren.Count; } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs index cab4d561f..cc4742937 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs @@ -268,7 +268,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat navController.RemovePageRequested += OnRemovePageRequested; // If there is already stuff on the stack we need to push it - foreach (Page page in navController.StackCopy.Reverse()) + foreach (Page page in navController.Pages) { PushViewAsync(page, false); } @@ -461,7 +461,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Task OnPopViewAsync(Page page, bool animated) { - Page pageToShow = ((INavigationPageController)Element).StackCopy.Skip(1).FirstOrDefault(); + Page pageToShow = ((INavigationPageController)Element).Peek(1); if (pageToShow == null) return Task.FromResult(false); diff --git a/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs index c02dc42b9..bf6540b93 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs @@ -109,7 +109,7 @@ namespace Xamarin.Forms.Platform.Android newNavController.RemovePageRequested += OnRemovePageRequested; // If there is already stuff on the stack we need to push it - foreach(Page page in newNavController.StackCopy.Reverse()) + foreach (Page page in newNavController.Pages) { PushViewAsync(page, false); } @@ -130,7 +130,7 @@ namespace Xamarin.Forms.Platform.Android protected virtual Task OnPopViewAsync(Page page, bool animated) { - Page pageToShow = ((INavigationPageController)Element).StackCopy.Skip(1).FirstOrDefault(); + Page pageToShow = ((INavigationPageController)Element).Peek(1); if (pageToShow == null) return Task.FromResult(false); diff --git a/Xamarin.Forms.Platform.WP8/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.WP8/NavigationPageRenderer.cs index 1637b1d00..e2837f05a 100644 --- a/Xamarin.Forms.Platform.WP8/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.WP8/NavigationPageRenderer.cs @@ -139,7 +139,7 @@ namespace Xamarin.Forms.Platform.WinPhone var platform = Element.Platform as Platform; if (platform != null) { - if (e.Page == ((INavigationPageController)Element).StackCopy.LastOrDefault()) + if (e.Page == ((INavigationPageController)Element).Pages.FirstOrDefault()) ((IPageController)e.Page).IgnoresContainerArea = true; e.Task = platform.PushCore(e.Page, Element, e.Animated, e.Realize).ContinueWith((t, o) => true, null); } diff --git a/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs index 593a6747e..d1fdee0a4 100644 --- a/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs @@ -397,7 +397,7 @@ namespace Xamarin.Forms.Platform.WinRT void OnPopRequested(object sender, NavigationRequestedEventArgs e) { - var newCurrent = (Page)PageController.InternalChildren[PageController.InternalChildren.Count - 2]; + var newCurrent = ((INavigationPageController)Element).Peek(1); SetPage(newCurrent, e.Animated, true); } @@ -418,8 +418,10 @@ namespace Xamarin.Forms.Platform.WinRT void PushExistingNavigationStack() { - for (int i = ((INavigationPageController)Element).StackCopy.Count - 1; i >= 0; i--) - SetPage(((INavigationPageController)Element).StackCopy.ElementAt(i), false, false); + foreach (var page in ((INavigationPageController)Element).Pages) + { + SetPage(page, false, false); + } } void SetPage(Page page, bool isAnimated, bool isPopping) diff --git a/Xamarin.Forms.Platform.iOS/Renderers/NavigationRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/NavigationRenderer.cs index 6b698f22b..1465b8dc9 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/NavigationRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/NavigationRenderer.cs @@ -211,7 +211,7 @@ namespace Xamarin.Forms.Platform.iOS UpdateBarTextColor(); // If there is already stuff on the stack we need to push it - ((INavigationPageController)navPage).StackCopy.Reverse().ForEach(async p => await PushPageAsync(p, false)); + ((INavigationPageController)navPage).Pages.ForEach(async p => await PushPageAsync(p, false)); _tracker = new VisualElementTracker(this); @@ -617,7 +617,7 @@ namespace Xamarin.Forms.Platform.iOS if (containerController == null) return; var currentChild = containerController.Child; - var firstPage = ((INavigationPageController)Element).StackCopy.LastOrDefault(); + var firstPage = ((INavigationPageController)Element).Pages.FirstOrDefault(); if ((firstPage != pageBeingRemoved && currentChild != firstPage && NavigationPage.GetHasBackButton(currentChild)) || _parentMasterDetailPage == null) return; diff --git a/docs/Xamarin.Forms.Core/Xamarin.Forms/INavigationPageController.xml b/docs/Xamarin.Forms.Core/Xamarin.Forms/INavigationPageController.xml index b72feb0b2..95aeae7eb 100644 --- a/docs/Xamarin.Forms.Core/Xamarin.Forms/INavigationPageController.xml +++ b/docs/Xamarin.Forms.Core/Xamarin.Forms/INavigationPageController.xml @@ -26,6 +26,42 @@ To be added. + + + + Property + + 2.0.0.0 + + + System.Collections.Generic.IEnumerable<Xamarin.Forms.Page> + + + To be added. + To be added. + To be added. + + + + + + Method + + 2.0.0.0 + + + Xamarin.Forms.Page + + + + + + To be added. + To be added. + To be added. + To be added. + + @@ -108,22 +144,6 @@ To be added. - - - - Property - - 2.0.0.0 - - - System.Collections.Generic.Stack<Xamarin.Forms.Page> - - - For internal use by platform renderers. - To be added. - To be added. - - diff --git a/docs/Xamarin.Forms.Core/Xamarin.Forms/NavigationPage.xml b/docs/Xamarin.Forms.Core/Xamarin.Forms/NavigationPage.xml index 77e203f54..1094d154c 100644 --- a/docs/Xamarin.Forms.Core/Xamarin.Forms/NavigationPage.xml +++ b/docs/Xamarin.Forms.Core/Xamarin.Forms/NavigationPage.xml @@ -450,7 +450,7 @@ System.Diagnostics.DebuggerStepThrough - System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<PopAsync>d__38)) + System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<PopAsync>d__39)) @@ -548,7 +548,7 @@ System.Diagnostics.DebuggerStepThrough - System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<PopToRootAsync>d__46)) + System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<PopToRootAsync>d__47)) @@ -605,7 +605,7 @@ System.Diagnostics.DebuggerStepThrough - System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<PushAsync>d__48)) + System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<PushAsync>d__49)) @@ -844,6 +844,42 @@ public class MyPage : NavigationPage To be added. + + + + Property + + 2.0.0.0 + + + System.Collections.Generic.IEnumerable<Xamarin.Forms.Page> + + + To be added. + To be added. + To be added. + + + + + + Method + + 2.0.0.0 + + + Xamarin.Forms.Page + + + + + + To be added. + To be added. + To be added. + To be added. + + @@ -856,7 +892,7 @@ public class MyPage : NavigationPage System.Diagnostics.DebuggerStepThrough - System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<Xamarin-Forms-INavigationPageController-PopAsyncInner>d__63)) + System.Runtime.CompilerServices.AsyncStateMachine(typeof(Xamarin.Forms.NavigationPage/<Xamarin-Forms-INavigationPageController-PopAsyncInner>d__64)) @@ -874,22 +910,6 @@ public class MyPage : NavigationPage To be added. - - - - Property - - 2.0.0.0 - - - System.Collections.Generic.Stack<Xamarin.Forms.Page> - - - Internal - To be added. - To be added. - -