Don't try to remove Views that about to be removed with the root View

Summary:
We keep a list of FlatShadowNodes that mount to Views that we want to delete, and only flush it at the end of an update cycle. This results in a situation where a root view is being removed before children are removed, which results in a crash in NativeViewHierarchyManager because a View that we are trying to remove no longer exists. There are a few approaches to fix the issue:
a) make a check if a View exists before removing it. While works, it removes a bug protection when we erroneously trying to remove a View that no longer exists (such as this). I'd prefer to keep the check in place
b) flush the views-to-drop queue. This works, but does some extra work in UI thread (namely, removing Views that would be removed anyway)
c) trim the views-to-drop queue to remove any Views that will be removed anyway. This does a tiny bit of extra work in BG thread, but less work in UI thread.

This diff implements option c).

Reviewed By: ahmedre

Differential Revision: D2990105
This commit is contained in:
Denis Koroskin 2016-02-29 19:27:09 -08:00 committed by Ahmed El-Helw
parent 75117fc91a
commit 0b560d3d3d
2 changed files with 15 additions and 0 deletions

View File

@ -389,6 +389,12 @@ public class FlatUIImplementation extends UIImplementation {
mStateBuilder.applyUpdates(eventDispatcher, (FlatRootShadowNode) cssNode);
}
@Override
public void removeRootView(int rootViewTag) {
mStateBuilder.removeRootView(rootViewTag);
super.removeRootView(rootViewTag);
}
@Override
public void setJSResponder(int possiblyVirtualReactTag, boolean blockNativeResponder) {
ReactShadowNode node = resolveShadowNode(possiblyVirtualReactTag);

View File

@ -116,6 +116,15 @@ import com.facebook.react.uimanager.events.EventDispatcher;
mOperationsQueue.enqueueProcessLayoutRequests();
}
/* package */ void removeRootView(int rootViewTag) {
// Don't remove Views that are connected to a View that we are about to remove.
for (int i = mViewsToDrop.size() - 1; i >= 0; --i) {
if (mViewsToDrop.get(i).getRootNode().getReactTag() == rootViewTag) {
mViewsToDrop.remove(i);
}
}
}
/**
* Adds a DrawCommand for current mountable node.
*/