From bf7601fde102114eda1144b48d2cfbc8dd7c95c1 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 6 Apr 2018 15:01:19 -0700 Subject: [PATCH] Avoid holding references to ReactShadowNode after a tree is commited Reviewed By: achen1 Differential Revision: D7495721 fbshipit-source-id: 33d5bba5040729f891455a9c330234fe25130b02 --- .../react/fabric/FabricUIManager.java | 3 ++ .../react/uimanager/ReactShadowNodeImpl.java | 35 ++++++------------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 2eda32a0e..50e24738b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -329,6 +329,9 @@ public class FabricUIManager implements UIManager { boolean frameDidChange = node.dispatchUpdates(absoluteX, absoluteY, mUIViewOperationQueue, null); } + // Set the reference to the OriginalReactShadowNode to NULL, as the tree is already committed + // and we do not need to hold references to the previous tree anymore + node.setOriginalReactShadowNode(null); node.markUpdateSeen(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java index 1e92ab478..7d08bd08e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java @@ -131,13 +131,16 @@ public class ReactShadowNodeImpl implements ReactShadowNode mViewClassName = original.mViewClassName; mThemedContext = original.mThemedContext; mShouldNotifyOnLayout = original.mShouldNotifyOnLayout; - mNodeUpdated = original.mNodeUpdated; mIsLayoutOnly = original.mIsLayoutOnly; mNativeParent = original.mNativeParent; - mScreenX = original.mScreenX; - mScreenY = original.mScreenY; - mScreenWidth = original.mScreenWidth; - mScreenHeight = original.mScreenHeight; + // Cloned nodes should be always updated. + mNodeUpdated = true; + // "cached" screen coordinates are not cloned because FabricJS not always clone the last + // ReactShadowNode that was rendered in the screen. + mScreenX = 0; + mScreenY = 0; + mScreenWidth = 0; + mScreenHeight = 0; arraycopy(original.mPadding, 0, mPadding, 0, original.mPadding.length); arraycopy(original.mPaddingIsPercent, 0, mPaddingIsPercent, 0, original.mPaddingIsPercent.length); mNewProps = null; @@ -148,6 +151,7 @@ public class ReactShadowNodeImpl implements ReactShadowNode private void replaceChild(ReactShadowNodeImpl newNode, int childIndex) { mChildren.remove(childIndex); mChildren.add(childIndex, newNode); + newNode.mParent = this; } /** @@ -164,14 +168,7 @@ public class ReactShadowNodeImpl implements ReactShadowNode copy.mNativeChildren = mNativeChildren == null ? null : new ArrayList<>(mNativeChildren); copy.mTotalNativeChildren = mTotalNativeChildren; copy.mChildren = mChildren == null ? null : new ArrayList<>(mChildren); - copy.mYogaNode.setData(this); - if (mChildren != null) { - for (ReactShadowNode child : mChildren) { - if (child.getOriginalReactShadowNode() == null) { - child.setOriginalReactShadowNode(child); - } - } - } + copy.mYogaNode.setData(copy); return copy; } @@ -182,7 +179,7 @@ public class ReactShadowNodeImpl implements ReactShadowNode copy.mNativeChildren = null; copy.mChildren = null; copy.mTotalNativeChildren = 0; - copy.mYogaNode.setData(this); + copy.mYogaNode.setData(copy); return copy; } @@ -306,16 +303,6 @@ public class ReactShadowNodeImpl implements ReactShadowNode + toString() + "')"); } - // TODO: T26729293 This is a temporary code that will be replaced as part of T26729293. - YogaNode parent = childYogaNode.getOwner(); - if (parent != null) { - for (int k = 0; k < parent.getChildCount(); k++) { - if (parent.getChildAt(k) == childYogaNode) { - parent.removeChildAt(k); - break; - } - } - } mYogaNode.addChildAt(childYogaNode, i); } markUpdated();