From 94cb4bf90c7c4c26500cdec190d3988b9bdf0092 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Thu, 24 Oct 2019 17:25:56 -0700 Subject: [PATCH] In modules or classes that call ReactApplicationContext.getJSModule, ensure that there's still a CatalystInstance alive Summary: In D18032458 we introduce getReactApplicationContextIfActiveOrWarn. In this diff, modules that access a JS or Native module through ReactApplicationContext need to check if the CatalystInstance is still alive before continuing. Modules that don't derive from `ReactContextBaseJavaModule` manually check for the catalyst impl and log their own SoftExceptions. In this diff we also introduce SoftExceptions that by contract never cause crashes, even in debug mode. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D18112989 fbshipit-source-id: 868f01f388aa2db3518db9f873f2afc2a62eed45 --- .../bridge/ReactNoCrashSoftException.java | 19 +++++++++++++++++++ .../modules/deviceinfo/DeviceInfoModule.java | 15 ++++++++++++--- .../uimanager/ReactAccessibilityDelegate.java | 13 ++++++++++--- .../react/views/text/ReactTextShadowNode.java | 18 ++++++++++++++---- .../react/views/text/ReactTextView.java | 7 ++++--- 5 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/ReactNoCrashSoftException.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactNoCrashSoftException.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactNoCrashSoftException.java new file mode 100644 index 0000000000..46c749fb6f --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactNoCrashSoftException.java @@ -0,0 +1,19 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.bridge; + +/** + * Extends RuntimeException so that it may be caught by a {@link ReactSoftExceptionListener}. Any + * {@link ReactSoftExceptionListener} that catches a ReactNoCrashSoftException should log it only + * and not crash, no matter what. + */ +public class ReactNoCrashSoftException extends RuntimeException { + public ReactNoCrashSoftException(String detailMessage) { + super(detailMessage); + } +} 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 8e421a507a..c612182f4c 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 @@ -12,6 +12,8 @@ import androidx.annotation.Nullable; import com.facebook.react.bridge.BaseJavaModule; import com.facebook.react.bridge.LifecycleEventListener; import com.facebook.react.bridge.ReactApplicationContext; +import com.facebook.react.bridge.ReactNoCrashSoftException; +import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.modules.core.DeviceEventManagerModule; import com.facebook.react.turbomodule.core.interfaces.TurboModule; @@ -77,9 +79,16 @@ public class DeviceInfoModule extends BaseJavaModule return; } - mReactApplicationContext - .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class) - .emit("didUpdateDimensions", DisplayMetricsHolder.getDisplayMetricsNativeMap(mFontScale)); + if (mReactApplicationContext.hasActiveCatalystInstance()) { + mReactApplicationContext + .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class) + .emit("didUpdateDimensions", DisplayMetricsHolder.getDisplayMetricsNativeMap(mFontScale)); + } else { + ReactSoftException.logSoftException( + NAME, + new ReactNoCrashSoftException( + "No active CatalystInstance, cannot emitUpdateDimensionsEvent")); + } } @Override 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 056ef71dfc..bf69ff00cd 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java @@ -25,6 +25,8 @@ import com.facebook.react.R; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Dynamic; import com.facebook.react.bridge.ReactContext; +import com.facebook.react.bridge.ReactNoCrashSoftException; +import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.ReadableMapKeySetIterator; @@ -278,9 +280,14 @@ public class ReactAccessibilityDelegate extends AccessibilityDelegateCompat { final WritableMap event = Arguments.createMap(); event.putString("actionName", mAccessibilityActionsMap.get(action)); ReactContext reactContext = (ReactContext) host.getContext(); - reactContext - .getJSModule(RCTEventEmitter.class) - .receiveEvent(host.getId(), "topAccessibilityAction", event); + if (reactContext.hasActiveCatalystInstance()) { + reactContext + .getJSModule(RCTEventEmitter.class) + .receiveEvent(host.getId(), "topAccessibilityAction", event); + } else { + ReactSoftException.logSoftException( + TAG, new ReactNoCrashSoftException("Cannot get RCTEventEmitter, no CatalystInstance")); + } // In order to make Talkback announce the change of the adjustable's value, // schedule to send a TYPE_VIEW_SELECTED event after performing the scroll actions. 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 c1a7983530..28180b31f9 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 @@ -20,11 +20,14 @@ import android.widget.TextView; import androidx.annotation.Nullable; import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.Arguments; +import com.facebook.react.bridge.ReactNoCrashSoftException; +import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; import com.facebook.react.uimanager.NativeViewHierarchyOptimizer; import com.facebook.react.uimanager.ReactShadowNode; import com.facebook.react.uimanager.Spacing; +import com.facebook.react.uimanager.ThemedReactContext; import com.facebook.react.uimanager.UIViewOperationQueue; import com.facebook.react.uimanager.annotations.ReactProp; import com.facebook.react.uimanager.events.RCTEventEmitter; @@ -157,14 +160,21 @@ public class ReactTextShadowNode extends ReactBaseTextShadowNode { } if (mShouldNotifyOnTextLayout) { + ThemedReactContext themedReactContext = getThemedContext(); WritableArray lines = FontMetricsUtil.getFontMetrics( - text, layout, sTextPaintInstance, getThemedContext()); + text, layout, sTextPaintInstance, themedReactContext); WritableMap event = Arguments.createMap(); event.putArray("lines", lines); - getThemedContext() - .getJSModule(RCTEventEmitter.class) - .receiveEvent(getReactTag(), "topTextLayout", event); + if (themedReactContext.hasActiveCatalystInstance()) { + themedReactContext + .getJSModule(RCTEventEmitter.class) + .receiveEvent(getReactTag(), "topTextLayout", event); + } else { + ReactSoftException.logSoftException( + "ReactTextShadowNode", + new ReactNoCrashSoftException("Cannot get RCTEventEmitter, no CatalystInstance")); + } } if (mNumberOfLines != UNSET && mNumberOfLines < layout.getLineCount()) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java index f28095ff24..02ff950015 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java @@ -110,13 +110,14 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie return; } - if (!getReactContext().hasCatalystInstance()) { + ReactContext reactContext = getReactContext(); + if (!reactContext.hasCatalystInstance()) { // In bridgeless mode there's no Catalyst instance; in that case, bail. // TODO (T45503888): Figure out how to support nested views from JS or cpp. return; } - UIManagerModule uiManager = getReactContext().getNativeModule(UIManagerModule.class); + UIManagerModule uiManager = reactContext.getNativeModule(UIManagerModule.class); Spanned text = (Spanned) getText(); Layout layout = getLayout(); @@ -253,7 +254,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie WritableMap event = Arguments.createMap(); event.putArray("inlineViews", inlineViewInfoArray2); - getReactContext() + reactContext .getJSModule(RCTEventEmitter.class) .receiveEvent(getId(), "topInlineViewLayout", event); }