Skip to content

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

Merged
PureWeen merged 19 commits into
dotnet:mainfrom
devanathan-vaithiyanathan:fix-13810
Feb 9, 2025
Merged

[Android] Fix Flickering issue when calling Navigation.PopAsync#24887
PureWeen merged 19 commits into
dotnet:mainfrom
devanathan-vaithiyanathan:fix-13810

Conversation

@devanathan-vaithiyanathan

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

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

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

@rmarinho rmarinho left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

/rebase

@PureWeen

PureWeen commented Sep 25, 2024

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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

ygl-rg commented Oct 17, 2024

Copy link
Copy Markdown

will this ptach be merged into dotnet 8 maui?

@jfversluis

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

/rebase

@jsuarezruiz

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz jsuarezruiz left a comment

Copy link
Copy Markdown
Contributor

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
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen

PureWeen commented Jan 6, 2025

Copy link
Copy Markdown
Member

/rebase

@PureWeen

PureWeen commented Jan 6, 2025

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

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

Changes Applied

InternalChildren.Remove(page);

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

PureWeen commented Feb 7, 2025

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member

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
@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).