Improve MountingManager diagnostics around View removal

Summary:
It is possible (most recently, if there are bugs in LayoutAnimations, but also in general) to issue a `removeViewAt` MountItem that removes the incorrect view if, for whatever reason, the View hierarchy has become "corrupt"
and Views are out of order. I added two heuristics to catch if that happens: check the tag of the View being removed if possible, and ensure that all deleted views do not have a parent. This will turn weird visual glitches into
hard crashes, which we want once the UI has become corrupt.

My only concern here is with perf; maybe we could put these behind a debug-only flag or something, but it's probably not a huge deal.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22130186

fbshipit-source-id: 0942b019c3449d68edfb9db1fe8130ea351d1d8f
This commit is contained in:
Joshua Gross 2020-06-19 11:11:08 -07:00 коммит произвёл Facebook GitHub Bot
Родитель 11ed9c2573
Коммит d183fd327b
3 изменённых файлов: 27 добавлений и 3 удалений

Просмотреть файл

@ -224,7 +224,7 @@ public class MountingManager {
}
@UiThread
public void removeViewAt(int parentTag, int index) {
public void removeViewAt(int tag, int parentTag, int index) {
UiThreadUtil.assertOnUiThread();
ViewState viewState = getNullableViewState(parentTag);
@ -244,6 +244,22 @@ public class MountingManager {
ViewGroupManager<ViewGroup> viewGroupManager = getViewGroupManager(viewState);
// Verify that the view we're about to remove has the same tag we expect
if (tag != -1) {
View view = viewGroupManager.getChildAt(parentView, index);
if (view != null && view.getId() != tag) {
throw new IllegalStateException(
"Tried to delete view ["
+ tag
+ "] of parent ["
+ parentTag
+ "] at index "
+ index
+ ", but got view tag "
+ view.getId());
}
}
try {
viewGroupManager.removeViewAt(parentView, index);
} catch (RuntimeException e) {
@ -397,6 +413,13 @@ public class MountingManager {
}
View view = viewState.mView;
ViewParent parentView = view.getParent();
if (parentView != null) {
throw new IllegalStateException(
"Cannot delete view that is still attached to parent: [" + reactTag + "]");
}
if (view != null) {
dropView(view);
} else {

Просмотреть файл

@ -49,8 +49,9 @@ public class RemoveDeleteMultiMountItem implements MountItem {
int flags = mMetadata[i + FLAGS_INDEX];
if ((flags & REMOVE_FLAG) != 0) {
int parentTag = mMetadata[i + PARENT_TAG_INDEX];
int tag = mMetadata[i + TAG_INDEX];
int index = mMetadata[i + VIEW_INDEX_INDEX];
mountingManager.removeViewAt(parentTag, index);
mountingManager.removeViewAt(tag, parentTag, index);
}
}

Просмотреть файл

@ -24,7 +24,7 @@ public class RemoveMountItem implements MountItem {
@Override
public void execute(@NonNull MountingManager mountingManager) {
mountingManager.removeViewAt(mParentReactTag, mIndex);
mountingManager.removeViewAt(-1, mParentReactTag, mIndex);
}
public int getParentReactTag() {