Remove ReactImageView Legacy Background Path (#46167)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46167

## This Diff

This removes the legacy path from ReactImageView and its view manager.

## This Stack

This removes the non-Style-applicator background management paths of the different native components. There have been multiple conflicting changes, and bugs added bc harder to reason about, which motivates making this change as soon as possible. This also lets us formalize guarantees that BaseViewManager may safely manipulate background styling of all built in native components.

There is one still known issue, where BackgroundStyleApplicator does not propagate I18nManager derived layout direction to borders (compared to Android derived root direction). This is mostly an issue for apps that with LTR and RTL context, or force a layout direction, which I would guess is relatively rare, so my plan is to forward fix this later this by enabling set_android_layout_direction which will solve that problem mopre generically.

Changelog: [Internal]

Reviewed By: cortinico

Differential Revision: D61657253

fbshipit-source-id: 96cf1160e466de78c2f133f0e4fb2d9b2e7cf478

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageManager.kt

@@ -16,12 +16,10 @@ import com.facebook.drawee.controller.AbstractDraweeControllerBuilder
 import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.common.ReactConstants
-import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.module.annotations.ReactModule
 import com.facebook.react.uimanager.BackgroundStyleApplicator
 import com.facebook.react.uimanager.LengthPercentage
 import com.facebook.react.uimanager.LengthPercentageType
-import com.facebook.react.uimanager.PixelUtil.dpToPx
 import com.facebook.react.uimanager.SimpleViewManager
 import com.facebook.react.uimanager.ThemedReactContext
 import com.facebook.react.uimanager.ViewProps
@@ -141,15 +139,7 @@ public constructor(
 
   @ReactProp(name = "borderColor", customType = "Color")
   public fun setBorderColor(view: ReactImageView, borderColor: Int?) {
-    if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBorderColor(view, LogicalEdge.ALL, borderColor)
-    } else {
-      if (borderColor == null) {
-        view.setBorderColor(Color.TRANSPARENT)
-      } else {
-        view.setBorderColor(borderColor)
-      }
-    }
+    BackgroundStyleApplicator.setBorderColor(view, LogicalEdge.ALL, borderColor)
   }
 
   @ReactProp(name = "overlayColor", customType = "Color")
@@ -163,11 +153,7 @@ public constructor(
 
   @ReactProp(name = "borderWidth")
   public fun setBorderWidth(view: ReactImageView, borderWidth: Float) {
-    if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBorderWidth(view, LogicalEdge.ALL, borderWidth)
-    } else {
-      view.setBorderWidth(borderWidth)
-    }
+    BackgroundStyleApplicator.setBorderWidth(view, LogicalEdge.ALL, borderWidth)
   }
 
   @ReactPropGroup(
@@ -180,24 +166,10 @@ public constructor(
               ViewProps.BORDER_BOTTOM_LEFT_RADIUS],
       defaultFloat = Float.NaN)
   public fun setBorderRadius(view: ReactImageView, index: Int, borderRadius: Float) {
-    if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) {
-      val radius =
-          if (borderRadius.isNaN()) null
-          else LengthPercentage(borderRadius, LengthPercentageType.POINT)
-      BackgroundStyleApplicator.setBorderRadius(view, BorderRadiusProp.values()[index], radius)
-    } else {
-      val convertedBorderRadius =
-          if (!borderRadius.isNaN()) {
-            borderRadius.dpToPx()
-          } else {
-            borderRadius
-          }
-      if (index == 0) {
-        view.setBorderRadius(convertedBorderRadius)
-      } else {
-        view.setBorderRadius(convertedBorderRadius, index - 1)
-      }
-    }
+    val radius =
+        if (borderRadius.isNaN()) null
+        else LengthPercentage(borderRadius, LengthPercentageType.POINT)
+    BackgroundStyleApplicator.setBorderRadius(view, BorderRadiusProp.values()[index], radius)
   }
 
   @ReactProp(name = ViewProps.RESIZE_MODE)
@@ -262,20 +234,14 @@ public constructor(
 
   @ReactProp(name = ViewProps.BOX_SHADOW, customType = "BoxShadow")
   public fun setBoxShadow(view: ReactImageView, shadows: ReadableArray?): Unit {
-    if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBoxShadow(view, shadows)
-    }
+    BackgroundStyleApplicator.setBoxShadow(view, shadows)
   }
 
   public override fun setBackgroundColor(
       view: ReactImageView,
       @ColorInt backgroundColor: Int
   ): Unit {
-    if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBackgroundColor(view, backgroundColor)
-    } else {
-      super.setBackgroundColor(view, backgroundColor)
-    }
+    BackgroundStyleApplicator.setBackgroundColor(view, backgroundColor)
   }
 
   public override fun getExportedCustomDirectEventTypeConstants(): MutableMap<String, Any> =

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.kt

@@ -28,7 +28,6 @@ import com.facebook.drawee.controller.AbstractDraweeControllerBuilder
 import com.facebook.drawee.controller.ControllerListener
 import com.facebook.drawee.controller.ForwardingControllerListener
 import com.facebook.drawee.drawable.AutoRotateDrawable
-import com.facebook.drawee.drawable.RoundedColorDrawable
 import com.facebook.drawee.drawable.ScalingUtils
 import com.facebook.drawee.generic.GenericDraweeHierarchyBuilder
 import com.facebook.drawee.generic.RoundingParams
@@ -49,17 +48,13 @@ import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.common.annotations.VisibleForTesting
 import com.facebook.react.common.build.ReactBuildConfig
 import com.facebook.react.config.ReactFeatureFlags
-import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags.enableBackgroundStyleApplicator
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags.loadVectorDrawablesOnImages
-import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags.useNewReactImageViewBackgroundDrawing
 import com.facebook.react.modules.fresco.ReactNetworkImageRequest
 import com.facebook.react.uimanager.BackgroundStyleApplicator
-import com.facebook.react.uimanager.FloatUtil.floatsEqual
 import com.facebook.react.uimanager.LengthPercentage
 import com.facebook.react.uimanager.LengthPercentageType
 import com.facebook.react.uimanager.PixelUtil.dpToPx
 import com.facebook.react.uimanager.PixelUtil.pxToDp
-import com.facebook.react.uimanager.Spacing
 import com.facebook.react.uimanager.UIManagerHelper
 import com.facebook.react.uimanager.style.BorderRadiusProp
 import com.facebook.react.uimanager.style.LogicalEdge
@@ -76,8 +71,6 @@ import com.facebook.react.views.imagehelper.ImageSource
 import com.facebook.react.views.imagehelper.ImageSource.Companion.getTransparentBitmapImageSource
 import com.facebook.react.views.imagehelper.MultiSourceHelper.getBestSourceForSize
 import com.facebook.react.views.imagehelper.ResourceDrawableIdHelper.Companion.instance
-import com.facebook.react.views.view.ReactViewBackgroundManager
-import com.facebook.yoga.YogaConstants
 import kotlin.math.abs
 
 /**
@@ -97,13 +90,7 @@ public class ReactImageView(
   private var cachedImageSource: ImageSource? = null
   private var defaultImageDrawable: Drawable? = null
   private var loadingImageDrawable: Drawable? = null
-  private var backgroundImageDrawable: RoundedColorDrawable? = null
-  private var backgroundColor = 0x00000000
-  private var borderColor = 0
   private var overlayColor = 0
-  private var borderWidth = 0f
-  private var borderRadius = YogaConstants.UNDEFINED
-  private var borderCornerRadii: FloatArray? = null
   private var scaleType = defaultValue()
   private var tileMode = defaultTileMode()
   private var isDirty = false
@@ -115,11 +102,9 @@ public class ReactImageView(
   private var progressiveRenderingEnabled = false
   private var headers: ReadableMap? = null
   private var resizeMultiplier = 1.0f
-  private val reactBackgroundManager = ReactViewBackgroundManager(this)
   private var resizeMethod = ImageResizeMethod.AUTO
 
   init {
-    reactBackgroundManager.setOverflow("hidden")
     // Workaround Android bug where ImageView visibility is not propagated to the Drawable, so you
     // have to manually update visibility. Will be resolved once we move to VitoView.
     setLegacyVisibilityHandlingEnabled(true)
@@ -213,26 +198,11 @@ public class ReactImageView(
   }
 
   public override fun setBackgroundColor(backgroundColor: Int) {
-    if (enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBackgroundColor(this, backgroundColor)
-    } else if (useNewReactImageViewBackgroundDrawing()) {
-      reactBackgroundManager.backgroundColor = backgroundColor
-    } else if (this.backgroundColor != backgroundColor) {
-      this.backgroundColor = backgroundColor
-      backgroundImageDrawable = RoundedColorDrawable(backgroundColor)
-      isDirty = true
-    }
+    BackgroundStyleApplicator.setBackgroundColor(this, backgroundColor)
   }
 
   public fun setBorderColor(borderColor: Int) {
-    if (enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBorderColor(this, LogicalEdge.ALL, borderColor)
-    } else if (useNewReactImageViewBackgroundDrawing()) {
-      reactBackgroundManager.setBorderColor(Spacing.ALL, borderColor)
-    } else if (this.borderColor != borderColor) {
-      this.borderColor = borderColor
-      isDirty = true
-    }
+    BackgroundStyleApplicator.setBorderColor(this, LogicalEdge.ALL, borderColor)
   }
 
   public fun setOverlayColor(overlayColor: Int) {
@@ -243,49 +213,21 @@ public class ReactImageView(
   }
 
   public fun setBorderWidth(borderWidth: Float) {
-    val newBorderWidth = borderWidth.dpToPx()
-    if (enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.setBorderWidth(this, LogicalEdge.ALL, borderWidth)
-    } else if (useNewReactImageViewBackgroundDrawing()) {
-      reactBackgroundManager.setBorderWidth(Spacing.ALL, newBorderWidth)
-    } else if (!floatsEqual(this.borderWidth, newBorderWidth)) {
-      this.borderWidth = newBorderWidth
-      isDirty = true
-    }
+    BackgroundStyleApplicator.setBorderWidth(this, LogicalEdge.ALL, borderWidth)
   }
 
   public fun setBorderRadius(borderRadius: Float) {
-    if (enableBackgroundStyleApplicator()) {
-      val radius =
-          if (borderRadius.isNaN()) null
-          else LengthPercentage(borderRadius.pxToDp(), LengthPercentageType.POINT)
-      BackgroundStyleApplicator.setBorderRadius(this, BorderRadiusProp.BORDER_RADIUS, radius)
-    } else if (useNewReactImageViewBackgroundDrawing()) {
-      reactBackgroundManager.setBorderRadius(borderRadius)
-    } else if (!floatsEqual(this.borderRadius, borderRadius)) {
-      this.borderRadius = borderRadius
-      isDirty = true
-    }
+    val radius =
+        if (borderRadius.isNaN()) null
+        else LengthPercentage(borderRadius.pxToDp(), LengthPercentageType.POINT)
+    BackgroundStyleApplicator.setBorderRadius(this, BorderRadiusProp.BORDER_RADIUS, radius)
   }
 
   public fun setBorderRadius(borderRadius: Float, position: Int) {
-    if (enableBackgroundStyleApplicator()) {
-      val radius =
-          if (borderRadius.isNaN()) null
-          else LengthPercentage(borderRadius.pxToDp(), LengthPercentageType.POINT)
-      BackgroundStyleApplicator.setBorderRadius(this, BorderRadiusProp.values()[position], radius)
-    } else if (useNewReactImageViewBackgroundDrawing()) {
-      reactBackgroundManager.setBorderRadius(borderRadius, position + 1)
-    } else {
-      if (borderCornerRadii == null) {
-        borderCornerRadii = FloatArray(4) { YogaConstants.UNDEFINED }
-      }
-
-      if (!floatsEqual(borderCornerRadii?.get(position), borderRadius)) {
-        borderCornerRadii?.set(position, borderRadius)
-        isDirty = true
-      }
-    }
+    val radius =
+        if (borderRadius.isNaN()) null
+        else LengthPercentage(borderRadius.pxToDp(), LengthPercentageType.POINT)
+    BackgroundStyleApplicator.setBorderRadius(this, BorderRadiusProp.values()[position], radius)
   }
 
   public fun setScaleType(scaleType: ScalingUtils.ScaleType) {
@@ -395,11 +337,7 @@ public class ReactImageView(
   public override fun hasOverlappingRendering(): Boolean = false
 
   public override fun onDraw(canvas: Canvas) {
-    if (enableBackgroundStyleApplicator()) {
-      BackgroundStyleApplicator.clipToPaddingBox(this, canvas)
-    } else if (useNewReactImageViewBackgroundDrawing()) {
-      reactBackgroundManager.maybeClipToPaddingBox(canvas)
-    }
+    BackgroundStyleApplicator.clipToPaddingBox(this, canvas)
     super.onDraw(canvas)
   }
 
@@ -439,22 +377,8 @@ public class ReactImageView(
       hierarchy.setPlaceholderImage(loadingImageDrawable, ScalingUtils.ScaleType.CENTER)
     }
 
-    getCornerRadii(computedCornerRadii)
-
     val roundingParams = hierarchy.roundingParams
     if (roundingParams != null) {
-      roundingParams.setCornersRadii(
-          computedCornerRadii[0],
-          computedCornerRadii[1],
-          computedCornerRadii[2],
-          computedCornerRadii[3])
-
-      backgroundImageDrawable?.let { background ->
-        background.setBorder(borderColor, borderWidth)
-        roundingParams.cornersRadii?.let { background.radii = it }
-        hierarchy.setBackgroundImage(background)
-      }
-      roundingParams.setBorder(borderColor, borderWidth)
       if (overlayColor != Color.TRANSPARENT) {
         roundingParams.setOverlayColor(overlayColor)
       } else {
@@ -578,16 +502,6 @@ public class ReactImageView(
     }
   }
 
-  private fun getCornerRadii(computedCorners: FloatArray) {
-    val defaultBorderRadius = if (!YogaConstants.isUndefined(borderRadius)) borderRadius else 0f
-
-    val radii = borderCornerRadii ?: FloatArray(4) { Float.NaN }
-    computedCorners[0] = if (!YogaConstants.isUndefined(radii[0])) radii[0] else defaultBorderRadius
-    computedCorners[1] = if (!YogaConstants.isUndefined(radii[1])) radii[1] else defaultBorderRadius
-    computedCorners[2] = if (!YogaConstants.isUndefined(radii[2])) radii[2] else defaultBorderRadius
-    computedCorners[3] = if (!YogaConstants.isUndefined(radii[3])) radii[3] else defaultBorderRadius
-  }
-
   @VisibleForTesting
   public fun setControllerListener(controllerListener: ControllerListener<ImageInfo>?) {
     controllerForTesting = controllerListener
@@ -709,8 +623,6 @@ public class ReactImageView(
   public companion object {
     public const val REMOTE_IMAGE_FADE_DURATION_MS: Int = 300
 
-    private val computedCornerRadii = FloatArray(4)
-
     // Fresco lacks support for repeating images, see https://github.com/facebook/fresco/issues/1575
     // We implement it here as a postprocessing step.
     private val tileMatrix = Matrix()

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/image/ReactImagePropertyTest.kt

@@ -19,7 +19,7 @@ import com.facebook.react.bridge.JavaOnlyMap
 import com.facebook.react.bridge.ReactTestHelper.createMockCatalystInstance
 import com.facebook.react.bridge.WritableArray
 import com.facebook.react.bridge.WritableMap
-import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
+import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
 import com.facebook.react.uimanager.DisplayMetricsHolder
 import com.facebook.react.uimanager.ReactStylesDiffMap
 import com.facebook.react.uimanager.ThemedReactContext
@@ -38,13 +38,7 @@ import org.mockito.Mockito.mockStatic
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.RuntimeEnvironment
 
-/**
- * Verify that [ScalingUtils] properties are being applied correctly by [ReactImageManager].
- *
- * TODO T195191609: The border rendering tests rely on introspecting Fresco tree, which is no longer
- * relevant when "useNewReactImageViewBackgroundDrawing" is enabled. These should be replaced by
- * screenshot tests over the existing RNTester examples.
- */
+/** Verify that [ScalingUtils] properties are being applied correctly by [ReactImageManager]. */
 @RunWith(RobolectricTestRunner::class)
 class ReactImagePropertyTest {
 
@@ -53,7 +47,6 @@ class ReactImagePropertyTest {
   private lateinit var themeContext: ThemedReactContext
   private lateinit var arguments: MockedStatic<Arguments>
   private lateinit var rnLog: MockedStatic<RNLog>
-  private lateinit var featureFlags: MockedStatic<ReactNativeFeatureFlags>
 
   @Before
   fun setup() {
@@ -64,12 +57,6 @@ class ReactImagePropertyTest {
     rnLog = mockStatic(RNLog::class.java)
     rnLog.`when`<Boolean> { RNLog.w(any(), anyString()) }.thenAnswer {}
 
-    // Avoid trying to load ReactNativeFeatureFlags JNI library
-    featureFlags = mockStatic(ReactNativeFeatureFlags::class.java)
-    featureFlags
-        .`when`<Boolean> { ReactNativeFeatureFlags.useNewReactImageViewBackgroundDrawing() }
-        .thenAnswer { false }
-
     SoLoader.setInTestMode()
     context = BridgeReactContext(RuntimeEnvironment.getApplication())
     catalystInstanceMock = createMockCatalystInstance()
@@ -77,6 +64,8 @@ class ReactImagePropertyTest {
     themeContext = ThemedReactContext(context, context, null, -1)
     Fresco.initialize(context)
     DisplayMetricsHolder.setWindowDisplayMetrics(DisplayMetrics())
+
+    ReactNativeFeatureFlagsForTests.setUp()
   }
 
   @After
@@ -84,55 +73,12 @@ class ReactImagePropertyTest {
     DisplayMetricsHolder.setWindowDisplayMetrics(null)
     arguments.close()
     rnLog.close()
-    featureFlags.close()
   }
 
   private fun buildStyles(vararg keysAndValues: Any?): ReactStylesDiffMap {
     return ReactStylesDiffMap(JavaOnlyMap.of(*keysAndValues))
   }
 
-  @Test
-  fun testBorderColor() {
-    val viewManager = ReactImageManager()
-    val view = viewManager.createViewInstance(themeContext)
-    viewManager.updateProperties(
-        view,
-        buildStyles("src", JavaOnlyArray.of(JavaOnlyMap.of("uri", "http://mysite.com/mypic.jpg"))))
-    viewManager.updateProperties(view, buildStyles("borderColor", Color.argb(0, 0, 255, 255)))
-    var borderColor = view.hierarchy.roundingParams!!.borderColor
-    assertThat(Color.alpha(borderColor)).isEqualTo(0)
-    assertThat(Color.red(borderColor)).isEqualTo(0)
-    assertThat(Color.green(borderColor)).isEqualTo(255)
-    assertThat(Color.blue(borderColor)).isEqualTo(255)
-    viewManager.updateProperties(view, buildStyles("borderColor", Color.argb(0, 255, 50, 128)))
-    borderColor = view.hierarchy.roundingParams!!.borderColor
-    assertThat(Color.alpha(borderColor)).isEqualTo(0)
-    assertThat(Color.red(borderColor)).isEqualTo(255)
-    assertThat(Color.green(borderColor)).isEqualTo(50)
-    assertThat(Color.blue(borderColor)).isEqualTo(128)
-    viewManager.updateProperties(view, buildStyles("borderColor", null))
-    borderColor = view.hierarchy.roundingParams!!.borderColor
-    assertThat(Color.alpha(borderColor)).isEqualTo(0)
-    assertThat(Color.red(borderColor)).isEqualTo(0)
-    assertThat(Color.green(borderColor)).isEqualTo(0)
-    assertThat(Color.blue(borderColor)).isEqualTo(0)
-  }
-
-  @Test
-  fun testRoundedCorners() {
-    val viewManager = ReactImageManager()
-    val view = viewManager.createViewInstance(themeContext)
-    viewManager.updateProperties(
-        view,
-        buildStyles("src", JavaOnlyArray.of(JavaOnlyMap.of("uri", "http://mysite.com/mypic.jpg"))))
-
-    // We can't easily verify if rounded corner was honored or not, this tests simply verifies
-    // we're not crashing..
-    viewManager.updateProperties(view, buildStyles("borderRadius", 10.0))
-    viewManager.updateProperties(view, buildStyles("borderRadius", 0.0))
-    viewManager.updateProperties(view, buildStyles("borderRadius", null))
-  }
-
   @Test
   fun testAccessibilityFocus() {
     val viewManager = ReactImageManager()