From 86c38739a71a33b8d5bd55f679a7299e79858960 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Sat, 13 Jun 2020 18:29:30 -0700 Subject: [PATCH] Avoid using ViewManager childCount to detect errors, since it's not reliable Summary: Similar to D21756178, we cannot rely on childCount since it can return zero when there are actually valid children. This both causes more exceptions than necessary when the operation would work, and pollutes error messages since the information is not strictly reliable. Instead, we just try to get a View and thrown an exception when it's null, or in loops, loop until we hit a null child. `getChildAt` doesn't throw exceptions, it just returns null when we're out-of-bounds. This can impact custom ViewGroups like BottomSheets, and other ViewGroups that might do interesting/weird things with children, including ReactClippingViewManager. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22035569 fbshipit-source-id: 43e98d81178faaf720face98fc84e78743f292c3 --- .../react/uimanager/NativeViewHierarchyManager.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index 749202eeaad..a5e9e13ace4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -284,11 +284,16 @@ public class NativeViewHierarchyManager { StringBuilder stringBuilder = new StringBuilder(); if (null != viewToManage) { - stringBuilder.append("View tag:" + viewToManage.getId() + "\n"); + stringBuilder.append( + "View tag:" + + viewToManage.getId() + + " View Type:" + + viewToManage.getClass().toString() + + "\n"); stringBuilder.append(" children(" + viewManager.getChildCount(viewToManage) + "): [\n"); - for (int index = 0; index < viewManager.getChildCount(viewToManage); index += 16) { + for (int index = 0; viewManager.getChildAt(viewToManage, index) != null; index += 16) { for (int innerOffset = 0; - ((index + innerOffset) < viewManager.getChildCount(viewToManage)) && innerOffset < 16; + viewManager.getChildAt(viewToManage, index + innerOffset) != null && innerOffset < 16; innerOffset++) { stringBuilder.append( viewManager.getChildAt(viewToManage, index + innerOffset).getId() + ","); @@ -396,7 +401,7 @@ public class NativeViewHierarchyManager { + constructManageChildrenErrorMessage( viewToManage, viewManager, indicesToRemove, viewsToAdd, tagsToDelete)); } - if (indexToRemove >= viewManager.getChildCount(viewToManage)) { + if (viewManager.getChildAt(viewToManage, indexToRemove) == null) { if (mRootTags.get(tag) && viewManager.getChildCount(viewToManage) == 0) { // This root node has already been removed (likely due to a threading issue caused by // async js execution). Ignore this root removal.