From dfa8eb0558338f18ea01f294a64d355f6deeff06 Mon Sep 17 00:00:00 2001 From: Lulu Wu Date: Wed, 31 Mar 2021 06:32:08 -0700 Subject: [PATCH] Rename "hasActiveCatalystInstance" to "hasActiveReactInstance" for clarification Summary: Sometimes ```hasActiveCatalystInstance()``` is used to check if it's safe to access the CatalystInstance, which will still crash in Venice. Previously we mitigate this by changing ```reactContext.hasActiveCatalystInstance()``` to ```reactContext.hasActiveCatalystInstance() || reactContext.isBridgeless()```. To solve this for all and good the plan is: 1, Rename ```hasActiveCatalystInstance()``` to ```hasActiveReactInstance()``` so it won't sounds like CatalystInstance-only. 2, Implement hasActiveReactInstance() for Venice. D27343867 3, Remove previous mitigation. D27343952 This diff is the first step, by xbgs there are **58** non-generated callsites of ```hasActiveCatalystInstance()``` in code base which are all renamed in this diff. Changelog: [Android][Changed] - Rename "hasActiveCatalystInstance" to "hasActiveReactInstance" Reviewed By: mdvacca Differential Revision: D27335055 fbshipit-source-id: 5b8ff5e09b79a492e910bb8f197e70fa1360bcef --- .../testing/idledetection/ReactIdleDetectionUtil.java | 2 +- .../java/com/facebook/react/ReactInstanceManager.java | 8 ++++---- .../main/java/com/facebook/react/bridge/ReactContext.java | 7 ++++--- .../facebook/react/bridge/ReactContextBaseJavaModule.java | 2 +- .../facebook/react/devsupport/DevSupportManagerBase.java | 2 +- .../com/facebook/react/jstasks/HeadlessJsTaskContext.java | 2 +- .../accessibilityinfo/AccessibilityInfoModule.java | 2 +- .../facebook/react/modules/appstate/AppStateModule.java | 2 +- .../react/modules/datepicker/DatePickerDialogModule.java | 4 ++-- .../react/modules/deviceinfo/DeviceInfoModule.java | 2 +- .../com/facebook/react/modules/dialog/DialogModule.java | 4 ++-- .../react/uimanager/ReactAccessibilityDelegate.java | 2 +- .../com/facebook/react/uimanager/UIManagerHelper.java | 2 +- .../react/uimanager/events/ReactEventEmitter.java | 2 +- .../facebook/react/views/text/ReactTextShadowNode.java | 2 +- .../react/animated/NativeAnimatedNodeTraversalTest.java | 2 +- .../facebook/react/modules/dialog/DialogModuleTest.java | 2 +- .../react/modules/network/NetworkingModuleTest.java | 2 +- .../facebook/react/modules/timing/TimingModuleTest.java | 2 +- 19 files changed, 27 insertions(+), 26 deletions(-) diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactIdleDetectionUtil.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactIdleDetectionUtil.java index e7410dc91b0db5..4cdbf83a058dda 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactIdleDetectionUtil.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/idledetection/ReactIdleDetectionUtil.java @@ -79,7 +79,7 @@ public void doFrame(long frameTimeNanos) { } private static void waitForJSIdle(ReactContext reactContext) { - if (!reactContext.hasActiveCatalystInstance()) { + if (!reactContext.hasActiveReactInstance()) { return; } final CountDownLatch latch = new CountDownLatch(1); diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index b6703a490450d2..2b97586881aec9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -518,7 +518,7 @@ public void onNewIntent(Intent intent) { private void toggleElementInspector() { ReactContext currentContext = getCurrentReactContext(); - if (currentContext != null && currentContext.hasActiveCatalystInstance()) { + if (currentContext != null && currentContext.hasActiveReactInstance()) { currentContext .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class) .emit("toggleElementInspector", null); @@ -859,7 +859,7 @@ public void detachRootView(ReactRoot reactRoot) { if (mAttachedReactRoots.contains(reactRoot)) { ReactContext currentContext = getCurrentReactContext(); mAttachedReactRoots.remove(reactRoot); - if (currentContext != null && currentContext.hasActiveCatalystInstance()) { + if (currentContext != null && currentContext.hasActiveReactInstance()) { detachViewFromInstance(reactRoot, currentContext.getCatalystInstance()); } } @@ -894,7 +894,7 @@ public List getOrCreateViewManagers( ReactApplicationContext context; synchronized (mReactContextLock) { context = (ReactApplicationContext) getCurrentReactContext(); - if (context == null || !context.hasActiveCatalystInstance()) { + if (context == null || !context.hasActiveReactInstance()) { return null; } } @@ -920,7 +920,7 @@ public List getOrCreateViewManagers( ReactApplicationContext context; synchronized (mReactContextLock) { context = (ReactApplicationContext) getCurrentReactContext(); - if (context == null || !context.hasActiveCatalystInstance()) { + if (context == null || !context.hasActiveReactInstance()) { return null; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index 28d024fd4b002b..184619890efe38 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -170,7 +170,8 @@ public CatalystInstance getCatalystInstance() { return Assertions.assertNotNull(mCatalystInstance); } - public boolean hasActiveCatalystInstance() { + /** @return true if there is an non-null, alive react native instance */ + public boolean hasActiveReactInstance() { return mCatalystInstance != null && !mCatalystInstance.isDestroyed(); } @@ -184,7 +185,7 @@ public LifecycleState getLifecycleState() { public void addLifecycleEventListener(final LifecycleEventListener listener) { mLifecycleEventListeners.add(listener); - if (hasActiveCatalystInstance() || isBridgeless()) { + if (hasActiveReactInstance() || isBridgeless()) { switch (mLifecycleState) { case BEFORE_CREATE: case BEFORE_RESUME: @@ -452,7 +453,7 @@ public JavaScriptContextHolder getJavaScriptContextHolder() { } public @Nullable JSIModule getJSIModule(JSIModuleType moduleType) { - if (!hasActiveCatalystInstance()) { + if (!hasActiveReactInstance()) { throw new IllegalStateException( "Unable to retrieve a JSIModule if CatalystInstance is not active."); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java index eb2111e4db3f16..ecae0c9ed5a34e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java @@ -52,7 +52,7 @@ protected final ReactApplicationContext getReactApplicationContext() { */ @ThreadConfined(ANY) protected @Nullable final ReactApplicationContext getReactApplicationContextIfActiveOrWarn() { - if (mReactApplicationContext.hasActiveCatalystInstance() + if (mReactApplicationContext.hasActiveReactInstance() || mReactApplicationContext.isBridgeless()) { return mReactApplicationContext; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java index c430958706428e..694724292b420d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java @@ -915,7 +915,7 @@ public void run() { @Nullable ReactContext context = mCurrentContext; if (context == null - || (!context.isBridgeless() && !context.hasActiveCatalystInstance())) { + || (!context.isBridgeless() && !context.hasActiveReactInstance())) { return; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java b/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java index b6f60da4e17781..28c86fb7ad85b2 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/jstasks/HeadlessJsTaskContext.java @@ -107,7 +107,7 @@ private synchronized void startTask(final HeadlessJsTaskConfig taskConfig, int t } mActiveTasks.add(taskId); mActiveTaskConfigs.put(taskId, new HeadlessJsTaskConfig(taskConfig)); - if (reactContext.hasActiveCatalystInstance()) { + if (reactContext.hasActiveReactInstance()) { reactContext .getJSModule(AppRegistry.class) .startHeadlessTask(taskId, taskConfig.getTaskKey(), taskConfig.getData()); diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/accessibilityinfo/AccessibilityInfoModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/accessibilityinfo/AccessibilityInfoModule.java index cf45e01e8c8437..bc923fed684d63 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/accessibilityinfo/AccessibilityInfoModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/accessibilityinfo/AccessibilityInfoModule.java @@ -56,7 +56,7 @@ public void onChange(boolean selfChange) { @Override public void onChange(boolean selfChange, Uri uri) { - if (getReactApplicationContext().hasActiveCatalystInstance()) { + if (getReactApplicationContext().hasActiveReactInstance()) { AccessibilityInfoModule.this.updateAndSendReduceMotionChangeEvent(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/appstate/AppStateModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/appstate/AppStateModule.java index d01ca87d50069f..4b5c08a4c31e62 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/appstate/AppStateModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/appstate/AppStateModule.java @@ -100,7 +100,7 @@ private void sendEvent(String eventName, @Nullable Object data) { // We don't gain anything interesting from logging here, and it's an extremely common // race condition for an AppState event to be triggered as the Catalyst instance is being // set up or torn down. So, just fail silently here. - if (!reactApplicationContext.hasActiveCatalystInstance()) { + if (!reactApplicationContext.hasActiveReactInstance()) { return; } reactApplicationContext.getJSModule(RCTDeviceEventEmitter.class).emit(eventName, data); diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogModule.java index 20cb825893816b..429fca866448ca 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogModule.java @@ -61,7 +61,7 @@ public DatePickerDialogListener(final Promise promise) { @Override public void onDateSet(DatePicker view, int year, int month, int day) { - if (!mPromiseResolved && getReactApplicationContext().hasActiveCatalystInstance()) { + if (!mPromiseResolved && getReactApplicationContext().hasActiveReactInstance()) { WritableMap result = new WritableNativeMap(); result.putString("action", ACTION_DATE_SET); result.putInt("year", year); @@ -74,7 +74,7 @@ public void onDateSet(DatePicker view, int year, int month, int day) { @Override public void onDismiss(DialogInterface dialog) { - if (!mPromiseResolved && getReactApplicationContext().hasActiveCatalystInstance()) { + if (!mPromiseResolved && getReactApplicationContext().hasActiveReactInstance()) { WritableMap result = new WritableNativeMap(); result.putString("action", ACTION_DISMISSED); mPromise.resolve(result); diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/deviceinfo/DeviceInfoModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/deviceinfo/DeviceInfoModule.java index 5662ccf89a698c..100682485065f9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/deviceinfo/DeviceInfoModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/deviceinfo/DeviceInfoModule.java @@ -83,7 +83,7 @@ public void emitUpdateDimensionsEvent() { return; } - if (mReactApplicationContext.hasActiveCatalystInstance()) { + if (mReactApplicationContext.hasActiveReactInstance()) { // Don't emit an event to JS if the dimensions haven't changed WritableNativeMap displayMetrics = DisplayMetricsHolder.getDisplayMetricsNativeMap(mFontScale); diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java index 918a30663dcde6..78b91ee2bd0611 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java @@ -130,7 +130,7 @@ public AlertFragmentListener(Callback callback) { public void onClick(DialogInterface dialog, int which) { if (!mCallbackConsumed) { if (getReactApplicationContext().isBridgeless() - || getReactApplicationContext().hasActiveCatalystInstance()) { + || getReactApplicationContext().hasActiveReactInstance()) { mCallback.invoke(ACTION_BUTTON_CLICKED, which); mCallbackConsumed = true; } @@ -141,7 +141,7 @@ public void onClick(DialogInterface dialog, int which) { public void onDismiss(DialogInterface dialog) { if (!mCallbackConsumed) { if (getReactApplicationContext().isBridgeless() - || getReactApplicationContext().hasActiveCatalystInstance()) { + || getReactApplicationContext().hasActiveReactInstance()) { mCallback.invoke(ACTION_DISMISSED); mCallbackConsumed = true; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java index 5ff4eb75ee7dfc..afbe05610b0ff8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java @@ -292,7 +292,7 @@ public boolean performAccessibilityAction(View host, int action, Bundle args) { final WritableMap event = Arguments.createMap(); event.putString("actionName", mAccessibilityActionsMap.get(action)); ReactContext reactContext = (ReactContext) host.getContext(); - if (reactContext.hasActiveCatalystInstance()) { + if (reactContext.hasActiveReactInstance()) { final int reactTag = host.getId(); final int surfaceId = UIManagerHelper.getSurfaceId(reactContext); UIManager uiManager = UIManagerHelper.getUIManager(reactContext, reactTag); diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java index f68548d86ba645..b2b82aca343f29 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java @@ -72,7 +72,7 @@ private static UIManager getUIManager( } // TODO T60461551: add tests to verify emission of events when the ReactContext is being turn // down. - if (!context.hasActiveCatalystInstance()) { + if (!context.hasActiveReactInstance()) { ReactSoftException.logSoftException( "UIManagerHelper", new ReactNoCrashSoftException( diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java index 49b05347a2040c..641b8459ec1329 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java @@ -89,7 +89,7 @@ private RCTEventEmitter getEventEmitter(int reactTag) { int type = ViewUtil.getUIManagerType(reactTag); assert type == UIManagerType.DEFAULT; if (mRCTEventEmitter == null) { - if (mReactContext.hasActiveCatalystInstance()) { + if (mReactContext.hasActiveReactInstance()) { mRCTEventEmitter = mReactContext.getJSModule(RCTEventEmitter.class); } else { ReactSoftException.logSoftException( diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java index 5dbd0796e53363..53111f412758ff 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java @@ -111,7 +111,7 @@ public long measure( text, layout, sTextPaintInstance, themedReactContext); WritableMap event = Arguments.createMap(); event.putArray("lines", lines); - if (themedReactContext.hasActiveCatalystInstance()) { + if (themedReactContext.hasActiveReactInstance()) { themedReactContext .getJSModule(RCTEventEmitter.class) .receiveEvent(getReactTag(), "topTextLayout", event); diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java index fc08cc8177bbb4..8c624bb62a71df 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java @@ -90,7 +90,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { mFrameTimeNanos = INITIAL_FRAME_TIME_NANOS; mReactApplicationContextMock = mock(ReactApplicationContext.class); - PowerMockito.when(mReactApplicationContextMock.hasActiveCatalystInstance()) + PowerMockito.when(mReactApplicationContextMock.hasActiveReactInstance()) .thenAnswer( new Answer() { @Override diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java index 619b464d1d7c0b..6f12eee70cecef 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java @@ -60,7 +60,7 @@ public void setUp() throws Exception { mActivity = mActivityController.create().start().resume().get(); final ReactApplicationContext context = PowerMockito.mock(ReactApplicationContext.class); - PowerMockito.when(context.hasActiveCatalystInstance()).thenReturn(true); + PowerMockito.when(context.hasActiveReactInstance()).thenReturn(true); PowerMockito.when(context, "getCurrentActivity").thenReturn(mActivity); mDialogModule = new DialogModule(context); diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java index b62d42b83d4893..321a9d87c8b314 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java @@ -96,7 +96,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { CatalystInstance reactInstance = mock(CatalystInstance.class); ReactApplicationContext reactContext = mock(ReactApplicationContext.class); when(reactContext.getCatalystInstance()).thenReturn(reactInstance); - when(reactContext.hasActiveCatalystInstance()).thenReturn(true); + when(reactContext.hasActiveReactInstance()).thenReturn(true); when(reactContext.getJSModule(any(Class.class))).thenReturn(mEmitter); mNetworkingModule = new NetworkingModule(reactContext, "", mHttpClient); } diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/timing/TimingModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/timing/TimingModuleTest.java index 7a98a0f2ba86e3..7164b8489af447 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/timing/TimingModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/timing/TimingModuleTest.java @@ -75,7 +75,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { CatalystInstance reactInstance = mock(CatalystInstance.class); ReactApplicationContext reactContext = mock(ReactApplicationContext.class); when(reactContext.getCatalystInstance()).thenReturn(reactInstance); - when(reactContext.hasActiveCatalystInstance()).thenReturn(true); + when(reactContext.hasActiveReactInstance()).thenReturn(true); mCurrentTimeNs = 0; mPostFrameCallbackHandler = new PostFrameCallbackHandler();