Remove instanceHandle, pass event target instead + add dispatchToEmptyTarget

Summary:
Removes the concept of instance handle. Instead we pass the event target
to createNode and don't pass it to subsequent clones.

The life time of the event target is managed by native (the event emitter).
It has to be released manually.

Reviewed By: shergin

Differential Revision: D8688330

fbshipit-source-id: e11b61f147ea9ca4dfb453fe07063ed06f24b7ac
This commit is contained in:
Sebastian Markbage
2018-06-29 15:30:30 -07:00
committed by Facebook Github Bot
parent 7b1bd4d4f3
commit 5d9326be29
16 changed files with 134 additions and 142 deletions

View File

@@ -14,12 +14,17 @@ public interface FabricBinding {
void installFabric(JavaScriptContextHolder jsContext, FabricUIManager fabricModule);
long createEventTarget(long jsContextNativePointer, long instanceHandlePointer);
void releaseEventTarget(long jsContextNativePointer, long eventTargetPointer);
void releaseEventHandler(long jsContextNativePointer, long eventHandlerPointer);
void dispatchEventToEmptyTarget(
long jsContextNativePointer,
long eventHandlerPointer,
String type,
NativeMap payload
);
void dispatchEventToTarget(
long jsContextNativePointer,
long eventHandlerPointer,

View File

@@ -106,7 +106,7 @@ public class FabricUIManager implements UIManager, JSHandler {
@Nullable
@DoNotStrip
public ReactShadowNode createNode(
int reactTag, String viewName, int rootTag, ReadableNativeMap props, long instanceHandle) {
int reactTag, String viewName, int rootTag, ReadableNativeMap props, long eventTarget) {
if (DEBUG) {
FLog.d(TAG, "createNode \n\ttag: " + reactTag +
"\n\tviewName: " + viewName +
@@ -119,7 +119,7 @@ public class FabricUIManager implements UIManager, JSHandler {
ReactShadowNode rootNode = getRootNode(rootTag);
node.setRootTag(rootNode.getReactTag());
node.setViewClassName(viewName);
node.setInstanceHandle(instanceHandle);
node.setInstanceHandle(eventTarget);
node.setReactTag(reactTag);
node.setThemedContext(rootNode.getThemedContext());
@@ -157,7 +157,7 @@ public class FabricUIManager implements UIManager, JSHandler {
*/
@Nullable
@DoNotStrip
public ReactShadowNode cloneNode(ReactShadowNode node, long instanceHandle) {
public ReactShadowNode cloneNode(ReactShadowNode node) {
if (DEBUG) {
FLog.d(TAG, "cloneNode \n\tnode: " + node);
}
@@ -166,7 +166,7 @@ public class FabricUIManager implements UIManager, JSHandler {
"FabricUIManager.cloneNode")
.flush();
try {
ReactShadowNode clone = node.mutableCopy(instanceHandle);
ReactShadowNode clone = node.mutableCopy(node.getInstanceHandle());
assertReactShadowNodeCopy(node, clone);
return clone;
} catch (Throwable t) {
@@ -184,7 +184,7 @@ public class FabricUIManager implements UIManager, JSHandler {
*/
@Nullable
@DoNotStrip
public ReactShadowNode cloneNodeWithNewChildren(ReactShadowNode node, long instanceHandle) {
public ReactShadowNode cloneNodeWithNewChildren(ReactShadowNode node) {
if (DEBUG) {
FLog.d(TAG, "cloneNodeWithNewChildren \n\tnode: " + node);
}
@@ -193,7 +193,7 @@ public class FabricUIManager implements UIManager, JSHandler {
"FabricUIManager.cloneNodeWithNewChildren")
.flush();
try {
ReactShadowNode clone = node.mutableCopyWithNewChildren(instanceHandle);
ReactShadowNode clone = node.mutableCopyWithNewChildren(node.getInstanceHandle());
assertReactShadowNodeCopy(node, clone);
return clone;
} catch (Throwable t) {
@@ -212,7 +212,7 @@ public class FabricUIManager implements UIManager, JSHandler {
@Nullable
@DoNotStrip
public ReactShadowNode cloneNodeWithNewProps(
ReactShadowNode node, @Nullable ReadableNativeMap newProps, long instanceHandle) {
ReactShadowNode node, @Nullable ReadableNativeMap newProps) {
if (DEBUG) {
FLog.d(TAG, "cloneNodeWithNewProps \n\tnode: " + node + "\n\tprops: " + newProps);
}
@@ -221,7 +221,7 @@ public class FabricUIManager implements UIManager, JSHandler {
"FabricUIManager.cloneNodeWithNewProps")
.flush();
try {
ReactShadowNode clone = node.mutableCopyWithNewProps(instanceHandle,
ReactShadowNode clone = node.mutableCopyWithNewProps(node.getInstanceHandle(),
newProps == null ? null : new ReactStylesDiffMap(newProps));
assertReactShadowNodeCopy(node, clone);
return clone;
@@ -242,7 +242,7 @@ public class FabricUIManager implements UIManager, JSHandler {
@Nullable
@DoNotStrip
public ReactShadowNode cloneNodeWithNewChildrenAndProps(
ReactShadowNode node, ReadableNativeMap newProps, long instanceHandle) {
ReactShadowNode node, ReadableNativeMap newProps) {
if (DEBUG) {
FLog.d(TAG, "cloneNodeWithNewChildrenAndProps \n\tnode: " + node + "\n\tnewProps: " + newProps);
}
@@ -252,7 +252,7 @@ public class FabricUIManager implements UIManager, JSHandler {
.flush();
try {
ReactShadowNode clone =
node.mutableCopyWithNewChildrenAndProps(instanceHandle,
node.mutableCopyWithNewChildrenAndProps(node.getInstanceHandle(),
newProps == null ? null : new ReactStylesDiffMap(newProps));
assertReactShadowNodeCopy(node, clone);
return clone;
@@ -610,16 +610,9 @@ public class FabricUIManager implements UIManager, JSHandler {
@Nullable
@DoNotStrip
public long createEventTarget(int reactTag) {
public long getEventTarget(int reactTag) {
long instanceHandle = mNativeViewHierarchyManager.getInstanceHandle(reactTag);
long context = mJSContext.get();
long eventTarget = mBinding.createEventTarget(context, instanceHandle);
if (DEBUG) {
FLog.d(
TAG,
"Created EventTarget: " + eventTarget + " for tag: " + reactTag + " with instanceHandle: " + instanceHandle);
}
return eventTarget;
return instanceHandle;
}
@DoNotStrip

View File

@@ -49,7 +49,7 @@ public class FabricEventEmitter implements RCTEventEmitter, Closeable {
@Override
public void receiveEvent(int reactTag, String eventName, @Nullable WritableMap params) {
try {
long eventTarget = mFabricUIManager.createEventTarget(reactTag);
long eventTarget = mFabricUIManager.getEventTarget(reactTag);
mScheduler.scheduleWork(new FabricUIManagerWork(eventTarget, eventName, params));
} catch (IllegalViewOperationException e) {
FLog.e(TAG, "Unable to emmit event for tag " + reactTag, e);
@@ -80,7 +80,10 @@ public class FabricEventEmitter implements RCTEventEmitter, Closeable {
FLog.e(TAG, "Error sending event " + mEventName, t);
//TODO: manage exception properly
} finally{
mFabricUIManager.releaseEventTarget(mEventTarget);
// TODO(dvacca): We need to only release this after all shadow nodes
// have been released. The easiest way would be to adopt the event
// emitter approach from the C++ Fabric. For now, we'll just leak.
// mFabricUIManager.releaseEventTarget(mEventTarget);
}
}
}

View File

@@ -28,15 +28,20 @@ public class FabricJSCBinding implements FabricBinding {
private static native HybridData initHybrid();
@Override
public native long createEventTarget(long jsContextNativePointer, long instanceHandlePointer);
@Override
public native void releaseEventTarget(long jsContextNativePointer, long eventTargetPointer);
@Override
public native void releaseEventHandler(long jsContextNativePointer, long eventHandlerPointer);
@Override
public native void dispatchEventToEmptyTarget(
long jsContextNativePointer,
long eventHandlerPointer,
String type,
NativeMap payload
);
@Override
public native void dispatchEventToTarget(
long jsContextNativePointer,

View File

@@ -103,9 +103,9 @@ JSValueRef createNode(JSContextRef ctx, JSObjectRef function, JSObjectRef thisOb
int rootTag = (int)JSC_JSValueToNumber(ctx, arguments[2], NULL);
auto props = JSC_JSValueIsNull(ctx, arguments[3]) ? local_ref<ReadableNativeMap::jhybridobject>(nullptr) :
JSValueToReadableMapViaJSON(ctx, arguments[3]);;
auto instanceHandle = (void *)arguments[4];
auto eventTarget = (void *)arguments[4];
auto node = createNode(manager, reactTag, viewName.get(), rootTag, props.get(), (jlong)instanceHandle);
auto node = createNode(manager, reactTag, viewName.get(), rootTag, props.get(), (jlong)eventTarget);
return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(node.get()));
}
@@ -117,11 +117,10 @@ JSValueRef cloneNode(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObj
static auto cloneNode =
jni::findClassStatic("com/facebook/react/fabric/FabricUIManager")
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject, jlong)>("cloneNode");
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject)>("cloneNode");
auto previousNode = JSValueToJShadowNode(ctx, arguments[0]);
auto instanceHandle = (void *)arguments[1];
auto newNode = cloneNode(manager, previousNode.get(), (jlong)instanceHandle);
auto newNode = cloneNode(manager, previousNode.get());
return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get()));
}
@@ -133,11 +132,10 @@ JSValueRef cloneNodeWithNewChildren(JSContextRef ctx, JSObjectRef function, JSOb
static auto cloneNodeWithNewChildren =
jni::findClassStatic("com/facebook/react/fabric/FabricUIManager")
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject, jlong)>("cloneNodeWithNewChildren");
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject)>("cloneNodeWithNewChildren");
auto previousNode = JSValueToJShadowNode(ctx, arguments[0]);
auto instanceHandle = (void *)arguments[1];
auto newNode = cloneNodeWithNewChildren(manager, previousNode.get(), (jlong)instanceHandle);
auto newNode = cloneNodeWithNewChildren(manager, previousNode.get());
return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get()));
}
@@ -149,12 +147,11 @@ JSValueRef cloneNodeWithNewProps(JSContextRef ctx, JSObjectRef function, JSObjec
static auto cloneNodeWithNewProps =
jni::findClassStatic("com/facebook/react/fabric/FabricUIManager")
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject, ReadableNativeMap::javaobject, jlong)>("cloneNodeWithNewProps");
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject, ReadableNativeMap::javaobject)>("cloneNodeWithNewProps");
auto previousNode = JSValueToJShadowNode(ctx, arguments[0]);
auto props = JSValueToReadableMapViaJSON(ctx, arguments[1]);
auto instanceHandle = (void *)arguments[2];
auto newNode = cloneNodeWithNewProps(manager, previousNode.get(), props.get(), (jlong)instanceHandle);
auto newNode = cloneNodeWithNewProps(manager, previousNode.get(), props.get());
return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get()));
}
@@ -166,12 +163,11 @@ JSValueRef cloneNodeWithNewChildrenAndProps(JSContextRef ctx, JSObjectRef functi
static auto cloneNodeWithNewChildrenAndProps =
jni::findClassStatic("com/facebook/react/fabric/FabricUIManager")
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject, ReadableNativeMap::javaobject, jlong)>("cloneNodeWithNewChildrenAndProps");
->getMethod<alias_ref<JShadowNode>(JShadowNode::javaobject, ReadableNativeMap::javaobject)>("cloneNodeWithNewChildrenAndProps");
auto previousNode = JSValueToJShadowNode(ctx, arguments[0]);
auto props = JSValueToReadableMapViaJSON(ctx, arguments[1]);
auto instanceHandle = (void *)arguments[2];
auto newNode = cloneNodeWithNewChildrenAndProps(manager, previousNode.get(), props.get(), (jlong)instanceHandle);
auto newNode = cloneNodeWithNewChildrenAndProps(manager, previousNode.get(), props.get());
return JSC_JSObjectMake(ctx, classRef, makePlainGlobalRef(newNode.get()));
}
@@ -293,25 +289,11 @@ jni::local_ref<FabricJSCBinding::jhybriddata> 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);
// This is now a noop.
}
void FabricJSCBinding::releaseEventHandler(
@@ -324,6 +306,36 @@ void FabricJSCBinding::releaseEventHandler(
JSC_JSValueUnprotect(context, value);
}
void FabricJSCBinding::dispatchEventToEmptyTarget(
jlong jsContextNativePointer,
jlong eventHandlerPointer,
std::string type,
NativeMap *payloadMap
) {
JSContextRef context = (JSContextRef)jsContextNativePointer;
JSObjectRef eventHandler = (JSObjectRef)((void *)eventHandlerPointer);
JSValueRef eventTarget = JSC_JSValueMakeNull(context);
JSObjectRef thisArg = (JSObjectRef)JSC_JSValueMakeUndefined(context);
JSStringRef typeStr = JSC_JSStringCreateWithUTF8CString(context, type.c_str());
JSValueRef typeRef = JSC_JSValueMakeString(context, typeStr);
JSC_JSStringRelease(context, typeStr);
JSValueRef payloadRef = ReadableMapToJSValueViaJSON(context, payloadMap);
JSValueRef args[] = {eventTarget, typeRef, payloadRef};
JSValueRef exn;
JSValueRef result = JSC_JSObjectCallAsFunction(
context,
eventHandler,
thisArg,
3,
args,
&exn
);
if (!result) {
// TODO: Handle error in exn
}
}
void FabricJSCBinding::dispatchEventToTarget(
jlong jsContextNativePointer,
jlong eventHandlerPointer,
@@ -392,9 +404,9 @@ void FabricJSCBinding::registerNatives() {
registerHybrid({
makeNativeMethod("initHybrid", FabricJSCBinding::initHybrid),
makeNativeMethod("installFabric", FabricJSCBinding::installFabric),
makeNativeMethod("createEventTarget", FabricJSCBinding::createEventTarget),
makeNativeMethod("releaseEventTarget", FabricJSCBinding::releaseEventTarget),
makeNativeMethod("releaseEventHandler", FabricJSCBinding::releaseEventHandler),
makeNativeMethod("dispatchEventToEmptyTarget", FabricJSCBinding::dispatchEventToEmptyTarget),
makeNativeMethod("dispatchEventToTarget", FabricJSCBinding::dispatchEventToTarget),
});
}

View File

@@ -27,12 +27,17 @@ private:
static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jclass>);
jlong createEventTarget(jlong jsContextNativePointer, jlong instanceHandlePointer);
void releaseEventTarget(jlong jsContextNativePointer, jlong eventTargetPointer);
void releaseEventHandler(jlong jsContextNativePointer, jlong eventHandlerPointer);
void dispatchEventToEmptyTarget(
jlong jsContextNativePointer,
jlong eventHandlerPointer,
std::string type,
NativeMap *payload
);
void dispatchEventToTarget(
jlong jsContextNativePointer,
jlong eventHandlerPointer,

View File

@@ -103,7 +103,7 @@ public class FabricUIManagerTest {
ReactShadowNode child = createViewNode();
node.addChildAt(child, 0);
ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node, 0);
ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node);
assertThat(clonedNode).isNotSameAs(node);
assertThat(clonedNode.getOriginalReactShadowNode()).isSameAs(node);
@@ -112,27 +112,12 @@ public class FabricUIManagerTest {
assertThat(clonedNode.getChildAt(0)).isEqualTo(child);
}
@Test
public void testCloneWithInstanceHandle() {
ReactShadowNode node = createViewNode();
long oldInstanceHandle = node.getInstanceHandle();
long newInstanceHandle = oldInstanceHandle + 1;
ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node, newInstanceHandle);
assertThat(clonedNode).isNotSameAs(node);
assertThat(clonedNode.getInstanceHandle()).isSameAs(newInstanceHandle);
assertThat(node.getInstanceHandle()).isSameAs(oldInstanceHandle);
}
@Test
public void testDefaultSpacingCloning() {
ReactShadowNode node = createViewNode();
node.setDefaultPadding(Spacing.LEFT, 10);
ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node, 0);
ReactShadowNode clonedNode = mFabricUIManager.cloneNode(node);
node.setDefaultPadding(Spacing.LEFT, 20);
assertThat(clonedNode.getStylePadding(Spacing.LEFT).value).isEqualTo(10f, Offset.offset(0.01f));
@@ -165,7 +150,7 @@ public class FabricUIManagerTest {
ReactShadowNode child = createViewNode();
node.addChildAt(child, 0);
ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildren(node, randomInstanceHandle());
ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildren(node);
assertThat(clonedNode.getChildCount()).isZero();
assertSameFields(clonedNode, node);
@@ -176,7 +161,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, randomInstanceHandle());
ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewProps(node, props);
}
@Test
@@ -184,7 +169,7 @@ public class FabricUIManagerTest {
ReactShadowNode node = createViewNode();
ReadableNativeMap props = null;
ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildrenAndProps(node, props, randomInstanceHandle());
ReactShadowNode clonedNode = mFabricUIManager.cloneNodeWithNewChildrenAndProps(node, props);
assertThat(clonedNode.getChildCount()).isZero();
}
@@ -299,10 +284,10 @@ public class FabricUIManagerTest {
mFabricUIManager.appendChildToSet(childSet, container);
mFabricUIManager.completeRoot(rootTag, childSet);
ReactShadowNode aaClone = mFabricUIManager.cloneNodeWithNewProps(aa, null, 0);
ReactShadowNode aClone = mFabricUIManager.cloneNodeWithNewChildren(a, 0);
ReactShadowNode aaClone = mFabricUIManager.cloneNodeWithNewProps(aa, null);
ReactShadowNode aClone = mFabricUIManager.cloneNodeWithNewChildren(a);
mFabricUIManager.appendChild(aClone, aaClone);
ReactShadowNode containerClone = mFabricUIManager.cloneNodeWithNewChildren(container, 0);
ReactShadowNode containerClone = mFabricUIManager.cloneNodeWithNewChildren(container);
mFabricUIManager.appendChild(containerClone, b);
mFabricUIManager.appendChild(containerClone, aClone);
List<ReactShadowNode> childSet2 = mFabricUIManager.createChildSet(rootTag);