Skip to content

Commit ffb3737

Browse files
javachefacebook-github-bot
authored andcommitted
Schedule OnViewAttachMountItems in GuardedRunnable (#52143)
Summary: Pull Request resolved: #52143 Noticed that some of the `IndexOutOfBoundsException` crashes we've been tracking we're not being reported as soft errors because they were not running wrapped by the RN ExceptionHandler Changelog: [Internal] Reviewed By: lenaic Differential Revision: D77017423 fbshipit-source-id: 760297a0c5ee3d58577931829a31d312dacffdf1
1 parent e297fe1 commit ffb3737

1 file changed

Lines changed: 46 additions & 47 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java

Lines changed: 46 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.facebook.common.logging.FLog;
2222
import com.facebook.infer.annotation.Assertions;
2323
import com.facebook.infer.annotation.ThreadConfined;
24+
import com.facebook.react.bridge.GuardedRunnable;
2425
import com.facebook.react.bridge.ReactNoCrashSoftException;
2526
import com.facebook.react.bridge.ReactSoftExceptionLogger;
2627
import com.facebook.react.bridge.ReadableArray;
@@ -189,57 +190,55 @@ private void addRootView(@NonNull final View rootView) {
189190
mTagToViewState.put(mSurfaceId, new ViewState(mSurfaceId, rootView, mRootViewManager, true));
190191

191192
Runnable runnable =
192-
() -> {
193-
// The CPU has ticked since `addRootView` was called, so the surface could technically
194-
// have already stopped here.
195-
if (isStopped()) {
196-
return;
197-
}
193+
new GuardedRunnable(Assertions.assertNotNull(mThemedReactContext)) {
194+
@Override
195+
public void runGuarded() {
196+
// The CPU has ticked since `addRootView` was called, so the surface could technically
197+
// have already stopped here.
198+
if (isStopped()) {
199+
return;
200+
}
198201

199-
if (rootView.getId() == mSurfaceId) {
200-
ReactSoftExceptionLogger.logSoftException(
201-
TAG,
202-
new IllegalViewOperationException(
203-
"Race condition in addRootView detected. Trying to set an id of ["
204-
+ mSurfaceId
205-
+ "] on the RootView, but that id has already been set. "));
206-
} else if (rootView.getId() != View.NO_ID) {
207-
FLog.e(
208-
TAG,
209-
"Trying to add RootTag to RootView that already has a tag: existing tag: [%d] new"
210-
+ " tag: [%d]",
211-
rootView.getId(),
212-
mSurfaceId);
213-
// This behavior can not be guaranteed in hybrid apps that have a native android layer
214-
// over
215-
// which reactRootViews are added and the native views need to have ids on them in order
216-
// to
217-
// work.
218-
// Hence this can cause unnecessary crashes at runtime for hybrid apps.
219-
// So converting this to a soft exception such that pure react-native devs can still see
220-
// the
221-
// warning while hybrid apps continue to run without crashes
222-
ReactSoftExceptionLogger.logSoftException(
223-
TAG,
224-
new IllegalViewOperationException(
225-
"Trying to add a root view with an explicit id already set. React Native uses"
226-
+ " the id field to track react tags and will overwrite this field. If that"
227-
+ " is fine, explicitly overwrite the id field to View.NO_ID before calling"
228-
+ " addRootView."));
229-
}
230-
rootView.setId(mSurfaceId);
202+
if (rootView.getId() == mSurfaceId) {
203+
ReactSoftExceptionLogger.logSoftException(
204+
TAG,
205+
new IllegalViewOperationException(
206+
"Race condition in addRootView detected. Trying to set an id of ["
207+
+ mSurfaceId
208+
+ "] on the RootView, but that id has already been set. "));
209+
} else if (rootView.getId() != View.NO_ID) {
210+
FLog.e(
211+
TAG,
212+
"Trying to add RootTag to RootView that already has a tag: existing tag: [%d] new"
213+
+ " tag: [%d]",
214+
rootView.getId(),
215+
mSurfaceId);
216+
// This behavior can not be guaranteed in hybrid apps that have a native android layer
217+
// over which reactRootViews are added and the native views need to have ids on them
218+
// in order to work. Hence this can cause unnecessary crashes at runtime for hybrid
219+
// apps. So converting this to a soft exception such that pure react-native devs can
220+
// still see the warning while hybrid apps continue to run without crashes
221+
ReactSoftExceptionLogger.logSoftException(
222+
TAG,
223+
new IllegalViewOperationException(
224+
"Trying to add a root view with an explicit id already set. React Native uses"
225+
+ " the id field to track react tags and will overwrite this field. If"
226+
+ " that is fine, explicitly overwrite the id field to View.NO_ID before"
227+
+ " calling addRootView."));
228+
}
229+
rootView.setId(mSurfaceId);
231230

232-
if (rootView instanceof ReactRoot) {
233-
((ReactRoot) rootView).setRootViewTag(mSurfaceId);
234-
}
231+
if (rootView instanceof ReactRoot) {
232+
((ReactRoot) rootView).setRootViewTag(mSurfaceId);
233+
}
235234

236-
executeMountItemsOnViewAttach();
235+
executeMountItemsOnViewAttach();
237236

238-
// By doing this after `executeMountItemsOnViewAttach`, we ensure
239-
// that any operations scheduled while processing this queue are
240-
// also added to the queue, instead of being processed immediately
241-
// through the queue in `MountItemDispatcher`.
242-
mRootViewAttached = true;
237+
// By doing this after `executeMountItemsOnViewAttach`, we ensure that any operations
238+
// scheduled while processing this queue are also added to the queue, instead of being
239+
// processed immediately through the queue in `MountItemDispatcher`.
240+
mRootViewAttached = true;
241+
}
243242
};
244243

245244
if (UiThreadUtil.isOnUiThread()) {

0 commit comments

Comments
 (0)