From 8529b1ee913229ba0696399a3132c2bd4034eead Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 1 Jun 2018 17:47:46 -0700 Subject: [PATCH] Implement release of FabricUIManager resources Reviewed By: achen1 Differential Revision: D8232155 fbshipit-source-id: 6683c692a830f5a73aab2c606167e54d668ae5c2 --- .../idledetection/ReactBridgeIdleSignaler.java | 5 +++++ .../react/bridge/CatalystInstanceImpl.java | 11 ++++++++--- .../com/facebook/react/bridge/JSIModule.java | 12 ++++++++++++ .../facebook/react/bridge/JSIModuleHolder.java | 7 +++++++ .../react/bridge/JSIModuleRegistry.java | 5 +++++ .../NotThreadSafeBridgeIdleDebugListener.java | 5 +++++ .../facebook/react/fabric/FabricUIManager.java | 17 ++++++++++++----- .../debug/DidJSUpdateUiDuringFrameDetector.java | 5 +++++ 8 files changed, 59 insertions(+), 8 deletions(-) diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactBridgeIdleSignaler.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactBridgeIdleSignaler.java index 6509897a3..05b666249 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactBridgeIdleSignaler.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactBridgeIdleSignaler.java @@ -30,6 +30,11 @@ public class ReactBridgeIdleSignaler implements NotThreadSafeBridgeIdleDebugList mBridgeIdleSemaphore.release(); } + @Override + public void onBridgeDestroyed() { + // Do nothing + } + @Override public void onTransitionToBridgeBusy() { mIsBridgeIdle = false; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index b247b9b54..71c6f25ba 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -338,10 +338,14 @@ public class CatalystInstanceImpl implements CatalystInstance { @Override public void run() { mNativeModuleRegistry.notifyJSInstanceDestroy(); + mJSIModuleRegistry.notifyJSInstanceDestroy(); boolean wasIdle = (mPendingJSCalls.getAndSet(0) == 0); - if (!wasIdle && !mBridgeIdleListeners.isEmpty()) { + if (!mBridgeIdleListeners.isEmpty()) { for (NotThreadSafeBridgeIdleDebugListener listener : mBridgeIdleListeners) { - listener.onTransitionToBridgeIdle(); + if (!wasIdle) { + listener.onTransitionToBridgeIdle(); + } + listener.onBridgeDestroyed(); } } AsyncTask.execute( @@ -351,7 +355,8 @@ public class CatalystInstanceImpl implements CatalystInstance { // Kill non-UI threads from neutral third party // potentially expensive, so don't run on UI thread - // contextHolder is used as a lock to guard against other users of the JS VM having + // contextHolder is used as a lock to guard against other users of the JS VM + // having // the VM destroyed underneath them, so notify them before we resetNative mJavaScriptContextHolder.clear(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModule.java index 5b9e24954..8ee96fee2 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModule.java @@ -4,4 +4,16 @@ package com.facebook.react.bridge; * Marker interface used to represent a JSI Module. */ public interface JSIModule { + + /** + * This is called at the end of {@link CatalystApplicationFragment#createCatalystInstance()} + * after the CatalystInstance has been created, in order to initialize NativeModules that require + * the CatalystInstance or JS modules. + */ + void initialize(); + + /** + * Called before {CatalystInstance#onHostDestroy} + */ + void onCatalystInstanceDestroy(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleHolder.java index 8b35a5577..14d0f97eb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleHolder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleHolder.java @@ -16,8 +16,15 @@ public class JSIModuleHolder { return mModule; } mModule = mSpec.getJSIModuleProvider().get(); + mModule.initialize(); } } return mModule; } + + public void notifyJSInstanceDestroy() { + if (mModule != null) { + mModule.onCatalystInstanceDestroy(); + } + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java index cb4eb7478..dbca620dc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java @@ -25,4 +25,9 @@ public class JSIModuleRegistry { } } + public void notifyJSInstanceDestroy() { + for (JSIModuleHolder moduleHolder : mModules.values()) { + moduleHolder.notifyJSInstanceDestroy(); + } + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/NotThreadSafeBridgeIdleDebugListener.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/NotThreadSafeBridgeIdleDebugListener.java index a10fa2953..2531b9793 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NotThreadSafeBridgeIdleDebugListener.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NotThreadSafeBridgeIdleDebugListener.java @@ -28,4 +28,9 @@ public interface NotThreadSafeBridgeIdleDebugListener { * Called when the bridge was in an idle state and executes a JS call or callback. */ void onTransitionToBridgeBusy(); + + /** + * Called when the bridge is destroyed + */ + void onBridgeDestroyed(); } 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 4442a8b15..0a35242d1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -29,7 +29,6 @@ import com.facebook.react.fabric.events.FabricEventEmitter; import com.facebook.react.modules.i18nmanager.I18nUtil; import com.facebook.react.uimanager.DisplayMetricsHolder; import com.facebook.react.uimanager.NativeViewHierarchyManager; -import com.facebook.react.uimanager.OnLayoutEvent; import com.facebook.react.uimanager.ReactRootViewTagGenerator; import com.facebook.react.uimanager.ReactShadowNode; import com.facebook.react.uimanager.ReactShadowNodeImpl; @@ -84,10 +83,6 @@ public class FabricUIManager implements UIManager, JSHandler { mFabricReconciler = new FabricReconciler(mUIViewOperationQueue); mEventDispatcher = eventDispatcher; mJSContext = jsContext; - - FabricEventEmitter eventEmitter = - new FabricEventEmitter(reactContext, this); - eventDispatcher.registerEventEmitter(FABRIC, eventEmitter); } public void setBinding(FabricBinding binding) { @@ -538,4 +533,16 @@ public class FabricUIManager implements UIManager, JSHandler { mBinding.dispatchEventToTarget(mJSContext.get(), mEventHandlerPointer, eventTarget, name, (WritableNativeMap) params); } + @Override + public void initialize() { + FabricEventEmitter eventEmitter = + new FabricEventEmitter(mReactApplicationContext, this); + mEventDispatcher.registerEventEmitter(FABRIC, eventEmitter); + } + + @Override + public void onCatalystInstanceDestroy() { + mBinding.releaseEventHandler(mJSContext.get(), mEventHandlerPointer); + } + } diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/debug/DidJSUpdateUiDuringFrameDetector.java b/ReactAndroid/src/main/java/com/facebook/react/modules/debug/DidJSUpdateUiDuringFrameDetector.java index 37bf6596f..f0a65ff1c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/debug/DidJSUpdateUiDuringFrameDetector.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/debug/DidJSUpdateUiDuringFrameDetector.java @@ -42,6 +42,11 @@ public class DidJSUpdateUiDuringFrameDetector implements NotThreadSafeBridgeIdle mTransitionToBusyEvents.add(System.nanoTime()); } + @Override + public synchronized void onBridgeDestroyed() { + // do nothing + } + @Override public synchronized void onViewHierarchyUpdateEnqueued() { mViewHierarchyUpdateEnqueuedEvents.add(System.nanoTime());