From 8a760480a24e9d5b8670964e393455f40b9af0bf Mon Sep 17 00:00:00 2001 From: Samantha Houts Date: Tue, 7 Aug 2018 14:45:28 -0700 Subject: [PATCH] [Android] Prevent white flash when switching MainPage to new MasterDetailPage (#3283) fixes #1373 * Test rapid MDP swaps * [Android] Prevent white screen flash * [Android] Prevent crashes on rapid page swaps * Shorten test time * Preserve old code just in case * Remove flaky test from main run Confirmed that it doesn't ever crash on UITest * More info in 1601 test * Update NavigateToTestPage to await test cases page This lets the page render and then clean itself up when we switch to the test case, which allows the test case for 1601 to actually render with a compressed layout after my change to Platform. * Remove call to FragmentManager.ExecutePendingTransactions in MasterDetailContainer this code was only added to prevent the page flicker, and it caused more issues than it solved. * SetPage will now delay removing views until the new views are in place --- .../Issue1601.cs | 2 ++ Xamarin.Forms.Controls/App.cs | 34 ++++++++++++------- .../Bugzilla44596SplashPage.cs | 2 +- .../AppCompat/MasterDetailContainer.cs | 12 ------- .../AppCompat/Platform.cs | 31 +++++++++++++++-- 5 files changed, 52 insertions(+), 29 deletions(-) diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1601.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1601.cs index 6a1b786b4..2152bae9d 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1601.cs +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1601.cs @@ -49,7 +49,9 @@ namespace Xamarin.Forms.Controls.Issues { RunningApp.Screenshot("Start G1601"); RunningApp.WaitForElement(q => q.Marked("CRASH!")); + RunningApp.Screenshot("I see the button"); RunningApp.Tap (q => q.Marked ("CRASH!")); + RunningApp.Screenshot("Didn't crash!"); } #endif } diff --git a/Xamarin.Forms.Controls/App.cs b/Xamarin.Forms.Controls/App.cs index fad0f6094..bb290f57c 100644 --- a/Xamarin.Forms.Controls/App.cs +++ b/Xamarin.Forms.Controls/App.cs @@ -32,9 +32,7 @@ namespace Xamarin.Forms.Controls SetMainPage(CreateDefaultMainPage()); - //TestBugzilla44596(); - - //TestBugzilla45702(); + //TestMainPageSwitches(); } protected override void OnStart() @@ -42,23 +40,28 @@ namespace Xamarin.Forms.Controls //TestIssue2393(); } - void TestBugzilla44596() + async Task TestBugzilla44596() { + await Task.Delay(50); // verify that there is no gray screen displayed between the blue splash and red MasterDetailPage. - SetMainPage(new Bugzilla44596SplashPage(() => + SetMainPage(new Bugzilla44596SplashPage(async () => { var newTabbedPage = new TabbedPage(); - newTabbedPage.Children.Add(new ContentPage { BackgroundColor = Color.Red, Content = new Label { Text = "yay" } }); + newTabbedPage.Children.Add(new ContentPage { BackgroundColor = Color.Red, Content = new Label { Text = "Success" } }); MainPage = new MasterDetailPage { Master = new ContentPage { Title = "Master", BackgroundColor = Color.Red }, Detail = newTabbedPage }; + + await Task.Delay(50); + SetMainPage(CreateDefaultMainPage()); })); } - void TestBugzilla45702() + async Task TestBugzilla45702() { + await Task.Delay(50); // verify that there is no crash when switching MainPage from MDP inside NavPage SetMainPage(new Bugzilla45702()); } @@ -91,6 +94,13 @@ namespace Xamarin.Forms.Controls //SetMainPage(new Issues.Issue2004()); } + async Task TestMainPageSwitches() + { + await TestBugzilla45702(); + + await TestBugzilla44596(); + } + public Page CreateDefaultMainPage() { var layout = new StackLayout { BackgroundColor = Color.Red }; @@ -123,8 +133,7 @@ namespace Xamarin.Forms.Controls string page = parts[1].Trim(); var pageForms = Activator.CreateInstance(Type.GetType(page)); - var appLinkPageGallery = pageForms as AppLinkPageGallery; - if (appLinkPageGallery != null) + if (pageForms is AppLinkPageGallery appLinkPageGallery) { appLinkPageGallery.ShowLabel = true; (MainPage as MasterDetailPage)?.Detail.Navigation.PushAsync((pageForms as Page)); @@ -178,8 +187,7 @@ namespace Xamarin.Forms.Controls static async Task LoadResource(string filename) { - string assemblystring; - Assembly assembly = GetAssembly(out assemblystring); + Assembly assembly = GetAssembly(out string assemblystring); Stream stream = assembly.GetManifestResourceStream($"{assemblystring}.{filename}"); string text; @@ -198,9 +206,9 @@ namespace Xamarin.Forms.Controls // Set up a delegate to handle the navigation to the test page EventHandler toTestPage = null; - toTestPage = delegate (object sender, EventArgs e) + toTestPage = async delegate (object sender, EventArgs e) { - Current.MainPage.Navigation.PushModalAsync(TestCases.GetTestCases()); + await Current.MainPage.Navigation.PushModalAsync(TestCases.GetTestCases()); TestCases.TestCaseScreen.PageToAction[test](); Current.MainPage.Appearing -= toTestPage; }; diff --git a/Xamarin.Forms.Controls/Bugzilla44596SplashPage.cs b/Xamarin.Forms.Controls/Bugzilla44596SplashPage.cs index ba7cdc461..6b2fa596b 100644 --- a/Xamarin.Forms.Controls/Bugzilla44596SplashPage.cs +++ b/Xamarin.Forms.Controls/Bugzilla44596SplashPage.cs @@ -18,7 +18,7 @@ namespace Xamarin.Forms.Controls protected async override void OnAppearing() { base.OnAppearing(); - await Task.Delay(2000); + await Task.Delay(500); FinishedLoading?.Invoke(); } } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs index 70c65511c..5d497183e 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs @@ -116,18 +116,6 @@ namespace Xamarin.Forms.Platform.Android.AppCompat transaction.CommitAllowingStateLossEx(); _currentFragment = fragment; - - new Handler(Looper.MainLooper).PostAtFrontOfQueue(() => - { - if (_pageContainer == null) - { - // The view we're hosting in the fragment was never created (possibly we're already - // navigating to another page?) so there's nothing to commit - return; - } - - FragmentManager.ExecutePendingTransactionsEx(); - }); } } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs b/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs index db7a3aef2..1e8d1d426 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs @@ -206,6 +206,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat void IPlatformLayout.OnLayout(bool changed, int l, int t, int r, int b) { + if (Page == null) + return; + if (changed) { LayoutRootPage(Page, r - l, b - t); @@ -262,12 +265,16 @@ namespace Xamarin.Forms.Platform.Android.AppCompat { var layout = false; + var viewsToRemove = new List(); + var renderersToDispose = new List(); + if (Page != null) { - _renderer.RemoveAllViews(); + for (int i = 0; i < _renderer.ChildCount; i++) + viewsToRemove.Add(_renderer.GetChildAt(i)); - foreach (IVisualElementRenderer rootRenderer in _navModel.Roots.Select(Android.Platform.GetRenderer)) - rootRenderer.Dispose(); + foreach (var root in _navModel.Roots) + renderersToDispose.Add(Android.Platform.GetRenderer(root)); _navModel = new NavigationModel(); @@ -275,7 +282,10 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } if (newRoot == null) + { + Cleanup(viewsToRemove, renderersToDispose); return; + } _navModel.Push(newRoot, null); @@ -283,10 +293,25 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Page.Platform = this; AddChild(Page, layout); + Cleanup(viewsToRemove, renderersToDispose); + Application.Current.NavigationProxy.Inner = this; } + + void Cleanup(List viewsToRemove, List renderersToDispose) + { + foreach (var view in viewsToRemove) + _renderer?.RemoveView(view); + + foreach (IVisualElementRenderer rootRenderer in renderersToDispose) + rootRenderer?.Dispose(); + } + void AddChild(Page page, bool layout = false) { + if (page == null) + return; + if (Android.Platform.GetRenderer(page) != null) return;