Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).
Apache License 2.0
1.36k stars 76 forks source link

Crash due to activity.onBackPressed() when removing fragment hosting a WebView in AndroidView {} + FADE #280

Closed uvaysss closed 11 months ago

uvaysss commented 11 months ago

Recently updated the target/compile sdk to 33 and did get some weird navigation issues. When I try to call requireActivity().onBackPressed() from a Fragment my app freezes for some time and closes. And the weirdest thing is that this happens only with FragmentTransaction.TRANSIT_FRAGMENT_FADE.

I'm overriding methods of DefaultFragmentStateChanger onForwardNavigation, onBackwardNavigation, onReplaceNavigation.

uvaysss commented 11 months ago

Also I tried updating to the last recommendation with onBackPressedDispatcher, but nothing has changed, looks like the animation is to long (weird, I know), but looks like a race condition of animation.

Zhuinden commented 11 months ago

Hello, unfortunately I can't really do much without a minimal sample that reproduces this. TRANSIT_FRAGMENT_FADE is built into fragments, and shouldn't cause freezes at all.

If you overrode onForwardNavigation, onBackwardNavigation, onReplaceNavigation, make sure you don't call super.onForwardNavigation and so on.

uvaysss commented 11 months ago

Could you, please, just set this animation to any of your project to reproduce it? I will later prepare some sample project for this use case.

Zhuinden commented 11 months ago

I have been using this animation and did not encounter this issue.

Zhuinden commented 11 months ago

Actually, now that I recall, the one time I did have some lag was when my fragmentKey.toString() took non-negligible time.

uvaysss commented 11 months ago

Sorry for the delay.

I think I found the pattern, it looks like it happens when I navigate back from a "heavy" screen, in my case it's a screen with webview. And it doesn't happen consistent, sometimes the navigation works fine.

I have an open source project where you can reproduce the bug - https://github.com/Jawziyya/azkar-android/tree/feature/hadith.

Sorry, but the app is only in russian language, but you will find a Settings menu on the main screen, it will open a webview from where you can than than navigate back with close button. Here is the settings screen - https://github.com/Jawziyya/azkar-android/tree/feature/hadith/app/src/main/java/io/jawziyya/azkar/ui/settings.

Also I have noticed it happens only when I navigate back through calling requireActivity().onBackPressed(), when I go back by the system navigation it's fine, weird.

uvaysss commented 11 months ago

Also I've notice now, that it look more like app crash, but I don't see any logs about it, maybe there is some system crash.

EDIT. found in the logs

Exception thrown during dispatchAppVisibility Window{a79cf22 u0 io.jawziyya.azkar/io.jawziyya.azkar.ui.AppActivity EXITING} android.os.DeadObjectException

Zhuinden commented 11 months ago

Your coroutine scope should be protected val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)

And you are not setting the disposition strategy on the Fragment's ComposeView setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)

Maybe one of these is the cause

uvaysss commented 11 months ago

I have there an extensions function, that sets the strategy createComposeView

uvaysss commented 11 months ago

Also I don't see a problem not having the right dispatcher, because in this case I'm not calling it.

Zhuinden commented 11 months ago

I enabled hardware acceleration on your WebView.

            webView.clipToOutline = true
            webView.setLayerType(View.LAYER_TYPE_HARDWARE, null) // <----
            webView.layoutParams = ViewGroup.LayoutParams(
                ViewGroup.LayoutParams.MATCH_PARENT,
                ViewGroup.LayoutParams.MATCH_PARENT
            )

By adding `webView.setLayerType(View.LAYER_TYPE_HARDWARE, null) it works.

I don't know why it happened, but it is not caused by Simple-Stack. Anyway, if you add that line, it will stop crashing.

uvaysss commented 11 months ago

This is not definitely a simple stack issue.

The funny thing is, that I noticed it also happens when I call requireActivity()::onBackPressed from the material component IconButton onclick, if I use an another custom button with clickable modifier it works fine.. Also I checked if it's called maybe twice, but no, the log show only 1 call of the method..

Zhuinden commented 11 months ago

This is not definitely a simple stack issue.

The funny thing is, that I noticed it also happens when I call requireActivity()::onBackPressed from the material component IconButton onclick, if I use an another custom button with clickable modifier it works fine.. Also I checked if it's called maybe twice, but no, the log show only 1 call of the method..

well as I mentioned, if you add hardware acceleration to the WebView, it stops crashing

I found that this also happened in React Native projects for some reason. One of the fixes said "set 0.99 alpha" which is just funny.

uvaysss commented 11 months ago

That's so weird.

Anyway, thank you for help and quick response!

uvaysss commented 11 months ago

I found that by removing webView.clipToOutline = true fixes the issue too. I think that's very weird that this things are somehow connected, but still they should kept in mind.

I've just recently added this clip function call, because of https://issuetracker.google.com/issues/242463987.

I don't know which solution would be best, but I will probably choose not to use the material component and just add some custom AppBar to solve this bug.