Fix padding flickering visible during initial render of text input

Summary: This diff fixes a race condition that caused a flicker during initial rendering of TextInput in Fabric The root cause is that the TextInput's State is sometimes initialized with no information from the Theme, this causes text input to be rendered with 0 padding. Later ReactTextInput manager updates TextInput state with theme data and the TextInput is re-rendered with the correct padding information. changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D22034849 fbshipit-source-id: a2fa91f34a8340878f2ec8d231ef6c86cee08f05
2020-06-13 22:18:06 -07:00 · 2020-06-13 22:18:06 -07:00 · 6d6268e903
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@ -14,6 +14,10 @@ import static com.facebook.react.fabric.mounting.LayoutMetricsConversions.getMax
 import static com.facebook.react.fabric.mounting.LayoutMetricsConversions.getMinSize;
 import static com.facebook.react.fabric.mounting.LayoutMetricsConversions.getYogaMeasureMode;
 import static com.facebook.react.fabric.mounting.LayoutMetricsConversions.getYogaSize;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_BOTTOM_INDEX;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_END_INDEX;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_START_INDEX;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_TOP_INDEX;
 import static com.facebook.react.uimanager.common.UIManagerType.FABRIC;

 import android.annotation.SuppressLint;
@ -73,6 +77,7 @@ import com.facebook.react.uimanager.ReactRoot;
 import com.facebook.react.uimanager.ReactRootViewTagGenerator;
 import com.facebook.react.uimanager.StateWrapper;
 import com.facebook.react.uimanager.ThemedReactContext;
+import com.facebook.react.uimanager.UIManagerHelper;
 import com.facebook.react.uimanager.ViewManagerPropertyUpdater;
 import com.facebook.react.uimanager.ViewManagerRegistry;
 import com.facebook.react.uimanager.events.EventDispatcher;
@ -496,6 +501,32 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
        attachmentsPositions);
  }

+  /**
+   * @param surfaceID {@link int} surface ID
+   * @param defaultTextInputPadding {@link float[]} output parameter will contain the default theme
+   *     padding used by RN Android TextInput.
+   * @return if theme data is available in the output parameters.
+   */
+  @DoNotStrip
+  public boolean getThemeData(int surfaceID, float[] defaultTextInputPadding) {
+    ThemedReactContext themedReactContext = mReactContextForRootTag.get(surfaceID);
+    if (themedReactContext == null) {
+      // TODO T68526882: Review if this should cause a crash instead.
+      ReactSoftException.logSoftException(
+          TAG,
+          new ReactNoCrashSoftException(
+              "Unable to find ThemedReactContext associated to surfaceID: " + surfaceID));
+      return false;
+    }
+    float[] defaultTextInputPaddingForTheme =
+        UIManagerHelper.getDefaultTextInputPadding(themedReactContext);
+    defaultTextInputPadding[0] = defaultTextInputPaddingForTheme[PADDING_START_INDEX];
+    defaultTextInputPadding[1] = defaultTextInputPaddingForTheme[PADDING_END_INDEX];
+    defaultTextInputPadding[2] = defaultTextInputPaddingForTheme[PADDING_TOP_INDEX];
+    defaultTextInputPadding[3] = defaultTextInputPaddingForTheme[PADDING_BOTTOM_INDEX];
+    return true;
+  }
+
  @Override
  @UiThread
  @ThreadConfined(UI)
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp
@ -43,9 +43,6 @@ struct JMountItem : public JavaClass<JMountItem> {
      "Lcom/facebook/react/fabric/mounting/mountitems/MountItem;";
 };

-static constexpr auto UIManagerJavaDescriptor =
-    "com/facebook/react/fabric/FabricUIManager";
-
 struct RemoveDeleteMetadata {
  Tag tag;
  Tag parentTag;
@ -366,7 +363,7 @@ local_ref<JMountItem::javaobject> createUpdateEventEmitterMountItem(
  cEventEmitter->eventEmitter = eventEmitter;

  static auto updateEventEmitterInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint, jobject)>(
              "updateEventEmitterMountItem");

@ -387,7 +384,7 @@ local_ref<JMountItem::javaobject> createUpdatePropsMountItem(
  local_ref<ReadableMap::javaobject> readableMap =
      castReadableMap(ReadableNativeMap::newObjectCxxArgs(newProps));
  static auto updatePropsInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint, ReadableMap::javaobject)>(
              "updatePropsMountItem");

@ -404,7 +401,7 @@ local_ref<JMountItem::javaobject> createUpdateLayoutMountItem(
  if (newChildShadowView.layoutMetrics != EmptyLayoutMetrics &&
      oldChildShadowView.layoutMetrics != newChildShadowView.layoutMetrics) {
    static auto updateLayoutInstruction =
-        jni::findClassStatic(UIManagerJavaDescriptor)
+        jni::findClassStatic(Binding::UIManagerJavaDescriptor)
            ->getMethod<alias_ref<JMountItem>(
                jint, jint, jint, jint, jint, jint)>("updateLayoutMountItem");
    auto layoutMetrics = newChildShadowView.layoutMetrics;
@ -436,7 +433,7 @@ local_ref<JMountItem::javaobject> createUpdatePaddingMountItem(
  }

  static auto updateLayoutInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint, jint, jint, jint, jint)>(
              "updatePaddingMountItem");

@ -457,7 +454,7 @@ local_ref<JMountItem::javaobject> createInsertMountItem(
    const jni::global_ref<jobject> &javaUIManager,
    const ShadowViewMutation &mutation) {
  static auto insertInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint, jint, jint)>(
              "insertMountItem");

@ -472,7 +469,7 @@ local_ref<JMountItem::javaobject> createUpdateStateMountItem(
    const jni::global_ref<jobject> &javaUIManager,
    const ShadowViewMutation &mutation) {
  static auto updateStateInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint, jobject)>(
              "updateStateMountItem");

@ -498,7 +495,7 @@ local_ref<JMountItem::javaobject> createRemoveMountItem(
    const jni::global_ref<jobject> &javaUIManager,
    const ShadowViewMutation &mutation) {
  static auto removeInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint, jint, jint)>(
              "removeMountItem");

@ -513,7 +510,7 @@ local_ref<JMountItem::javaobject> createDeleteMountItem(
    const jni::global_ref<jobject> &javaUIManager,
    const ShadowViewMutation &mutation) {
  static auto deleteInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jint)>("deleteMountItem");

  return deleteInstruction(javaUIManager, mutation.oldChildShadowView.tag);
@ -536,7 +533,7 @@ local_ref<JMountItem::javaobject> createRemoveAndDeleteMultiMountItem(
  }

  static auto removeDeleteMultiInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(jintArray)>(
              "removeDeleteMultiMountItem");

@ -561,7 +558,7 @@ local_ref<JMountItem::javaobject> createCreateMountItem(
    const ShadowViewMutation &mutation,
    const Tag surfaceId) {
  static auto createJavaInstruction =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(
              jstring, ReadableMap::javaobject, jobject, jint, jint, jboolean)>(
              "createMountItem");
@ -850,7 +847,7 @@ void Binding::schedulerDidFinishTransaction(
  }

  static auto createMountItemsBatchContainer =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<alias_ref<JMountItem>(
              jint, jtypeArray<JMountItem::javaobject>, jint, jint)>(
              "createBatchMountItem");
@ -864,17 +861,18 @@ void Binding::schedulerDidFinishTransaction(
      position,
      commitNumber);

-  static auto scheduleMountItem = jni::findClassStatic(UIManagerJavaDescriptor)
-                                      ->getMethod<void(
-                                          JMountItem::javaobject,
-                                          jint,
-                                          jlong,
-                                          jlong,
-                                          jlong,
-                                          jlong,
-                                          jlong,
-                                          jlong,
-                                          jlong)>("scheduleMountItem");
+  static auto scheduleMountItem =
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
+          ->getMethod<void(
+              JMountItem::javaobject,
+              jint,
+              jlong,
+              jlong,
+              jlong,
+              jlong,
+              jlong,
+              jlong,
+              jlong)>("scheduleMountItem");

  auto finishTransactionEndTime = telemetryTimePointNow();

@ -903,7 +901,7 @@ void Binding::onAnimationStarted() {
  }

  static auto layoutAnimationsStartedJNI =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<void()>("onAnimationStarted");

  layoutAnimationsStartedJNI(localJavaUIManager);
@ -916,7 +914,7 @@ void Binding::onAllAnimationsComplete() {
  }

  static auto allAnimationsCompleteJNI =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<void()>("onAllAnimationsComplete");

  allAnimationsCompleteJNI(localJavaUIManager);
@ -947,7 +945,7 @@ void Binding::schedulerDidRequestPreliminaryViewAllocation(
  }

  static auto preallocateView =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<void(
              jint, jint, jstring, ReadableMap::javaobject, jobject, jboolean)>(
              "preallocateView");
@ -988,7 +986,7 @@ void Binding::schedulerDidDispatchCommand(
  }

  static auto dispatchCommand =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<void(jint, jstring, ReadableArray::javaobject)>(
              "dispatchCommand");

@ -1013,7 +1011,7 @@ void Binding::schedulerDidSetJSResponder(
  }

  static auto setJSResponder =
-      jni::findClassStatic(UIManagerJavaDescriptor)
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
          ->getMethod<void(jint, jint, jboolean)>("setJSResponder");

  setJSResponder(
@ -1031,8 +1029,9 @@ void Binding::schedulerDidClearJSResponder() {
    return;
  }

-  static auto clearJSResponder = jni::findClassStatic(UIManagerJavaDescriptor)
-                                     ->getMethod<void()>("clearJSResponder");
+  static auto clearJSResponder =
+      jni::findClassStatic(Binding::UIManagerJavaDescriptor)
+          ->getMethod<void()>("clearJSResponder");

  clearJSResponder(localJavaUIManager);
 }
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h
@ -32,6 +32,9 @@ class Binding : public jni::HybridClass<Binding>,
  constexpr static const char *const kJavaDescriptor =
      "Lcom/facebook/react/fabric/Binding;";

+  constexpr static auto UIManagerJavaDescriptor =
+      "com/facebook/react/fabric/FabricUIManager";
+
  static void registerNatives();

 private:
--- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java
@ -13,7 +13,9 @@ import static com.facebook.react.uimanager.common.ViewUtil.getUIManagerType;
 import android.content.Context;
 import android.content.ContextWrapper;
 import android.view.View;
+import android.widget.EditText;
 import androidx.annotation.Nullable;
+import androidx.core.view.ViewCompat;
 import com.facebook.react.bridge.CatalystInstance;
 import com.facebook.react.bridge.JSIModuleType;
 import com.facebook.react.bridge.ReactContext;
@ -27,6 +29,11 @@ import com.facebook.react.uimanager.events.EventDispatcherProvider;
 /** Helper class for {@link UIManager}. */
 public class UIManagerHelper {

+  public static final int PADDING_START_INDEX = 0;
+  public static final int PADDING_END_INDEX = 1;
+  public static final int PADDING_TOP_INDEX = 2;
+  public static final int PADDING_BOTTOM_INDEX = 3;
+
  /** @return a {@link UIManager} that can handle the react tag received by parameter. */
  @Nullable
  public static UIManager getUIManagerForReactTag(ReactContext context, int reactTag) {
@ -110,4 +117,18 @@ public class UIManagerHelper {
    }
    return (ReactContext) context;
  }
+
+  /**
+   * @return the default padding used by Android EditText's. This method returns the padding in an
+   *     array to avoid extra classloading during hot-path of RN Android.
+   */
+  public static float[] getDefaultTextInputPadding(ThemedReactContext context) {
+    EditText editText = new EditText(context);
+    float[] padding = new float[4];
+    padding[PADDING_START_INDEX] = PixelUtil.toDIPFromPixel(ViewCompat.getPaddingStart(editText));
+    padding[PADDING_END_INDEX] = PixelUtil.toDIPFromPixel(ViewCompat.getPaddingEnd(editText));
+    padding[PADDING_TOP_INDEX] = PixelUtil.toDIPFromPixel(editText.getPaddingTop());
+    padding[PADDING_BOTTOM_INDEX] = PixelUtil.toDIPFromPixel(editText.getPaddingBottom());
+    return padding;
+  }
 }
--- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
@ -7,6 +7,10 @@

 package com.facebook.react.views.textinput;

+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_BOTTOM_INDEX;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_END_INDEX;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_START_INDEX;
+import static com.facebook.react.uimanager.UIManagerHelper.PADDING_TOP_INDEX;
 import static com.facebook.react.uimanager.UIManagerHelper.getReactContext;

 import android.content.Context;
@ -31,7 +35,6 @@ import android.widget.EditText;
 import android.widget.TextView;
 import androidx.annotation.Nullable;
 import androidx.core.content.ContextCompat;
-import androidx.core.view.ViewCompat;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.Dynamic;
@ -1248,7 +1251,6 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
      ReactContext reactContext = UIManagerHelper.getReactContext(view);
      if (reactContext instanceof ThemedReactContext) {
        ThemedReactContext themedReactContext = (ThemedReactContext) reactContext;
-        EditText editText = createInternalEditText(themedReactContext);

        // Even though we check `data["textChanged"].empty()` before using the value in C++,
        // state updates crash without this value on key exception. It's unintuitive why
@ -1256,13 +1258,12 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
        // so leave this here until we can figure out a better way of key-existence-checking in C++.
        update.putNull("textChanged");

-        update.putDouble(
-            "themePaddingStart", PixelUtil.toDIPFromPixel(ViewCompat.getPaddingStart(editText)));
-        update.putDouble(
-            "themePaddingEnd", PixelUtil.toDIPFromPixel(ViewCompat.getPaddingEnd(editText)));
-        update.putDouble("themePaddingTop", PixelUtil.toDIPFromPixel(editText.getPaddingTop()));
-        update.putDouble(
-            "themePaddingBottom", PixelUtil.toDIPFromPixel(editText.getPaddingBottom()));
+        // TODO T68526882 review is themePadding can be removed from TextInput
+        float[] padding = UIManagerHelper.getDefaultTextInputPadding(themedReactContext);
+        update.putDouble("themePaddingStart", padding[PADDING_START_INDEX]);
+        update.putDouble("themePaddingEnd", padding[PADDING_END_INDEX]);
+        update.putDouble("themePaddingTop", padding[PADDING_TOP_INDEX]);
+        update.putDouble("themePaddingBottom", padding[PADDING_BOTTOM_INDEX]);

        stateWrapper.updateState(update);
      } else {
--- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputComponentDescriptor.h
+++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputComponentDescriptor.h
@ -7,6 +7,7 @@

 #pragma once

+#include <fbjni/fbjni.h>
 #include <react/core/ConcreteComponentDescriptor.h>
 #include "AndroidTextInputShadowNode.h"

@ -48,6 +49,26 @@ class AndroidTextInputComponentDescriptor final
      defaultThemePaddingEnd = ((YGValue)theme[YGEdgeEnd]).value;
      defaultThemePaddingTop = ((YGValue)theme[YGEdgeTop]).value;
      defaultThemePaddingBottom = ((YGValue)theme[YGEdgeBottom]).value;
+    } else {
+      const jni::global_ref<jobject> &fabricUIManager =
+          contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");
+
+      auto env = jni::Environment::current();
+      auto defaultTextInputPaddingArray = env->NewFloatArray(4);
+      static auto getThemeData =
+          jni::findClassStatic(UIManagerJavaDescriptor)
+              ->getMethod<jboolean(jint, jfloatArray)>("getThemeData");
+
+      if (getThemeData(
+              fabricUIManager, surfaceId, defaultTextInputPaddingArray)) {
+        jfloat *defaultTextInputPadding =
+            env->GetFloatArrayElements(defaultTextInputPaddingArray, 0);
+        defaultThemePaddingStart = defaultTextInputPadding[0];
+        defaultThemePaddingEnd = defaultTextInputPadding[1];
+        defaultThemePaddingTop = defaultTextInputPadding[2];
+        defaultThemePaddingBottom = defaultTextInputPadding[3];
+      }
+      env->DeleteLocalRef(defaultTextInputPaddingArray);
    }

    return std::make_shared<AndroidTextInputShadowNode::ConcreteState>(
@ -177,6 +198,10 @@ class AndroidTextInputComponentDescriptor final
  }

 private:
+  // TODO T68526882: Unify with Binding::UIManagerJavaDescriptor
+  constexpr static auto UIManagerJavaDescriptor =
+      "com/facebook/react/fabric/FabricUIManager";
+
  SharedTextLayoutManager textLayoutManager_;
  mutable better::map<int, YGStyle::Edges> surfaceIdToThemePaddingMap_;
 };