Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix crash in removeViewAt by fixing incorrect error-detection

Browse Source 
Summary:
As a followup to D21750097 and D21735940, it seems that ANY uses of childCount will be incorrect as they are "often" reported, incorrectly, as just 0.

This is likely due to exotic cases like (1) the ViewManager childCount being overridden, or (2) special ViewManagers like BottomSheet where the childCount may actually be 1/0 even though many children are inserted.

Anyway, as a more generic fix, let's only rethrow an existing Exception with additional diagnostics instead of trying to detect when this /would/ crash.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D21756178

fbshipit-source-id: 17ffb2ed531978bae8d4db19d7b87ec62397e44b
This commit is contained in:
Joshua Gross
2020-05-27 22:53:31 -07:00
committed by Facebook GitHub Bot
parent 7e559465bd
commit 8da92d746b
1 changed files with 23 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java
+23 -5
View File
@@ -244,18 +244,36 @@ public class MountingManager {
ViewGroupManager<ViewGroup> viewGroupManager = getViewGroupManager(viewState);
if (viewGroupManager.getChildCount(parentView) <= index) {
try {
viewGroupManager.removeViewAt(parentView, index);
} catch (RuntimeException e) {
// Note: `getChildCount` may not always be accurate!
// We don't currently have a good explanation other than, in situations where you
// would empirically expect to see childCount > 0, the childCount is reported as 0.
// This is likely due to a ViewManager overriding getChildCount or some other methods
// in a way that is strictly incorrect, but potentially only visible here.
// The failure mode is actually that in `removeViewAt`, a NullPointerException is
// thrown when we try to perform an operation on a View that doesn't exist, and
// is therefore null.
// We try to add some extra diagnostics here, but we always try to remove the View
// from the hierarchy first because detecting by looking at childCount will not work.
//
// Note that the lesson here is that `getChildCount` is not /required/ to adhere to
// any invariants. If you add 9 children to a parent, the `getChildCount` of the parent
// may not be equal to 9. This apparently causes no issues with Android and is common
// enough that we shouldn't try to change this invariant, without a lot of thought.
int childCount = viewGroupManager.getChildCount(parentView);
throw new IllegalStateException(
"Cannot remove child at index "
+ index
+ " from parent ViewGroup ["
+ parentView.getId()
+ "], only "
+ parentView.getChildCount()
+ " children in parent");
+ childCount
+ " children in parent. Warning: childCount may be incorrect!",
e);
}
viewGroupManager.removeViewAt(parentView, index);
}
@UiThread