Add conditional return in handleRemoveView

[bartlomiejbloniarz]

Summary:

I was recently working on an issue in Reanimated where z-index of some views was broken after a Layout Animation was used. The issue was that in some cases we were calling the removeView function on a already removed view. On plain Android this wouldn't be an issue, since the removeView function ignores such calls. Unfortunately, the ReactViewGroup.java implementation maintains a counter of views with user defined z-index. This counter is decremented whenever a call to removeView is made, even if the view is not a child of this ViewGroup. This PR adds an additional check in the handleRemoveView function to unify the removeView behavior between Android and react-native.

Changelog:

[ANDROID] [CHANGED] - Changed the handleRemoveView function in ReactViewGroup.java to ignore calls for Views that are not children of this ViewGroup

Test Plan:

I tested if the rn-tester app behaves correctly after those changes.

[javache]

Does this issue only affect the old architecture or does it repro on the new arch too? (zonder is handled differently there)

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	18,012,530	-40,959
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,371,043	-36,878
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 1ea269f
Branch: main

[bartlomiejbloniarz]

I tested it only on the old architecture, but I think this issue should only be present there.

[fabriziocucci]

Hey @bartlomiejbloniarz, thanks for submitting this! 🙂

I was just trying to repro on a physical Android device using rn-tester and your Expo snack but I've encountered a couple of problems:

1. when running react-native run-android --mode HermesDebug I'm hitting this as soon I tap on the Trigger Layout Animation button:

2. when running react-native run-android --mode JscDebug, I can't repro the issue!

NOTE: the react-native-reanimated is the one in the Expo snack, i.e. 3.6.2.

Do you happen to have a branch where you added an exemple in rn-tester for testing purposes?

[bartlomiejbloniarz]

Hi @fabriziocucci. There might have been a small miscommunication on my part. I tested the code from the snack only within our ReanimatedExampleApp. Then I applied those changes in the forked react-native repo and tested if the rn-tester app still behaves correctly (i.e. if views are removed normally).

After some testing I think the issue that you are seeing is caused by the fact that our compatibility with bridgeless is still work in progress, so using reanmiated in the rn-tester app will produce those errors.

[facebook-github-bot]

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@javache merged this pull request in 0d7a92b.