Fix Image payloads that Props 2.0 sends wrong or drops (#59077)

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

@@ -369,8 +369,12 @@ public class ReactImageView(
     // no worth marking as dirty if it already rendered.
   }
 
+  /** Sets the request headers. A change rebuilds the image request on the next update. */
   public fun setHeaders(headers: ReadableMap?) {
-    this.headers = headers
+    if (this.headers != headers) {
+      this.headers = headers
+      isDirty = true
+    }
   }
 
   // Disable rasterizing to offscreen layer in order to preserve background effects like box-shadow

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

@@ -15,14 +15,17 @@ import android.graphics.Color
 import android.util.DisplayMetrics
 import com.facebook.common.logging.FLog
 import com.facebook.drawee.backends.pipeline.Fresco
+import com.facebook.drawee.backends.pipeline.PipelineDraweeControllerBuilder
 import com.facebook.drawee.drawable.ScalingUtils
+import com.facebook.imagepipeline.request.ImageRequest
 import com.facebook.react.bridge.Arguments
 import com.facebook.react.bridge.BridgeReactContext
 import com.facebook.react.bridge.CatalystInstance
 import com.facebook.react.bridge.JavaOnlyMap
 import com.facebook.react.bridge.ReactTestHelper.createMockCatalystInstance
 import com.facebook.react.common.ReactConstants
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
+import com.facebook.react.modules.fresco.ReactNetworkImageRequest
 import com.facebook.react.uimanager.BackgroundStyleApplicator
 import com.facebook.react.uimanager.DisplayMetricsHolder
 import com.facebook.react.uimanager.LengthPercentage
@@ -40,12 +43,15 @@ import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.MockedStatic
+import org.mockito.Mockito.RETURNS_SELF
 import org.mockito.Mockito.anyString
 import org.mockito.Mockito.mockStatic
 import org.mockito.kotlin.any
+import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
 import org.mockito.kotlin.reset
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.RuntimeEnvironment
@@ -281,4 +287,27 @@ class ReactImagePropertyTest {
     viewManager.setHeaders(mockView, headers)
     verify(mockView).setHeaders(headers)
   }
+
+  @Test
+  fun testHeadersChangeRebuildsImageRequest() {
+    val controllerBuilder = mock<PipelineDraweeControllerBuilder>(defaultAnswer = RETURNS_SELF)
+    val viewManager = ReactImageManager(controllerBuilder)
+    val view = viewManager.createViewInstance(themeContext)
+    val sources = Arguments.createArray()
+    sources.pushMap(JavaOnlyMap.of("uri", "https://example.com/image.png"))
+    viewManager.setSource(view, sources)
+    viewManager.setHeaders(view, JavaOnlyMap.of("Authorization", "token-1"))
+    view.maybeUpdateView()
+
+    viewManager.setHeaders(view, JavaOnlyMap.of("Authorization", "token-2"))
+    view.maybeUpdateView()
+    viewManager.setHeaders(view, JavaOnlyMap.of("Authorization", "token-2"))
+    view.maybeUpdateView()
+
+    val requests = argumentCaptor<ImageRequest>()
+    verify(controllerBuilder, times(2)).setImageRequest(requests.capture())
+    assertThat(requests.firstValue.sourceUri).isEqualTo(requests.secondValue.sourceUri)
+    assertThat((requests.secondValue as ReactNetworkImageRequest).headers)
+        .isEqualTo(JavaOnlyMap.of("Authorization", "token-2"))
+  }
 }

packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp

@@ -12,6 +12,9 @@
 #include <react/renderer/debug/debugStringConvertibleUtils.h>
 #include <react/renderer/graphics/Color.h>
 
+#include <algorithm>
+#include <tuple>
+
 namespace facebook::react {
 
 ImageProps::ImageProps(
@@ -145,6 +148,30 @@ static folly::dynamic convertEdgeInsets(const EdgeInsets& edgeInsets) {
   return edgeInsetsResult;
 }
 
+// ImageSource::operator== compares only the request identity: type and URI.
+static bool hasSamePayload(const ImageSource& lhs, const ImageSource& rhs) {
+  return std::tie(
+             lhs.type,
+             lhs.uri,
+             lhs.bundle,
+             lhs.scale,
+             lhs.size,
+             lhs.body,
+             lhs.method,
+             lhs.cache,
+             lhs.headers) ==
+      std::tie(
+             rhs.type,
+             rhs.uri,
+             rhs.bundle,
+             rhs.scale,
+             rhs.size,
+             rhs.body,
+             rhs.method,
+             rhs.cache,
+             rhs.headers);
+}
+
 ComponentName ImageProps::getDiffPropsImplementationTarget() const {
   return "Image";
 }
@@ -158,11 +185,14 @@ folly::dynamic ImageProps::getDiffProps(const Props* prevProps) const {
 
   folly::dynamic result = ViewProps::getDiffProps(oldProps);
 
-  if (sources != oldProps->sources) {
+  if (!std::ranges::equal(sources, oldProps->sources, hasSamePayload)) {
     auto sourcesArray = folly::dynamic::array();
     for (const auto& source : sources) {
       sourcesArray.push_back(toDynamic(source));
     }
+    if (!sources.empty()) {
+      result["headers"] = sourcesArray[0]["headers"];
+    }
     result["source"] = sourcesArray;
   }
 
@@ -221,7 +251,8 @@ folly::dynamic ImageProps::getDiffProps(const Props* prevProps) const {
     SharedColor prevTintColorValue =
         oldProps->tintColor.value_or(SharedColor{});
     if (tintColorValue != prevTintColorValue) {
-      result["tintColor"] = *tintColorValue;
+      result["tintColor"] = tintColorValue ? folly::dynamic(*tintColorValue)
+                                           : folly::dynamic(nullptr);
     }
   }
 

packages/react-native/ReactCommon/react/renderer/components/image/tests/ImagePropsDiffTest.cpp

@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include <gtest/gtest.h>
+
+#include <react/renderer/components/image/ImageProps.h>
+#include <react/test_utils/PropsPayloads.h>
+
+#ifdef RN_SERIALIZABLE_STATE
+
+namespace facebook::react {
+
+namespace {
+
+using folly::dynamic;
+
+// Props 2.0 serializes every field of a source, JS only the ones it sets.
+void expectPayloads(const std::vector<PropsCommit>& commits) {
+  expectPayloadsMatchRawProps<ImageProps>(commits, {"source"});
+}
+
+} // namespace
+
+// ReactImageView.setSource reads width and height at the top level of each
+// source when there are several, and ReactImageManager reads the request
+// headers from the top-level headers prop, which JS derives from the first
+// source.
+TEST(ImagePropsDiffTest, multipleSourcesCarrySizesAndHeaders) {
+  dynamic headers = dynamic::object("Authorization", "token");
+  expectPayloads({{dynamic::object(
+      "source",
+      dynamic::array(
+          dynamic::object("uri", "https://a")("width", 10)("height", 20)(
+              "headers", headers),
+          dynamic::object("uri", "https://b")("width", 30)("height", 40)))(
+      "headers", headers)}});
+}
+
+// ReactImageManager ignores a null headers prop, so only an empty map clears
+// the headers of the previous source.
+TEST(ImagePropsDiffTest, clearsHeadersWhenTheNewSourceHasNone) {
+  dynamic headers = dynamic::object("Authorization", "old");
+  expectPayloads({
+      {dynamic::object(
+          "source",
+          dynamic::array(
+              dynamic::object("uri", "https://a")("headers", headers)))(
+          "headers", headers)},
+      {dynamic::object(
+           "source", dynamic::array(dynamic::object("uri", "https://b")))(
+           "headers", nullptr),
+       dynamic::object("headers", dynamic::object())},
+  });
+}
+
+// ReactImageManager clears the tint only for null; 0 is a transparent SRC_IN
+// tint that hides the image.
+TEST(ImagePropsDiffTest, removingTintColorEmitsNull) {
+  expectPayloads({
+      {dynamic::object("tintColor", static_cast<int32_t>(0xFFFF0000))},
+      {dynamic::object("tintColor", nullptr)},
+  });
+}
+
+// ReactImageView reads the cache policy and the sizes of each source, and
+// ReactImageManager the headers; a source with the same URI can change them.
+TEST(ImagePropsDiffTest, emitsASameUriSourceWhenItsOtherFieldsChange) {
+  dynamic headers = dynamic::object("Authorization", "old");
+  dynamic source = dynamic::object("uri", "https://a")("width", 10)(
+      "height", 20)("scale", 1)("cache", "default")("headers", headers);
+  std::vector<PropsCommit> commits{
+      {dynamic::object("source", dynamic::array(source))("headers", headers)}};
+  dynamic changes = dynamic::object("cache", "reload")("width", 30)("scale", 2);
+  for (const auto& [field, value] : changes.items()) {
+    source[field] = value;
+    // Props 2.0 sends the first source's headers whenever the sources change.
+    commits.push_back(
+        {dynamic::object("source", dynamic::array(source)),
+         dynamic::object("headers", headers)});
+  }
+  source["headers"] = dynamic::object("Authorization", "new");
+  commits.push_back({dynamic::object("source", dynamic::array(source))(
+      "headers", source["headers"])});
+  expectPayloads(commits);
+}
+
+} // namespace facebook::react
+
+#endif

packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h

@@ -62,10 +62,8 @@ class ImageSource {
     imageSourceResult["bundle"] = bundle;
     imageSourceResult["scale"] = scale;
 
-    folly::dynamic sizeResult = folly::dynamic::object();
-    sizeResult["width"] = size.width;
-    sizeResult["height"] = size.height;
-    imageSourceResult["size"] = sizeResult;
+    imageSourceResult["width"] = size.width;
+    imageSourceResult["height"] = size.height;
 
     imageSourceResult["body"] = body;
     imageSourceResult["method"] = method;

packages/react-native/ReactCommon/react/test_utils/PropsPayloads.h

@@ -0,0 +1,131 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <gtest/gtest.h>
+
+#include <react/renderer/core/DynamicPropsUtilities.h>
+#include <react/renderer/core/PropsParserContext.h>
+#include <react/renderer/core/RawProps.h>
+#include <react/renderer/core/RawPropsParser.h>
+#include <react/utils/ContextContainer.h>
+
+#include <algorithm>
+#include <cmath>
+#include <memory>
+#include <set>
+#include <string>
+#include <vector>
+
+#ifdef RN_SERIALIZABLE_STATE
+
+namespace facebook::react {
+
+/*
+ * One commit: the props JS sends (a removed prop is null), and the Props 2.0
+ * values that intentionally differ from what Props 1.5 sends.
+ */
+struct PropsCommit {
+  folly::dynamic props;
+  folly::dynamic intendedDifferences = folly::dynamic::object();
+};
+
+namespace detail {
+
+inline bool payloadValuesMatch(const folly::dynamic &actual, const folly::dynamic &expected)
+{
+  if (actual.isNumber() && expected.isNumber()) {
+    // Props 2.0 stores floats, so 0.998 comes back as 0.99800002574...
+    auto tolerance = 1e-6 * std::max(1.0, std::abs(expected.asDouble()));
+    return std::abs(actual.asDouble() - expected.asDouble()) <= tolerance;
+  }
+  if (actual.isArray() && expected.isArray()) {
+    return actual.size() == expected.size() &&
+        std::equal(actual.begin(), actual.end(), expected.begin(), payloadValuesMatch);
+  }
+  if (actual.isObject() && expected.isObject()) {
+    return actual.size() == expected.size() && std::ranges::all_of(expected.items(), [&](const auto &item) {
+             return actual.count(item.first) != 0 && payloadValuesMatch(actual[item.first], item.second);
+           });
+  }
+  return actual == expected;
+}
+
+// Each element carries the fields of the expected element, and may add more.
+inline bool elementsContainFields(const folly::dynamic &actual, const folly::dynamic &expected)
+{
+  if (!actual.isArray() || !expected.isArray() || actual.size() != expected.size()) {
+    return payloadValuesMatch(actual, expected);
+  }
+  for (size_t i = 0; i < expected.size(); i++) {
+    for (const auto &[field, value] : expected[i].items()) {
+      if (actual[i].count(field) == 0 || !payloadValuesMatch(actual[i][field], value)) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+} // namespace detail
+
+/*
+ * Applies the commits in order and checks that each Props 2.0 payload
+ * (`getDiffProps`) has the keys, nulls and values of the Props 1.5 payload
+ * (accumulated raw props), apart from the commit's intended differences. The
+ * elements of the arrays under `keysWithAddedFields` may carry fields that JS
+ * does not send.
+ */
+template <typename PropsT>
+void expectPayloadsMatchRawProps(
+    const std::vector<PropsCommit> &commits,
+    const std::set<std::string> &keysWithAddedFields = {})
+{
+  ContextContainer contextContainer{};
+  PropsParserContext parserContext{-1, contextContainer};
+
+  auto previous = std::make_shared<const PropsT>();
+  folly::dynamic previousRawProps = folly::dynamic::object();
+  for (size_t i = 0; i < commits.size(); i++) {
+    const auto &commit = commits[i];
+    auto rawProps = RawProps(commit.props);
+    auto parser = RawPropsParser();
+    parser.template prepare<PropsT>();
+    rawProps.parse(parser);
+    auto props = std::make_shared<const PropsT>(parserContext, *previous, rawProps);
+    auto accumulatedRawProps = mergeDynamicProps(previousRawProps, commit.props, NullValueStrategy::Override);
+
+    auto actual = props->getDiffProps(i == 0 ? nullptr : previous.get());
+    auto expected = i == 0 ? accumulatedRawProps : diffDynamicProps(previousRawProps, accumulatedRawProps);
+    for (const auto &[key, value] : commit.intendedDifferences.items()) {
+      expected[key] = value;
+    }
+
+    std::set<std::string> keys;
+    for (const auto &payload : {actual, expected}) {
+      for (const auto &key : payload.keys()) {
+        keys.insert(key.asString());
+      }
+    }
+    for (const auto &key : keys) {
+      bool matches = actual.count(key) != 0 && expected.count(key) != 0 &&
+          (keysWithAddedFields.contains(key) ? detail::elementsContainFields(actual[key], expected[key])
+                                             : detail::payloadValuesMatch(actual[key], expected[key]));
+      EXPECT_TRUE(matches) << "commit " << i << " " << commit.props << ", key '" << key
+                           << "'\n  Props 2.0: " << actual.getDefault(key, "(absent)")
+                           << "\n  expected:  " << expected.getDefault(key, "(absent)");
+    }
+
+    previous = props;
+    previousRawProps = accumulatedRawProps;
+  }
+}
+
+} // namespace facebook::react
+
+#endif