Skip to content
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

App Crash on Android due to ScrollViewHandler.DisconnectHandler call during Scroll Animation while navigating away from Page. #22443

Closed
PATRICKdallat opened this issue May 16, 2024 · 9 comments · Fixed by #22492
Labels
area-controls-scrollview ScrollView p/1 Work that is critical for the release, but we could probably ship without platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working

Comments

@PATRICKdallat
Copy link

PATRICKdallat commented May 16, 2024

Description

The android application experiences a crash when a user attempts to navigate back (i.e. removing the page from the stack) while a scroll animation is in progress and the ScrollViewHandler has been disconnected.

Here are three suggested improvements:

  1. Refine the RequestScrollTo CommandMapping by adding a null-check for the VirtualView within the OnFinish Action Callback. This will prevent potential null reference errors.

  2. Prior to disposing of the ScrollView from the ScrollViewHandler.DisconnectHandler, cancel the _scrollCompletionSource. This will ensure that no tasks are left open, improving resource management.

  3. Create a publicly (or protected) accessible method to cancel the ScrollView Scroll animation task. This will provide greater flexibility and control over the Scroll animation.

Steps to Reproduce

  1. Clone the reproduction project repository
  2. Run on Android device
  3. Navigate to Second Page
  4. On Appearing of Second Page navigate back to main page (before scroll animation finishes)
  5. App will crash

Link to public reproduction project repository

https://github.com/PATRICKdallat/ScrollViewHandlerCrash

Version with bug

8.0.10 SR3

Is this a regression from previous behavior?

No, this is something new

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

Replacing the CommandMapper for IScrollView.RequestScrollTo

` ScrollViewHandler.CommandMapper.ReplaceMapping<IScrollView, ScrollViewHandler>(nameof(IScrollView.RequestScrollTo), (handler, _, args) =>
{
var context = handler.PlatformView.Context;

        if (args is not ScrollToRequest request || context is null)
        {
            return;
        }
    
        var horizontalOffsetDevice = (int)context.ToPixels(request.HorizontalOffset);
        var verticalOffsetDevice = (int)context.ToPixels(request.VerticalOffset);
    
        handler.PlatformView.ScrollTo(horizontalOffsetDevice, verticalOffsetDevice,
            request.Instant, () => handler?.VirtualView?.ScrollFinished());
    });`

Relevant log output

Android.Runtime.RuntimeNativeMethods.monodroid_debugger_unhandled_exception (Unknown Source:0) Android.Runtime.JNINativeWrapper._unhandled_exception (JNINativeWrapper.g.cs:13) Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V (JNINativeWrapper.g.cs:126) Microsoft.Maui.Handlers.ViewHandler<Microsoft.Maui.IScrollView,Microsoft.Maui.Platform.MauiScrollView>.get_VirtualView (ViewHandlerOfT.cs:42) Microsoft.Maui.Handlers.ScrollViewHandler.Microsoft.Maui.Handlers.IScrollViewHandler.get_VirtualView (ScrollViewHandler.cs:53) Microsoft.Maui.Handlers.ScrollViewHandler. (ScrollViewHandler.Android.cs:150) Microsoft.Maui.Platform.MauiScrollView. (MauiScrollView.cs:288) Android.Animation.AnimatorEventDispatcher.OnAnimationEnd (Animator.cs:86) Android.Animation.Animator.IAnimatorListenerInvoker.n_OnAnimationEnd_Landroid_animation_Animator_ (Android.Animation.Animator.cs:142) Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V (JNINativeWrapper.g.cs:125)

@PATRICKdallat PATRICKdallat added the t/bug Something isn't working label May 16, 2024
Copy link
Contributor

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

@RoiChen001 RoiChen001 added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels May 16, 2024
@RoiChen001
Copy link
Collaborator

RoiChen001 commented May 16, 2024

Can repro this issue at Android platform on the latest 17.10 Preview 7(8.0.7&8.0.10&8.0.40).

@PATRICKdallat
Copy link
Author

Yeah I have been able to recreate the issue using the latest .NET 9 Preview 3, and I can still see the problematic issue within the ScrollViewHandler.Android.cs line 150.

@PureWeen PureWeen added p/1 Work that is critical for the release, but we could probably ship without area-controls-scrollview ScrollView labels May 16, 2024
kubaflo added a commit to kubaflo/maui that referenced this issue May 17, 2024
kubaflo added a commit to kubaflo/maui that referenced this issue May 17, 2024
@PATRICKdallat
Copy link
Author

@kubaflo
Unfortunately this is not a fix, this would be covering up the issue.
The underlying issue that the ScrollTo task is still alive as we have not cancelled and the task has not completed.

Can you take a look at the improvements listed above and that should address this issue.
Let me know if there is anything you are not sure on and I am happy to help.

@PATRICKdallat
Copy link
Author

@PureWeen
I believe I am addressing the right person with this question, if not, point me in the right direction of someone who can clarify this.
I would just like some clarification for myself and my team, when should a ViewHandler be Disconnected (ViewHandler.DisconnectHandler)?

PureWeen pushed a commit that referenced this issue May 21, 2024
…ix (#22492)

* Fix #22443 & UITest

* Update ScrollViewHandler.Android.cs & move tests

* Fixed the UI test
@PATRICKdallat
Copy link
Author

@PureWeen
Thank you @kubaflo for the fix for eliminating the chance of a crash on scroll view finish callback.

However, can this ticket be reopened as the _scrollCompletionSource has not been cancelled/completed issue has not been addressed.

@PATRICKdallat
Copy link
Author

@kubaflo

Making the change from the merged PR does not resolve the issue.
App still crashes but the exception is now around the PlatformView rather than the VirtualView.

Please attempt the same steps as above with the updated repo -> https://github.com/PATRICKdallat/ScrollViewHandlerCrash

@kubaflo
Copy link
Contributor

kubaflo commented May 22, 2024

@PATRICKdallat I think you should try with the latest nightly build

@PATRICKdallat
Copy link
Author

@kubaflo
Would you be able to answer the following - when should a ViewHandler be Disconnected (ViewHandler.DisconnectHandler)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-scrollview ScrollView p/1 Work that is critical for the release, but we could probably ship without platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants