From 80f13412e548c8666b6ad770e6d3d5c54a717bc2 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 10 Jul 2020 23:32:37 -0700 Subject: [PATCH] Expose `resolveCustomDirectEventName` from UIManager interface Summary: Expose `resolveCustomDirectEventName` from UIManager interface for Bridgeless Mode+NativeAnimatedModule. We cannot remove the interface from the Non-Fabric UIManagerModule yet, as it would break downstream open-source dependencies, so I just marked it deprecated. Note that this still doesn't totally fix issues with Bridgeless mode: generally I have to navigate to a Bridgeless surface, exit it, and then navigate back, and only on the 2nd navigation does everything seem to work properly... Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22483508 fbshipit-source-id: 685126e7e51aa5d0fd60ad5d4ecc44e8c6c3029d --- .../react/animated/NativeAnimatedModule.java | 4 + .../animated/NativeAnimatedNodesManager.java | 84 ++++++++++++++++--- .../com/facebook/react/bridge/UIManager.java | 5 ++ .../react/fabric/FabricUIManager.java | 13 +++ .../react/uimanager/UIManagerModule.java | 24 ++++-- 5 files changed, 113 insertions(+), 17 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index 52729b14fa3..453bf70e9dd 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -304,6 +304,10 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec mNumNonFabricAnimations++; } + if (mNodesManager != null) { + mNodesManager.initializeEventListenerForUIManagerType(mUIManagerType); + } + // Subscribe to UIManager (Fabric or non-Fabric) lifecycle events if we haven't yet if ((mInitializedForFabric && mUIManagerType == UIManagerType.FABRIC) || (mInitializedForNonFabric && mUIManagerType == UIManagerType.DEFAULT)) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index f1c76b780d1..179a6f0a766 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -9,8 +9,8 @@ package com.facebook.react.animated; import android.util.SparseArray; import androidx.annotation.Nullable; +import androidx.annotation.UiThread; import com.facebook.common.logging.FLog; -import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.JSApplicationCausedNativeException; @@ -25,7 +25,7 @@ import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.bridge.WritableMap; import com.facebook.react.uimanager.IllegalViewOperationException; import com.facebook.react.uimanager.UIManagerHelper; -import com.facebook.react.uimanager.UIManagerModule; +import com.facebook.react.uimanager.common.UIManagerType; import com.facebook.react.uimanager.events.Event; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.react.uimanager.events.EventDispatcherListener; @@ -62,23 +62,51 @@ import java.util.Queue; // Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time // there will be only one driver per mapping so all code code should be optimized around that. private final Map> mEventDrivers = new HashMap<>(); - private final UIManagerModule.CustomEventNamesResolver mCustomEventNamesResolver; private final ReactApplicationContext mReactApplicationContext; private int mAnimatedGraphBFSColor = 0; private int mNumInconsistentFrames = 0; // Used to avoid allocating a new array on every frame in `runUpdates` and `onEventDispatch`. private final List mRunUpdateNodeList = new LinkedList<>(); + private boolean mEventListenerInitializedForFabric = false; + private boolean mEventListenerInitializedForNonFabric = false; + public NativeAnimatedNodesManager(ReactApplicationContext reactApplicationContext) { mReactApplicationContext = reactApplicationContext; + } - UIManagerModule uiManager = - Assertions.assertNotNull(reactApplicationContext.getNativeModule(UIManagerModule.class)); + /** + * Initialize event listeners for Fabric UIManager or non-Fabric UIManager, exactly once. Once + * Fabric is the only UIManager, this logic can be simplified. This is only called on the JS + * thread. + * + * @param uiManagerType + */ + @UiThread + public void initializeEventListenerForUIManagerType(@UIManagerType final int uiManagerType) { + if ((uiManagerType == UIManagerType.FABRIC && mEventListenerInitializedForFabric) + || (uiManagerType == UIManagerType.DEFAULT && mEventListenerInitializedForNonFabric)) { + return; + } - uiManager.getEventDispatcher().addListener(this); - // TODO T64216139 Remove dependency of UIManagerModule when the Constants are not in Native - // anymore - mCustomEventNamesResolver = uiManager.getDirectEventNamesResolver(); + final NativeAnimatedNodesManager self = this; + mReactApplicationContext.runOnUiQueueThread( + new Runnable() { + @Override + public void run() { + UIManager uiManager = + UIManagerHelper.getUIManager(mReactApplicationContext, uiManagerType); + if (uiManager != null) { + uiManager.getEventDispatcher().addListener(self); + + if (uiManagerType == UIManagerType.FABRIC) { + mEventListenerInitializedForFabric = true; + } else { + mEventListenerInitializedForNonFabric = true; + } + } + } + }); } /*package*/ @Nullable @@ -90,6 +118,7 @@ import java.util.Queue; return mActiveAnimations.size() > 0 || mUpdatedNodes.size() > 0; } + @UiThread public void createAnimatedNode(int tag, ReadableMap config) { if (mAnimatedNodes.get(tag) != null) { throw new JSApplicationIllegalArgumentException( @@ -129,11 +158,13 @@ import java.util.Queue; mUpdatedNodes.put(tag, node); } + @UiThread public void dropAnimatedNode(int tag) { mAnimatedNodes.remove(tag); mUpdatedNodes.remove(tag); } + @UiThread public void startListeningToAnimatedNodeValue(int tag, AnimatedNodeValueListener listener) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -143,6 +174,7 @@ import java.util.Queue; ((ValueAnimatedNode) node).setValueListener(listener); } + @UiThread public void stopListeningToAnimatedNodeValue(int tag) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -152,6 +184,7 @@ import java.util.Queue; ((ValueAnimatedNode) node).setValueListener(null); } + @UiThread public void setAnimatedNodeValue(int tag, double value) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -163,6 +196,7 @@ import java.util.Queue; mUpdatedNodes.put(tag, node); } + @UiThread public void setAnimatedNodeOffset(int tag, double offset) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -173,6 +207,7 @@ import java.util.Queue; mUpdatedNodes.put(tag, node); } + @UiThread public void flattenAnimatedNodeOffset(int tag) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -182,6 +217,7 @@ import java.util.Queue; ((ValueAnimatedNode) node).flattenOffset(); } + @UiThread public void extractAnimatedNodeOffset(int tag) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -191,6 +227,7 @@ import java.util.Queue; ((ValueAnimatedNode) node).extractOffset(); } + @UiThread public void startAnimatingNode( int animationId, int animatedNodeTag, ReadableMap animationConfig, Callback endCallback) { AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); @@ -228,6 +265,7 @@ import java.util.Queue; mActiveAnimations.put(animationId, animation); } + @UiThread private void stopAnimationsForNode(AnimatedNode animatedNode) { // in most of the cases there should never be more than a few active animations running at the // same time. Therefore it does not make much sense to create an animationId -> animation @@ -248,6 +286,7 @@ import java.util.Queue; } } + @UiThread public void stopAnimation(int animationId) { // in most of the cases there should never be more than a few active animations running at the // same time. Therefore it does not make much sense to create an animationId -> animation @@ -272,6 +311,7 @@ import java.util.Queue; // when the animation is already over. } + @UiThread public void connectAnimatedNodes(int parentNodeTag, int childNodeTag) { AnimatedNode parentNode = mAnimatedNodes.get(parentNodeTag); if (parentNode == null) { @@ -302,6 +342,7 @@ import java.util.Queue; mUpdatedNodes.put(childNodeTag, childNode); } + @UiThread public void connectAnimatedNodeToView(int animatedNodeTag, int viewTag) { AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); if (node == null) { @@ -336,6 +377,7 @@ import java.util.Queue; mUpdatedNodes.put(animatedNodeTag, node); } + @UiThread public void disconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag) { AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); if (node == null) { @@ -352,6 +394,7 @@ import java.util.Queue; propsAnimatedNode.disconnectFromView(viewTag); } + @UiThread public void getValue(int tag, Callback callback) { AnimatedNode node = mAnimatedNodes.get(tag); if (node == null || !(node instanceof ValueAnimatedNode)) { @@ -361,6 +404,7 @@ import java.util.Queue; callback.invoke(((ValueAnimatedNode) node).getValue()); } + @UiThread public void restoreDefaultValues(int animatedNodeTag) { AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); // Restoring default values needs to happen before UIManager operations so it is @@ -380,6 +424,7 @@ import java.util.Queue; propsAnimatedNode.restoreDefaultValues(); } + @UiThread public void addAnimatedEventToView(int viewTag, String eventName, ReadableMap eventMapping) { int nodeTag = eventMapping.getInt("animatedValueTag"); AnimatedNode node = mAnimatedNodes.get(nodeTag); @@ -411,6 +456,7 @@ import java.util.Queue; } } + @UiThread public void removeAnimatedEventFromView(int viewTag, String eventName, int animatedValueTag) { String key = viewTag + eventName; if (mEventDrivers.containsKey(key)) { @@ -429,6 +475,7 @@ import java.util.Queue; } } + @UiThread @Override public void onEventDispatch(final Event event) { // Events can be dispatched from any thread so we have to make sure handleEvent is run from the @@ -446,10 +493,25 @@ import java.util.Queue; } } + @UiThread private void handleEvent(Event event) { if (!mEventDrivers.isEmpty()) { // If the event has a different name in native convert it to it's JS name. - String eventName = mCustomEventNamesResolver.resolveCustomEventName(event.getEventName()); + // TODO T64216139 Remove dependency of UIManagerModule when the Constants are not in Native + // anymore + if (mReactApplicationContext == null) { + return; + } + UIManager uiManager = + UIManagerHelper.getUIManagerForReactTag(mReactApplicationContext, event.getViewTag()); + if (uiManager == null) { + return; + } + String eventName = uiManager.resolveCustomDirectEventName(event.getEventName()); + if (eventName == null) { + eventName = ""; + } + List driversForKey = mEventDrivers.get(event.getViewTag() + eventName); if (driversForKey != null) { for (EventAnimationDriver driver : driversForKey) { @@ -475,6 +537,7 @@ import java.util.Queue; * sub-graph of *active* nodes. This is done by adding node to the BFS queue only if all its * "predecessors" have already been visited. */ + @UiThread public void runUpdates(long frameTimeNanos) { UiThreadUtil.assertOnUiThread(); boolean hasFinishedAnimations = false; @@ -517,6 +580,7 @@ import java.util.Queue; } } + @UiThread private void updateNodes(List nodes) { int activeNodesCount = 0; int updatedNodesCount = 0; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java index 1707c623edb..0bd6a2c649a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java @@ -123,4 +123,9 @@ public interface UIManager extends JSIModule, PerformanceCounter { * @param event parameters */ void receiveEvent(int reactTag, String eventName, @Nullable WritableMap event); + + /** Resolves Direct Event name exposed to JS from the one known to the Native side. */ + @Deprecated + @Nullable + String resolveCustomDirectEventName(@Nullable String eventName); } 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 57409a31601..1ab26f3349d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -1056,6 +1056,19 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // TODO T31905686: Remove this method and add support for multi-threading performance counters } + @Override + @Deprecated + @Nullable + public String resolveCustomDirectEventName(@Nullable String eventName) { + if (eventName == null) { + return null; + } + if (eventName.substring(0, 3).equals("top")) { + return "on" + eventName.substring(3); + } + return eventName; + } + // Called from Binding.cpp @DoNotStrip @AnyThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 7cb37afd116..2336cf82953 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -357,20 +357,30 @@ public class UIManagerModule extends ReactContextBaseJavaModule } /** Resolves Direct Event name exposed to JS from the one known to the Native side. */ + @Deprecated public CustomEventNamesResolver getDirectEventNamesResolver() { return new CustomEventNamesResolver() { @Override - public @Nullable String resolveCustomEventName(String eventName) { - Map customEventType = - (Map) mCustomDirectEvents.get(eventName); - if (customEventType != null) { - return customEventType.get("registrationName"); - } - return eventName; + public @Nullable String resolveCustomEventName(@Nullable String eventName) { + return resolveCustomDirectEventName(eventName); } }; } + @Override + @Deprecated + @Nullable + public String resolveCustomDirectEventName(@Nullable String eventName) { + if (eventName != null) { + Map customEventType = + (Map) mCustomDirectEvents.get(eventName); + if (customEventType != null) { + return customEventType.get("registrationName"); + } + } + return eventName; + } + @Override public void profileNextBatch() { mUIImplementation.profileNextBatch();