Skip to content

[Android] Fix Flickering issue when calling Navigation.PopAsync #24887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Feb 9, 2025

Conversation

devanathan-vaithiyanathan
Copy link
Contributor

@devanathan-vaithiyanathan devanathan-vaithiyanathan commented Sep 24, 2024

Root Cause

The flickering during navigation occurred because, when performing PopAsync, the removed page’s handlers were set to null. This caused flickering as the navigation was processed.

Description of Change

The fix involves avoiding the removal of page handlers during PopAsync, which eliminates the flickering issue during navigation.

Issues Fixed

Fixes #13810

Validated the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Output Screenshot

Before

366413807-f0592d6d-6b89-48b0-bab8-eed46d20336c.mp4

After

366413828-b47e24f4-68f0-44a0-b1bf-ed8dd24d1dcf.mp4

@devanathan-vaithiyanathan devanathan-vaithiyanathan changed the title changes added [Android] Fix Flickering issue when calling Navigation.PopAsync Sep 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Sep 24, 2024
@devanathan-vaithiyanathan devanathan-vaithiyanathan marked this pull request as ready for review September 24, 2024 07:50
@devanathan-vaithiyanathan devanathan-vaithiyanathan requested a review from a team as a code owner September 24, 2024 07:50
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho rmarinho self-requested a review September 24, 2024 18:04
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems is making the Windows Device tests fail , I run few times and got same results. Maybe this is causing some side effects on Windows. But maybe also rebase just to make sure branch is updated with latest .

@PureWeen
Copy link
Member

/rebase

@PureWeen
Copy link
Member

PureWeen commented Sep 25, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the device tests on windows are crashing.

On CI you can check the logs here to see where it's crashing

image

@jsuarezruiz
Copy link
Contributor

It looks like the device tests on windows are crashing.

On CI you can check the logs here to see where it's crashing

image

This is the failing test NavigatedFiresAfterSwitchingFlyoutItems.

at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0(Int32 hr)
   at ABI.Microsoft.UI.Xaml.Controls.IContentPresenterMethods.set_Content(IObjectReference _obj, Object value)
   at Microsoft.UI.Xaml.Controls.ContentPresenter.set_Content(Object value)
   at Microsoft.Maui.Platform.StackNavigationManager.OnNavigated(Object sender, NavigationEventArgs e)
   at WinRT._EventSource_global__Microsoft_UI_Xaml_Navigation_NavigatedEventHandler.EventState.<GetEventInvoke>b__1_0(Object sender, NavigationEventArgs e)
   at ABI.Microsoft.UI.Xaml.Navigation.NavigatedEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e)
--- End of stack trace from previous location ---
   at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0(Int32 hr)
   at ABI.Microsoft.UI.Xaml.Controls.IFrameMethods.GoBack(IObjectReference _obj, NavigationTransitionInfo transitionInfoOverride)
   at Microsoft.UI.Xaml.Controls.Frame.GoBack(NavigationTransitionInfo transitionInfoOverride)
   at Microsoft.Maui.Platform.StackNavigationManager.NavigateTo(NavigationRequest args)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.RequestNavigation(ShellSectionHandler handler, IStackNavigation view, Object arg3)
   at Microsoft.Maui.CommandMapper`2.<>c__DisplayClass6_0.<Add>b__0(IElementHandler h, IElement v, Object o)
   at Microsoft.Maui.CommandMapper.InvokeCore(String key, IElementHandler viewHandler, IElement virtualView, Object args)
   at Microsoft.Maui.CommandMapper.Invoke(IElementHandler viewHandler, IElement virtualView, String property, Object args)
   at Microsoft.Maui.Handlers.ElementHandler.Invoke(String command, Object args)
   at Microsoft.Maui.Controls.ShellSection.Microsoft.Maui.IStackNavigation.RequestNavigation(NavigationRequest eventArgs)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SyncNavigationStack(Boolean animated, NavigationRequestedEventArgs e)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.OnNavigationRequested(Object sender, NavigationRequestedEventArgs e)
   at Microsoft.Maui.Controls.ShellSection.InvokeNavigationRequest(NavigationRequestedEventArgs args)
   at Microsoft.Maui.Controls.ShellSection.OnPopAsync(Boolean animated)
   at Microsoft.Maui.Controls.ShellSection.GoToAsync(ShellNavigationRequest request, ShellRouteParameters queryData, IServiceProvider services, Nullable`1 animate, Boolean isRelativePopping)
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass3_0.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass2_0`1.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
   at Microsoft.Maui.DeviceTests.ShellTests.<>c__DisplayClass32_0.<<NavigatedFiresAfterSwitchingFlyoutItems>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass16_1`1.<<CreateHandlerAndAddToWindow>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass30_0`1.<<SetupWindowForTests>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass30_0`1.<<SetupWindowForTests>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass3_0.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass2_0`1.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass16_0`1.<<CreateHandlerAndAddToWindow>

@devanathan-vaithiyanathan
Copy link
Contributor Author

It looks like the device tests on windows are crashing.
On CI you can check the logs here to see where it's crashing
image

This is the failing test NavigatedFiresAfterSwitchingFlyoutItems.

at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0(Int32 hr)
   at ABI.Microsoft.UI.Xaml.Controls.IContentPresenterMethods.set_Content(IObjectReference _obj, Object value)
   at Microsoft.UI.Xaml.Controls.ContentPresenter.set_Content(Object value)
   at Microsoft.Maui.Platform.StackNavigationManager.OnNavigated(Object sender, NavigationEventArgs e)
   at WinRT._EventSource_global__Microsoft_UI_Xaml_Navigation_NavigatedEventHandler.EventState.<GetEventInvoke>b__1_0(Object sender, NavigationEventArgs e)
   at ABI.Microsoft.UI.Xaml.Navigation.NavigatedEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e)
--- End of stack trace from previous location ---
   at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0(Int32 hr)
   at ABI.Microsoft.UI.Xaml.Controls.IFrameMethods.GoBack(IObjectReference _obj, NavigationTransitionInfo transitionInfoOverride)
   at Microsoft.UI.Xaml.Controls.Frame.GoBack(NavigationTransitionInfo transitionInfoOverride)
   at Microsoft.Maui.Platform.StackNavigationManager.NavigateTo(NavigationRequest args)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.RequestNavigation(ShellSectionHandler handler, IStackNavigation view, Object arg3)
   at Microsoft.Maui.CommandMapper`2.<>c__DisplayClass6_0.<Add>b__0(IElementHandler h, IElement v, Object o)
   at Microsoft.Maui.CommandMapper.InvokeCore(String key, IElementHandler viewHandler, IElement virtualView, Object args)
   at Microsoft.Maui.CommandMapper.Invoke(IElementHandler viewHandler, IElement virtualView, String property, Object args)
   at Microsoft.Maui.Handlers.ElementHandler.Invoke(String command, Object args)
   at Microsoft.Maui.Controls.ShellSection.Microsoft.Maui.IStackNavigation.RequestNavigation(NavigationRequest eventArgs)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SyncNavigationStack(Boolean animated, NavigationRequestedEventArgs e)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.OnNavigationRequested(Object sender, NavigationRequestedEventArgs e)
   at Microsoft.Maui.Controls.ShellSection.InvokeNavigationRequest(NavigationRequestedEventArgs args)
   at Microsoft.Maui.Controls.ShellSection.OnPopAsync(Boolean animated)
   at Microsoft.Maui.Controls.ShellSection.GoToAsync(ShellNavigationRequest request, ShellRouteParameters queryData, IServiceProvider services, Nullable`1 animate, Boolean isRelativePopping)
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass3_0.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass2_0`1.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
   at Microsoft.Maui.DeviceTests.ShellTests.<>c__DisplayClass32_0.<<NavigatedFiresAfterSwitchingFlyoutItems>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass16_1`1.<<CreateHandlerAndAddToWindow>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass30_0`1.<<SetupWindowForTests>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass30_0`1.<<SetupWindowForTests>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass3_0.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass2_0`1.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass16_0`1.<<CreateHandlerAndAddToWindow>

We'll run the DeviceTests using NET9 and provide an update.

@ygl-rg
Copy link

ygl-rg commented Oct 17, 2024

will this ptach be merged into dotnet 8 maui?

@jfversluis
Copy link
Member

@ygl-rg I think at this point with .NET 9 being close to being released this will be .NET 9 only

@devanathan-vaithiyanathan devanathan-vaithiyanathan changed the base branch from net9.0 to main October 22, 2024 09:30
@devanathan-vaithiyanathan
Copy link
Contributor Author

It looks like the device tests on windows are crashing.
On CI you can check the logs here to see where it's crashing
image

This is the failing test NavigatedFiresAfterSwitchingFlyoutItems.

at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0(Int32 hr)
   at ABI.Microsoft.UI.Xaml.Controls.IContentPresenterMethods.set_Content(IObjectReference _obj, Object value)
   at Microsoft.UI.Xaml.Controls.ContentPresenter.set_Content(Object value)
   at Microsoft.Maui.Platform.StackNavigationManager.OnNavigated(Object sender, NavigationEventArgs e)
   at WinRT._EventSource_global__Microsoft_UI_Xaml_Navigation_NavigatedEventHandler.EventState.<GetEventInvoke>b__1_0(Object sender, NavigationEventArgs e)
   at ABI.Microsoft.UI.Xaml.Navigation.NavigatedEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e)
--- End of stack trace from previous location ---
   at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0(Int32 hr)
   at ABI.Microsoft.UI.Xaml.Controls.IFrameMethods.GoBack(IObjectReference _obj, NavigationTransitionInfo transitionInfoOverride)
   at Microsoft.UI.Xaml.Controls.Frame.GoBack(NavigationTransitionInfo transitionInfoOverride)
   at Microsoft.Maui.Platform.StackNavigationManager.NavigateTo(NavigationRequest args)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.RequestNavigation(ShellSectionHandler handler, IStackNavigation view, Object arg3)
   at Microsoft.Maui.CommandMapper`2.<>c__DisplayClass6_0.<Add>b__0(IElementHandler h, IElement v, Object o)
   at Microsoft.Maui.CommandMapper.InvokeCore(String key, IElementHandler viewHandler, IElement virtualView, Object args)
   at Microsoft.Maui.CommandMapper.Invoke(IElementHandler viewHandler, IElement virtualView, String property, Object args)
   at Microsoft.Maui.Handlers.ElementHandler.Invoke(String command, Object args)
   at Microsoft.Maui.Controls.ShellSection.Microsoft.Maui.IStackNavigation.RequestNavigation(NavigationRequest eventArgs)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SyncNavigationStack(Boolean animated, NavigationRequestedEventArgs e)
   at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.OnNavigationRequested(Object sender, NavigationRequestedEventArgs e)
   at Microsoft.Maui.Controls.ShellSection.InvokeNavigationRequest(NavigationRequestedEventArgs args)
   at Microsoft.Maui.Controls.ShellSection.OnPopAsync(Boolean animated)
   at Microsoft.Maui.Controls.ShellSection.GoToAsync(ShellNavigationRequest request, ShellRouteParameters queryData, IServiceProvider services, Nullable`1 animate, Boolean isRelativePopping)
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass3_0.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass2_0`1.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
   at Microsoft.Maui.DeviceTests.ShellTests.<>c__DisplayClass32_0.<<NavigatedFiresAfterSwitchingFlyoutItems>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass16_1`1.<<CreateHandlerAndAddToWindow>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass30_0`1.<<SetupWindowForTests>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass30_0`1.<<SetupWindowForTests>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass3_0.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.Dispatching.DispatcherExtensions.<>c__DisplayClass2_0`1.<<DispatchAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.Maui.DeviceTests.ControlsHandlerTestBase.<>c__DisplayClass16_0`1.<<CreateHandlerAndAddToWindow>

We'll run the DeviceTests using NET9 and provide an update.

I ran the device tests using the latest MAUI source with our fix and confirmed that the mentioned test passed successfully. Let me know if you have any concerns.

@devanathan-vaithiyanathan
Copy link
Contributor Author

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a device test failing related with pop navigating.
image

@sheiksyedm sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
@jfversluis jfversluis added the area-navigation NavigationPage label Dec 10, 2024
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/rebase

@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen dismissed stale reviews from jsuarezruiz and rmarinho January 6, 2025 10:42

Changes Applied

@@ -402,9 +402,6 @@ void FireAppearing(Page page)
void RemoveFromInnerChildren(Element page)
{
InternalChildren.Remove(page);

// TODO For NET9 we should remove this because the DisconnectHandlers will take care of it
page.Handler = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this change is causing the winui device tests to crash.

you can download the crash dump from the artifacts and analyze it inside VS to see the exception.

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! @devanathan-vaithiyanathan can you please fix it? It is an annoying bug that would be great to have fixed as soon as possible. I can help you if you want :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PureWeen I have updated the code to include null checks for MauiContext to address the WinUI device test crashes. Could you please review the changes and let me know if you have any further concerns?

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -19,7 +19,7 @@ public partial class TabbedPage
WFrame? _navigationFrame;
bool _connectedToHandler;
WFrame NavigationFrame => _navigationFrame ?? throw new ArgumentNullException(nameof(NavigationFrame));
IMauiContext MauiContext => this.Handler?.MauiContext ?? throw new InvalidOperationException("MauiContext cannot be null here");
IMauiContext? MauiContext => this.Handler?.MauiContext;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to leave the exception

Can you just add a method to check nullability?

bool HasHandler => this.Handler?.MauiContext is not null

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PureWeen MauiContext reference is used in multiple places in this file, and I have ensured that null checks are added to prevent exceptions. Do we need to assign this.Handler?.MauiContext based on HasHandler in each place?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to take a different approach here.

we should be no longer listening to the SelectionChanged event after the handler is disconnected but it looks like we are

image

Can we try to figure out here why the subscription still exists after disconnect is called?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PureWeen Upon checking further, the SelectionChanged event was unhooked properly during navigation, but the _navigationFrame.Navigated event was not. Since I am unable to reproduce the exact crash, based on the call stack from the dump files, I suspect that the issue occurs because the Navigated event was not unhooked properly. I have now unhooked the Navigated event correctly. If the Navigated event is properly unhooked during the disconnect handling, it will prevent the UpdateCurrentPageContent method from being called, even when triggered from the selection changing event. Could you please review and share your concerns?

@PureWeen
Copy link
Member

PureWeen commented Feb 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!!! Thank you

@PureWeen PureWeen merged commit 2075725 into dotnet:main Feb 9, 2025
124 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-navigation NavigationPage community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Flickering when calling Navigation.PopAsync(false).