From 871f294bcbed43f757606b8fb97c01abbb063b97 Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Tue, 28 Mar 2023 03:08:13 -0700 Subject: [PATCH] Fix dispatching into incorrect UIManager type when event's target is a root view (#36659) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36659 ## Changelog: [Android][Fixed] - Fix dispatching into incorrect UIManager type when event's target is a root view There is a function, called [ViewUtil.getUIManagerType](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/ViewUtil.java#L22), which infers whether we are using the new or old architecture, given a View tag ID, with the assumption being that all New Architecture tags are even (and therefore, we are in the New Architecture iff the view tag is even). See [here for more context](https://github.com/facebook/react/pull/12587/files). This function was used to find out which type of event dispatcher to dispatch to, when going down the chain of event dispatching function calls on Android. The problem is, that there may be cases, when this odd/even assumption breaks, in particular when the target view ID is equal to `1`, meaning that it's a root view ID and there is nothing else mounted there yet. It's very rare that this happens in practice, but still is possible that user interacts with the screen before anything is mounted there (or if there is nothing mounted there by design). Reviewed By: javache Differential Revision: D44421739 fbshipit-source-id: fd5ba3c882f6c7d3c9543ebc2ec30ba000f7ca4f --- .../react/uimanager/common/ViewUtil.java | 39 ++++++++++++++++--- .../react/uimanager/events/Event.java | 22 +---------- .../uimanager/events/ReactEventEmitter.java | 2 +- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/ViewUtil.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/ViewUtil.java index be03cc75edc..5e5955b158a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/ViewUtil.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/ViewUtil.java @@ -16,20 +16,47 @@ public class ViewUtil { * Counter for uniquely identifying views. - % 2 === 0 means it is a Fabric tag. See * https://github.com/facebook/react/pull/12587 * - * @param reactTag {@link } + * @param viewTag {@link int} tag of the view this is event is dispatched to */ @UIManagerType - public static int getUIManagerType(int reactTag) { - if (reactTag % 2 == 0) return FABRIC; + public static int getUIManagerType(int viewTag) { + if (viewTag % 2 == 0) return FABRIC; return DEFAULT; } /** - * @param reactTag {@link int} react tag + * Version of getUIManagerType that uses both surfaceId and viewTag heuristics + * + * @param viewTag {@link int} tag of the view this is event is dispatched to + * @param surfaceId {@link int} ID of the corresponding surface + */ + @UIManagerType + public static int getUIManagerType(int viewTag, int surfaceId) { + // We have a contract that Fabric events *always* have a SurfaceId passed in, and non-Fabric + // events NEVER have a SurfaceId passed in (the default/placeholder of -1 is passed in instead). + // + // Why does this matter? + // Events can be sent to Views that are part of the View hierarchy *but not directly managed + // by React Native*. For example, embedded custom hierarchies, Litho hierarchies, etc. + // In those cases it's important to know that the Event should be sent to the Fabric or + // non-Fabric UIManager, and we cannot use the ViewTag for inference since it's not controlled + // by RN and is essentially a random number. + // At some point it would be great to pass the SurfaceContext here instead. + @UIManagerType int uiManagerType = (surfaceId == -1 ? DEFAULT : FABRIC); + if (uiManagerType == DEFAULT && !ViewUtil.isRootTag(viewTag)) { + // TODO (T123064648): Some events for Fabric still didn't have the surfaceId set, so if it's + // not a React RootView, double check if the tag belongs to Fabric. + if (viewTag % 2 == 0) return FABRIC; + } + return uiManagerType; + } + + /** + * @param viewTag {@link int} react tag * @return if the react tag received by parameter is a RootTag or not. */ @Deprecated - public static boolean isRootTag(int reactTag) { - return reactTag % 10 == 1; + public static boolean isRootTag(int viewTag) { + return viewTag % 10 == 1; } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java index 2178af11f7a..2bfff544b8e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java @@ -70,27 +70,7 @@ public abstract class Event { protected void init(int surfaceId, int viewTag, long timestampMs) { mSurfaceId = surfaceId; mViewTag = viewTag; - - // We infer UIManagerType. Even though it's not passed in explicitly, we have a - // contract that Fabric events *always* have a SurfaceId passed in, and non-Fabric events - // NEVER have a SurfaceId passed in (the default/placeholder of -1 is passed in instead). - // - // Why does this matter? - // Events can be sent to Views that are part of the View hierarchy *but not directly managed - // by React Native*. For example, embedded custom hierarchies, Litho hierarchies, etc. - // In those cases it's important to know that the Event should be sent to the Fabric or - // non-Fabric UIManager, and we cannot use the ViewTag for inference since it's not controlled - // by RN and is essentially a random number. - // At some point it would be great to pass the SurfaceContext here instead. - @UIManagerType - int uiManagerType = (surfaceId == -1 ? UIManagerType.DEFAULT : UIManagerType.FABRIC); - if (uiManagerType == UIManagerType.DEFAULT && !ViewUtil.isRootTag(viewTag)) { - // TODO (T123064648): Some events for Fabric still didn't have the surfaceId set, so if it's - // not a React RootView, double check if the tag belongs to Fabric. - uiManagerType = ViewUtil.getUIManagerType(viewTag); - } - mUIManagerType = uiManagerType; - + mUIManagerType = ViewUtil.getUIManagerType(viewTag, surfaceId); mTimestampMs = timestampMs; mInitialized = true; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java index b1098cb0f63..b7a0fb44925 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java @@ -135,7 +135,7 @@ public class ReactEventEmitter implements RCTModernEventEmitter { int customCoalesceKey, @Nullable WritableMap event, @EventCategoryDef int category) { - @UIManagerType int uiManagerType = ViewUtil.getUIManagerType(targetReactTag); + @UIManagerType int uiManagerType = ViewUtil.getUIManagerType(targetReactTag, surfaceId); if (uiManagerType == UIManagerType.FABRIC && mFabricEventEmitter != null) { mFabricEventEmitter.receiveEvent( surfaceId,