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
This commit is contained in:
Joshua Gross
2020-06-13 18:31:41 -07:00
committed by Facebook GitHub Bot
parent ccf5c86bd7
commit 86c38739a7
@@ -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.