Refactor creation of views in Fabric Android

Summary:
This diff refactors the createViewInstance method in order to ensure that viewID is set before props are updated in the view.
This is necessary because there are components that deliver events at the same time their props are set. This means that some components might not have their viewId set correctly when events are delivered.
Since viewId is used to determine if a view belongs to Fabric or Paper, there are cases when the events are not delivered to the right renderer

changelog: [internal]

Reviewed By: JoshuaGross

Differential Revision: D25667987

fbshipit-source-id: 4acfa8f80d66e9e59514354481957d7d3b571248

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java

@@ -533,7 +533,7 @@ public class MountingManager {
       // View Managers are responsible for dealing with initial state and props.
       view =
           viewManager.createView(
-              themedReactContext, propsDiffMap, stateWrapper, mJSResponderHandler);
+              reactTag, themedReactContext, propsDiffMap, stateWrapper, mJSResponderHandler);
       view.setId(reactTag);
     }
 

ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java

@@ -278,7 +278,7 @@ public class NativeViewHierarchyManager {
     try {
       ViewManager viewManager = mViewManagers.get(className);
 
-      View view = viewManager.createView(themedContext, null, null, mJSResponderHandler);
+      View view = viewManager.createView(tag, themedContext, null, null, mJSResponderHandler);
       mTagsToViews.put(tag, view);
       mTagsToViewManagers.put(tag, viewManager);
 

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java

@@ -67,19 +67,14 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode>
     return null;
   }
 
-  /** Creates a view and installs event emitters on it. */
-  private final @NonNull T createView(
-      @NonNull ThemedReactContext reactContext, JSResponderHandler jsResponderHandler) {
-    return createView(reactContext, null, null, jsResponderHandler);
-  }
-
   /** Creates a view with knowledge of props and state. */
   public @NonNull T createView(
+      int reactTag,
       @NonNull ThemedReactContext reactContext,
       @Nullable ReactStylesDiffMap props,
       @Nullable StateWrapper stateWrapper,
       JSResponderHandler jsResponderHandler) {
-    T view = createViewInstance(reactContext, props, stateWrapper);
+    T view = createViewInstance(reactTag, reactContext, props, stateWrapper);
     if (view instanceof ReactInterceptingViewGroup) {
       ((ReactInterceptingViewGroup) view).setOnInterceptTouchEventListener(jsResponderHandler);
     }
@@ -129,13 +124,18 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode>
    * that will call createViewInstance for you. Override it if you need props upon creation of the
    * view.
    *
-   * @param reactContext
+   * @param reactTag reactTag that should be set as ID of the view instance
+   * @param reactContext ReactContext used to initialize view instance
+   * @param initialProps initial props for the view instance
+   * @param stateWrapper initial state for the view instance
    */
   protected @NonNull T createViewInstance(
+      int reactTag,
       @NonNull ThemedReactContext reactContext,
       @Nullable ReactStylesDiffMap initialProps,
       @Nullable StateWrapper stateWrapper) {
     T view = createViewInstance(reactContext);
+    view.setId(reactTag);
     addEventEmitters(reactContext, view);
     if (initialProps != null) {
       updateProperties(view, initialProps);

ReactAndroid/src/test/java/com/facebook/react/uimanager/SimpleViewPropertyTest.java

@@ -36,6 +36,8 @@ public class SimpleViewPropertyTest {
 
   @Rule public PowerMockRule rule = new PowerMockRule();
 
+  private static int sViewTag = 2;
+
   private static class ConcreteViewManager extends SimpleViewManager<View> {
 
     @ReactProp(name = "foo")
@@ -75,7 +77,9 @@ public class SimpleViewPropertyTest {
 
   @Test
   public void testOpacity() {
-    View view = mManager.createView(mThemedContext, buildStyles(), null, new JSResponderHandler());
+    View view =
+        mManager.createView(
+            sViewTag, mThemedContext, buildStyles(), null, new JSResponderHandler());
 
     mManager.updateProperties(view, buildStyles());
     assertThat(view.getAlpha()).isEqualTo(1.0f);
@@ -89,7 +93,9 @@ public class SimpleViewPropertyTest {
 
   @Test
   public void testBackgroundColor() {
-    View view = mManager.createView(mThemedContext, buildStyles(), null, new JSResponderHandler());
+    View view =
+        mManager.createView(
+            sViewTag, mThemedContext, buildStyles(), null, new JSResponderHandler());
 
     mManager.updateProperties(view, buildStyles());
     assertThat(view.getBackground()).isEqualTo(null);