From 95d05fc415888a8793d9775d3cf51e01dbfc894d Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 10 Jul 2020 23:32:37 -0700 Subject: [PATCH] NativeAnimatedModule: subscribe to Fabric lifecycle events in more cases Summary: The previous assumption was that any animations would result in `addAnimatedEventToView` being called. That's not true in all cases, so sometimes we never subscribed to Fabric lifecycle events, preventing animations from working on some screens and for Venice. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22483509 fbshipit-source-id: c97576675902b4b9e1d4e659c7c1e24c5fe92946 --- .../react/animated/NativeAnimatedModule.java | 121 +++++++++++------- .../react/uimanager/UIManagerModule.java | 4 +- 2 files changed, 78 insertions(+), 47 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 f85a092952b..52729b14fa3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -104,6 +104,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec private volatile boolean mFabricBatchCompleted = false; private boolean mInitializedForFabric = false; + private boolean mInitializedForNonFabric = false; private @UIManagerType int mUIManagerType = UIManagerType.DEFAULT; private int mNumFabricAnimations = 0; private int mNumNonFabricAnimations = 0; @@ -145,12 +146,8 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec public void initialize() { ReactApplicationContext reactApplicationContext = getReactApplicationContextIfActiveOrWarn(); - // TODO T59412313 Implement this API on FabricUIManager to use in bridgeless mode - if (reactApplicationContext != null && !reactApplicationContext.isBridgeless()) { + if (reactApplicationContext != null) { reactApplicationContext.addLifecycleEventListener(this); - UIManagerModule uiManager = - Assertions.assertNotNull(reactApplicationContext.getNativeModule(UIManagerModule.class)); - uiManager.addUIManagerEventListener(this); } } @@ -292,6 +289,73 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec mNodesManager = nodesManager; } + /** + * Given a viewTag, detect if we're running in Fabric or non-Fabric and attach an event listener + * to the correct UIManager, if necessary. This is expected to only be called from the JS thread, + * and not concurrently. + * + * @param viewTag + */ + private void initializeLifecycleEventListenersForViewTag(final int viewTag) { + mUIManagerType = ViewUtil.getUIManagerType(viewTag); + if (mUIManagerType == UIManagerType.FABRIC) { + mNumFabricAnimations++; + } else { + mNumNonFabricAnimations++; + } + + // Subscribe to UIManager (Fabric or non-Fabric) lifecycle events if we haven't yet + if ((mInitializedForFabric && mUIManagerType == UIManagerType.FABRIC) + || (mInitializedForNonFabric && mUIManagerType == UIManagerType.DEFAULT)) { + return; + } + + ReactApplicationContext reactApplicationContext = getReactApplicationContext(); + if (reactApplicationContext != null) { + @Nullable + UIManager uiManager = UIManagerHelper.getUIManager(reactApplicationContext, mUIManagerType); + if (uiManager != null) { + uiManager.addUIManagerEventListener(this); + if (mUIManagerType == UIManagerType.FABRIC) { + mInitializedForFabric = true; + } else { + mInitializedForNonFabric = true; + } + } + } + } + + /** + * Given a viewTag and the knowledge that a "disconnect" or "stop"-type imperative command is + * being executed, decrement the number of inflight animations and possibly switch UIManager + * modes. + * + * @param viewTag + */ + private void decrementInFlightAnimationsForViewTag(final int viewTag) { + @UIManagerType int animationManagerType = ViewUtil.getUIManagerType(viewTag); + if (animationManagerType == UIManagerType.FABRIC) { + mNumFabricAnimations--; + } else { + mNumNonFabricAnimations--; + } + + // Should we switch to a different animation mode? + // This can be useful when navigating between Fabric and non-Fabric screens: + // If there are ongoing Fabric animations from a previous screen, + // and we tear down the current non-Fabric screen, we should expect + // the animation mode to switch back - and vice-versa. + if (mNumNonFabricAnimations == 0 + && mNumFabricAnimations > 0 + && mUIManagerType != UIManagerType.FABRIC) { + mUIManagerType = UIManagerType.FABRIC; + } else if (mNumFabricAnimations == 0 + && mNumNonFabricAnimations > 0 + && mUIManagerType != UIManagerType.DEFAULT) { + mUIManagerType = UIManagerType.DEFAULT; + } + } + @Override public void createAnimatedNode(final double tagDouble, final ReadableMap config) { final int tag = (int) tagDouble; @@ -582,6 +646,8 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec + viewTag); } + initializeLifecycleEventListenersForViewTag(viewTag); + mOperations.add( new UIThreadOperation() { @Override @@ -613,6 +679,8 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec + viewTag); } + decrementInFlightAnimationsForViewTag(viewTag); + mOperations.add( new UIThreadOperation() { @Override @@ -668,26 +736,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec + eventMapping.toHashMap().toString()); } - mUIManagerType = ViewUtil.getUIManagerType(viewTag); - if (mUIManagerType == UIManagerType.FABRIC) { - mNumFabricAnimations++; - } else { - mNumNonFabricAnimations++; - } - - // Subscribe to FabricUIManager lifecycle events if we haven't yet - if (!mInitializedForFabric && mUIManagerType == UIManagerType.FABRIC) { - ReactApplicationContext reactApplicationContext = getReactApplicationContext(); - if (reactApplicationContext != null) { - @Nullable - UIManager uiManager = - UIManagerHelper.getUIManager(reactApplicationContext, UIManagerType.FABRIC); - if (uiManager != null) { - uiManager.addUIManagerEventListener(this); - mInitializedForFabric = true; - } - } - } + initializeLifecycleEventListenersForViewTag(viewTag); mOperations.add( new UIThreadOperation() { @@ -724,27 +773,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec + animatedValueTag); } - @UIManagerType int animationManagerType = ViewUtil.getUIManagerType(viewTag); - if (animationManagerType == UIManagerType.FABRIC) { - mNumFabricAnimations--; - } else { - mNumNonFabricAnimations--; - } - - // Should we switch to a different animation mode? - // This can be useful when navigating between Fabric and non-Fabric screens: - // If there are ongoing Fabric animations from a previous screen, - // and we tear down the current non-Fabric screen, we should expect - // the animation mode to switch back - and vice-versa. - if (mNumNonFabricAnimations == 0 - && mNumFabricAnimations > 0 - && mUIManagerType != UIManagerType.FABRIC) { - mUIManagerType = UIManagerType.FABRIC; - } else if (mNumFabricAnimations == 0 - && mNumNonFabricAnimations > 0 - && mUIManagerType != UIManagerType.DEFAULT) { - mUIManagerType = UIManagerType.DEFAULT; - } + decrementInFlightAnimationsForViewTag(viewTag); mOperations.add( new UIThreadOperation() { 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 52750844ed8..7cb37afd116 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -51,6 +51,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; /** * Native module to allow JS to create and update native Views. @@ -119,7 +120,8 @@ public class UIManagerModule extends ReactContextBaseJavaModule private final UIImplementation mUIImplementation; private final MemoryTrimCallback mMemoryTrimCallback = new MemoryTrimCallback(); private final List mListeners = new ArrayList<>(); - private final List mUIManagerListeners = new ArrayList<>(); + private final CopyOnWriteArrayList mUIManagerListeners = + new CopyOnWriteArrayList<>(); private @Nullable Map mViewManagerConstantsCache; private volatile int mViewManagerConstantsCacheSize;