Refactor calls to UIManagerHelper.getUIManager

Summary: This diff refactors the usages of UIManagerHelper.getUIManager() to make sure we always consider null objects. Some of the callsites were throwing a NullPointerExcetpion, now they throw a more explicit exception. changelog: [internal] Reviewed By: makovkastar Differential Revision: D19383064 fbshipit-source-id: 1806a37528e80cab1c8fdff5eb631aaf47bde819
facebook · Jan 23, 2020 · 39089b4 · 39089b4
1 parent edcbfb9
commit 39089b4
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 21 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java
@@ -1104,12 +1104,16 @@ private void attachRootViewToInstance(final ReactRoot reactRoot) {
     Log.d(ReactConstants.TAG, "ReactInstanceManager.attachRootViewToInstance()");
     Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "attachRootViewToInstance");
 
-    // UIManager is technically Nullable here, but if we can't get a UIManager
-    // at this point, something has probably gone horribly wrong so it's probably best
-    // to throw a NullPointerException.
+    @Nullable
     UIManager uiManager =
         UIManagerHelper.getUIManager(mCurrentReactContext, reactRoot.getUIManagerType());
 
+    // If we can't get a UIManager something has probably gone horribly wrong
+    if (uiManager == null) {
+      throw new IllegalStateException(
+          "Unable to attache a rootView to ReactInstance when UIManager is not properly initialized.");
+    }
+
     @Nullable Bundle initialProperties = reactRoot.getAppProperties();
 
     final int rootTag =

diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java
@@ -34,6 +34,7 @@
 import com.facebook.react.bridge.ReactContext;
 import com.facebook.react.bridge.ReactMarker;
 import com.facebook.react.bridge.ReactMarkerConstants;
+import com.facebook.react.bridge.UIManager;
 import com.facebook.react.bridge.UiThreadUtil;
 import com.facebook.react.bridge.WritableMap;
 import com.facebook.react.bridge.WritableNativeMap;
@@ -125,6 +126,7 @@ private void init() {
 
   @Override
   protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
+    // TODO: T60453649 - Add test automation to verify behavior of onMeasure
     setAllowImmediateUIOperationExecution(false);
 
     if (mUseSurface) {
@@ -441,8 +443,13 @@ private void updateRootLayoutSpecs(final int widthMeasureSpec, final int heightM
     final ReactContext reactApplicationContext = mReactInstanceManager.getCurrentReactContext();
 
     if (reactApplicationContext != null) {
-      UIManagerHelper.getUIManager(reactApplicationContext, getUIManagerType())
-          .updateRootLayoutSpecs(getRootViewTag(), widthMeasureSpec, heightMeasureSpec);
+      @Nullable
+      UIManager uiManager =
+          UIManagerHelper.getUIManager(reactApplicationContext, getUIManagerType());
+      // Ignore calling updateRootLayoutSpecs if UIManager is not properly initialized.
+      if (uiManager != null) {
+        uiManager.updateRootLayoutSpecs(getRootViewTag(), widthMeasureSpec, heightMeasureSpec);
+      }
     }
   }
 
@@ -474,8 +481,13 @@ private void setAllowImmediateUIOperationExecution(boolean flag) {
       return;
     }
 
-    UIManagerHelper.getUIManager(reactApplicationContext, getUIManagerType())
-        .setAllowImmediateUIOperationExecution(flag);
+    @Nullable
+    UIManager uiManager = UIManagerHelper.getUIManager(reactApplicationContext, getUIManagerType());
+    // Ignore calling setAllowImmediateUIOperationExecution if UIManager is not properly
+    // initialized.
+    if (uiManager != null) {
+      uiManager.setAllowImmediateUIOperationExecution(flag);
+    }
   }
 
   /**

diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java
@@ -680,22 +680,18 @@ public void dispatchViewManagerCommand(
       int reactTag, Dynamic commandId, @Nullable ReadableArray commandArgs) {
     // TODO: this is a temporary approach to support ViewManagerCommands in Fabric until
     // the dispatchViewManagerCommand() method is supported by Fabric JS API.
+    @Nullable
+    UIManager uiManager =
+        UIManagerHelper.getUIManager(
+            getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag));
+    if (uiManager == null) {
+      return;
+    }
+
     if (commandId.getType() == ReadableType.Number) {
-      final int commandIdNum = commandId.asInt();
-      UIManager uiManager =
-          UIManagerHelper.getUIManager(
-              getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag));
-      if (uiManager != null) {
-        uiManager.dispatchCommand(reactTag, commandIdNum, commandArgs);
-      }
+      uiManager.dispatchCommand(reactTag, commandId.asInt(), commandArgs);
     } else if (commandId.getType() == ReadableType.String) {
-      final String commandIdStr = commandId.asString();
-      UIManager uiManager =
-          UIManagerHelper.getUIManager(
-              getReactApplicationContext(), ViewUtil.getUIManagerType(reactTag));
-      if (uiManager != null) {
-        uiManager.dispatchCommand(reactTag, commandIdStr, commandArgs);
-      }
+      uiManager.dispatchCommand(reactTag, commandId.asString(), commandArgs);
     }
   }