From fc10d0d3bf5f585f3923f7a0be1f2f64eb22547b Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Tue, 27 Sep 2016 13:47:57 -0700 Subject: [PATCH] Fix modals freezing on close with Nodes Summary: Modals were doing nothing (and sometimes crashing) when they were being closed. The reason for this was due to the fact that the parent being removed was not necessarily the view's parent. Consequently, trying to inform said parent that its child was removed failed, because said parent wasn't a view, and therefore had no record in mViewsToTags. Reviewed By: sriramramani Differential Revision: D3928850 --- .../react/flat/FlatUIImplementation.java | 29 +++++++++++++++---- .../react/flat/FlatUIViewOperationQueue.java | 16 +++++++--- .../com/facebook/react/flat/StateBuilder.java | 12 +++++--- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java index fb2c7cd3c..5363a3128 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIImplementation.java @@ -342,9 +342,7 @@ public class FlatUIImplementation extends UIImplementation { --moveFromIndex; moveFromChildIndex = (moveFromIndex == -1) ? -1 : mMoveProxy.getMoveFrom(moveFromIndex); } else if (removeFromChildIndex > moveFromChildIndex) { - removeChild( - removeChildAt(parentNode, removeFromChildIndex, prevIndex), - parentNode.getReactTag()); + removeChild(removeChildAt(parentNode, removeFromChildIndex, prevIndex), parentNode); prevIndex = removeFromChildIndex; --removeFromIndex; @@ -361,19 +359,38 @@ public class FlatUIImplementation extends UIImplementation { * Unregisters given element and all of its children from ShadowNodeRegistry, * and drops all Views used by it and its children. */ - private void removeChild(ReactShadowNode child, int parentReactTag) { + private void removeChild(ReactShadowNode child, ReactShadowNode parentNode) { if (child instanceof FlatShadowNode) { FlatShadowNode node = (FlatShadowNode) child; if (node.mountsToView() && node.isBackingViewCreated()) { + int tag = -1; + + // this tag is used to remove the reference to this dropping view if it it's clipped. + // we need to figure out the correct "view parent" tag to do this. note that this is + // not necessarily getParent().getReactTag(), since getParent() may represent something + // that's not a View - we need to find the first View (what would represent + // view.getParent() on the ui thread), which is what this code is finding. + ReactShadowNode tmpNode = parentNode; + while (tmpNode != null) { + if (tmpNode instanceof FlatShadowNode) { + FlatShadowNode flatTmpNode = (FlatShadowNode) tmpNode; + if (flatTmpNode.mountsToView() && flatTmpNode.isBackingViewCreated()) { + tag = flatTmpNode.getReactTag(); + break; + } + } + tmpNode = tmpNode.getParent(); + } + // this will recursively drop all subviews - mStateBuilder.dropView(node, parentReactTag); + mStateBuilder.dropView(node, tag); removeShadowNode(node); return; } } for (int i = 0, childCount = child.getChildCount(); i != childCount; ++i) { - removeChild(child.getChildAt(i), child.getReactTag()); + removeChild(child.getChildAt(i), child); } removeShadowNode(child); diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java index a0772e450..fe904ae4b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java @@ -11,6 +11,8 @@ package com.facebook.react.flat; import javax.annotation.Nullable; +import java.util.ArrayList; + import android.util.SparseIntArray; import android.view.View; import android.view.ViewGroup; @@ -207,8 +209,12 @@ import com.facebook.react.uimanager.UIViewOperationQueue; private final SparseIntArray mViewsToDrop; - private DropViews(SparseIntArray viewsToDrop) { - mViewsToDrop = viewsToDrop; + private DropViews(ArrayList viewsToDrop, ArrayList parentsForViewsToDrop) { + SparseIntArray sparseIntArray = new SparseIntArray(); + for (int i = 0, count = viewsToDrop.size(); i < count; i++) { + sparseIntArray.put(viewsToDrop.get(i), parentsForViewsToDrop.get(i)); + } + mViewsToDrop = sparseIntArray; } @Override @@ -478,8 +484,10 @@ import com.facebook.react.uimanager.UIViewOperationQueue; new SetPadding(reactTag, paddingLeft, paddingTop, paddingRight, paddingBottom)); } - public void enqueueDropViews(SparseIntArray viewsToDrop) { - enqueueUIOperation(new DropViews(viewsToDrop)); + public void enqueueDropViews( + ArrayList viewsToDrop, + ArrayList parentsOfViewsToDrop) { + enqueueUIOperation(new DropViews(viewsToDrop, parentsOfViewsToDrop)); } public void enqueueMeasureVirtualView( diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java index 0351cd81f..200f0d525 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -51,7 +51,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; private final ArrayList mViewsToDetachAllChildrenFrom = new ArrayList<>(); private final ArrayList mViewsToDetach = new ArrayList<>(); - private final SparseIntArray mViewsToDrop = new SparseIntArray(); + private final ArrayList mViewsToDrop = new ArrayList<>(); + private final ArrayList mParentsForViewsToDrop = new ArrayList<>(); private final ArrayList mOnLayoutEvents = new ArrayList<>(); private final ArrayList mUpdateViewBoundsOperations = new ArrayList<>(); @@ -132,8 +133,9 @@ import com.facebook.react.uimanager.events.EventDispatcher; mOnLayoutEvents.clear(); if (mViewsToDrop.size() > 0) { - mOperationsQueue.enqueueDropViews(mViewsToDrop); + mOperationsQueue.enqueueDropViews(mViewsToDrop, mParentsForViewsToDrop); mViewsToDrop.clear(); + mParentsForViewsToDrop.clear(); } mOperationsQueue.enqueueProcessLayoutRequests(); @@ -141,7 +143,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; /* package */ void removeRootView(int rootViewTag) { // Note root view tags with a negative value. - mViewsToDrop.put(-rootViewTag, -1); + mViewsToDrop.add(-rootViewTag); + mParentsForViewsToDrop.add(-1); } /** @@ -229,7 +232,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; * @param node The node to drop the backing view for. */ /* package */ void dropView(FlatShadowNode node, int parentReactTag) { - mViewsToDrop.put(node.getReactTag(), parentReactTag); + mViewsToDrop.add(node.getReactTag()); + mParentsForViewsToDrop.add(parentReactTag); } /**