From 5d9326be29be8688f2238c72c18a9037f983c77d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 29 Jun 2018 15:30:30 -0700 Subject: [PATCH] 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 --- .../facebook/react/fabric/FabricBinding.java | 9 ++- .../react/fabric/FabricUIManager.java | 31 ++++---- .../fabric/events/FabricEventEmitter.java | 7 +- .../react/fabric/jsc/FabricJSCBinding.java | 11 ++- .../react/fabric/jsc/jni/FabricJSCBinding.cpp | 72 +++++++++++-------- .../react/fabric/jsc/jni/FabricJSCBinding.h | 9 ++- .../react/fabric/FabricUIManagerTest.java | 31 +++----- .../componentdescriptor/ComponentDescriptor.h | 2 +- .../ConcreteComponentDescriptor.h | 4 +- .../fabric/core/events/EventDispatcher.h | 2 - .../fabric/core/events/EventEmitter.cpp | 13 +--- ReactCommon/fabric/core/events/EventEmitter.h | 7 +- .../fabric/uimanager/FabricUIManager.cpp | 40 ++++++----- .../fabric/uimanager/FabricUIManager.h | 26 +++---- .../uimanager/SchedulerEventDispatcher.cpp | 10 +-- .../uimanager/SchedulerEventDispatcher.h | 2 - 16 files changed, 134 insertions(+), 142 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricBinding.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricBinding.java index 0636c17c24e..4db39a42522 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricBinding.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricBinding.java @@ -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, 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 05ce4025e3a..87d1850e674 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -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 diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java index 4a705bb7505..2d1c01d8692 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java @@ -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); } } } 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 38bbbfe636f..92e82d41e51 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 @@ -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, 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 9b18298562c..5717c930b68 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 @@ -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(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(JShadowNode::javaobject, jlong)>("cloneNode"); + ->getMethod(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(JShadowNode::javaobject, jlong)>("cloneNodeWithNewChildren"); + ->getMethod(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(JShadowNode::javaobject, ReadableNativeMap::javaobject, jlong)>("cloneNodeWithNewProps"); + ->getMethod(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(JShadowNode::javaobject, ReadableNativeMap::javaobject, jlong)>("cloneNodeWithNewChildrenAndProps"); + ->getMethod(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::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), }); } 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 40b7cbd1681..d47a0bb584d 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 @@ -27,12 +27,17 @@ private: static jni::local_ref initHybrid(jni::alias_ref); - 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, 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 ee7e96fb2f0..93f601c5ffa 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.java @@ -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 childSet2 = mFabricUIManager.createChildSet(rootTag); diff --git a/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h index 1718860b434..53312ed2fb9 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h @@ -85,7 +85,7 @@ public: * shadow nodes. */ virtual SharedEventEmitter createEventEmitter( - const InstanceHandle &instanceHandle, + const EventTarget &eventTarget, const Tag &tag ) const = 0; }; diff --git a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h index 2f1f7fcd9e0..82801a7753f 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h @@ -113,10 +113,10 @@ public: }; virtual SharedEventEmitter createEventEmitter( - const InstanceHandle &instanceHandle, + const EventTarget &eventTarget, const Tag &tag ) const override { - return std::make_shared(instanceHandle, tag, eventDispatcher_); + return std::make_shared(eventTarget, tag, eventDispatcher_); } protected: diff --git a/ReactCommon/fabric/core/events/EventDispatcher.h b/ReactCommon/fabric/core/events/EventDispatcher.h index 0f61a010eb1..11ba3580850 100644 --- a/ReactCommon/fabric/core/events/EventDispatcher.h +++ b/ReactCommon/fabric/core/events/EventDispatcher.h @@ -28,8 +28,6 @@ class EventDispatcher { public: - virtual EventTarget createEventTarget(const InstanceHandle &instanceHandle) const = 0; - /* * Dispatches "raw" event using some event-delivery infrastructure. */ diff --git a/ReactCommon/fabric/core/events/EventEmitter.cpp b/ReactCommon/fabric/core/events/EventEmitter.cpp index 9c61be13f0a..2e99199eea7 100644 --- a/ReactCommon/fabric/core/events/EventEmitter.cpp +++ b/ReactCommon/fabric/core/events/EventEmitter.cpp @@ -12,13 +12,10 @@ namespace facebook { namespace react { -EventEmitter::EventEmitter(const InstanceHandle &instanceHandle, const Tag &tag, const SharedEventDispatcher &eventDispatcher): - instanceHandle_(instanceHandle), +EventEmitter::EventEmitter(const EventTarget &eventTarget, const Tag &tag, const SharedEventDispatcher &eventDispatcher): + eventTarget_(eventTarget), tag_(tag), eventDispatcher_(eventDispatcher) { - if (eventDispatcher) { - eventTarget_ = createEventTarget(); - } } EventEmitter::~EventEmitter() { @@ -49,11 +46,5 @@ void EventEmitter::dispatchEvent( eventDispatcher->dispatchEvent(eventTarget_, type, extendedPayload, priority); } -EventTarget EventEmitter::createEventTarget() const { - const auto &eventDispatcher = eventDispatcher_.lock(); - assert(eventDispatcher); - return eventDispatcher->createEventTarget(instanceHandle_); -} - } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/core/events/EventEmitter.h b/ReactCommon/fabric/core/events/EventEmitter.h index d854b00ba10..3876be111c0 100644 --- a/ReactCommon/fabric/core/events/EventEmitter.h +++ b/ReactCommon/fabric/core/events/EventEmitter.h @@ -31,7 +31,7 @@ using SharedEventEmitter = std::shared_ptr; class EventEmitter { public: - EventEmitter(const InstanceHandle &instanceHandle, const Tag &tag, const SharedEventDispatcher &eventDispatcher); + EventEmitter(const EventTarget &eventTarget, const Tag &tag, const SharedEventDispatcher &eventDispatcher); virtual ~EventEmitter(); protected: @@ -48,12 +48,9 @@ protected: private: - EventTarget createEventTarget() const; - - InstanceHandle instanceHandle_; + mutable EventTarget eventTarget_ {nullptr}; Tag tag_; std::weak_ptr eventDispatcher_; - mutable EventTarget eventTarget_ {nullptr}; }; } // namespace react diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.cpp b/ReactCommon/fabric/uimanager/FabricUIManager.cpp index 06bbeb94449..463c537bcdb 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.cpp +++ b/ReactCommon/fabric/uimanager/FabricUIManager.cpp @@ -97,12 +97,12 @@ UIManagerDelegate *FabricUIManager::getDelegate() { return delegate_; } -void FabricUIManager::setCreateEventTargetFunction(std::function createEventTargetFunction) { - createEventTargetFunction_ = createEventTargetFunction; +void FabricUIManager::setDispatchEventToEmptyTargetFunction(std::function dispatchEventFunction) { + dispatchEventToEmptyTargetFunction_ = dispatchEventFunction; } -void FabricUIManager::setDispatchEventFunction(std::function dispatchEventFunction) { - dispatchEventFunction_ = dispatchEventFunction; +void FabricUIManager::setDispatchEventToTargetFunction(std::function dispatchEventFunction) { + dispatchEventToTargetFunction_ = dispatchEventFunction; } void FabricUIManager::setReleaseEventHandlerFunction(std::function releaseEventHandlerFunction) { @@ -113,12 +113,16 @@ void FabricUIManager::setReleaseEventTargetFunction(std::function(type), + const_cast(payload) + ); } -void FabricUIManager::dispatchEvent(const EventTarget &eventTarget, const std::string &type, const folly::dynamic &payload) const { - dispatchEventFunction_( +void FabricUIManager::dispatchEventToTarget(const EventTarget &eventTarget, const std::string &type, const folly::dynamic &payload) const { + dispatchEventToTargetFunction_( eventHandler_, eventTarget, const_cast(type), @@ -130,7 +134,7 @@ void FabricUIManager::releaseEventTarget(const EventTarget &eventTarget) const { releaseEventTargetFunction_(eventTarget); } -SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int rootTag, folly::dynamic props, InstanceHandle instanceHandle) { +SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int rootTag, folly::dynamic props, EventTarget eventTarget) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::createNode(tag: " << tag << ", name: " << viewName << ", rootTag: " << rootTag << ", props: " << props << ")"; ComponentName componentName = componentNameByReactViewName(viewName); @@ -141,7 +145,7 @@ SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int componentDescriptor->createShadowNode( tag, rootTag, - componentDescriptor->createEventEmitter(instanceHandle, tag), + componentDescriptor->createEventEmitter(eventTarget, tag), componentDescriptor->cloneProps(nullptr, rawProps) ); @@ -154,7 +158,7 @@ SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int return shadowNode; } -SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode, InstanceHandle instanceHandle) { +SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNode(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; @@ -162,7 +166,7 @@ SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode, componentDescriptor->cloneShadowNode( shadowNode, nullptr, - componentDescriptor->createEventEmitter(instanceHandle, shadowNode->getTag()), + componentDescriptor->createEventEmitter(nullptr /*TODO(shergin)*/, shadowNode->getTag()), nullptr ); @@ -170,7 +174,7 @@ SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode, return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNode &shadowNode, InstanceHandle instanceHandle) { +SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNode &shadowNode) { 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]; @@ -179,7 +183,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNod componentDescriptor->cloneShadowNode( shadowNode, nullptr, - componentDescriptor->createEventEmitter(instanceHandle, shadowNode->getTag()), + componentDescriptor->createEventEmitter(nullptr /*TODO(shergin)*/, shadowNode->getTag()), ShadowNode::emptySharedShadowNodeSharedList() ); @@ -187,7 +191,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNod return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode &shadowNode, folly::dynamic props, InstanceHandle instanceHandle) { +SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode &shadowNode, folly::dynamic props) { 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]; @@ -197,7 +201,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode & componentDescriptor->cloneShadowNode( shadowNode, componentDescriptor->cloneProps(shadowNode->getProps(), rawProps), - componentDescriptor->createEventEmitter(instanceHandle, shadowNode->getTag()), + componentDescriptor->createEventEmitter(nullptr /*TODO(shergin)*/, shadowNode->getTag()), nullptr ); @@ -205,7 +209,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode & return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedShadowNode &shadowNode, folly::dynamic props, InstanceHandle instanceHandle) { +SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedShadowNode &shadowNode, folly::dynamic props) { 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]; @@ -215,7 +219,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedS componentDescriptor->cloneShadowNode( shadowNode, componentDescriptor->cloneProps(shadowNode->getProps(), rawProps), - componentDescriptor->createEventEmitter(instanceHandle, shadowNode->getTag()), + componentDescriptor->createEventEmitter(nullptr /*TODO(shergin)*/, shadowNode->getTag()), ShadowNode::emptySharedShadowNodeSharedList() ); diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.h b/ReactCommon/fabric/uimanager/FabricUIManager.h index 3f66913332f..e33501e3197 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.h +++ b/ReactCommon/fabric/uimanager/FabricUIManager.h @@ -18,8 +18,8 @@ namespace facebook { namespace react { -using CreateEventTargetFunction = EventTarget (InstanceHandle instanceHandle); -using DispatchEventFunction = void (EventHandler eventHandler, EventTarget eventTarget, std::string type, folly::dynamic payload); +using DispatchEventToEmptyTargetFunction = void (EventHandler eventHandler, std::string type, folly::dynamic payload); +using DispatchEventToTargetFunction = void (EventHandler eventHandler, EventTarget eventTarget, std::string type, folly::dynamic payload); using ReleaseEventHandlerFunction = void (EventHandler eventHandler); using ReleaseEventTargetFunction = void (EventTarget eventTarget); @@ -44,24 +44,24 @@ public: /* * Registers callback functions. */ - void setCreateEventTargetFunction(std::function createEventTargetFunction); - void setDispatchEventFunction(std::function dispatchEventFunction); + void setDispatchEventToEmptyTargetFunction(std::function dispatchEventFunction); + void setDispatchEventToTargetFunction(std::function dispatchEventFunction); void setReleaseEventHandlerFunction(std::function releaseEventHandlerFunction); void setReleaseEventTargetFunction(std::function releaseEventTargetFunction); #pragma mark - Native-facing Interface - EventTarget createEventTarget(const InstanceHandle &instanceHandle) const; - void dispatchEvent(const EventTarget &eventTarget, const std::string &type, const folly::dynamic &payload) const; + void dispatchEventToEmptyTarget(const std::string &type, const folly::dynamic &payload) const; + void dispatchEventToTarget(const EventTarget &eventTarget, const std::string &type, const folly::dynamic &payload) const; void releaseEventTarget(const EventTarget &eventTarget) const; #pragma mark - JavaScript/React-facing Interface - SharedShadowNode createNode(Tag reactTag, std::string viewName, Tag rootTag, folly::dynamic props, InstanceHandle instanceHandle); - SharedShadowNode cloneNode(const SharedShadowNode &node, InstanceHandle instanceHandle); - SharedShadowNode cloneNodeWithNewChildren(const SharedShadowNode &node, InstanceHandle instanceHandle); - SharedShadowNode cloneNodeWithNewProps(const SharedShadowNode &node, folly::dynamic props, InstanceHandle instanceHandle); - SharedShadowNode cloneNodeWithNewChildrenAndProps(const SharedShadowNode &node, folly::dynamic newProps, InstanceHandle instanceHandle); + SharedShadowNode createNode(Tag reactTag, std::string viewName, Tag rootTag, folly::dynamic props, EventTarget eventTarget); + 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); void appendChild(const SharedShadowNode &parentNode, const SharedShadowNode &childNode); SharedShadowNodeUnsharedList createChildSet(Tag rootTag); void appendChildToSet(const SharedShadowNodeUnsharedList &childSet, const SharedShadowNode &childNode); @@ -73,8 +73,8 @@ private: SharedComponentDescriptorRegistry componentDescriptorRegistry_; UIManagerDelegate *delegate_; EventHandler eventHandler_; - std::function createEventTargetFunction_; - std::function dispatchEventFunction_; + std::function dispatchEventToEmptyTargetFunction_; + std::function dispatchEventToTargetFunction_; std::function releaseEventHandlerFunction_; std::function releaseEventTargetFunction_; }; diff --git a/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.cpp b/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.cpp index 77d6942535f..f261499326b 100644 --- a/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.cpp +++ b/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.cpp @@ -26,11 +26,6 @@ void SchedulerEventDispatcher::setUIManager(std::shared_ptrcreateEventTarget(instanceHandle); -} - void SchedulerEventDispatcher::dispatchEvent( const EventTarget &eventTarget, const std::string &type, @@ -41,14 +36,15 @@ void SchedulerEventDispatcher::dispatchEvent( return; } // TODO: Schedule the event based on priority. - uiManager_->dispatchEvent(eventTarget, normalizeEventType(type), payload); + uiManager_->dispatchEventToTarget(eventTarget, normalizeEventType(type), payload); } void SchedulerEventDispatcher::releaseEventTarget(const EventTarget &eventTarget) const { if (!uiManager_) { return; } - uiManager_->releaseEventTarget(eventTarget); + // TODO(shergin): This needs to move to the destructor of EventEmitter. For now we'll leak. + // uiManager_->releaseEventTarget(eventTarget); } } // namespace react diff --git a/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.h b/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.h index 847946b57b0..e89735ad998 100644 --- a/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.h +++ b/ReactCommon/fabric/uimanager/SchedulerEventDispatcher.h @@ -31,8 +31,6 @@ public: #pragma mark - EventDispatcher - EventTarget createEventTarget(const InstanceHandle &instanceHandle) const override; - void dispatchEvent( const EventTarget &eventTarget, const std::string &type,