From 33c390a7da99639e55dd4e74cc6b303a9410b4f7 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Sat, 6 Feb 2021 23:02:02 -0800 Subject: [PATCH] Deallocate EventDispatcher in FabricUIManager when StaticViewConfigs are enabled Summary: This diff refactor the initialization and deallocation of EventDispatcher in FabricUIManager when StaticViewConfigs are enabled. The goal of this diff is to make sure that the EventDispatcher is deallocated correctly when using StaticViewConfigs changelog: [internal] Reviewed By: JoshuaGross Differential Revision: D26166413 fbshipit-source-id: e5bdad7ba923edc677c6b73f3a4d1271941f41cc --- .../react/fabric/FabricJSIModuleProvider.java | 24 +++++++---------- .../react/fabric/FabricUIManager.java | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java index c4db712c77c..27382dbef01 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java @@ -32,7 +32,6 @@ import com.facebook.react.uimanager.UIManagerModule; import com.facebook.react.uimanager.ViewManagerRegistry; import com.facebook.react.uimanager.events.BatchEventDispatchedListener; import com.facebook.react.uimanager.events.EventDispatcher; -import com.facebook.react.uimanager.events.EventDispatcherImpl; import com.facebook.systrace.Systrace; public class FabricJSIModuleProvider implements JSIModuleProvider { @@ -86,25 +85,22 @@ public class FabricJSIModuleProvider implements JSIModuleProvider { private FabricUIManager createUIManager(@NonNull EventBeatManager eventBeatManager) { Systrace.beginSection( Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricJSIModuleProvider.createUIManager"); - EventDispatcher eventDispatcher = getEventDispatcher(); - FabricUIManager fabricUIManager = - new FabricUIManager( - mReactApplicationContext, mViewManagerRegistry, eventDispatcher, eventBeatManager); - Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); - return fabricUIManager; - } - - private EventDispatcher getEventDispatcher() { - EventDispatcher eventDispatcher; + FabricUIManager fabricUIManager; if (enableExperimentalStaticViewConfigs) { - eventDispatcher = new EventDispatcherImpl(mReactApplicationContext); + fabricUIManager = + new FabricUIManager(mReactApplicationContext, mViewManagerRegistry, eventBeatManager); } else { + // TODO T83943316: Remove this code once StaticViewConfigs are enabled by default UIManagerModule nativeModule = Assertions.assertNotNull(mReactApplicationContext.getNativeModule(UIManagerModule.class)); - eventDispatcher = nativeModule.getEventDispatcher(); + EventDispatcher eventDispatcher = nativeModule.getEventDispatcher(); + fabricUIManager = + new FabricUIManager( + mReactApplicationContext, mViewManagerRegistry, eventDispatcher, eventBeatManager); } - return eventDispatcher; + Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); + return fabricUIManager; } // TODO T31905686: eager load Fabric classes, this is temporary and it will be removed 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 42a5dfa7a86..fdbed90e7fa 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -79,6 +79,7 @@ import com.facebook.react.uimanager.UIManagerHelper; import com.facebook.react.uimanager.ViewManagerPropertyUpdater; import com.facebook.react.uimanager.ViewManagerRegistry; import com.facebook.react.uimanager.events.EventDispatcher; +import com.facebook.react.uimanager.events.EventDispatcherImpl; import com.facebook.react.views.text.TextLayoutManager; import com.facebook.systrace.Systrace; import java.util.ArrayList; @@ -143,6 +144,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { */ private volatile boolean mDestroyed = false; + // TODO T83943316: Delete this variable once StaticViewConfigs are enabled by default + private volatile boolean mShouldDeallocateEventDispatcher = false; + private boolean mDriveCxxAnimations = false; private long mRunStartTime = 0l; @@ -159,6 +163,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // from C++ to exceed 9,999 and it should be obvious what's going on when analyzing performance. private int mCurrentSynchronousCommitNumber = 10000; + // TODO T83943316: Deprecate and delete this constructor once StaticViewConfigs are enabled by + // default public FabricUIManager( ReactApplicationContext reactContext, ViewManagerRegistry viewManagerRegistry, @@ -168,6 +174,20 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mReactApplicationContext = reactContext; mMountingManager = new MountingManager(viewManagerRegistry); mEventDispatcher = eventDispatcher; + mShouldDeallocateEventDispatcher = false; + mEventBeatManager = eventBeatManager; + mReactApplicationContext.addLifecycleEventListener(this); + } + + public FabricUIManager( + ReactApplicationContext reactContext, + ViewManagerRegistry viewManagerRegistry, + EventBeatManager eventBeatManager) { + mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext); + mReactApplicationContext = reactContext; + mMountingManager = new MountingManager(viewManagerRegistry); + mEventDispatcher = new EventDispatcherImpl(reactContext); + mShouldDeallocateEventDispatcher = true; mEventBeatManager = eventBeatManager; mReactApplicationContext.addLifecycleEventListener(this); } @@ -308,6 +328,13 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mBinding = null; ViewManagerPropertyUpdater.clear(); + + // When using ReactFeatureFlags.enableExperimentalStaticViewConfigs enabled, FabriUIManager is + // responsible for initializing and deallocating EventDispatcher. + // TODO T83943316: Remove this IF once StaticViewConfigs are enabled by default + if (mShouldDeallocateEventDispatcher) { + mEventDispatcher.onCatalystInstanceDestroyed(); + } } @DoNotStrip