From 6cc597e6e4013ef0fa0bc1d5e0c7df611aef1773 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 May 2018 20:19:03 -0700 Subject: [PATCH] Alternative Instance Handle Approach without JSWeakRef Reviewed By: fkgozali Differential Revision: D8003736 fbshipit-source-id: 597378555cc3f9c0ae95e8927460a3c813ebfb45 --- .../react/fabric/FabricUIManager.java | 14 ++++-- .../react/fabric/jsc/FabricJSCBinding.java | 4 ++ .../react/fabric/jsc/jni/FabricJSCBinding.cpp | 47 ++++++++++++++----- .../react/fabric/jsc/jni/FabricJSCBinding.h | 4 ++ .../react/fabric/FabricUIManagerTest.java | 16 +++---- .../fabric/uimanager/FabricUIManager.cpp | 12 +++-- .../fabric/uimanager/FabricUIManager.h | 8 ++-- 7 files changed, 73 insertions(+), 32 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 c7a539e3e..0eabd1820 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -74,7 +74,7 @@ public class FabricUIManager implements UIManager { /** Creates a new {@link ReactShadowNode} */ @Nullable public ReactShadowNode createNode( - int reactTag, String viewName, int rootTag, ReadableNativeMap props, int instanceHandle) { + int reactTag, String viewName, int rootTag, ReadableNativeMap props, long instanceHandle) { if (DEBUG) { Log.d(TAG, "createNode \n\ttag: " + reactTag + "\n\tviewName: " + viewName + @@ -123,11 +123,12 @@ public class FabricUIManager implements UIManager { * including its children set (note that the children nodes will not be cloned). */ @Nullable - public ReactShadowNode cloneNode(ReactShadowNode node) { + public ReactShadowNode cloneNode(ReactShadowNode node, long instanceHandle) { if (DEBUG) { Log.d(TAG, "cloneNode \n\tnode: " + node); } try { + // TODO: Pass new instanceHandle ReactShadowNode clone = node.mutableCopy(); assertReactShadowNodeCopy(node, clone); return clone; @@ -143,11 +144,12 @@ public class FabricUIManager implements UIManager { * children set will be empty. */ @Nullable - public ReactShadowNode cloneNodeWithNewChildren(ReactShadowNode node) { + public ReactShadowNode cloneNodeWithNewChildren(ReactShadowNode node, long instanceHandle) { if (DEBUG) { Log.d(TAG, "cloneNodeWithNewChildren \n\tnode: " + node); } try { + // TODO: Pass new instanceHandle ReactShadowNode clone = node.mutableCopyWithNewChildren(); assertReactShadowNodeCopy(node, clone); return clone; @@ -164,11 +166,12 @@ public class FabricUIManager implements UIManager { */ @Nullable public ReactShadowNode cloneNodeWithNewProps( - ReactShadowNode node, @Nullable ReadableNativeMap newProps) { + ReactShadowNode node, @Nullable ReadableNativeMap newProps, long instanceHandle) { if (DEBUG) { Log.d(TAG, "cloneNodeWithNewProps \n\tnode: " + node + "\n\tprops: " + newProps); } try { + // TODO: Pass new instanceHandle ReactShadowNode clone = node.mutableCopyWithNewProps(newProps == null ? null : new ReactStylesDiffMap(newProps)); assertReactShadowNodeCopy(node, clone); @@ -187,11 +190,12 @@ public class FabricUIManager implements UIManager { */ @Nullable public ReactShadowNode cloneNodeWithNewChildrenAndProps( - ReactShadowNode node, ReadableNativeMap newProps) { + ReactShadowNode node, ReadableNativeMap newProps, long instanceHandle) { if (DEBUG) { Log.d(TAG, "cloneNodeWithNewChildrenAndProps \n\tnode: " + node + "\n\tnewProps: " + newProps); } try { + // TODO: Pass new instanceHandle ReactShadowNode clone = node.mutableCopyWithNewChildrenAndProps( newProps == null ? null : new ReactStylesDiffMap(newProps)); diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/FabricJSCBinding.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/FabricJSCBinding.java index 8ae05ff65..bc2c8e926 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/FabricJSCBinding.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/FabricJSCBinding.java @@ -27,6 +27,10 @@ public class FabricJSCBinding implements FabricBinding { private static native HybridData initHybrid(); + private native long createEventTarget(long jsContextNativePointer, long instanceHandlePointer); + + private native void releaseEventTarget(long jsContextNativePointer, long eventTargetPointer); + private native void installFabric(long jsContextNativePointer, Object fabricModule); public FabricJSCBinding() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.cpp index 27aac53bb..efc4db619 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.cpp @@ -88,16 +88,16 @@ JSValueRef createNode(JSContextRef ctx, JSObjectRef function, JSObjectRef thisOb static auto createNode = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") - ->getMethod(jint, jstring, jint, ReadableNativeMap::javaobject, jint)>("createNode"); + ->getMethod(jint, jstring, jint, ReadableNativeMap::javaobject, jlong)>("createNode"); int reactTag = (int)JSC_JSValueToNumber(ctx, arguments[0], NULL); auto viewName = JSValueToJString(ctx, arguments[1]); int rootTag = (int)JSC_JSValueToNumber(ctx, arguments[2], NULL); auto props = JSC_JSValueIsNull(ctx, arguments[3]) ? local_ref(nullptr) : JSValueToReadableMapViaJSON(ctx, arguments[3]);; - int instanceHandle = (int)JSC_JSValueToNumber(ctx, arguments[4], NULL); + auto instanceHandle = (void *)arguments[4]; - auto node = createNode(manager, reactTag, viewName.get(), rootTag, props.get(), instanceHandle); + auto node = createNode(manager, reactTag, viewName.get(), rootTag, props.get(), (jlong)instanceHandle); return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(node.get())); } @@ -109,10 +109,11 @@ JSValueRef cloneNode(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObj static auto cloneNode = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") - ->getMethod(JShadowNode::javaobject)>("cloneNode"); + ->getMethod(JShadowNode::javaobject, jlong)>("cloneNode"); auto previousNode = JSValueToJShadowNode(ctx, arguments[0]); - auto newNode = cloneNode(manager, previousNode.get()); + auto instanceHandle = (void *)arguments[1]; + auto newNode = cloneNode(manager, previousNode.get(), (jlong)instanceHandle); return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get())); } @@ -124,10 +125,11 @@ JSValueRef cloneNodeWithNewChildren(JSContextRef ctx, JSObjectRef function, JSOb static auto cloneNodeWithNewChildren = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") - ->getMethod(JShadowNode::javaobject)>("cloneNodeWithNewChildren"); + ->getMethod(JShadowNode::javaobject, jlong)>("cloneNodeWithNewChildren"); auto previousNode = JSValueToJShadowNode(ctx, arguments[0]); - auto newNode = cloneNodeWithNewChildren(manager, previousNode.get()); + auto instanceHandle = (void *)arguments[1]; + auto newNode = cloneNodeWithNewChildren(manager, previousNode.get(), (jlong)instanceHandle); return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get())); } @@ -139,11 +141,12 @@ JSValueRef cloneNodeWithNewProps(JSContextRef ctx, JSObjectRef function, JSObjec static auto cloneNodeWithNewProps = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") - ->getMethod(JShadowNode::javaobject, ReadableNativeMap::javaobject)>("cloneNodeWithNewProps"); + ->getMethod(JShadowNode::javaobject, ReadableNativeMap::javaobject, jlong)>("cloneNodeWithNewProps"); auto previousNode = JSValueToJShadowNode(ctx, arguments[0]); auto props = JSValueToReadableMapViaJSON(ctx, arguments[1]); - auto newNode = cloneNodeWithNewProps(manager, previousNode.get(), props.get()); + auto instanceHandle = (void *)arguments[2]; + auto newNode = cloneNodeWithNewProps(manager, previousNode.get(), props.get(), (jlong)instanceHandle); return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get())); } @@ -155,11 +158,12 @@ JSValueRef cloneNodeWithNewChildrenAndProps(JSContextRef ctx, JSObjectRef functi static auto cloneNodeWithNewChildrenAndProps = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") - ->getMethod(JShadowNode::javaobject, ReadableNativeMap::javaobject)>("cloneNodeWithNewChildrenAndProps"); + ->getMethod(JShadowNode::javaobject, ReadableNativeMap::javaobject, jlong)>("cloneNodeWithNewChildrenAndProps"); auto previousNode = JSValueToJShadowNode(ctx, arguments[0]); auto props = JSValueToReadableMapViaJSON(ctx, arguments[1]); - auto newNode = cloneNodeWithNewChildrenAndProps(manager, previousNode.get(), props.get()); + auto instanceHandle = (void *)arguments[2]; + auto newNode = cloneNodeWithNewChildrenAndProps(manager, previousNode.get(), props.get(), (jlong)instanceHandle); return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get())); } @@ -266,6 +270,27 @@ jni::local_ref FabricJSCBinding::initHybrid( return makeCxxInstance(); } +jlong FabricJSCBinding::createEventTarget( + jlong jsContextNativePointer, + jlong instanceHandlePointer +) { + JSContextRef context = (JSContextRef)jsContextNativePointer; + JSValueRef value = (JSValueRef)instanceHandlePointer; + // Retain a strong reference to this object. + JSC_JSValueProtect(context, value); + return (jlong)((void *)value); +} + +void FabricJSCBinding::releaseEventTarget( + jlong jsContextNativePointer, + jlong eventTargetPointer +) { + JSContextRef context = (JSContextRef)jsContextNativePointer; + JSValueRef value = (JSValueRef)((void *)eventTargetPointer); + // Release this object. + JSC_JSValueUnprotect(context, value); +} + void FabricJSCBinding::installFabric(jlong jsContextNativePointer, jni::alias_ref fabricModule) { JSContextRef context = (JSContextRef)jsContextNativePointer; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.h index 6327ec690..a6b9857b0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.h @@ -26,6 +26,10 @@ private: static jni::local_ref initHybrid(jni::alias_ref); + jlong createEventTarget(jlong jsContextNativePointer, jlong instanceHandlePointer); + + void releaseEventTarget(jlong jsContextNativePointer, jlong eventTargetPointer); + void installFabric(jlong jsContextNativePointer, jni::alias_ref fabricModule); }; diff --git a/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.java b/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.java index 159352c77..69f029361 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.java @@ -97,7 +97,7 @@ public class FabricUIManagerTest { ReactShadowNode child = createViewNode(); node.addChildAt(child, 0); - ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node); + ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node, 0); assertThat(clonedNode).isNotSameAs(node); assertThat(clonedNode.getOriginalReactShadowNode()).isSameAs(node); @@ -111,7 +111,7 @@ public class FabricUIManagerTest { ReactShadowNode node = createViewNode(); node.setDefaultPadding(Spacing.LEFT, 10); - ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node); + ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node, 0); node.setDefaultPadding(Spacing.LEFT, 20); assertThat(clonedNode.getStylePadding(Spacing.LEFT).value).isEqualTo(10f, Offset.offset(0.01f)); @@ -144,7 +144,7 @@ public class FabricUIManagerTest { ReactShadowNode child = createViewNode(); node.addChildAt(child, 0); - ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildren(node); + ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildren(node, 0); assertThat(clonedNode.getChildCount()).isZero(); assertSameFields(clonedNode, node); @@ -155,7 +155,7 @@ public class FabricUIManagerTest { ReactShadowNode node = createViewNode(); ReadableNativeMap props = null; // TODO(ayc): Figure out how to create a Native map from tests. - ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewProps(node, props); + ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewProps(node, props, 0); } @Test @@ -163,7 +163,7 @@ public class FabricUIManagerTest { ReactShadowNode node = createViewNode(); ReadableNativeMap props = null; - ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildrenAndProps(node, props); + ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildrenAndProps(node, props, 0); assertThat(clonedNode.getChildCount()).isZero(); } @@ -260,10 +260,10 @@ public class FabricUIManagerTest { mFabricUIManager.appendChildToSet(childSet, container); mFabricUIManager.completeRoot(rootTag, childSet); - ReactShadowNode aaClone = mFabricUIManager.cloneNodeWithNewProps(aa, null); - ReactShadowNode aClone = mFabricUIManager.cloneNodeWithNewChildren(a); + ReactShadowNode aaClone = mFabricUIManager.cloneNodeWithNewProps(aa, null, 0); + ReactShadowNode aClone = mFabricUIManager.cloneNodeWithNewChildren(a, 0); mFabricUIManager.appendChild(aClone, aaClone); - ReactShadowNode containerClone = mFabricUIManager.cloneNodeWithNewChildren(container); + ReactShadowNode containerClone = mFabricUIManager.cloneNodeWithNewChildren(container, 0); mFabricUIManager.appendChild(containerClone, b); mFabricUIManager.appendChild(containerClone, aClone); List childSet2 = mFabricUIManager.createChildSet(rootTag); diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.cpp b/ReactCommon/fabric/uimanager/FabricUIManager.cpp index da5af6f7a..1e74831ea 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.cpp +++ b/ReactCommon/fabric/uimanager/FabricUIManager.cpp @@ -115,10 +115,11 @@ SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int return shadowNode; } -SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) { +SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode, void *instanceHandle) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNode(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; + // TODO: Retain new instanceHandle SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode(shadowNode); @@ -126,11 +127,12 @@ SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNode &shadowNode) { +SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNode &shadowNode, void *instanceHandle) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildren(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; // Assuming semantic: Cloning with same props but empty children. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; + // TODO: Retain new instanceHandle SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode( shadowNode, @@ -142,12 +144,13 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNod return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode &shadowNode, folly::dynamic props) { +SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode &shadowNode, folly::dynamic props, void *instanceHandle) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNodeWithNewProps(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ", props: " << props << ")"; // Assuming semantic: Cloning with same children and specified props. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; RawProps rawProps = rawPropsFromDynamic(props); + // TODO: Retain new instanceHandle SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode( shadowNode, @@ -159,12 +162,13 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode & return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedShadowNode &shadowNode, folly::dynamic props) { +SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedShadowNode &shadowNode, folly::dynamic props, void *instanceHandle) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildrenAndProps(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ", props: " << props << ")"; // Assuming semantic: Cloning with empty children and specified props. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; RawProps rawProps = rawPropsFromDynamic(props); + // TODO: Retain new instanceHandle SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode( shadowNode, diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.h b/ReactCommon/fabric/uimanager/FabricUIManager.h index c2b3dea9b..1fa11b521 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.h +++ b/ReactCommon/fabric/uimanager/FabricUIManager.h @@ -36,10 +36,10 @@ public: #pragma mark - JavaScript/React-facing Interface SharedShadowNode createNode(Tag reactTag, std::string viewName, Tag rootTag, folly::dynamic props, void *instanceHandle); - SharedShadowNode cloneNode(const SharedShadowNode &node); - SharedShadowNode cloneNodeWithNewChildren(const SharedShadowNode &node); - SharedShadowNode cloneNodeWithNewProps(const SharedShadowNode &node, folly::dynamic props); - SharedShadowNode cloneNodeWithNewChildrenAndProps(const SharedShadowNode &node, folly::dynamic newProps); + SharedShadowNode cloneNode(const SharedShadowNode &node, void *instanceHandle); + SharedShadowNode cloneNodeWithNewChildren(const SharedShadowNode &node, void *instanceHandle); + SharedShadowNode cloneNodeWithNewProps(const SharedShadowNode &node, folly::dynamic props, void *instanceHandle); + SharedShadowNode cloneNodeWithNewChildrenAndProps(const SharedShadowNode &node, folly::dynamic newProps, void *instanceHandle); void appendChild(const SharedShadowNode &parentNode, const SharedShadowNode &childNode); SharedShadowNodeUnsharedList createChildSet(Tag rootTag); void appendChildToSet(const SharedShadowNodeUnsharedList &childSet, const SharedShadowNode &childNode);