fix(android): crash while drawing in dispatchGetDisplayList

[hannojg]

Description

Fixes #3321

Changes

The applyTransformation callback is called while we are drawing in android, as can be seen in this stack:

The event we dispatch might be intercepted by reanimated which will cause to synchronously execute a mount transaction, which can alter the native view hierarchy, causing the mentioned issue.

The fix here is to move the dispatching off of the drawing stack, and post to the next queue spot.

I think mFragment.view is always present? If not we can also use something like UIThreadUtils if thats preferred.

Test code and steps to reproduce

/

Checklist

• Included code example that can be used to test this change
• Updated TS types
• Updated documentation:
  • https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
  • https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
  • https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
  • https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
• Ensured that CI passes

[hannojg]

I am wondering if we are worried that animations could be delayed by one frame when using reanimated screens, but i am not sure if thats really a problem here?

[hannojg]

An alternative would be to shift the responsibility to either reanimated or rn-core to check to only mount if we are not drawing currently.
But then i think in android we want to avoid changing anything in the native view hierarchy while we are drawing, so it seems fair that this code here should be explicit about it?

[RodolfoSilva]

LGTM!

[kkafar]

I'm rather reluctant to land this change in screens.

This is an event sent to JS asynchronously and meant to be consumed in JS.

The fact that reanimated could hijack it & run sync commit w/ mounting is reanimated implementation detail / operation model & I don't think this lib should rely / adjust to it, especially that likely this can be handled on the other side.

I'll talk this through with reanimated guys.

[RodolfoSilva]

I totally agree with you @kkafar, just wanted to let you know this PR fixes the issue for me. But I'd also appreciate it if this could be solved on the Reanimated side.

[kkafar]

Initial conclusions are that we'll want to investigate it further, before we settle on any solution. Will be back once we have some decisions.

[RodolfoSilva]

@kkafar this patch fix the issue in the reanimated package:

software-mansion/react-native-reanimated#8422 (comment)

[kkafar]

I'm doing something entirely different atm., but it has just crossed my mind that potentially posting the onTransitionProgress event might lead to situation, where onTransitionProgress is delivered after onTransitionEnd. Haven't confirmed it by testing yet, just noting a thought experiment.

This makes me even more reluctant to land it.