From f671f8868f8bf8e7976263e5b2665c2005df7329 Mon Sep 17 00:00:00 2001 From: "Andrew Chen (Eng)" Date: Mon, 22 Oct 2018 17:26:15 -0700 Subject: [PATCH] Don't crash when ReactRootView's id is already set Summary: This is a bandaid fix to address a crash with a stack trace involving ``` com.facebook.react.uimanager.IllegalViewOperationException: Trying to add a root view with an explicit id already set. React Native uses the id field to track react tags and will overwrite this field. If that is fine, explicitly overwrite the id field to View.NO_ID before calling addRootView. 0+com.facebook.react.uimanager.NativeViewHierarchyManager.addRootViewGroup(NativeViewHierarchyManager.java:546) [inlined] 1+com.facebook.react.uimanager.NativeViewHierarchyManager.addRootView(NativeViewHierarchyManager.java:538) [inlined] 2+com.facebook.react.uimanager.UIViewOperationQueue.addRootView(UIViewOperationQueue.java:678) [inlined] 3+com.facebook.react.uimanager.UIImplementation.registerRootView(UIImplementation.java:216) [inlined] 4+com.facebook.react.uimanager.UIManagerModule.addRootView(UIManagerModule.java:355) 5+com.facebook.react.ReactInstanceManager.attachRootViewToInstance(ReactInstanceManager.java:1032) 6+com.facebook.react.ReactInstanceManager.attachRootView(ReactInstanceManager.java:726) [inlined] 7+com.facebook.react.ReactRootView.attachToReactInstanceManager(ReactRootView.java:524) 8+com.facebook.react.ReactRootView.startReactApplication(ReactRootView.java:377) ``` This crash seems to be happening because the root view's id is set to a non NO_ID value, but further up in the stack trace, UIManager.addRootView() is called which always sets the root view's id to NO_ID. It's not clear how this error is happening, but in this code path it's expected that addRootView should always ensure that the id is always reset. In order to avoid crashing for this, let's remove the exception and log the issue instead. In the future, we should not be overloading the android view id field with these types of react native implementation details. The react tag should be stored as a view tag instead. Reviewed By: mmmulani Differential Revision: D10499861 fbshipit-source-id: 4dffedab4e7a34eee7f64bb43ec8209699521c73 --- .../react/uimanager/NativeViewHierarchyManager.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index 430f2478a2b..280034983eb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -543,10 +543,12 @@ public class NativeViewHierarchyManager { ViewGroup view, ThemedReactContext themedContext) { if (view.getId() != View.NO_ID) { - throw new IllegalViewOperationException( - "Trying to add a root view with an explicit id already set. React Native uses " + - "the id field to track react tags and will overwrite this field. If that is fine, " + - "explicitly overwrite the id field to View.NO_ID before calling addRootView."); + themedContext.handleException( + new IllegalViewOperationException( + "Trying to add a root view with an explicit id (" + view.getId() + ") already " + + "set. React Native uses the id field to track react tags and will overwrite this field. " + + "If that is fine, explicitly overwrite the id field to View.NO_ID before calling " + + "addRootView.")); } mTagsToViews.put(tag, view);